Core: Introduce AuthConfig #10161
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| /* | ||
| * 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. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package org.apache.iceberg.rest.auth; | ||
|
|
||
| import java.util.Map; | ||
| import javax.annotation.Nullable; | ||
| import org.apache.iceberg.rest.ResourcePaths; | ||
| import org.immutables.value.Value; | ||
|
|
||
| /** | ||
| * The purpose of this class is to hold configuration options for {@link | ||
| * org.apache.iceberg.rest.auth.OAuth2Util.AuthSession}. | ||
| */ | ||
| @Value.Style(redactedMask = "****") | ||
| @SuppressWarnings("ImmutablesStyle") | ||
| @Value.Immutable | ||
| public interface AuthConfig { | ||
nastra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| @Nullable | ||
| @Value.Redacted | ||
| String token(); | ||
|
|
||
| @Nullable | ||
| String tokenType(); | ||
|
|
||
| @Nullable | ||
| @Value.Redacted | ||
| String credential(); | ||
|
|
||
| @Value.Default | ||
| default String scope() { | ||
| return OAuth2Properties.CATALOG_SCOPE; | ||
| } | ||
|
|
||
| @Value.Lazy | ||
| @Nullable | ||
| default Long expiresAtMillis() { | ||
| return OAuth2Util.expiresAtMillis(token()); | ||
| } | ||
|
|
||
| @Value.Default | ||
| default boolean keepRefreshed() { | ||
| return true; | ||
| } | ||
|
|
||
| @Nullable | ||
| @Value.Default | ||
| default String oauth2ServerUri() { | ||
| return ResourcePaths.tokens(); | ||
| } | ||
|
|
||
| Map<String, String> optionalOAuthParams(); | ||
|
|
||
| static ImmutableAuthConfig.Builder builder() { | ||
| return ImmutableAuthConfig.builder(); | ||
| } | ||
| } | ||
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 |
|---|---|---|
|
|
@@ -458,32 +458,11 @@ public static class AuthSession { | |
| private static final long MAX_REFRESH_WINDOW_MILLIS = 300_000; // 5 minutes | ||
| private static final long MIN_REFRESH_WAIT_MILLIS = 10; | ||
| private volatile Map<String, String> headers; | ||
| private volatile AuthConfig config; | ||
|
|
||
| public AuthSession(Map<String, String> baseHeaders, AuthConfig config) { | ||
| this.headers = RESTUtil.merge(baseHeaders, authHeaders(config.token())); | ||
| this.config = config; | ||
|
Comment on lines
-471
to
+465
Contributor
There was a problem hiding this comment. Since it's public, unfortunately I don't think we can remove this constructor just yet, I think we'll have to deprecate like the other constructors.
Contributor
Author
There was a problem hiding this comment. @amogh-jahagirdar that constructor was introduced with 5f655a3 which didn't make it into any release and only exists on |
||
| } | ||
|
|
||
| /** @deprecated since 1.5.0, will be removed in 1.6.0 */ | ||
|
|
@@ -494,13 +473,14 @@ public AuthSession( | |
| String tokenType, | ||
| String credential, | ||
| String scope) { | ||
| this( | ||
| baseHeaders, | ||
| AuthConfig.builder() | ||
| .token(token) | ||
| .tokenType(tokenType) | ||
| .credential(credential) | ||
| .scope(scope) | ||
| .build()); | ||
| } | ||
|
|
||
| /** @deprecated since 1.6.0, will be removed in 1.7.0 */ | ||
|
|
@@ -512,50 +492,55 @@ public AuthSession( | |
| String credential, | ||
| String scope, | ||
| String oauth2ServerUri) { | ||
| this( | ||
| baseHeaders, | ||
| AuthConfig.builder() | ||
| .token(token) | ||
| .tokenType(tokenType) | ||
| .credential(credential) | ||
| .scope(scope) | ||
| .oauth2ServerUri(oauth2ServerUri) | ||
| .build()); | ||
| } | ||
|
|
||
| public Map<String, String> headers() { | ||
| return headers; | ||
| } | ||
|
|
||
| public String token() { | ||
| return config.token(); | ||
| } | ||
|
|
||
| public String tokenType() { | ||
| return config.tokenType(); | ||
| } | ||
|
|
||
| public Long expiresAtMillis() { | ||
| return config.expiresAtMillis(); | ||
| } | ||
|
|
||
| public String scope() { | ||
| return config.scope(); | ||
| } | ||
|
|
||
| public synchronized void stopRefreshing() { | ||
| this.config = ImmutableAuthConfig.copyOf(config).withKeepRefreshed(false); | ||
| } | ||
|
|
||
| public String credential() { | ||
| return config.credential(); | ||
| } | ||
|
|
||
| public String oauth2ServerUri() { | ||
| return config.oauth2ServerUri(); | ||
| } | ||
|
|
||
| public Map<String, String> optionalOAuthParams() { | ||
| return config.optionalOAuthParams(); | ||
| } | ||
|
|
||
| public AuthConfig config() { | ||
| return config; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
|
|
@@ -569,14 +554,7 @@ static void setTokenRefreshNumRetries(int retries) { | |
| * @return A new {@link AuthSession} with empty headers. | ||
| */ | ||
| public static AuthSession empty() { | ||
| return new AuthSession(ImmutableMap.of(), AuthConfig.builder().build()); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -586,7 +564,7 @@ public static AuthSession empty() { | |
| * @return interval to wait before calling refresh again, or null if no refresh is needed | ||
| */ | ||
| public Pair<Integer, TimeUnit> refresh(RESTClient client) { | ||
| if (token() != null && config.keepRefreshed()) { | ||
| AtomicReference<OAuthTokenResponse> ref = new AtomicReference<>(null); | ||
| boolean isSuccessful = | ||
| Tasks.foreach(ref) | ||
|
|
@@ -612,10 +590,13 @@ public Pair<Integer, TimeUnit> refresh(RESTClient client) { | |
| } | ||
|
|
||
| OAuthTokenResponse response = ref.get(); | ||
| this.config = | ||
| AuthConfig.builder() | ||
| .from(config()) | ||
| .token(response.token()) | ||
| .tokenType(response.issuedTokenType()) | ||
| .build(); | ||
| this.headers = RESTUtil.merge(headers, authHeaders(config.token())); | ||
|
|
||
| if (response.expiresInSeconds() != null) { | ||
| return Pair.of(response.expiresInSeconds(), TimeUnit.SECONDS); | ||
|
|
@@ -626,21 +607,34 @@ public Pair<Integer, TimeUnit> refresh(RESTClient client) { | |
| } | ||
|
|
||
| private OAuthTokenResponse refreshCurrentToken(RESTClient client) { | ||
| if (null != expiresAtMillis() && expiresAtMillis() <= System.currentTimeMillis()) { | ||
| // the token has already expired, attempt to refresh using the credential | ||
| return refreshExpiredToken(client); | ||
| } else { | ||
| // attempt a normal refresh | ||
| return refreshToken( | ||
| client, | ||
| headers(), | ||
| token(), | ||
| tokenType(), | ||
| scope(), | ||
| oauth2ServerUri(), | ||
| optionalOAuthParams()); | ||
| } | ||
| } | ||
|
|
||
| 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(), | ||
| optionalOAuthParams()); | ||
| } | ||
|
|
||
| return null; | ||
|
|
@@ -693,12 +687,11 @@ public static AuthSession fromAccessToken( | |
| AuthSession session = | ||
| new AuthSession( | ||
| parent.headers(), | ||
| AuthConfig.builder() | ||
| .from(parent.config()) | ||
| .token(token) | ||
| .tokenType(OAuth2Properties.ACCESS_TOKEN_TYPE) | ||
| .build()); | ||
|
|
||
| long startTimeMillis = System.currentTimeMillis(); | ||
| Long expiresAtMillis = session.expiresAtMillis(); | ||
|
|
@@ -766,12 +759,12 @@ private static AuthSession fromTokenResponse( | |
| AuthSession session = | ||
| new AuthSession( | ||
| parent.headers(), | ||
| AuthConfig.builder() | ||
| .from(parent.config()) | ||
| .token(response.token()) | ||
| .tokenType(response.issuedTokenType()) | ||
| .credential(credential) | ||
| .build()); | ||
|
|
||
| Long expiresAtMillis = session.expiresAtMillis(); | ||
| if (null == expiresAtMillis && response.expiresInSeconds() != null) { | ||
|
|
||
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.
I'd consider
SCOPEas one of the optional params. It is optional in both token exchange flow and client credential flow.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.
yes
SCOPEis optional, but the signer client uses a custom scope, which we need to provide hereThere 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.
We can get the scope from the optional OAuth parameters for signer. Also a rest catalog isn't necessary with signer. But I'm OK with either one.