-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding support for base_url for Okta api #3316
Changes from 2 commits
e78d769
fede390
b8e831d
cf39e40
7f91eac
1a291c8
489479c
2c6bf0d
fc33402
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ package okta | |
import ( | ||
"fmt" | ||
"net/url" | ||
"strings" | ||
|
||
"time" | ||
|
||
|
@@ -13,35 +12,39 @@ import ( | |
"github.com/hashicorp/vault/logical/framework" | ||
) | ||
|
||
const ( | ||
defaultBaseURL = "okta.com" | ||
previewBaseURL = "oktapreview.com" | ||
) | ||
|
||
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", | ||
}, | ||
"org_name": &framework.FieldSchema{ | ||
Type: framework.TypeString, | ||
Description: "Name of the organization to be used in the Okta API.", | ||
}, | ||
"token": &framework.FieldSchema{ | ||
Type: framework.TypeString, | ||
Description: "Okta admin API token (DEPRECATED)", | ||
Description: "(DEPRECATED) Okta admin API token", | ||
}, | ||
"api_token": &framework.FieldSchema{ | ||
Type: framework.TypeString, | ||
Description: "Okta API key.", | ||
}, | ||
"base_url": &framework.FieldSchema{ | ||
Type: framework.TypeString, | ||
Description: `The API endpoint to use. Useful if you | ||
are using Okta development accounts. (DEPRECATED)`, | ||
Type: framework.TypeString, | ||
Default: defaultBaseURL, | ||
Description: `The base domain to use for the Okta API. Default is "okta.com".`, | ||
}, | ||
"production": &framework.FieldSchema{ | ||
Type: framework.TypeBool, | ||
Default: true, | ||
Description: `If set, production API URL prefix will be used to communicate with Okta and if not set, a preview production API URL prefix will be used. Defaults to true.`, | ||
Description: `(DEPRECATED) Use base_url.`, | ||
}, | ||
"ttl": &framework.FieldSchema{ | ||
Type: framework.TypeDurationSecond, | ||
|
@@ -100,14 +103,16 @@ func (b *backend) pathConfigRead( | |
Data: map[string]interface{}{ | ||
"organization": cfg.Org, | ||
"org_name": cfg.Org, | ||
"production": *cfg.Production, | ||
"ttl": cfg.TTL, | ||
"max_ttl": cfg.MaxTTL, | ||
}, | ||
} | ||
if cfg.BaseURL != "" { | ||
resp.Data["base_url"] = cfg.BaseURL | ||
} | ||
if cfg.Production != nil { | ||
resp.Data["production"] = *cfg.Production | ||
} | ||
|
||
return resp, nil | ||
} | ||
|
@@ -153,22 +158,18 @@ func (b *backend) pathConfigWrite( | |
return logical.ErrorResponse("api_token is missing"), nil | ||
} | ||
|
||
baseURL, ok := d.GetOk("base_url") | ||
if ok { | ||
baseURLString := baseURL.(string) | ||
if len(baseURLString) != 0 { | ||
_, err = url.Parse(baseURLString) | ||
if err != nil { | ||
return logical.ErrorResponse(fmt.Sprintf("Error parsing given base_url: %s", err)), nil | ||
} | ||
cfg.BaseURL = baseURLString | ||
} | ||
} 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
_, err = url.Parse(fmt.Sprintf("https://%s,%s", cfg.Org, baseURL)) | ||
if err != nil { | ||
return logical.ErrorResponse(fmt.Sprintf("Error parsing given base_url: %s", err)), nil | ||
} | ||
cfg.BaseURL = baseURL | ||
|
||
productionRaw := d.Get("production").(bool) | ||
cfg.Production = &productionRaw | ||
productionRaw, ok := d.GetOk("production") | ||
if ok { | ||
production := productionRaw.(bool) | ||
cfg.Production = &production | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. I played around with this dance a bit and ended up hear. I like your suggestion better. |
||
} | ||
|
||
ttl, ok := d.GetOk("ttl") | ||
if ok { | ||
|
@@ -207,16 +208,18 @@ func (b *backend) pathConfigExistenceCheck( | |
|
||
// OktaClient creates a basic okta client connection | ||
func (c *ConfigEntry) OktaClient() *okta.Client { | ||
production := true | ||
if c.Production != nil { | ||
production = *c.Production | ||
} | ||
baseURL := defaultBaseURL | ||
if c.BaseURL != "" { | ||
if strings.Contains(c.BaseURL, "oktapreview.com") { | ||
production = false | ||
baseURL = c.BaseURL | ||
} | ||
if c.Production != nil { | ||
if !*c.Production { | ||
baseURL = previewBaseURL | ||
} | ||
} | ||
return okta.NewClient(cleanhttp.DefaultClient(), c.Org, c.Token, production) | ||
// We validate config on input and errors are only returned when parsing URLs | ||
client, _ := okta.NewClientWithDomain(cleanhttp.DefaultClient(), c.Org, baseURL, c.Token) | ||
return client | ||
} | ||
|
||
// ConfigEntry for Okta | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe update this to say "Use org_name instead."