Skip to content

Commit 42af733

Browse files
collado-mikecollado-mike
authored
Update DefaultOAuth2ApiService to support multiple token types and client secret without id (#952)
* Update DefaultOAuth2ApiService to support multiple token types and client secret without id * Address PR comments * Update TokenBroker interface to accept requestedTokenType * Add check for requested token type
1 parent 889ab7c commit 42af733

File tree

7 files changed

+457
-39
lines changed

7 files changed

+457
-39
lines changed

‎quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTRSAKeyPairTest.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.apache.polaris.service.auth.TokenBroker;
4242
import org.apache.polaris.service.auth.TokenRequestValidator;
4343
import org.apache.polaris.service.auth.TokenResponse;
44+
import org.apache.polaris.service.types.TokenType;
4445
import org.junit.jupiter.api.Test;
4546
import org.mockito.Mockito;
4647

@@ -76,7 +77,11 @@ public void testSuccessfulTokenGeneration() throws Exception {
7677
new JWTRSAKeyPair(metastoreManager, session, 420, publicFileLocation, privateFileLocation);
7778
TokenResponse token =
7879
tokenBroker.generateFromClientSecrets(
79-
clientId, mainSecret, TokenRequestValidator.CLIENT_CREDENTIALS, scope);
80+
clientId,
81+
mainSecret,
82+
TokenRequestValidator.CLIENT_CREDENTIALS,
83+
scope,
84+
TokenType.ACCESS_TOKEN);
8085
assertThat(token).isNotNull();
8186
assertThat(token.getExpiresIn()).isEqualTo(420);
8287

‎quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.polaris.service.auth.TokenBroker;
3535
import org.apache.polaris.service.auth.TokenRequestValidator;
3636
import org.apache.polaris.service.auth.TokenResponse;
37+
import org.apache.polaris.service.types.TokenType;
3738
import org.junit.jupiter.api.Test;
3839
import org.mockito.Mockito;
3940

@@ -64,7 +65,11 @@ public void testJWTSymmetricKeyGenerator() {
6465
new JWTSymmetricKeyBroker(metastoreManager, metaStoreSession, 666, () -> "polaris");
6566
TokenResponse token =
6667
generator.generateFromClientSecrets(
67-
clientId, mainSecret, TokenRequestValidator.CLIENT_CREDENTIALS, "PRINCIPAL_ROLE:TEST");
68+
clientId,
69+
mainSecret,
70+
TokenRequestValidator.CLIENT_CREDENTIALS,
71+
"PRINCIPAL_ROLE:TEST",
72+
TokenType.ACCESS_TOKEN);
6873
assertThat(token).isNotNull();
6974

7075
JWTVerifier verifier = JWT.require(Algorithm.HMAC256("polaris")).withIssuer("polaris").build();

‎service/common/src/main/java/org/apache/polaris/service/auth/DefaultOAuth2ApiService.java

+25-30
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ public class DefaultOAuth2ApiService implements IcebergRestOAuth2ApiService {
4343

4444
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultOAuth2ApiService.class);
4545

46-
private static final String CLIENT_CREDENTIALS = "client_credentials";
4746
private static final String BEARER = "bearer";
4847

4948
private final TokenBrokerFactory tokenBrokerFactory;
@@ -75,43 +74,39 @@ public Response getToken(
7574
if (!tokenBroker.supportsRequestedTokenType(requestedTokenType)) {
7675
return OAuthUtils.getResponseFromError(OAuthTokenErrorResponse.Error.invalid_request);
7776
}
78-
if (authHeader == null && clientId == null) {
77+
if (authHeader == null && clientSecret == null) {
7978
return OAuthUtils.getResponseFromError(OAuthTokenErrorResponse.Error.invalid_client);
8079
}
81-
if (authHeader != null && clientId == null && authHeader.startsWith("Basic ")) {
80+
// token exchange with client id and client secret in the authorization header means the client
81+
// has previously attempted to refresh an access token, but refreshing was not supported by the
82+
// token broker. Accept the client id and secret and treat it as a new token request
83+
if (authHeader != null && clientSecret == null && authHeader.startsWith("Basic ")) {
8284
String credentials = new String(Base64.decodeBase64(authHeader.substring(6)), UTF_8);
8385
if (!credentials.contains(":")) {
84-
return OAuthUtils.getResponseFromError(OAuthTokenErrorResponse.Error.invalid_client);
86+
return OAuthUtils.getResponseFromError(OAuthTokenErrorResponse.Error.invalid_request);
8587
}
8688
LOGGER.debug("Found credentials in auth header - treating as client_credentials");
8789
String[] parts = credentials.split(":", 2);
88-
clientId = parts[0];
89-
clientSecret = parts[1];
90+
if (parts.length == 2) {
91+
clientId = parts[0];
92+
clientSecret = parts[1];
93+
} else {
94+
LOGGER.debug("Don't know how to parse Basic auth header");
95+
return OAuthUtils.getResponseFromError(OAuthTokenErrorResponse.Error.invalid_request);
96+
}
97+
}
98+
TokenResponse tokenResponse;
99+
if (clientSecret != null) {
100+
tokenResponse =
101+
tokenBroker.generateFromClientSecrets(
102+
clientId, clientSecret, grantType, scope, requestedTokenType);
103+
} else if (subjectToken != null) {
104+
tokenResponse =
105+
tokenBroker.generateFromToken(
106+
subjectTokenType, subjectToken, grantType, scope, requestedTokenType);
107+
} else {
108+
return OAuthUtils.getResponseFromError(OAuthTokenErrorResponse.Error.invalid_request);
90109
}
91-
TokenResponse tokenResponse =
92-
switch (subjectTokenType) {
93-
case TokenType.ID_TOKEN,
94-
TokenType.REFRESH_TOKEN,
95-
TokenType.JWT,
96-
TokenType.SAML1,
97-
TokenType.SAML2 ->
98-
new TokenResponse(OAuthTokenErrorResponse.Error.invalid_request);
99-
case TokenType.ACCESS_TOKEN -> {
100-
// token exchange with client id and client secret means the client has previously
101-
// attempted to refresh
102-
// an access token, but refreshing was not supported by the token broker. Accept the
103-
// client id and
104-
// secret and treat it as a new token request
105-
if (clientId != null && clientSecret != null) {
106-
yield tokenBroker.generateFromClientSecrets(
107-
clientId, clientSecret, CLIENT_CREDENTIALS, scope);
108-
} else {
109-
yield tokenBroker.generateFromToken(subjectTokenType, subjectToken, grantType, scope);
110-
}
111-
}
112-
case null ->
113-
tokenBroker.generateFromClientSecrets(clientId, clientSecret, grantType, scope);
114-
};
115110
if (tokenResponse == null) {
116111
return OAuthUtils.getResponseFromError(OAuthTokenErrorResponse.Error.unsupported_grant_type);
117112
}

‎service/common/src/main/java/org/apache/polaris/service/auth/JWTBroker.java

+14-3
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,15 @@ public String getScope() {
9898

9999
@Override
100100
public TokenResponse generateFromToken(
101-
TokenType tokenType, String subjectToken, String grantType, String scope) {
102-
if (!TokenType.ACCESS_TOKEN.equals(tokenType)) {
101+
TokenType subjectTokenType,
102+
String subjectToken,
103+
String grantType,
104+
String scope,
105+
TokenType requestedTokenType) {
106+
if (!TokenType.ACCESS_TOKEN.equals(requestedTokenType)) {
107+
return new TokenResponse(OAuthTokenErrorResponse.Error.invalid_request);
108+
}
109+
if (!TokenType.ACCESS_TOKEN.equals(subjectTokenType)) {
103110
return new TokenResponse(OAuthTokenErrorResponse.Error.invalid_request);
104111
}
105112
if (StringUtils.isBlank(subjectToken)) {
@@ -121,7 +128,11 @@ public TokenResponse generateFromToken(
121128

122129
@Override
123130
public TokenResponse generateFromClientSecrets(
124-
String clientId, String clientSecret, String grantType, String scope) {
131+
String clientId,
132+
String clientSecret,
133+
String grantType,
134+
String scope,
135+
TokenType requestedTokenType) {
125136
// Initial sanity checks
126137
TokenRequestValidator validator = new TokenRequestValidator();
127138
Optional<OAuthTokenErrorResponse.Error> initialValidationResponse =

‎service/common/src/main/java/org/apache/polaris/service/auth/NoneTokenBrokerFactory.java

+10-2
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,21 @@ public boolean supportsRequestedTokenType(TokenType tokenType) {
4242

4343
@Override
4444
public TokenResponse generateFromClientSecrets(
45-
String clientId, String clientSecret, String grantType, String scope) {
45+
String clientId,
46+
String clientSecret,
47+
String grantType,
48+
String scope,
49+
TokenType requestedTokenType) {
4650
return null;
4751
}
4852

4953
@Override
5054
public TokenResponse generateFromToken(
51-
TokenType tokenType, String subjectToken, String grantType, String scope) {
55+
TokenType subjectTokenType,
56+
String subjectToken,
57+
String grantType,
58+
String scope,
59+
TokenType requestedTokenType) {
5260
return null;
5361
}
5462

‎service/common/src/main/java/org/apache/polaris/service/auth/TokenBroker.java

+68-2
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,77 @@ public interface TokenBroker {
3434

3535
boolean supportsRequestedTokenType(TokenType tokenType);
3636

37+
/**
38+
* Generate a token from client secrets without specifying the requested token type
39+
*
40+
* @param clientId
41+
* @param clientSecret
42+
* @param grantType
43+
* @param scope
44+
* @return the response indicating an error or the requested token
45+
* @deprecated - use the method with the requested token type
46+
*/
47+
@Deprecated
48+
default TokenResponse generateFromClientSecrets(
49+
final String clientId,
50+
final String clientSecret,
51+
final String grantType,
52+
final String scope) {
53+
return generateFromClientSecrets(
54+
clientId, clientSecret, grantType, scope, TokenType.ACCESS_TOKEN);
55+
}
56+
57+
/**
58+
* Generate a token from client secrets
59+
*
60+
* @param clientId
61+
* @param clientSecret
62+
* @param grantType
63+
* @param scope
64+
* @param requestedTokenType
65+
* @return the response indicating an error or the requested token
66+
*/
3767
TokenResponse generateFromClientSecrets(
38-
final String clientId, final String clientSecret, final String grantType, final String scope);
68+
final String clientId,
69+
final String clientSecret,
70+
final String grantType,
71+
final String scope,
72+
TokenType requestedTokenType);
73+
74+
/**
75+
* Generate a token from an existing token of a specified type without specifying the requested
76+
* token type
77+
*
78+
* @param subjectTokenType
79+
* @param subjectToken
80+
* @param grantType
81+
* @param scope
82+
* @return the response indicating an error or the requested token
83+
* @deprecated - use the method with the requested token type
84+
*/
85+
@Deprecated
86+
default TokenResponse generateFromToken(
87+
TokenType subjectTokenType, String subjectToken, final String grantType, final String scope) {
88+
return generateFromToken(
89+
subjectTokenType, subjectToken, grantType, scope, TokenType.ACCESS_TOKEN);
90+
}
3991

92+
/**
93+
* Generate a token from an existing token of a specified type
94+
*
95+
* @param subjectTokenType
96+
* @param subjectToken
97+
* @param grantType
98+
* @param scope
99+
* @param requestedTokenType
100+
* @return the response indicating an error or the requested token
101+
*/
40102
TokenResponse generateFromToken(
41-
TokenType tokenType, String subjectToken, final String grantType, final String scope);
103+
TokenType subjectTokenType,
104+
String subjectToken,
105+
final String grantType,
106+
final String scope,
107+
TokenType requestedTokenType);
42108

43109
DecodedToken verify(String token);
44110

0 commit comments

Comments
 (0)