Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Ensure jwks_uri or jwks can be set when using private_key_jwt #1648

Closed
wants to merge 2 commits into from
Closed

Ensure jwks_uri or jwks can be set when using private_key_jwt #1648

wants to merge 2 commits into from

Conversation

virgofx
Copy link
Contributor

@virgofx virgofx commented Jul 27, 2023 •
edited
Loading

Small bugfix to ensure jwks_uri can be used and additionally adds the field into documentation for the resource and error message.

/cc @MikeMondragon-okta @duytiennguyen-okta

Testing

The following app uses jwks_uri. Tested creation and updating. Updating log shown below:

terraform state show 'resource123'
# resource123:
resource "okta_app_oauth" "default" {
    accessibility_self_service = false
    admin_note                 = <<-EOT
        Old
    EOT
    app_links_json             = jsonencode(
        {
            oidc_client_link = true
        }
    )
    app_settings_json          = jsonencode({})
    auto_key_rotation          = true
    auto_submit_toolbar        = false
    client_id                  = "XXXXXXXXX"
    consent_method             = "TRUSTED"
    grant_types                = [
        "client_credentials",
    ]
    hide_ios                   = true
    hide_web                   = true
    id                         = "XXXXXXXXX"
    implicit_assignment        = false
    issuer_mode                = "CUSTOM_URL"
    jwks_uri                   = "https://example.com"
    label                      = "XXXX"
    login_mode                 = "DISABLED"
    login_scopes               = []
    logo                       = "5ede509fb78728daf69f9720660b7490da1903f91ce44df8337049d4d9b4b81f"
    logo_url                   = "https://ok14static.oktacdn.com/fs/bco/4/fs06l2v85f2YdeKbw697"
    name                       = "oidc_client"
    omit_secret                = true
    pkce_required              = false
    post_logout_redirect_uris  = []
    redirect_uris              = []
    response_types             = [
        "token",
    ]
    sign_on_mode               = "OPENID_CONNECT"
    status                     = "ACTIVE"
    token_endpoint_auth_method = "private_key_jwt"
    type                       = "service"
    user_name_template         = "${source.login}"
    user_name_template_type    = "BUILT_IN"
    wildcard_redirect          = "DISABLED"
}

Updating:

resource123 will be updated in-place
  ~ resource "okta_app_oauth" "resource123" {
      ~ admin_note                 = <<-EOT
          - Old
          + New
        EOT
        id                         = "XXXXXXX"
        name                       = "oidc_client"
        # (30 unchanged attributes hidden)
    }

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

virgofx
virgofx commented Jul 27, 2023
View reviewed changes
website/docs/r/app_oauth.html.markdown
@@ -190,7 +192,7 @@ Valid values: `"CUSTOM_URL"`,`"ORG_URL"` or `"DYNAMIC"`. Default is `"ORG_URL"`.

## Timeouts

The `timeouts` block allows you to specify custom [timeouts](https://www.terraform.io/language/resources/syntax#operation-timeouts) for certain actions:
The `timeouts` block allows you to specify custom [timeouts](https://www.terraform.io/language/resources/syntax#operation-timeouts) for certain actions:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Result of formatting - Must have had trailing whitespace

virgofx
virgofx commented Jul 27, 2023
View reviewed changes
website/docs/r/app_oauth.html.markdown
Comment on lines +108 to +109
- `jwks_uri` - (Optional) URL of the custom authorization server's JSON Web Key Set document.

Copy link
Contributor Author

@virgofx virgofx Jul 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ensure this property is documented in Okta documentation for this resource. This was missed during addition of the original logic.

virgofx
virgofx commented Jul 27, 2023
View reviewed changes
okta/resource_okta_app_oauth.go
Comment on lines +846 to +849
_, jwks := d.GetOk("jwks")
_, jwks_uri := d.GetOk("jwks_uri")
if !(jwks || jwks_uri) && d.Get("token_endpoint_auth_method").(string) == "private_key_jwt" {
return errors.New("'jwks' or 'jwks_uri' is required when 'token_endpoint_auth_method' is 'private_key_jwt'")
Copy link
Contributor Author

@virgofx virgofx Jul 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Update the logic to ensure that if private_key_jwt is specified we allow either jwks or jwks_uri

virgofx
virgofx commented Jul 27, 2023
View reviewed changes
okta/provider_test.go
var (
testAccProvidersFactories map[string]func() (*schema.Provider, error)
)
var testAccProvidersFactories map[string]func() (*schema.Provider, error)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Result of make fmt

virgofx
virgofx commented Jul 27, 2023
View reviewed changes
okta/data_source_okta_group_rule.go
@@ -57,7 +57,7 @@ func dataSourceGroupRuleRead(ctx context.Context, d *schema.ResourceData, m inte
} else {
ruleName, nameOk := d.GetOk("name")
if nameOk {
var name = ruleName.(string)
name := ruleName.(string)
Copy link
Contributor Author

@virgofx virgofx Jul 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Result of make fmt

duytiennguyen-okta
duytiennguyen-okta approved these changes Jul 27, 2023
View reviewed changes
Copy link
Contributor

@duytiennguyen-okta duytiennguyen-okta left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm. Thanks @virgofx

@virgofx virgofx changed the title Ensurejwks_uri or jwks can be set when using private_key_jwt Ensure jwks_uri or jwks can be set when using private_key_jwt Jul 27, 2023
duytiennguyen-okta added a commit that referenced this pull request Jul 27, 2023
@duytiennguyen-okta
bring in virgofx's work from #1648
fca4b48
@monde
Copy link
Collaborator

monde commented Jul 27, 2023

merged into #1649

@monde monde closed this Jul 27, 2023
@monde monde mentioned this pull request Aug 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@duytiennguyen-okta duytiennguyen-okta duytiennguyen-okta approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Set jwks-uri on
3 participants
@virgofx @monde @duytiennguyen-okta