Skip to content

fix(bqjdbc): add default OAuth client id/secret#12946

Open
logachev wants to merge 1 commit intomainfrom
kirl/user_auth_default_client
Open

fix(bqjdbc): add default OAuth client id/secret#12946
logachev wants to merge 1 commit intomainfrom
kirl/user_auth_default_client

Conversation

@logachev
Copy link
Copy Markdown
Contributor

b/500714643

This OAuth ClientId/Secret is used also by BQ CLI and other drivers, so this is not a real "secret" (e.g. can be found in BQ CLI py files).

@logachev logachev requested review from a team as code owners April 28, 2026 22:43
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces default OAuth client credentials in BigQueryJdbcUrlUtility and updates the DataSource getters to return these defaults when values are missing. It also includes a new test case to verify the default behavior. Feedback highlights that applying these defaults unconditionally may conflict with other authentication types and recommends using trim().isEmpty() for more robust string validation. Additionally, hardcoding credentials in the source code was identified as a security risk.

Comment on lines +943 to +945
return oAuthClientId != null && !oAuthClientId.isEmpty()
? oAuthClientId
: BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_ID;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The getter returns a default OAuth client ID unconditionally when the field is null or empty. This can lead to authentication conflicts when using other authentication types (e.g., Service Account) that do not require a client ID. Consider applying this default only when the OAuthType is set to User Account. Also, using trim().isEmpty() ensures that blank strings are correctly handled as empty.

Suggested change
return oAuthClientId != null && !oAuthClientId.isEmpty()
? oAuthClientId
: BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_ID;
return oAuthClientId != null && !oAuthClientId.trim().isEmpty()
? oAuthClientId
: BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_ID;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we not want to add trim().isEmpty()?

Comment on lines +953 to +955
return oAuthClientSecret != null && !oAuthClientSecret.isEmpty()
? oAuthClientSecret
: BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_SECRET;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Similar to getOAuthClientId, this getter returns a default secret unconditionally. This may cause issues with other authentication methods. Ensure that this default is only applied when appropriate for the selected OAuthType. Using trim().isEmpty() is also recommended here.

Suggested change
return oAuthClientSecret != null && !oAuthClientSecret.isEmpty()
? oAuthClientSecret
: BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_SECRET;
return oAuthClientSecret != null && !oAuthClientSecret.trim().isEmpty()
? oAuthClientSecret
: BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_SECRET;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same question here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants