feat(auth): add grant type and acr values to OIDC configs #10798
Conversation
WalkthroughThe recent updates enrich the OpenID Connect (OIDC) authentication with additional configuration options for Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Configuration
participant OidcConfigs as OidcConfigs
participant OidcProvider as OidcProvider
participant Pac4jClient as Pac4jClient
Config->>OidcConfigs: Load grantType and acrValues
OidcConfigs->>OidcProvider: Provide grantType and acrValues
OidcProvider->>Pac4jClient: Set grant_type and acr_values
Pac4jClient-->>OidcProvider: Client ready with configurations
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- datahub-frontend/app/auth/sso/oidc/OidcConfigs.java (1 hunks)
- datahub-frontend/app/auth/sso/oidc/OidcProvider.java (2 hunks)
- datahub-frontend/conf/application.conf (1 hunks)
Additional comments not posted (2)
datahub-frontend/app/auth/sso/oidc/OidcConfigs.java (1)
166-167: Validate default values for new OIDC configurations.The added lines correctly implement default values for
grantTypeandacrValues. It's important to ensure these defaults align with the expected security standards and OIDC specifications.datahub-frontend/conf/application.conf (1)
190-192: Confirm the correct setup of environment variables for new OIDC settings.The configuration lines for
auth.oidc.acrValuesandauth.oidc.grantTypeare correctly set up to be overridden by environment variables, which is crucial for flexible deployment configurations.
| @@ -75,6 +75,7 @@ private Client<OidcCredentials> createPac4jClient() { | |||
| oidcConfiguration.setPreferredJwsAlgorithm(preferred); | |||
| }); | |||
|
|
|||
| oidcConfiguration.setCustomParams(ImmutableMap.of("grant_type", _oidcConfigs.getGrantType(), "acr_values", _oidcConfigs.getAcrValues())); | |||
There was a problem hiding this comment.
Tip
Codebase Verification
Ensure proper handling of custom parameters in OIDC configuration.
The addition of grant_type and acr_values to the OIDC configuration is crucial for the new feature. However, there is no evidence of these parameters being validated before being set, which could lead to potential security issues. Please implement validation for these parameters to ensure they are correctly handled.
- Files to review:
datahub-frontend/app/auth/sso/oidc/OidcProvider.javadatahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.javadatahub-frontend/app/auth/sso/oidc/custom/CustomOidcAuthenticator.java
Analysis chain
Ensure proper handling of custom parameters in OIDC configuration.
The addition of grant_type and acr_values to the OIDC configuration is crucial for the new feature. However, ensure that these parameters are properly validated before being set to avoid potential security issues.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the provided grant_type and acr_values are validated.
# Test: Search for validation logic related to grant_type and acr_values.
rg --type java "(validate|check).*(grantType|acrValues)" datahub-frontend/app/auth/sso/oidc/
Length of output: 94
Script:
#!/bin/bash
# Description: Verify the provided grant_type and acr_values are validated.
# Search for any validation logic related to _oidcConfigs.
rg --type java "(validate|check).*(oidcConfigs|_oidcConfigs)" datahub-frontend/
# Search for validation logic within OIDC-related classes and files.
rg --type java "(validate|check)" datahub-frontend/app/auth/sso/oidc/
Length of output: 663
Script:
#!/bin/bash
# Description: Extract and examine validation logic within CustomOidcAuthenticator.java and OidcCallbackLogic.java.
# Extract the validate method from CustomOidcAuthenticator.java
ast-grep --lang java --pattern 'public void validate($_, $_) { $$$ }' datahub-frontend/app/auth/sso/oidc/custom/CustomOidcAuthenticator.java
# Extract the validation-related code from OidcCallbackLogic.java
rg --type java 'validate' datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java
Length of output: 323
| @@ -75,6 +75,7 @@ private Client<OidcCredentials> createPac4jClient() { | |||
| oidcConfiguration.setPreferredJwsAlgorithm(preferred); | |||
| }); | |||
|
|
|||
| oidcConfiguration.setCustomParams(ImmutableMap.of("grant_type", _oidcConfigs.getGrantType(), "acr_values", _oidcConfigs.getAcrValues())); | |||
There was a problem hiding this comment.
are we overriding the previous customParams set on line 69?
| @@ -163,6 +163,8 @@ public Builder from(final com.typesafe.config.Config configs) { | |||
| responseType = getOptional(configs, OIDC_RESPONSE_TYPE); | |||
| responseMode = getOptional(configs, OIDC_RESPONSE_MODE); | |||
| useNonce = getOptional(configs, OIDC_USE_NONCE).map(Boolean::parseBoolean); | |||
| grantType = getOptional(configs, OIDC_GRANT_TYPE, DEFAULT_OIDC_GRANT_TYPE); | |||
There was a problem hiding this comment.
where is DEFAULT_OIDC_GRANT_TYPE defined?
Checklist
Summary by CodeRabbit
New Features
grantTypeandacrValuesconfigurations in Single Sign-On (SSO) settings.Configuration
auth.oidc.acrValuesandauth.oidc.grantType.