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

feat(auth): add grant type and acr values to OIDC configs #10798

Closed
wants to merge 1 commit into from
Closed

feat(auth): add grant type and acr values to OIDC configs #10798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions datahub-frontend/app/auth/sso/oidc/OidcConfigs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is DEFAULT_OIDC_GRANT_TYPE defined?

acrValues = getOptional(configs, OIDC_ACR_VALUES, DEFAULT_OIDC_ACR_VALUES);
customParamResource = getOptional(configs, OIDC_CUSTOM_PARAM_RESOURCE);
readTimeout = getOptional(configs, OIDC_READ_TIMEOUT, DEFAULT_OIDC_READ_TIMEOUT);
extractJwtAccessTokenClaims =
Expand Down
3 changes: 2 additions & 1 deletion datahub-frontend/app/auth/sso/oidc/OidcProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public SsoProtocol protocol() {
return SsoProtocol.OIDC;
}

private Client<OidcCredentials> createPac4jClient() {
private Client<OidcCredentials, OidcProfile> createPac4jClient() {
final OidcConfiguration oidcConfiguration = new OidcConfiguration();
oidcConfiguration.setClientId(_oidcConfigs.getClientId());
oidcConfiguration.setSecret(_oidcConfigs.getClientSecret());
Expand All @@ -75,6 +75,7 @@ private Client<OidcCredentials> createPac4jClient() {
oidcConfiguration.setPreferredJwsAlgorithm(preferred);
});

oidcConfiguration.setCustomParams(ImmutableMap.of("grant_type", _oidcConfigs.getGrantType(), "acr_values", _oidcConfigs.getAcrValues()));
Copy link
Contributor

Choose a reason for hiding this comment

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

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.java
    • datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java
    • datahub-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

Copy link
Collaborator

Choose a reason for hiding this comment

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

are we overriding the previous customParams set on line 69?

final CustomOidcClient oidcClient = new CustomOidcClient(oidcConfiguration);
oidcClient.setName(OIDC_CLIENT_NAME);
oidcClient.setCallbackUrl(
Expand Down
3 changes: 3 additions & 0 deletions datahub-frontend/conf/application.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ auth.oidc.readTimeout = ${?AUTH_OIDC_READ_TIMEOUT}
auth.oidc.extractJwtAccessTokenClaims = ${?AUTH_OIDC_EXTRACT_JWT_ACCESS_TOKEN_CLAIMS} # Whether to extract claims from JWT access token. Defaults to false.
auth.oidc.preferredJwsAlgorithm = ${?AUTH_OIDC_PREFERRED_JWS_ALGORITHM} # Which jws algorithm to use

auth.oidc.acrValues = ${?AUTH_OIDC_ACR_VALUES}
auth.oidc.grantType = ${?AUTH_OIDC_GRANT_TYPE}

#
# By default, the callback URL that should be registered with the identity provider is computed as {$baseUrl}/callback/oidc.
# For example, the default callback URL for a local deployment of DataHub would be "http://localhost:9002/callback/oidc".
Expand Down
Loading