fix(bqjdbc): add default OAuth client id/secret#12946
Conversation
There was a problem hiding this comment.
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.
| return oAuthClientId != null && !oAuthClientId.isEmpty() | ||
| ? oAuthClientId | ||
| : BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_ID; |
There was a problem hiding this comment.
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.
| return oAuthClientId != null && !oAuthClientId.isEmpty() | |
| ? oAuthClientId | |
| : BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_ID; | |
| return oAuthClientId != null && !oAuthClientId.trim().isEmpty() | |
| ? oAuthClientId | |
| : BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_ID; |
There was a problem hiding this comment.
Do we not want to add trim().isEmpty()?
| return oAuthClientSecret != null && !oAuthClientSecret.isEmpty() | ||
| ? oAuthClientSecret | ||
| : BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_SECRET; |
There was a problem hiding this comment.
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.
| return oAuthClientSecret != null && !oAuthClientSecret.isEmpty() | |
| ? oAuthClientSecret | |
| : BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_SECRET; | |
| return oAuthClientSecret != null && !oAuthClientSecret.trim().isEmpty() | |
| ? oAuthClientSecret | |
| : BigQueryJdbcUrlUtility.DEFAULT_OAUTH_CLIENT_SECRET; |
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
pyfiles).