Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

ASR-14: accept full okta domains #14

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

ASR-14: accept full okta domains #14

wants to merge 2 commits into from

Conversation

thardt-praetorian
Copy link
Contributor

if the operator enters a full okta domain for the okta provider subdomain, we now truncate and accept the subdomain

Copy link
Contributor

@dkaman dkaman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple things before merge

@@ -62,6 +63,16 @@ func (Driver) New(opts map[string]string) (nozzle.Nozzle, error) {
return nil, fmt.Errorf("okta nozzle requires 'subdomain' config parameter")
}

// check if operator entered full domain instead of subdomain
if strings.Contains(subdomain, ".okta.com") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check may not work if the domain has .okta.com in the middle of it. We have a function in the Util package util.ValidateURLSuffix designed to check for this at the end of urls. can you swap to that and give it a test?

// check if operator entered full domain instead of subdomain
if strings.Contains(subdomain, ".okta.com") {
domains := strings.Split(subdomain, ".")
// i don't think sub-subdomains are legal for okta, but i could be wrong
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also before we merge to master can you delete this and if it represents more code/testing we need to do, create a ticket that references this line?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants