dimas-b commented on code in PR #952:
URL: https://github.com/apache/polaris/pull/952#discussion_r1947336357
##########
service/common/src/main/java/org/apache/polaris/service/auth/JWTBroker.java:
##########
@@ -98,8 +98,12 @@ public String getScope() {
@Override
public TokenResponse generateFromToken(
- TokenType tokenType, String subjectToken, String grantType, String
scope) {
- if (!TokenType.ACCESS_TOKEN.equals(tokenType)) {
+ TokenType subjectTokenType,
+ String subjectToken,
+ String grantType,
+ String scope,
+ TokenType requestedTokenType) {
+ if (!TokenType.ACCESS_TOKEN.equals(subjectTokenType)) {
Review Comment:
Shouldn't we validate `requestedTokenType`? We return
`TokenType.ACCESS_TOKEN` (line 123) even if it was not requested :thinking:
##########
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 client 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]
