lasdf1234 commented on code in PR #13676:
URL: https://github.com/apache/gravitino/pull/13676#discussion_r4237516624
##########
clients/client-java/src/main/java/org/apache/gravitino/client/DefaultOAuth2TokenProvider.java:
##########
@@ -61,6 +64,20 @@ public static class Builder
private String credential;
private String scope;
private String path;
+ private OAuth2ClientAuthenticationMethod authenticationMethod =
+ OAuth2ClientAuthenticationMethod.CLIENT_SECRET_POST;
+
+ /**
+ * Sets the OAuth2 client authentication method.
+ *
+ * @param authenticationMethod OAuth2 client authentication method.
+ * @return This Builder instance.
Review Comment:
**High — integration wiring still missing (Java)**
The builder API is the only way to select `CLIENT_SECRET_BASIC` today. Java
GFVS (`GravitinoVirtualFileSystemUtils`), Spark/Flink/Trino OAuth bootstrap
paths still build the provider without `withClientAuthenticationMethod`, so
deployments that only use `gravitino.client.oauth2.*` /
`fs.gravitino.client.oauth2.*` config cannot enable basic auth yet.
Worth either wiring a config property in this PR or documenting the
limitation explicitly in the PR description / client docs.
##########
clients/client-python/gravitino/auth/default_oauth2_token_provider.py:
##########
@@ -103,16 +111,45 @@ def _parse_credential(self):
def _fetch_token(self) -> str:
client_id, client_secret = self._parse_credential()
- client_credential_request = OAuth2ClientCredentialRequest(
- grant_type=CLIENT_CREDENTIALS,
- client_id=client_id,
- client_secret=client_secret,
- scope=self._scope,
- )
+ headers = {}
+
+ if (
+ self._authentication_method
+ == OAuth2ClientAuthenticationMethod.CLIENT_SECRET_BASIC
+ ):
+ if not client_id or not client_id.strip():
+ raise IllegalArgumentException(
+ "client_id must be set when using client_secret_basic
authentication"
+ )
+
+ encoded_client_id = quote_plus(client_id, safe="")
Review Comment:
**Medium — cross-check encoding with Java**
Python uses `quote_plus(..., safe="")` here. Please keep this consistent
with `OAuth2ClientUtil` (Java `URLEncoder`) — we have seen `*` handled
differently between the two. A shared test vector (same credential → same Basic
header) across languages would help.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
