Copilot commented on code in PR #13676:
URL: https://github.com/apache/gravitino/pull/13676#discussion_r4215566644
##########
clients/client-python/gravitino/auth/default_oauth2_token_provider.py:
##########
@@ -43,20 +46,22 @@ class DefaultOAuth2TokenProvider(OAuth2TokenProvider):
_scope: Optional[str]
_path: Optional[str]
_token: Optional[str]
-
+ _authentication_method: OAuth2ClientAuthenticationMethod
def __init__(
self,
uri: str = None,
credential: str = None,
scope: str = None,
+ authentication_method: OAuth2ClientAuthenticationMethod =
+ OAuth2ClientAuthenticationMethod.CLIENT_SECRET_POST,
path: str = None,
Review Comment:
This inserts `authentication_method` before the existing `path` argument, so
current positional callers such as `DefaultOAuth2TokenProvider(uri, credential,
scope, path)` now bind `path` to `authentication_method` and leave `_path` as
`None`; validation then fails before a token request. Move the new option after
`path` (or make it keyword-only) to preserve the existing Python API.
##########
clients/client-python/gravitino/auth/default_oauth2_token_provider.py:
##########
@@ -103,16 +108,39 @@ 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
+ ):
+ credentials = f"{client_id}:{client_secret}"
+ encoded_credentials = base64.b64encode(
+ credentials.encode("utf-8")
+ ).decode("utf-8")
+
+ headers["Authorization"] = f"Basic {encoded_credentials}"
Review Comment:
When the credential contains only a secret, `_parse_credential()` returns
`client_id=None`, but this branch still constructs `None:<secret>` and sends it
as Basic credentials. `client_secret_basic` requires a client ID; reject
missing or blank IDs here instead of emitting a malformed Authorization header.
##########
clients/client-java/src/main/java/org/apache/gravitino/client/OAuth2ClientAuthenticationMethod.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.
+ * See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
Review Comment:
The new Java source file has only a shortened license notice and omits the
repository's full Apache 2.0 header (including the license URL and terms),
unlike the existing client Java sources. Replace this header with the standard
project header before merging.
##########
clients/client-python/gravitino/auth/oauth2_client_authentication_method.py:
##########
@@ -0,0 +1,6 @@
+from enum import Enum
+
+
+class OAuth2ClientAuthenticationMethod(Enum):
+ CLIENT_SECRET_POST = "client_secret_post"
+ CLIENT_SECRET_BASIC = "client_secret_basic"
Review Comment:
This new Python source file is missing the repository-required Apache
license header; the surrounding auth modules, for example
`default_oauth2_token_provider.py:1-16`, include it. Add the standard header
before the import so the new file passes the repository's license checks.
--
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]
