fix(configuration): missing request_uris option #7033
Conversation
This fixes a missing option for OpenID Connect 1.0 clients 'redirect_uris'. This feature was effectively implemented but no way to configure it existed. Signed-off-by: James Elliott <james-d-elliott@users.noreply.github.com>
ArtifactsThese changes are published for testing on Buildkite, DockerHub and GitHub Container Registry. Docker Container
|
WalkthroughThis update introduces enhancements and a new feature to the OpenID Connect client configuration within the project. The Swagger UI version has been upgraded to improve the API documentation experience. A significant addition is the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
✅ Deploy Preview for authelia-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
config.template.ymlis excluded by:!**/*.ymldocs/static/schemas/latest/json-schema/configuration.jsonis excluded by:!**/*.jsondocs/static/schemas/v4.38/json-schema/configuration.jsonis excluded by:!**/*.jsoninternal/configuration/config.template.ymlis excluded by:!**/*.yml
Files selected for processing (10)
- cmd/authelia-scripts/cmd/gen.go (1 hunks)
- docs/content/configuration/identity-providers/openid-connect/clients.md (2 hunks)
- internal/configuration/schema/identity_providers.go (1 hunks)
- internal/configuration/schema/keys.go (1 hunks)
- internal/configuration/schema/types.go (1 hunks)
- internal/configuration/schema/types_test.go (1 hunks)
- internal/configuration/validator/const.go (2 hunks)
- internal/configuration/validator/identity_providers.go (2 hunks)
- internal/configuration/validator/identity_providers_test.go (7 hunks)
- internal/oidc/client.go (1 hunks)
Files skipped from review due to trivial changes (1)
- cmd/authelia-scripts/cmd/gen.go
Additional comments: 13
internal/configuration/schema/keys.go (1)
- 43-43: The addition of
"identity_providers.oidc.clients[].request_uris"to the schema keys list correctly implements the newrequest_urisconfiguration option for OpenID Connect clients. This change aligns with the PR objectives and is implemented correctly.internal/oidc/client.go (1)
- 28-28: The addition of the
RequestURIsfield to theNewClientfunction's configuration correctly implements the newrequest_urisconfiguration option for OpenID Connect clients. This change aligns with the PR objectives and is implemented correctly.internal/configuration/schema/types.go (1)
- 497-499: Renaming
IdentityProvidersOpenIDConnectClientRedirectURIstoIdentityProvidersOpenIDConnectClientURIscorrectly reflects the addition of therequest_urisconfiguration option and generalizes the type for both redirect URIs and request URIs. This change aligns with the PR objectives and is implemented correctly.internal/configuration/schema/identity_providers.go (1)
- 118-119: The addition of the
RequestURIsfield to theIdentityProvidersOpenIDConnectClientstruct is well-implemented, with appropriate JSON and schema tags. This change aligns with the PR's objectives and enhances the configurability of OpenID Connect clients.However, it's essential to ensure that this new configuration option integrates seamlessly with the existing system and does not introduce any unintended side effects. Testing and validation should cover scenarios where the
RequestURIsare specified, as well as backward compatibility with configurations that do not use this new field.internal/configuration/schema/types_test.go (1)
- 368-368: The renaming of
IdentityProvidersOpenIDConnectClientRedirectURIstoIdentityProvidersOpenIDConnectClientURIsis correctly reflected in the test cases. This change aligns with the PR's objective to introduce and utilize therequest_urisconfiguration option for OpenID Connect clients.docs/content/configuration/identity-providers/openid-connect/clients.md (1)
- 206-213: The
request_urisconfiguration option is correctly documented, including the requirement for URIs to use thehttpsscheme. This aligns with the PR objectives to enhance security and flexibility in OpenID Connect client configurations.internal/configuration/validator/const.go (2)
- 230-236: The addition of error formats for OIDC client request URIs is consistent with the PR objectives to enhance OpenID Connect client configurations by introducing the
request_urisoption. These error messages are well-defined and follow the existing pattern for error messages in this file.- 524-524: The addition of the
attrOIDCRequestURIsconstant aligns with the introduction of therequest_urisconfiguration option for OpenID Connect clients. This constant is correctly named and follows the naming convention of other OIDC-related constants in this file.internal/configuration/validator/identity_providers.go (1)
- 933-957: The addition of the
validateOIDCClientRequestURIsfunction enhances the validation of request URIs for OpenID Connect clients by ensuring they are absolute URLs, use the HTTPS scheme, and are unique. This is a crucial security measure for preventing potential vulnerabilities related to insecure request URIs. Good job on implementing these checks.internal/configuration/validator/identity_providers_test.go (4)
- 325-372: The test cases for validating OIDC client JWKS are comprehensive and well-structured, covering a wide range of scenarios including URL validation, key properties, and error handling for invalid configurations.
- 325-372: The test cases for validating OIDC issuer configuration are thorough and cover essential aspects such as key and certificate validation, algorithm support, and error handling for invalid configurations.
- 325-372: The test cases for validating OIDC lifespans configuration effectively cover scenarios related to default values, custom definitions, and error handling for invalid configurations.
- 325-372: The test cases for validating OIDC authorization policies are comprehensive, covering validation of policy names, default policies, custom rules, and error handling for invalid configurations.
| public: false | ||
| redirect_uris: | ||
| - 'https://oidc.example.com:8080/oauth2/callback' | ||
| request_uris: | ||
| - 'https://oidc.example.com:8080/oidc/request-object.jwk' | ||
| audience: | ||
| - 'https://app.example.com' | ||
| scopes: |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [17-17]
Consider using a hyphen in "provider specific" to form the compound adjective "provider-specific".
- For the provider specific configuration and information not related to clients see the [OpenID Connect 1.0 Provider](provider.md)
+ For the provider-specific configuration and information not related to clients see the [OpenID Connect 1.0 Provider](provider.md)📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [18-18]
A comma may improve readability after "clients".
- For the provider specific configuration and information not related to clients see the [OpenID Connect 1.0 Provider](provider.md)
+ For the provider specific configuration and information not related to clients, see the [OpenID Connect 1.0 Provider](provider.md)| @@ -365,7 +365,7 @@ func TestJSONSchema(t *testing.T) { | |||
| &AccessControlRuleMethods{}, | |||
| &AccessControlRuleRegex{}, | |||
| &AccessControlRuleSubjects{}, | |||
| &IdentityProvidersOpenIDConnectClientRedirectURIs{}, | |||
| &IdentityProvidersOpenIDConnectClientURIs{}, | |||
There was a problem hiding this comment.
Considering the introduction of the request_uris configuration option, it would be beneficial to include test cases that specifically validate its behavior and correct configuration. This could involve testing the validation logic for the URIs specified, ensuring they adhere to the https scheme, and verifying their correct usage within the OIDC client initialization process.
Would you like me to suggest a template for such test cases?
This fixes a missing option for OpenID Connect 1.0 clients 'request_uris'. This feature was effectively implemented but no way to configure it existed.
Summary by CodeRabbit
request_urisfor OpenID Connect clients, enabling the use of URIs for passing Authorize Request parameters.request_urisunder the OpenID Connect client configuration section.