Core: Support usage of Separate Authorization Server URI in Rest Catalog #8976
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,7 +134,8 @@ private static OAuthTokenResponse refreshToken( | |
| Map<String, String> headers, | ||
| String subjectToken, | ||
| String subjectTokenType, | ||
| String scope, | ||
| String oauth2ServerUri) { | ||
danielcweeks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Map<String, String> request = | ||
| tokenExchangeRequest( | ||
| subjectToken, | ||
|
|
@@ -143,7 +144,7 @@ private static OAuthTokenResponse refreshToken( | |
|
|
||
| OAuthTokenResponse response = | ||
| client.postForm( | ||
| oauth2ServerUri, | ||
| request, | ||
| OAuthTokenResponse.class, | ||
| headers, | ||
|
|
@@ -160,7 +161,8 @@ public static OAuthTokenResponse exchangeToken( | |
| String subjectTokenType, | ||
| String actorToken, | ||
| String actorTokenType, | ||
| String scope, | ||
| String oauth2ServerUri) { | ||
| Map<String, String> request = | ||
| tokenExchangeRequest( | ||
| subjectToken, | ||
|
|
@@ -171,7 +173,7 @@ public static OAuthTokenResponse exchangeToken( | |
|
|
||
| OAuthTokenResponse response = | ||
| client.postForm( | ||
| oauth2ServerUri, | ||
| request, | ||
| OAuthTokenResponse.class, | ||
| headers, | ||
|
|
@@ -181,15 +183,38 @@ public static OAuthTokenResponse exchangeToken( | |
| return response; | ||
| } | ||
|
|
||
| public static OAuthTokenResponse exchangeToken( | ||
| RESTClient client, | ||
| Map<String, String> headers, | ||
| String subjectToken, | ||
| String subjectTokenType, | ||
| String actorToken, | ||
| String actorTokenType, | ||
| String scope) { | ||
| return exchangeToken( | ||
| client, | ||
| headers, | ||
| subjectToken, | ||
| subjectTokenType, | ||
| actorToken, | ||
| actorTokenType, | ||
| scope, | ||
| ResourcePaths.tokens()); | ||
| } | ||
|
|
||
| public static OAuthTokenResponse fetchToken( | ||
| RESTClient client, | ||
| Map<String, String> headers, | ||
| String credential, | ||
| String scope, | ||
| String oauth2ServerUri) { | ||
| Map<String, String> request = | ||
| clientCredentialsRequest( | ||
| credential, scope != null ? ImmutableList.of(scope) : ImmutableList.of()); | ||
|
|
||
| OAuthTokenResponse response = | ||
| client.postForm( | ||
| oauth2ServerUri, | ||
| request, | ||
| OAuthTokenResponse.class, | ||
| headers, | ||
|
|
@@ -199,6 +224,12 @@ public static OAuthTokenResponse fetchToken( | |
| return response; | ||
| } | ||
|
|
||
| public static OAuthTokenResponse fetchToken( | ||
| RESTClient client, Map<String, String> headers, String credential, String scope) { | ||
|
|
||
| return fetchToken(client, headers, credential, scope, ResourcePaths.tokens()); | ||
| } | ||
|
|
||
| private static Map<String, String> tokenExchangeRequest( | ||
| String subjectToken, String subjectTokenType, List<String> scopes) { | ||
| return tokenExchangeRequest(subjectToken, subjectTokenType, null, null, scopes); | ||
|
|
@@ -361,7 +392,26 @@ public static class AuthSession { | |
| private final String credential; | ||
| private final String scope; | ||
| private volatile boolean keepRefreshed = true; | ||
| private final String oauth2ServerUri; | ||
|
|
||
| public AuthSession( | ||
danielcweeks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Map<String, String> baseHeaders, | ||
| String token, | ||
| String tokenType, | ||
| String credential, | ||
| String scope, | ||
| String oauth2ServerUri) { | ||
| this.headers = RESTUtil.merge(baseHeaders, authHeaders(token)); | ||
| this.token = token; | ||
| this.tokenType = tokenType; | ||
| this.expiresAtMillis = OAuth2Util.expiresAtMillis(token); | ||
| this.credential = credential; | ||
| this.scope = scope; | ||
| this.oauth2ServerUri = oauth2ServerUri; | ||
| } | ||
|
|
||
| /** @deprecated since 1.5.0, will be removed in 1.6.0 */ | ||
| @Deprecated | ||
| public AuthSession( | ||
|
Contributor
There was a problem hiding this comment. Looks like it is not used anywhere. Should we deprecate it? If that's so, can we add deprecation comment?
Contributor
Author
There was a problem hiding this comment. I noticed that too. I lack the context to make that decision, so I will leave this comment thread for others to opine
Contributor
There was a problem hiding this comment. It doesn't appear to be referenced anymore so you should add a deprecation warning and message (see deprecation docs) |
||
| Map<String, String> baseHeaders, | ||
| String token, | ||
|
|
@@ -374,6 +424,7 @@ public AuthSession( | |
| this.expiresAtMillis = OAuth2Util.expiresAtMillis(token); | ||
| this.credential = credential; | ||
| this.scope = scope; | ||
| this.oauth2ServerUri = ResourcePaths.tokens(); | ||
| } | ||
|
|
||
| public Map<String, String> headers() { | ||
|
|
@@ -404,6 +455,10 @@ public String credential() { | |
| return credential; | ||
| } | ||
|
|
||
| public String oauth2ServerUri() { | ||
| return oauth2ServerUri; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| static void setTokenRefreshNumRetries(int retries) { | ||
| tokenRefreshNumRetries = retries; | ||
|
|
@@ -415,7 +470,8 @@ static void setTokenRefreshNumRetries(int retries) { | |
| * @return A new {@link AuthSession} with empty headers. | ||
| */ | ||
| public static AuthSession empty() { | ||
| return new AuthSession( | ||
| ImmutableMap.of(), null, null, null, OAuth2Properties.CATALOG_SCOPE, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -470,14 +526,14 @@ private OAuthTokenResponse refreshCurrentToken(RESTClient client) { | |
| return refreshExpiredToken(client); | ||
| } else { | ||
| // attempt a normal refresh | ||
| return refreshToken(client, headers(), token, tokenType, scope, oauth2ServerUri); | ||
| } | ||
| } | ||
|
|
||
| private OAuthTokenResponse refreshExpiredToken(RESTClient client) { | ||
| if (credential != null) { | ||
| Map<String, String> basicHeaders = RESTUtil.merge(headers(), basicAuthHeaders(credential)); | ||
| return refreshToken(client, basicHeaders, token, tokenType, scope, oauth2ServerUri); | ||
| } | ||
|
|
||
| return null; | ||
|
|
@@ -533,7 +589,8 @@ public static AuthSession fromAccessToken( | |
| token, | ||
| OAuth2Properties.ACCESS_TOKEN_TYPE, | ||
| parent.credential(), | ||
| parent.scope(), | ||
| parent.oauth2ServerUri()); | ||
|
|
||
| long startTimeMillis = System.currentTimeMillis(); | ||
| Long expiresAtMillis = session.expiresAtMillis(); | ||
|
|
@@ -571,7 +628,8 @@ public static AuthSession fromCredential( | |
| AuthSession parent) { | ||
| long startTimeMillis = System.currentTimeMillis(); | ||
| OAuthTokenResponse response = | ||
| fetchToken( | ||
| client, parent.headers(), credential, parent.scope(), parent.oauth2ServerUri()); | ||
| return fromTokenResponse(client, executor, response, startTimeMillis, parent, credential); | ||
| } | ||
|
|
||
|
|
@@ -598,7 +656,8 @@ private static AuthSession fromTokenResponse( | |
| response.token(), | ||
| response.issuedTokenType(), | ||
| credential, | ||
| parent.scope(), | ||
| parent.oauth2ServerUri()); | ||
|
|
||
| Long expiresAtMillis = session.expiresAtMillis(); | ||
| if (null == expiresAtMillis && response.expiresInSeconds() != null) { | ||
|
|
@@ -627,7 +686,8 @@ public static AuthSession fromTokenExchange( | |
| tokenType, | ||
| parent.token(), | ||
| parent.tokenType(), | ||
| parent.scope(), | ||
| parent.oauth2ServerUri()); | ||
| return fromTokenResponse(client, executor, response, startTimeMillis, parent); | ||
| } | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Token endpoint URI" -> "OAuth Server URI" or just "Endpoint"?