rdblue commented on code in PR #6489:
URL: https://github.com/apache/iceberg/pull/6489#discussion_r1065296347
##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java:
##########
@@ -376,5 +404,127 @@ public Pair<Integer, TimeUnit> refresh(RESTClient client)
{
return null;
}
+
+ @SuppressWarnings("FutureReturnValueIgnored")
+ public static void scheduleTokenRefresh(
+ RESTClient client,
+ ScheduledExecutorService tokenRefreshExecutor,
+ AuthSession session,
+ long startTimeMillis,
+ long expiresIn,
+ TimeUnit unit) {
+ // convert expiration interval to milliseconds
+ long expiresInMillis = unit.toMillis(expiresIn);
+ // how much ahead of time to start the request to allow it to complete
+ long refreshWindowMillis = Math.min(expiresInMillis / 10,
MAX_REFRESH_WINDOW_MILLIS);
+ // how much time to wait before expiration
+ long waitIntervalMillis = expiresInMillis - refreshWindowMillis;
+ // how much time has already elapsed since the new token was issued
+ long elapsedMillis = System.currentTimeMillis() - startTimeMillis;
+ // how much time to actually wait
+ long timeToWait = Math.max(waitIntervalMillis - elapsedMillis,
MIN_REFRESH_WAIT_MILLIS);
+
+ tokenRefreshExecutor.schedule(
+ () -> {
+ long refreshStartTime = System.currentTimeMillis();
+ Pair<Integer, TimeUnit> expiration = session.refresh(client);
+ if (expiration != null) {
+ scheduleTokenRefresh(
+ client,
+ tokenRefreshExecutor,
+ session,
+ refreshStartTime,
+ expiration.first(),
+ expiration.second());
+ }
+ },
+ timeToWait,
+ TimeUnit.MILLISECONDS);
+ }
+
+ public static AuthSession sessionFromToken(
+ RESTClient client,
+ ScheduledExecutorService tokenRefreshExecutor,
+ String token,
+ @Nullable String credential,
+ Long defaultExpirationMillis,
+ AuthSession parent) {
+ Optional<JWT> jwt = JWT.of(token);
+
+ if (jwt.isPresent() && jwt.get().isExpired()) {
+ Preconditions.checkState(
+ null != credential, "Credential is required to refresh expired
token.");
+
+ // we add the credential to the Authorization header and perform a
token exchange to
+ // refresh the expired token
+ AuthSession session = new
AuthSession(OAuth2Util.basicAuthHeader(credential), null, null);
+
+ return AuthSession.sessionFromTokenExchange(
+ client,
+ tokenRefreshExecutor,
+ token,
+ OAuth2Properties.ACCESS_TOKEN_TYPE,
+ session,
+ OAuth2Properties.CATALOG_SCOPE);
+ } else {
+ AuthSession session =
+ new AuthSession(parent.headers(), token,
OAuth2Properties.ACCESS_TOKEN_TYPE);
+ Long expiresInMillis =
jwt.map(JWT::expiresInMillis).orElse(defaultExpirationMillis);
+
+ if (null != expiresInMillis) {
+ scheduleTokenRefresh(
+ client,
+ tokenRefreshExecutor,
+ session,
+ System.currentTimeMillis(),
+ expiresInMillis,
Review Comment:
I think this should pass in the expiration time directly, rather than
converting to an interval and passing that. The existing refresh method is
based on `expiresIn` because that's what we get back from the request, but if
we have a known timestamp instead it is more accurate to use it.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
