snazy commented on code in PR #2680: URL: https://github.com/apache/polaris/pull/2680#discussion_r2444039255
########## extensions/auth/opa/impl/src/test/java/org/apache/polaris/extension/auth/opa/token/StaticBearerTokenProviderTest.java: ########## @@ -0,0 +1,48 @@ +/* + * 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 org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +public class StaticBearerTokenProviderTest { + + @Test + public void testStaticBearerTokenProvider() { + String expectedToken = "static-bearer-token"; + StaticBearerTokenProvider provider = new StaticBearerTokenProvider(expectedToken); Review Comment: Mind add a try-with-resources in this test class as well? IntelliJ shows warnings because of unclosed resources here. ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaAuthorizationConfig.java: ########## @@ -0,0 +1,201 @@ +/* + * 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; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.common.base.Strings; +import io.smallrye.config.ConfigMapping; +import io.smallrye.config.WithDefault; +import java.net.URI; +import java.nio.file.Path; +import java.time.Duration; +import java.util.Optional; + +/** + * Configuration for OPA (Open Policy Agent) authorization. + * + * <p><strong>Beta Feature:</strong> OPA authorization is currently in Beta and is not a stable + * release. It may undergo breaking changes in future versions. Use with caution in production + * environments. + */ +@ConfigMapping(prefix = "polaris.authorization.opa") +public interface OpaAuthorizationConfig { + + /** Authentication types supported by OPA authorization */ + enum AuthenticationType { + NONE("none"), + BEARER("bearer"); + + private final String value; + + AuthenticationType(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } + + /** Bearer token configuration types */ + enum BearerTokenType { + STATIC_TOKEN("static-token"), + FILE_BASED("file-based"); + + private final String value; + + BearerTokenType(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } + + Optional<URI> policyUri(); + + Optional<AuthenticationConfig> auth(); + + Optional<HttpConfig> http(); Review Comment: These three properties don't need to be `Optional`s (simplifies the code quite a bit, smallrye-config/Quarkus would emit relevant startup errors for missing mandatory properties). ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/token/FileBearerTokenProvider.java: ########## @@ -0,0 +1,307 @@ +/* + * 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 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.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +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 ReadWriteLock lock = new ReentrantReadWriteLock(); + + 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 + */ + public FileBearerTokenProvider( + Path tokenFilePath, + Duration refreshInterval, + boolean jwtExpirationRefresh, + Duration jwtExpirationBuffer) { + this( + tokenFilePath, + refreshInterval, + jwtExpirationRefresh, + jwtExpirationBuffer, + Clock.systemUTC()); + } + + /** + * Create a new file-based token provider with JWT expiration support and custom clock. + * Package-private constructor for testing purposes. + * + * @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 (useful for testing) + */ + 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() { + if (closed) { + logger.warn("Token provider is closed, returning null"); + return null; + } + + // Check if we need to refresh + if (shouldRefresh()) { + refreshToken(); + } + + lock.readLock().lock(); + try { + return cachedToken; + } finally { + lock.readLock().unlock(); + } + } + + @Override + public void close() { + closed = true; + lock.writeLock().lock(); + try { + cachedToken = null; + logger.info("File token provider closed"); + } finally { + lock.writeLock().unlock(); + } + } + + private boolean shouldRefresh() { + return clock.instant().isAfter(nextRefresh); + } + + private void refreshToken() { + lock.writeLock().lock(); Review Comment: Nit, with all other `lock` usages removed, it can be simplified by changing ```java private final ReadWriteLock lock = new ReentrantReadWriteLock(); ``` to ```java private final AtomicBoolean refreshLock = new AtomicBoolean(); ``` and this in this function: ```java if (!refreshLock.compareAndSet(false, true)) { return; } try { // refresh code } finally { refreshLock.set(false); } ``` ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/token/FileBearerTokenProvider.java: ########## @@ -0,0 +1,307 @@ +/* + * 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 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.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +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 ReadWriteLock lock = new ReentrantReadWriteLock(); + + 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 + */ + public FileBearerTokenProvider( + Path tokenFilePath, + Duration refreshInterval, + boolean jwtExpirationRefresh, + Duration jwtExpirationBuffer) { + this( + tokenFilePath, + refreshInterval, + jwtExpirationRefresh, + jwtExpirationBuffer, + Clock.systemUTC()); + } + + /** + * Create a new file-based token provider with JWT expiration support and custom clock. + * Package-private constructor for testing purposes. + * + * @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 (useful for testing) + */ + 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() { + if (closed) { + logger.warn("Token provider is closed, returning null"); + return null; + } + + // Check if we need to refresh + if (shouldRefresh()) { + refreshToken(); + } + + lock.readLock().lock(); + try { + return cachedToken; + } finally { + lock.readLock().unlock(); + } + } + + @Override + public void close() { + closed = true; + lock.writeLock().lock(); + try { + cachedToken = null; + logger.info("File token provider closed"); + } finally { + lock.writeLock().unlock(); + } Review Comment: ```suggestion cachedToken = null; ``` ########## extensions/auth/opa/tests/src/intTest/java/org/apache/polaris/extension/auth/opa/OpaTestResource.java: ########## @@ -0,0 +1,137 @@ +/* + * 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; + +import io.quarkus.test.common.QuarkusTestResourceLifecycleManager; +import java.io.OutputStream; +import java.net.HttpURLConnection; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; +import org.apache.polaris.containerspec.ContainerSpecHelper; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.Wait; + +public class OpaTestResource implements QuarkusTestResourceLifecycleManager { + private static GenericContainer<?> opa; + private int mappedPort; + private Map<String, String> resourceConfig; Review Comment: Nit: unused (assigned, but not read) ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/token/FileBearerTokenProvider.java: ########## @@ -0,0 +1,307 @@ +/* + * 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 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.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +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 ReadWriteLock lock = new ReentrantReadWriteLock(); + + 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 + */ + public FileBearerTokenProvider( + Path tokenFilePath, + Duration refreshInterval, + boolean jwtExpirationRefresh, + Duration jwtExpirationBuffer) { + this( + tokenFilePath, + refreshInterval, + jwtExpirationRefresh, + jwtExpirationBuffer, + Clock.systemUTC()); + } + + /** + * Create a new file-based token provider with JWT expiration support and custom clock. + * Package-private constructor for testing purposes. + * + * @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 (useful for testing) + */ + 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() { + if (closed) { + logger.warn("Token provider is closed, returning null"); + return null; + } + + // Check if we need to refresh + if (shouldRefresh()) { + refreshToken(); + } + + lock.readLock().lock(); + try { + return cachedToken; + } finally { + lock.readLock().unlock(); + } + } + + @Override + public void close() { + closed = true; + lock.writeLock().lock(); + try { + cachedToken = null; + logger.info("File token provider closed"); + } finally { + lock.writeLock().unlock(); + } + } + + private boolean shouldRefresh() { + return clock.instant().isAfter(nextRefresh); + } + + private void refreshToken() { + lock.writeLock().lock(); + try { + // Double-check pattern - another thread might have refreshed while we waited for the lock + if (!shouldRefresh()) { + return; + } + + 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 { + lock.writeLock().unlock(); + } + } + + /** Calculate when the next refresh should occur based on JWT expiration or fixed interval. */ + private Instant calculateNextRefresh(@Nullable String token) { + if (token == null || !jwtExpirationRefresh) { + // Use fixed interval + return lastRefresh.plus(refreshInterval); + } + + // Attempt to parse as JWT and extract expiration + Optional<Instant> expiration = getJwtExpirationTime(token); + + if (expiration.isPresent()) { + // Refresh before expiration minus buffer + Instant refreshTime = expiration.get().minus(jwtExpirationBuffer); + + // Ensure refresh time is in the future and not too soon (at least 1 second) + Instant minRefreshTime = clock.instant().plus(Duration.ofSeconds(1)); + if (refreshTime.isBefore(minRefreshTime)) { + logger.warn( + "JWT expires too soon ({}), using minimum refresh interval instead", expiration.get()); + return lastRefresh.plus(refreshInterval); + } + + logger.debug( + "Using JWT expiration-based refresh: token expires at {}, refreshing at {}", + expiration.get(), + refreshTime); + return refreshTime; + } + + // Fall back to fixed interval (token is not a valid JWT or has no expiration) + logger.debug("Token is not a valid JWT or has no expiration, using fixed refresh interval"); + return lastRefresh.plus(refreshInterval); + } + + @Nullable + private String loadTokenFromFile() { + int attempts = 0; + long deadlineMs = + clock.millis() + + 3000; // 3 second deadline for retries (reasonable for CSI driver file mounts) + String currentCachedToken = cachedToken; // Snapshot of current cache + + while (true) { + try { + // Check if file is readable first + if (!Files.isReadable(tokenFilePath)) { + // Treat as transient error and retry (could be CSI driver not ready yet) + throw new IOException("File is not readable: " + tokenFilePath); + } + + String token = Files.readString(tokenFilePath, StandardCharsets.UTF_8).trim(); + if (!token.isEmpty()) { + return token; + } + + // Empty token - treat as transient issue (could be file being written) + throw new IOException("File contains only whitespace: " + tokenFilePath); + } catch (IOException e) { + // Treat file system exceptions as transient (file being rotated, temporary unavailability) + logger.debug( + "Transient error reading token file (attempt {}): {}", attempts + 1, e.getMessage()); + } + + // If we have a cached token and it's safe to use, fall back to it + if (currentCachedToken != null && !currentCachedToken.isEmpty()) { + logger.warn( + "Failed to read new token from file after {} attempts, using cached token", + attempts + 1); + return currentCachedToken; + } + + // No cached token available and we can't read from file + attempts++; + if (attempts >= 5 || clock.millis() > deadlineMs) { + // Return null to let refreshToken() decide how to handle this based on cached token state + logger.debug("Token unavailable after {} attempts", attempts); + return null; + } + + // Exponential backoff: 100ms, 200ms, 400ms, 800ms, 1000ms (reasonable for CSI driver + // scenarios) + long delayMs = Math.min(100L << (attempts - 1), 1000); + try { + Thread.sleep(delayMs); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + logger.debug("Interrupted while reading token, returning null"); + return null; + } + } + } Review Comment: I think, the whole function can be simplified like this: ```suggestion private String loadTokenFromFile() { try { String token = Files.readString(tokenFilePath, StandardCharsets.UTF_8).trim(); if (!token.isEmpty()) { return token; } } catch (IOException e) { logger.debug("Failed to read token from file", e); } return null; } ``` ########## extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/OpaAuthorizationConfig.java: ########## @@ -0,0 +1,170 @@ +/* + * 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; + +import static com.google.common.base.Preconditions.checkArgument; + +import io.smallrye.config.ConfigMapping; +import io.smallrye.config.WithDefault; +import java.util.Optional; + +/** + * Configuration for OPA (Open Policy Agent) authorization. + * + * <p><strong>Beta Feature:</strong> OPA authorization is currently in Beta and is not a stable + * release. It may undergo breaking changes in future versions. Use with caution in production + * environments. + */ +@ConfigMapping(prefix = "polaris.authorization.opa") +public interface OpaAuthorizationConfig { + Optional<String> url(); + + Optional<String> policyPath(); + + Optional<AuthenticationConfig> auth(); + + Optional<HttpConfig> http(); + + /** Validates the complete OPA configuration */ + default void validate() { + checkArgument(url().isPresent() && !url().get().isBlank(), "OPA URL cannot be null or empty"); + checkArgument( + policyPath().isPresent() && !policyPath().get().isBlank(), + "OPA policy path cannot be null or empty"); + checkArgument(auth().isPresent(), "Authentication configuration is required"); + + auth().get().validate(); + } + + /** HTTP client configuration for OPA communication. */ + interface HttpConfig { + @WithDefault("2000") + int timeoutMs(); + + @WithDefault("true") + boolean verifySsl(); + + Optional<String> trustStorePath(); + + Optional<String> trustStorePassword(); + } + + /** Authentication configuration for OPA communication. */ + interface AuthenticationConfig { + /** Type of authentication */ + @WithDefault("none") + String type(); + + /** Bearer token authentication configuration */ + Optional<BearerTokenConfig> bearer(); + + default void validate() { + switch (type()) { + case "bearer": + checkArgument( + bearer().isPresent(), "Bearer configuration is required when type is 'bearer'"); + bearer().get().validate(); + break; + case "none": + // No authentication - nothing to validate + break; + default: + throw new IllegalArgumentException( + "Invalid authentication type: " + type() + ". Supported types: 'bearer', 'none'"); + } + } + } + + interface BearerTokenConfig { + /** Type of bearer token configuration */ + @WithDefault("static-token") + String type(); Review Comment: Hm, not really. Given ``` polaris.authorization.opa.auth.bearer.file-based.path=/path/to/token/file ``` `BearerTokenConfig.staticToken()` will be empty and `BearerTokenConfig.fileBased()` will not be empty. Feels a bit redundant to require a type property. WDYT? ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/token/FileBearerTokenProvider.java: ########## @@ -0,0 +1,307 @@ +/* + * 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 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.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +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 ReadWriteLock lock = new ReentrantReadWriteLock(); + + 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 + */ + public FileBearerTokenProvider( + Path tokenFilePath, + Duration refreshInterval, + boolean jwtExpirationRefresh, + Duration jwtExpirationBuffer) { + this( + tokenFilePath, + refreshInterval, + jwtExpirationRefresh, + jwtExpirationBuffer, + Clock.systemUTC()); + } + + /** + * Create a new file-based token provider with JWT expiration support and custom clock. + * Package-private constructor for testing purposes. + * + * @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 (useful for testing) + */ + 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() { + if (closed) { + logger.warn("Token provider is closed, returning null"); + return null; + } + + // Check if we need to refresh + if (shouldRefresh()) { + refreshToken(); + } + + lock.readLock().lock(); + try { + return cachedToken; + } finally { + lock.readLock().unlock(); + } Review Comment: The read-lock isn't necessary, the field is already `volatile`. ```suggestion return cachedToken; ``` ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/token/FileBearerTokenProvider.java: ########## @@ -0,0 +1,307 @@ +/* + * 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 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.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +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 ReadWriteLock lock = new ReentrantReadWriteLock(); + + 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 + */ + public FileBearerTokenProvider( + Path tokenFilePath, + Duration refreshInterval, + boolean jwtExpirationRefresh, + Duration jwtExpirationBuffer) { + this( + tokenFilePath, + refreshInterval, + jwtExpirationRefresh, + jwtExpirationBuffer, + Clock.systemUTC()); + } + + /** + * Create a new file-based token provider with JWT expiration support and custom clock. + * Package-private constructor for testing purposes. + * + * @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 (useful for testing) + */ + 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() { + if (closed) { + logger.warn("Token provider is closed, returning null"); + return null; + } + + // Check if we need to refresh + if (shouldRefresh()) { + refreshToken(); + } + + lock.readLock().lock(); + try { + return cachedToken; + } finally { + lock.readLock().unlock(); + } + } + + @Override + public void close() { + closed = true; + lock.writeLock().lock(); + try { + cachedToken = null; + logger.info("File token provider closed"); + } finally { + lock.writeLock().unlock(); + } + } + + private boolean shouldRefresh() { + return clock.instant().isAfter(nextRefresh); + } + + private void refreshToken() { + lock.writeLock().lock(); + try { + // Double-check pattern - another thread might have refreshed while we waited for the lock + if (!shouldRefresh()) { + return; + } + + 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 { + lock.writeLock().unlock(); + } + } + + /** Calculate when the next refresh should occur based on JWT expiration or fixed interval. */ + private Instant calculateNextRefresh(@Nullable String token) { + if (token == null || !jwtExpirationRefresh) { + // Use fixed interval + return lastRefresh.plus(refreshInterval); + } + + // Attempt to parse as JWT and extract expiration + Optional<Instant> expiration = getJwtExpirationTime(token); + + if (expiration.isPresent()) { + // Refresh before expiration minus buffer + Instant refreshTime = expiration.get().minus(jwtExpirationBuffer); + + // Ensure refresh time is in the future and not too soon (at least 1 second) + Instant minRefreshTime = clock.instant().plus(Duration.ofSeconds(1)); + if (refreshTime.isBefore(minRefreshTime)) { + logger.warn( + "JWT expires too soon ({}), using minimum refresh interval instead", expiration.get()); + return lastRefresh.plus(refreshInterval); + } + + logger.debug( + "Using JWT expiration-based refresh: token expires at {}, refreshing at {}", + expiration.get(), + refreshTime); + return refreshTime; + } + + // Fall back to fixed interval (token is not a valid JWT or has no expiration) + logger.debug("Token is not a valid JWT or has no expiration, using fixed refresh interval"); + return lastRefresh.plus(refreshInterval); + } + + @Nullable + private String loadTokenFromFile() { + int attempts = 0; + long deadlineMs = Review Comment: Hm, this would block all requests. What I had in mind is to try only once (as before) and yield the previous token in case of an error. ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/token/FileBearerTokenProvider.java: ########## @@ -0,0 +1,307 @@ +/* + * 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 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.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +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 ReadWriteLock lock = new ReentrantReadWriteLock(); + + 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 + */ + public FileBearerTokenProvider( + Path tokenFilePath, + Duration refreshInterval, + boolean jwtExpirationRefresh, + Duration jwtExpirationBuffer) { + this( + tokenFilePath, + refreshInterval, + jwtExpirationRefresh, + jwtExpirationBuffer, + Clock.systemUTC()); + } + + /** + * Create a new file-based token provider with JWT expiration support and custom clock. + * Package-private constructor for testing purposes. + * + * @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 (useful for testing) + */ + 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() { + if (closed) { + logger.warn("Token provider is closed, returning null"); + return null; + } Review Comment: ```suggestion checkState(!closed, "Token provider is closed"); ``` ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaAuthorizationConfig.java: ########## @@ -0,0 +1,201 @@ +/* + * 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; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.common.base.Strings; +import io.smallrye.config.ConfigMapping; +import io.smallrye.config.WithDefault; +import java.net.URI; +import java.nio.file.Path; +import java.time.Duration; +import java.util.Optional; + +/** + * Configuration for OPA (Open Policy Agent) authorization. + * + * <p><strong>Beta Feature:</strong> OPA authorization is currently in Beta and is not a stable + * release. It may undergo breaking changes in future versions. Use with caution in production + * environments. + */ +@ConfigMapping(prefix = "polaris.authorization.opa") +public interface OpaAuthorizationConfig { + + /** Authentication types supported by OPA authorization */ + enum AuthenticationType { + NONE("none"), + BEARER("bearer"); + + private final String value; + + AuthenticationType(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } + + /** Bearer token configuration types */ + enum BearerTokenType { + STATIC_TOKEN("static-token"), + FILE_BASED("file-based"); + + private final String value; + + BearerTokenType(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } + + Optional<URI> policyUri(); + + Optional<AuthenticationConfig> auth(); + + Optional<HttpConfig> http(); + + /** Validates the complete OPA configuration */ + default void validate() { + checkArgument(policyUri().isPresent(), "OPA policy URI cannot be null"); + checkArgument(auth().isPresent(), "Authentication configuration is required"); + + auth().get().validate(); + } + + /** HTTP client configuration for OPA communication. */ + interface HttpConfig { + @WithDefault("2000") + int timeoutMs(); + + @WithDefault("true") + boolean verifySsl(); + + Optional<Path> trustStorePath(); + + Optional<String> trustStorePassword(); + } + + /** Authentication configuration for OPA communication. */ + interface AuthenticationConfig { + /** Type of authentication */ + @WithDefault("none") + AuthenticationType type(); + + /** Bearer token authentication configuration */ + Optional<BearerTokenConfig> bearer(); + + default void validate() { + switch (type()) { + case BEARER: + checkArgument( + bearer().isPresent(), "Bearer configuration is required when type is 'bearer'"); + bearer().get().validate(); + break; + case NONE: + // No authentication - nothing to validate + break; + default: + throw new IllegalArgumentException( + "Invalid authentication type: " + type() + ". Supported types: 'bearer', 'none'"); + } + } + } + + interface BearerTokenConfig { + /** Type of bearer token configuration */ + @WithDefault("static-token") + BearerTokenType type(); + + /** Static bearer token configuration */ + Optional<StaticTokenConfig> staticToken(); + + /** File-based bearer token configuration */ + Optional<FileBasedConfig> fileBased(); + + default void validate() { + switch (type()) { + case STATIC_TOKEN: + checkArgument( + staticToken().isPresent(), + "Static token configuration is required when type is 'static-token'"); + staticToken().get().validate(); + break; + case FILE_BASED: + checkArgument( + fileBased().isPresent(), + "File-based configuration is required when type is 'file-based'"); + fileBased().get().validate(); + break; + default: + throw new IllegalArgumentException( + "Invalid bearer token type: " + type() + ". Must be 'static-token' or 'file-based'"); + } + } + + /** Configuration for static bearer tokens */ + interface StaticTokenConfig { + /** Static bearer token value */ + String value(); + + default void validate() { + checkArgument( + !Strings.isNullOrEmpty(value()), "Static bearer token value cannot be null or empty"); + } + } + + /** Configuration for file-based bearer tokens */ + interface FileBasedConfig { + /** Path to file containing bearer token */ + Path path(); + + /** How often to refresh file-based bearer tokens (defaults to 5 minutes if not specified) */ + Optional<Duration> refreshInterval(); + + /** + * Whether to automatically detect JWT tokens and use their 'exp' field for refresh timing. If + * true and the token is a valid JWT with an 'exp' claim, the token will be refreshed based on + * the expiration time minus the buffer, rather than the fixed refresh interval. Defaults to + * true if not specified. + */ + Optional<Boolean> jwtExpirationRefresh(); + + /** + * Buffer time before JWT expiration to refresh the token. Only used when jwtExpirationRefresh + * is true and the token is a valid JWT. Defaults to 1 minute if not specified. + */ + Optional<Duration> jwtExpirationBuffer(); + + default void validate() { + checkArgument(path() != null, "Bearer token file path cannot be null"); Review Comment: `path` will always be present (it's mandatory now), so the check will always yield `true`. ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerFactory.java: ########## @@ -0,0 +1,170 @@ +/* + * 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; + +import io.smallrye.common.annotation.Identifier; +import jakarta.annotation.PostConstruct; +import jakarta.annotation.PreDestroy; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.io.IOException; +import java.net.URI; +import java.time.Duration; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.auth.PolarisAuthorizerFactory; +import org.apache.polaris.core.config.RealmConfig; +import org.apache.polaris.extension.auth.opa.token.BearerTokenProvider; +import org.apache.polaris.extension.auth.opa.token.FileBearerTokenProvider; +import org.apache.polaris.extension.auth.opa.token.StaticBearerTokenProvider; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Factory for creating OPA-based Polaris authorizer implementations. */ +@ApplicationScoped +@Identifier("opa") +class OpaPolarisAuthorizerFactory implements PolarisAuthorizerFactory { + + private static final Logger logger = LoggerFactory.getLogger(OpaPolarisAuthorizerFactory.class); + + private final OpaAuthorizationConfig opaConfig; + private CloseableHttpClient httpClient; + private BearerTokenProvider bearerTokenProvider; + + @Inject + public OpaPolarisAuthorizerFactory(OpaAuthorizationConfig opaConfig) { + this.opaConfig = opaConfig; + } + + @PostConstruct + public void initialize() { + // Validate configuration once during startup + opaConfig.validate(); + + // Create HTTP client once during startup + httpClient = createHttpClient(); + + // Setup authentication once during startup + setupAuthentication(opaConfig.auth().get()); + } + + @Override + public PolarisAuthorizer create(RealmConfig realmConfig) { + // All components are now pre-initialized, just create the authorizer + URI policyUri = opaConfig.policyUri().get(); + + return new OpaPolarisAuthorizer(policyUri, httpClient, bearerTokenProvider); + } + + @PreDestroy + public void cleanup() { + // Clean up bearer token provider resources + if (bearerTokenProvider != null) { + try { + bearerTokenProvider.close(); + logger.info("Bearer token provider closed successfully"); + } catch (Exception e) { + // Log but don't throw - we're shutting down anyway + logger.warn("Error closing bearer token provider: {}", e.getMessage(), e); + } + } + + // Clean up HTTP client resources + if (httpClient != null) { + try { + httpClient.close(); + logger.info("HTTP client closed successfully"); + } catch (IOException e) { + // Log but don't throw - we're shutting down anyway + logger.warn("Error closing HTTP client: {}", e.getMessage(), e); + } + } + } + + private CloseableHttpClient createHttpClient() { + try { + if (opaConfig.http().isEmpty()) { + throw new IllegalStateException("HTTP configuration is required"); + } + return OpaHttpClientFactory.createHttpClient(opaConfig.http().get()); Review Comment: Nit: ```suggestion if (opaConfig.http().isEmpty()) { throw new IllegalStateException("HTTP configuration is required"); } return OpaHttpClientFactory.createHttpClient(opaConfig.http().orElseThrow(() -> new IllegalStateException("HTTP configuration is required"))); ``` (irrelevant if `http()` is not an `Optional, see above) ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerFactory.java: ########## @@ -0,0 +1,170 @@ +/* + * 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; + +import io.smallrye.common.annotation.Identifier; +import jakarta.annotation.PostConstruct; +import jakarta.annotation.PreDestroy; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.io.IOException; +import java.net.URI; +import java.time.Duration; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.auth.PolarisAuthorizerFactory; +import org.apache.polaris.core.config.RealmConfig; +import org.apache.polaris.extension.auth.opa.token.BearerTokenProvider; +import org.apache.polaris.extension.auth.opa.token.FileBearerTokenProvider; +import org.apache.polaris.extension.auth.opa.token.StaticBearerTokenProvider; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Factory for creating OPA-based Polaris authorizer implementations. */ +@ApplicationScoped +@Identifier("opa") +class OpaPolarisAuthorizerFactory implements PolarisAuthorizerFactory { + + private static final Logger logger = LoggerFactory.getLogger(OpaPolarisAuthorizerFactory.class); + + private final OpaAuthorizationConfig opaConfig; + private CloseableHttpClient httpClient; + private BearerTokenProvider bearerTokenProvider; + + @Inject + public OpaPolarisAuthorizerFactory(OpaAuthorizationConfig opaConfig) { + this.opaConfig = opaConfig; + } + + @PostConstruct + public void initialize() { + // Validate configuration once during startup + opaConfig.validate(); + + // Create HTTP client once during startup + httpClient = createHttpClient(); + + // Setup authentication once during startup + setupAuthentication(opaConfig.auth().get()); + } + + @Override + public PolarisAuthorizer create(RealmConfig realmConfig) { + // All components are now pre-initialized, just create the authorizer + URI policyUri = opaConfig.policyUri().get(); + + return new OpaPolarisAuthorizer(policyUri, httpClient, bearerTokenProvider); + } + + @PreDestroy + public void cleanup() { + // Clean up bearer token provider resources + if (bearerTokenProvider != null) { + try { + bearerTokenProvider.close(); + logger.info("Bearer token provider closed successfully"); + } catch (Exception e) { + // Log but don't throw - we're shutting down anyway + logger.warn("Error closing bearer token provider: {}", e.getMessage(), e); + } + } + + // Clean up HTTP client resources + if (httpClient != null) { + try { + httpClient.close(); + logger.info("HTTP client closed successfully"); Review Comment: Nit ```suggestion logger.debug("HTTP client closed successfully"); ``` ########## runtime/defaults/src/main/resources/application.properties: ########## @@ -197,6 +197,40 @@ polaris.oidc.principal-roles-mapper.type=default # polaris.storage.gcp.token=token # polaris.storage.gcp.lifespan=PT1H +# Polaris authorization type settings +# Which authorizer to use: "internal" (PolarisAuthorizerImpl) or "opa" (OpaPolarisAuthorizer) +# polaris.authorization.type=internal + +# OPA Authorizer Configuration: effective only if polaris.authorization.type=opa +# NOTE: The OPA Authorizer is currently in Beta and is not a stable release. +# It may undergo breaking changes in future versions. +# polaris.authorization.opa.url=http://localhost:8181 +# polaris.authorization.opa.policy-path=/v1/data/polaris/authz/allow + +# OPA HTTP configuration +# polaris.authorization.opa.http.timeout-ms=5000 +# polaris.authorization.opa.http.verify-ssl=false Review Comment: Mind mentioning the trust-store settings here as well and document that using verify-ssl=true breaks production readiness? ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerFactory.java: ########## @@ -0,0 +1,170 @@ +/* + * 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; + +import io.smallrye.common.annotation.Identifier; +import jakarta.annotation.PostConstruct; +import jakarta.annotation.PreDestroy; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.io.IOException; +import java.net.URI; +import java.time.Duration; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.auth.PolarisAuthorizerFactory; +import org.apache.polaris.core.config.RealmConfig; +import org.apache.polaris.extension.auth.opa.token.BearerTokenProvider; +import org.apache.polaris.extension.auth.opa.token.FileBearerTokenProvider; +import org.apache.polaris.extension.auth.opa.token.StaticBearerTokenProvider; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Factory for creating OPA-based Polaris authorizer implementations. */ +@ApplicationScoped +@Identifier("opa") +class OpaPolarisAuthorizerFactory implements PolarisAuthorizerFactory { + + private static final Logger logger = LoggerFactory.getLogger(OpaPolarisAuthorizerFactory.class); + + private final OpaAuthorizationConfig opaConfig; + private CloseableHttpClient httpClient; + private BearerTokenProvider bearerTokenProvider; + + @Inject + public OpaPolarisAuthorizerFactory(OpaAuthorizationConfig opaConfig) { Review Comment: Nit: ```suggestion public OpaPolarisAuthorizerFactory(OpaAuthorizationConfig opaConfig, Clock clock) { ``` and pass that clock to `FileBearerTokenProvider`. (`Clock`'s produced via `ServiceProducers#clock`) ########## extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerFactory.java: ########## @@ -0,0 +1,170 @@ +/* + * 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; + +import io.smallrye.common.annotation.Identifier; +import jakarta.annotation.PostConstruct; +import jakarta.annotation.PreDestroy; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.io.IOException; +import java.net.URI; +import java.time.Duration; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.auth.PolarisAuthorizerFactory; +import org.apache.polaris.core.config.RealmConfig; +import org.apache.polaris.extension.auth.opa.token.BearerTokenProvider; +import org.apache.polaris.extension.auth.opa.token.FileBearerTokenProvider; +import org.apache.polaris.extension.auth.opa.token.StaticBearerTokenProvider; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Factory for creating OPA-based Polaris authorizer implementations. */ +@ApplicationScoped +@Identifier("opa") +class OpaPolarisAuthorizerFactory implements PolarisAuthorizerFactory { + + private static final Logger logger = LoggerFactory.getLogger(OpaPolarisAuthorizerFactory.class); + + private final OpaAuthorizationConfig opaConfig; + private CloseableHttpClient httpClient; + private BearerTokenProvider bearerTokenProvider; + + @Inject + public OpaPolarisAuthorizerFactory(OpaAuthorizationConfig opaConfig) { + this.opaConfig = opaConfig; + } + + @PostConstruct + public void initialize() { + // Validate configuration once during startup + opaConfig.validate(); + + // Create HTTP client once during startup + httpClient = createHttpClient(); + + // Setup authentication once during startup + setupAuthentication(opaConfig.auth().get()); + } + + @Override + public PolarisAuthorizer create(RealmConfig realmConfig) { + // All components are now pre-initialized, just create the authorizer + URI policyUri = opaConfig.policyUri().get(); + + return new OpaPolarisAuthorizer(policyUri, httpClient, bearerTokenProvider); + } + + @PreDestroy + public void cleanup() { + // Clean up bearer token provider resources + if (bearerTokenProvider != null) { + try { + bearerTokenProvider.close(); + logger.info("Bearer token provider closed successfully"); Review Comment: Nit ```suggestion logger.debug("Bearer token provider closed successfully"); ``` ########## gradle/projects.main.properties: ########## @@ -43,6 +43,8 @@ polaris-version=tools/version polaris-misc-types=tools/misc-types polaris-extensions-federation-hadoop=extensions/federation/hadoop polaris-extensions-federation-hive=extensions/federation/hive +polaris-extensions-auth-opa=extensions/auth/opa/impl +polaris-extensions-auth-opa-tests=extensions/auth/opa/tests Review Comment: Hm, I'd prefer to leave the integration tests in the runtime module. Having this module adds another expensive and (somewhat) long-running Quarkus build to CI duration. -- 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]
