Adding support for base_url for Okta api #3316
Conversation
| func pathConfig(b *backend) *framework.Path { | ||
| return &framework.Path{ | ||
| Pattern: `config`, | ||
| Fields: map[string]*framework.FieldSchema{ | ||
| "organization": &framework.FieldSchema{ | ||
| Type: framework.TypeString, | ||
| Description: "Okta organization to authenticate against (DEPRECATED)", | ||
| Description: "(DEPRECATED) Okta organization to authenticate against", |
There was a problem hiding this comment.
Maybe update this to say "Use org_name instead."
| } | ||
| } else if req.Operation == logical.CreateOperation { | ||
| cfg.BaseURL = d.Get("base_url").(string) | ||
| baseURL := d.Get("base_url").(string) |
There was a problem hiding this comment.
This code now requires base_url set at all times instead of honoring create/update. Not necessarily a problem, but a change in behavior. Also, the documentation lists it as optional.
There was a problem hiding this comment.
I guess this is the problem with providing a default value in the fields. I'll remove the default and only populate it when it is specified.
| productionRaw, ok := d.GetOk("production") | ||
| if ok { | ||
| production := productionRaw.(bool) | ||
| cfg.Production = &production |
There was a problem hiding this comment.
Because this loads cfg first if it's there, if they have production in the past, but now leave that value out and switch to a custom base url, there's a possibility of their base url being overwritten by the production value later on. I think if a base_url is provided we should nil out production.
There was a problem hiding this comment.
Sounds good. I played around with this dance a bit and ended up hear. I like your suggestion better.
|
I just realized that the previous PR that standardized the APIs across Okta features removed the ability to optionally query the groups from Okta. My latest commit reintroduces this behavior and addresses the previous comments. |
This adds back support for providing the base_url for Okta API requests. This will allow the backend to support domains such as okta-emea.com.
Fixes #3313