dimas-b commented on code in PR #952:
URL: https://github.com/apache/polaris/pull/952#discussion_r1947339254
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultOAuth2ApiService.java:
##########
@@ -75,43 +74,39 @@ public Response getToken(
if (!tokenBroker.supportsRequestedTokenType(requestedTokenType)) {
return
OAuthUtils.getResponseFromError(OAuthTokenErrorResponse.Error.invalid_request);
}
- if (authHeader == null && clientId == null) {
+ if (authHeader == null && clientSecret == null) {
return
OAuthUtils.getResponseFromError(OAuthTokenErrorResponse.Error.invalid_client);
}
- if (authHeader != null && clientId == null && authHeader.startsWith("Basic
")) {
+ // token exchange with client id and client secret in the authorization
header means the client
+ // has previously attempted to refresh an access token, but refreshing was
not supported by the
+ // token broker. Accept the client id and secret and treat it as a new
token request
Review Comment:
nit: The comment makes sense now (thanks for updating)... However, I'm not
sure this is the right place to discuss the behaviour of the client in general.
Polaris does not control these clients in principle and does not specify how
they should behave. If this refers to the current behaviour of REST Catalog
client from the Iceberg codebase, I think it would be much clearer if the
comment said that explicitly (referencing a version).
--
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]
