You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by et...@apache.org on 2025/03/26 06:15:56 UTC
(iceberg) branch main updated: AWS: Use correct parent session when calling delegate auth manager (#12582)
This is an automated email from the ASF dual-hosted git repository.
etudenhoefner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 07ad9a635c AWS: Use correct parent session when calling delegate auth manager (#12582)
07ad9a635c is described below
commit 07ad9a635c6e24959280f53b76ef3036e33d13ee
Author: Alexandre Dutra <ad...@users.noreply.github.com>
AuthorDate: Wed Mar 26 07:15:51 2025 +0100
AWS: Use correct parent session when calling delegate auth manager (#12582)
---
.../apache/iceberg/aws/RESTSigV4AuthManager.java | 27 +++--
.../apache/iceberg/aws/RESTSigV4AuthSession.java | 4 +
.../iceberg/aws/TestRESTSigV4AuthManager.java | 130 ++++++++++++++-------
3 files changed, 109 insertions(+), 52 deletions(-)
diff --git a/aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthManager.java b/aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthManager.java
index 061a6afe54..e975c9588e 100644
--- a/aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthManager.java
+++ b/aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthManager.java
@@ -19,6 +19,7 @@
package org.apache.iceberg.aws;
import java.util.Map;
+import java.util.Optional;
import org.apache.iceberg.catalog.SessionCatalog;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
@@ -33,7 +34,6 @@ import software.amazon.awssdk.auth.signer.Aws4Signer;
*
* <p>It takes a delegate AuthManager to handle double authentication cases, e.g. on top of OAuth2.
*/
-@SuppressWarnings("unused") // loaded by reflection
public class RESTSigV4AuthManager implements AuthManager {
private final Aws4Signer signer = Aws4Signer.create();
@@ -41,7 +41,7 @@ public class RESTSigV4AuthManager implements AuthManager {
private Map<String, String> catalogProperties = Map.of();
- public RESTSigV4AuthManager(String name, AuthManager delegate) {
+ public RESTSigV4AuthManager(String ignored, AuthManager delegate) {
this.delegate = Preconditions.checkNotNull(delegate, "Invalid delegate: null");
}
@@ -61,20 +61,33 @@ public class RESTSigV4AuthManager implements AuthManager {
}
@Override
- public AuthSession contextualSession(SessionCatalog.SessionContext context, AuthSession parent) {
+ public RESTSigV4AuthSession contextualSession(
+ SessionCatalog.SessionContext context, AuthSession parent) {
+ Preconditions.checkState(
+ parent instanceof RESTSigV4AuthSession, "Parent session is not SigV4: %s", parent);
AwsProperties contextProperties =
- new AwsProperties(RESTUtil.merge(catalogProperties, context.properties()));
+ new AwsProperties(
+ RESTUtil.merge(
+ catalogProperties,
+ // Use both context properties and credentials to create the AwsProperties instance
+ RESTUtil.merge(
+ Optional.ofNullable(context.properties()).orElseGet(Map::of),
+ Optional.ofNullable(context.credentials()).orElseGet(Map::of))));
+ RESTSigV4AuthSession sigV4Parent = (RESTSigV4AuthSession) parent;
return new RESTSigV4AuthSession(
- signer, delegate.contextualSession(context, parent), contextProperties);
+ signer, delegate.contextualSession(context, sigV4Parent.delegate()), contextProperties);
}
@Override
- public AuthSession tableSession(
+ public RESTSigV4AuthSession tableSession(
TableIdentifier table, Map<String, String> properties, AuthSession parent) {
+ Preconditions.checkState(
+ parent instanceof RESTSigV4AuthSession, "Parent session is not SigV4: %s", parent);
AwsProperties tableProperties =
new AwsProperties(RESTUtil.merge(catalogProperties, properties));
+ RESTSigV4AuthSession sigV4Parent = (RESTSigV4AuthSession) parent;
return new RESTSigV4AuthSession(
- signer, delegate.tableSession(table, properties, parent), tableProperties);
+ signer, delegate.tableSession(table, properties, sigV4Parent.delegate()), tableProperties);
}
@Override
diff --git a/aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthSession.java b/aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthSession.java
index f2e66cfb75..3fd52fa8bb 100644
--- a/aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthSession.java
+++ b/aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthSession.java
@@ -73,6 +73,10 @@ public class RESTSigV4AuthSession implements AuthSession {
this.credentialsProvider = awsProperties.restCredentialsProvider();
}
+ public AuthSession delegate() {
+ return delegate;
+ }
+
@Override
public HTTPRequest authenticate(HTTPRequest request) {
return sign(delegate.authenticate(request));
diff --git a/aws/src/test/java/org/apache/iceberg/aws/TestRESTSigV4AuthManager.java b/aws/src/test/java/org/apache/iceberg/aws/TestRESTSigV4AuthManager.java
index 68df9e2d1c..2bb5d0bfaf 100644
--- a/aws/src/test/java/org/apache/iceberg/aws/TestRESTSigV4AuthManager.java
+++ b/aws/src/test/java/org/apache/iceberg/aws/TestRESTSigV4AuthManager.java
@@ -20,27 +20,30 @@ package org.apache.iceberg.aws;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.when;
+import static org.assertj.core.api.InstanceOfAssertFactories.type;
import java.util.Map;
import org.apache.iceberg.catalog.SessionCatalog;
import org.apache.iceberg.catalog.TableIdentifier;
-import org.apache.iceberg.rest.HTTPClient;
import org.apache.iceberg.rest.RESTClient;
+import org.apache.iceberg.rest.auth.AuthConfig;
import org.apache.iceberg.rest.auth.AuthManager;
import org.apache.iceberg.rest.auth.AuthManagers;
import org.apache.iceberg.rest.auth.AuthProperties;
import org.apache.iceberg.rest.auth.AuthSession;
import org.apache.iceberg.rest.auth.NoopAuthManager;
import org.apache.iceberg.rest.auth.OAuth2Manager;
+import org.apache.iceberg.rest.auth.OAuth2Properties;
import org.apache.iceberg.rest.auth.OAuth2Util;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
+import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
+import software.amazon.awssdk.regions.Region;
class TestRESTSigV4AuthManager {
- private final Map<String, String> awsProperties =
+ private final Map<String, String> catalogProperties =
Map.of(
// CI environment doesn't have credentials, but a value must be set for signing
AwsProperties.REST_SIGNER_REGION,
@@ -48,7 +51,12 @@ class TestRESTSigV4AuthManager {
AwsProperties.REST_ACCESS_KEY_ID,
"id",
AwsProperties.REST_SECRET_ACCESS_KEY,
- "secret");
+ "secret",
+ // OAuth2 properties
+ OAuth2Properties.TOKEN,
+ "token1",
+ OAuth2Properties.SCOPE,
+ "scope1");
@Test
void create() {
@@ -105,65 +113,67 @@ class TestRESTSigV4AuthManager {
@Test
void initSession() {
- AuthManager delegate = Mockito.mock(AuthManager.class);
- when(delegate.initSession(any(), any())).thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
+ AuthManager delegate =
+ AuthManagers.loadAuthManager(
+ "test", Map.of(AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2));
RESTClient client = Mockito.mock(RESTClient.class);
AuthManager manager = new RESTSigV4AuthManager("test", delegate);
- AuthSession authSession = manager.initSession(client, awsProperties);
- assertThat(authSession)
- .isInstanceOf(RESTSigV4AuthSession.class)
- .extracting("delegate")
- .isInstanceOf(OAuth2Util.AuthSession.class);
+ AuthSession authSession = manager.initSession(client, catalogProperties);
+ checkSession(authSession, "us-west-2", "id", "secret", false);
}
@Test
void catalogSession() {
- AuthManager delegate = Mockito.mock(AuthManager.class);
- when(delegate.catalogSession(any(), any()))
- .thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
+ AuthManager delegate =
+ AuthManagers.loadAuthManager(
+ "test", Map.of(AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2));
RESTClient client = Mockito.mock(RESTClient.class);
AuthManager manager = new RESTSigV4AuthManager("test", delegate);
- AuthSession authSession = manager.catalogSession(client, awsProperties);
- assertThat(authSession)
- .isInstanceOf(RESTSigV4AuthSession.class)
- .extracting("delegate")
- .isInstanceOf(OAuth2Util.AuthSession.class);
+ AuthSession authSession = manager.catalogSession(client, catalogProperties);
+ checkSession(authSession, "us-west-2", "id", "secret", true);
}
@Test
void contextualSession() {
- AuthManager delegate = Mockito.mock(AuthManager.class);
- when(delegate.catalogSession(any(), any()))
- .thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
- when(delegate.contextualSession(any(), any()))
- .thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
+ AuthManager delegate =
+ AuthManagers.loadAuthManager(
+ "test", Map.of(AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2));
+ RESTClient client = Mockito.mock(RESTClient.class);
AuthManager manager = new RESTSigV4AuthManager("test", delegate);
- manager.catalogSession(Mockito.mock(HTTPClient.class), awsProperties);
- AuthSession authSession =
- manager.contextualSession(
- Mockito.mock(SessionCatalog.SessionContext.class), Mockito.mock(AuthSession.class));
- assertThat(authSession)
- .isInstanceOf(RESTSigV4AuthSession.class)
- .extracting("delegate")
- .isInstanceOf(OAuth2Util.AuthSession.class);
+ AuthSession catalogSession = manager.catalogSession(client, catalogProperties);
+ SessionCatalog.SessionContext context =
+ new SessionCatalog.SessionContext(
+ "context1",
+ "identity1",
+ Map.of(
+ AwsProperties.REST_ACCESS_KEY_ID,
+ "id2",
+ AwsProperties.REST_SECRET_ACCESS_KEY,
+ "secret2"),
+ Map.of(AwsProperties.REST_SIGNER_REGION, "us-east-1"));
+ AuthSession authSession = manager.contextualSession(context, catalogSession);
+ checkSession(authSession, "us-east-1", "id2", "secret2", true);
}
@Test
void tableSession() {
- AuthManager delegate = Mockito.mock(AuthManager.class);
- when(delegate.catalogSession(any(), any()))
- .thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
- when(delegate.tableSession(any(), any(), any()))
- .thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
+ AuthManager delegate =
+ AuthManagers.loadAuthManager(
+ "test", Map.of(AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2));
+ RESTClient client = Mockito.mock(RESTClient.class);
AuthManager manager = new RESTSigV4AuthManager("test", delegate);
- manager.catalogSession(Mockito.mock(HTTPClient.class), awsProperties);
+ AuthSession catalogSession = manager.catalogSession(client, catalogProperties);
+ Map<String, String> tableProperties =
+ Map.of(
+ AwsProperties.REST_ACCESS_KEY_ID,
+ "id2",
+ AwsProperties.REST_SECRET_ACCESS_KEY,
+ "secret2",
+ AwsProperties.REST_SIGNER_REGION,
+ "us-east-1");
AuthSession authSession =
- manager.tableSession(
- Mockito.mock(TableIdentifier.class), Map.of(), Mockito.mock(AuthSession.class));
- assertThat(authSession)
- .isInstanceOf(RESTSigV4AuthSession.class)
- .extracting("delegate")
- .isInstanceOf(OAuth2Util.AuthSession.class);
+ manager.tableSession(TableIdentifier.of("table1"), tableProperties, catalogSession);
+ checkSession(authSession, "us-east-1", "id2", "secret2", true);
}
@Test
@@ -173,4 +183,34 @@ class TestRESTSigV4AuthManager {
manager.close();
Mockito.verify(delegate).close();
}
+
+ private static void checkSession(
+ AuthSession authSession,
+ String expectedRegion,
+ String expectedAccessKeyId,
+ String expectedSecretAccessKey,
+ boolean expectedKeepRefreshed) {
+ assertThat(authSession).isInstanceOf(RESTSigV4AuthSession.class);
+ RESTSigV4AuthSession sigV4AuthSession = (RESTSigV4AuthSession) authSession;
+ // Check AWS properties present
+ assertThat(sigV4AuthSession).extracting("signingRegion").isEqualTo(Region.of(expectedRegion));
+ assertThat(sigV4AuthSession)
+ .extracting("credentialsProvider")
+ .asInstanceOf(type(AwsCredentialsProvider.class))
+ .extracting(AwsCredentialsProvider::resolveCredentials)
+ .isEqualTo(AwsBasicCredentials.create(expectedAccessKeyId, expectedSecretAccessKey));
+ // Check OAuth2 properties present in delegate
+ assertThat(sigV4AuthSession.delegate())
+ .asInstanceOf(type(OAuth2Util.AuthSession.class))
+ .extracting("config")
+ .asInstanceOf(type(AuthConfig.class))
+ .isEqualTo(
+ AuthConfig.builder()
+ .token("token1")
+ .tokenType(OAuth2Properties.ACCESS_TOKEN_TYPE)
+ .scope("scope1")
+ .keepRefreshed(expectedKeepRefreshed)
+ .optionalOAuthParams(Map.of(OAuth2Properties.SCOPE, "scope1"))
+ .build());
+ }
}