-
Notifications
You must be signed in to change notification settings - Fork 206
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
Added support for retrieving client secret from okta_app_oauth data source #1280
Conversation
9604d37
to
d891aae
Compare
d891aae
to
541dd43
Compare
@@ -195,6 +202,35 @@ func dataSourceAppOauthRead(ctx context.Context, d *schema.ResourceData, m inter | |||
if err != nil { | |||
return diag.Errorf("failed to list OAuth's app groups and users: %v", err) | |||
} | |||
skipClientSecrets := false // Do we ever need to skip doing this? |
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.
there's an omit_secret
attribute in the resource, we could follow the same pattern here?
terraform-provider-okta/okta/resource_okta_app_oauth.go
Lines 146 to 152 in 090c474
"omit_secret": { | |
Type: schema.TypeBool, | |
Optional: true, | |
// No ForceNew to avoid recreating when going from false => true | |
Description: "This tells the provider not to persist the application's secret to state. If this is ever changes from true => false your app will be recreated.", | |
Default: false, | |
}, |
terraform-provider-okta/okta/resource_okta_app_oauth.go
Lines 532 to 534 in 090c474
if d.Get("omit_secret").(bool) { | |
_ = d.Set("client_secret", "") | |
} |
@dkulchinsky I'll try to address this in a release this week. Last week my focus was pulled away on to another Okta project. |
@rickardp @dkulchinsky I'll do a separate PR and bring in this behavior. |
Fixes the problem for the specific case reported in #1279.
This works in the case that I have tested, but I am not sure if it is a good idea to fetch the secrets using this call (maybe it causes problems with token scopes) in which case maybe there is a need to opt in/out of this.
Draft PR because it currently serves as discussion material and I do not expect it to be merged in the current state. I can spend some time fixing it up based on review feedback if this is the correct path for solving this issue.