snazy commented on code in PR #2680: URL: https://github.com/apache/polaris/pull/2680#discussion_r2451830502
########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/token/FileBearerTokenProvider.java: ########## @@ -0,0 +1,223 @@ +/* + * 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.polaris.extension.auth.opa.token; + +import static com.google.common.base.Preconditions.checkState; + +import com.auth0.jwt.JWT; +import com.auth0.jwt.exceptions.JWTDecodeException; +import com.auth0.jwt.interfaces.DecodedJWT; +import jakarta.annotation.Nullable; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.util.Date; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A token provider that reads tokens from a file and automatically reloads them based on a + * configurable refresh interval or JWT expiration timing. + * + * <p>This is particularly useful in Kubernetes environments where tokens are mounted as files and + * refreshed by external systems (e.g., service account tokens, projected volumes, etc.). + * + * <p>The token file is expected to contain the bearer token as plain text. Leading and trailing + * whitespace will be trimmed. + * + * <p>If JWT expiration refresh is enabled and the token is a valid JWT with an 'exp' claim, the + * provider will automatically refresh the token based on the expiration time minus a configurable + * buffer, rather than using the fixed refresh interval. + */ +public class FileBearerTokenProvider implements BearerTokenProvider { + + private static final Logger logger = LoggerFactory.getLogger(FileBearerTokenProvider.class); + + private final Path tokenFilePath; + private final Duration refreshInterval; + private final boolean jwtExpirationRefresh; + private final Duration jwtExpirationBuffer; + private final Clock clock; + private final AtomicBoolean refreshLock = new AtomicBoolean(); + + private volatile String cachedToken; + private volatile Instant lastRefresh; + private volatile Instant nextRefresh; + private volatile boolean closed = false; + + /** + * Create a new file-based token provider with JWT expiration support. + * + * @param tokenFilePath path to the file containing the bearer token + * @param refreshInterval how often to check for token file changes (fallback for non-JWT tokens) + * @param jwtExpirationRefresh whether to use JWT expiration for refresh timing + * @param jwtExpirationBuffer buffer time before JWT expiration to refresh the token + * @param clock clock instance for time operations + */ + public FileBearerTokenProvider( + Path tokenFilePath, + Duration refreshInterval, + boolean jwtExpirationRefresh, + Duration jwtExpirationBuffer, + Clock clock) { + this.tokenFilePath = tokenFilePath; + this.refreshInterval = refreshInterval; + this.jwtExpirationRefresh = jwtExpirationRefresh; + this.jwtExpirationBuffer = jwtExpirationBuffer; + this.clock = clock; + this.lastRefresh = Instant.MIN; // Force initial load + this.nextRefresh = Instant.MIN; // Force initial refresh + + logger.debug( + "Created file token provider for path: {} with refresh interval: {}, JWT expiration refresh: {}, JWT buffer: {}", + tokenFilePath, + refreshInterval, + jwtExpirationRefresh, + jwtExpirationBuffer); + } + + @Override + @Nullable + public String getToken() { + checkState(!closed, "Token provider is closed"); + + // Check if we need to refresh + if (shouldRefresh()) { + refreshToken(); + } + + return cachedToken; + } + + @Override + public void close() { + closed = true; + cachedToken = null; + } + + private boolean shouldRefresh() { + return clock.instant().isAfter(nextRefresh); + } + + private void refreshToken() { + if (!refreshLock.compareAndSet(false, true)) { + return; + } + try { + String newToken = loadTokenFromFile(); + + // If we couldn't load a token and have no cached token, this is a fatal error + if (newToken == null && cachedToken == null) { + throw new RuntimeException( + "Unable to load bearer token from file: " + + tokenFilePath + + ". This is required for OPA authorization."); + } + + // Only update cached token if we successfully loaded a new one + if (newToken != null) { + cachedToken = newToken; + } + // If newToken is null but cachedToken exists, we keep using the cached token + + lastRefresh = clock.instant(); + + // Calculate next refresh time based on current token (may be cached) + nextRefresh = calculateNextRefresh(cachedToken); + + logger.debug( + "Token refreshed from file: {} (token present: {}), next refresh: {}", + tokenFilePath, + cachedToken != null && !cachedToken.isEmpty(), + nextRefresh); + } finally { + refreshLock.set(false); + } + } + + /** Calculate when the next refresh should occur based on JWT expiration or fixed interval. */ + private Instant calculateNextRefresh(@Nullable String token) { + if (token == null || !jwtExpirationRefresh) { Review Comment: With the above suggestion, `token` can never be `null`. ```suggestion private Instant calculateNextRefresh(String token) { if (!jwtExpirationRefresh) { ``` ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/token/FileBearerTokenProvider.java: ########## @@ -0,0 +1,223 @@ +/* + * 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.polaris.extension.auth.opa.token; + +import static com.google.common.base.Preconditions.checkState; + +import com.auth0.jwt.JWT; +import com.auth0.jwt.exceptions.JWTDecodeException; +import com.auth0.jwt.interfaces.DecodedJWT; +import jakarta.annotation.Nullable; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.util.Date; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A token provider that reads tokens from a file and automatically reloads them based on a + * configurable refresh interval or JWT expiration timing. + * + * <p>This is particularly useful in Kubernetes environments where tokens are mounted as files and + * refreshed by external systems (e.g., service account tokens, projected volumes, etc.). + * + * <p>The token file is expected to contain the bearer token as plain text. Leading and trailing + * whitespace will be trimmed. + * + * <p>If JWT expiration refresh is enabled and the token is a valid JWT with an 'exp' claim, the + * provider will automatically refresh the token based on the expiration time minus a configurable + * buffer, rather than using the fixed refresh interval. + */ +public class FileBearerTokenProvider implements BearerTokenProvider { + + private static final Logger logger = LoggerFactory.getLogger(FileBearerTokenProvider.class); + + private final Path tokenFilePath; + private final Duration refreshInterval; + private final boolean jwtExpirationRefresh; + private final Duration jwtExpirationBuffer; + private final Clock clock; + private final AtomicBoolean refreshLock = new AtomicBoolean(); + + private volatile String cachedToken; + private volatile Instant lastRefresh; + private volatile Instant nextRefresh; + private volatile boolean closed = false; + + /** + * Create a new file-based token provider with JWT expiration support. + * + * @param tokenFilePath path to the file containing the bearer token + * @param refreshInterval how often to check for token file changes (fallback for non-JWT tokens) + * @param jwtExpirationRefresh whether to use JWT expiration for refresh timing + * @param jwtExpirationBuffer buffer time before JWT expiration to refresh the token + * @param clock clock instance for time operations + */ + public FileBearerTokenProvider( + Path tokenFilePath, + Duration refreshInterval, + boolean jwtExpirationRefresh, + Duration jwtExpirationBuffer, + Clock clock) { + this.tokenFilePath = tokenFilePath; + this.refreshInterval = refreshInterval; + this.jwtExpirationRefresh = jwtExpirationRefresh; + this.jwtExpirationBuffer = jwtExpirationBuffer; + this.clock = clock; + this.lastRefresh = Instant.MIN; // Force initial load + this.nextRefresh = Instant.MIN; // Force initial refresh + + logger.debug( + "Created file token provider for path: {} with refresh interval: {}, JWT expiration refresh: {}, JWT buffer: {}", + tokenFilePath, + refreshInterval, + jwtExpirationRefresh, + jwtExpirationBuffer); + } + + @Override + @Nullable + public String getToken() { + checkState(!closed, "Token provider is closed"); + + // Check if we need to refresh + if (shouldRefresh()) { + refreshToken(); + } + + return cachedToken; + } + + @Override + public void close() { + closed = true; + cachedToken = null; + } + + private boolean shouldRefresh() { + return clock.instant().isAfter(nextRefresh); + } + + private void refreshToken() { + if (!refreshLock.compareAndSet(false, true)) { + return; + } + try { + String newToken = loadTokenFromFile(); + + // If we couldn't load a token and have no cached token, this is a fatal error + if (newToken == null && cachedToken == null) { + throw new RuntimeException( + "Unable to load bearer token from file: " + + tokenFilePath + + ". This is required for OPA authorization."); + } + + // Only update cached token if we successfully loaded a new one + if (newToken != null) { + cachedToken = newToken; + } Review Comment: Seems we need to shortcut here, otherwise we will retry too late. ```suggestion // Only update cached token if we successfully loaded a new one if (newToken == null) { logger.debug("Couldn't load new bearer token from {}, will retry.", tokenFilePath); return; } cachedToken = newToken; ``` -- 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]
