From fdd194156db7a213f50c8b5ce4fc61a4e2b9c17c Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Thu, 21 Nov 2024 12:42:47 +0100 Subject: [PATCH 1/6] Auth Manager API part 2: AuthManager, AuthSession --- .../org/apache/iceberg/rest/HTTPHeaders.java | 10 ++ .../apache/iceberg/rest/auth/AuthManager.java | 108 ++++++++++++++++++ .../iceberg/rest/auth/AuthManagers.java | 77 +++++++++++++ .../iceberg/rest/auth/AuthProperties.java | 37 ++++++ .../apache/iceberg/rest/auth/AuthSession.java | 50 ++++++++ .../iceberg/rest/auth/BasicAuthManager.java | 44 +++++++ .../iceberg/rest/auth/DefaultAuthSession.java | 52 +++++++++ .../iceberg/rest/auth/NoopAuthManager.java | 31 +++++ .../apache/iceberg/rest/TestHTTPHeaders.java | 12 ++ .../iceberg/rest/auth/TestAuthManagers.java | 88 ++++++++++++++ .../rest/auth/TestBasicAuthManager.java | 62 ++++++++++ .../rest/auth/TestDefaultAuthSession.java | 74 ++++++++++++ 12 files changed, 645 insertions(+) create mode 100644 core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/auth/AuthProperties.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/auth/AuthSession.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/auth/DefaultAuthSession.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java create mode 100644 core/src/test/java/org/apache/iceberg/rest/auth/TestAuthManagers.java create mode 100644 core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java create mode 100644 core/src/test/java/org/apache/iceberg/rest/auth/TestDefaultAuthSession.java diff --git a/core/src/main/java/org/apache/iceberg/rest/HTTPHeaders.java b/core/src/main/java/org/apache/iceberg/rest/HTTPHeaders.java index 35710bd9a9b7..23263899b95f 100644 --- a/core/src/main/java/org/apache/iceberg/rest/HTTPHeaders.java +++ b/core/src/main/java/org/apache/iceberg/rest/HTTPHeaders.java @@ -19,6 +19,7 @@ package org.apache.iceberg.rest; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; @@ -85,6 +86,15 @@ static HTTPHeaders of(HTTPHeader... headers) { return ImmutableHTTPHeaders.builder().addEntries(headers).build(); } + static HTTPHeaders of(Map headers) { + return ImmutableHTTPHeaders.builder() + .entries( + headers.entrySet().stream() + .map(e -> HTTPHeader.of(e.getKey(), e.getValue())) + .collect(Collectors.toList())) + .build(); + } + /** Represents an HTTP header as a name-value pair. */ @Value.Style(redactedMask = "****", depluralize = true) @Value.Immutable diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java new file mode 100644 index 000000000000..4e321f619723 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +import java.util.Map; +import org.apache.iceberg.catalog.SessionCatalog; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.rest.RESTClient; + +/** + * Manager for authentication sessions. This interface is used to create sessions for the catalog, + * the tables/views, and any other context that requires authentication. + * + *

Managers are usually stateful and may require initialization and cleanup. The manager is + * created by the catalog and is closed when the catalog is closed. + */ +public interface AuthManager extends AutoCloseable { + + /** + * Returns a temporary session to use for contacting the configuration endpoint only. Note that + * the returned session will be closed after the configuration endpoint is contacted, and should + * not be cached. + * + *

The provided REST client is a short-lived client; it should only be used to fetch initial + * credentials, if required, and must be discarded after that. + * + *

This method cannot return null. By default, it returns the catalog session. + */ + default AuthSession initSession(RESTClient initClient, Map properties) { + return mainSession(initClient, properties); + } + + /** + * Returns a long-lived session whose lifetime is tied to the owning catalog. This session serves + * as the parent session for all other sessions (contextual and table-specific). It is closed when + * the owning catalog is closed. + * + *

The provided REST client is a long-lived, shared client; if required, implementors may store + * it and reuse it for all subsequent requests to the authorization server, e.g. for renewing or + * refreshing credentials. It is not necessary to close it when {@link #close()} is called. + * + *

This method cannot return null. + * + *

It is not required to cache the returned session internally, as the catalog will keep it + * alive for the lifetime of the catalog. + */ + AuthSession mainSession(RESTClient sharedClient, Map properties); + + /** + * Returns a session for a specific context. + * + *

If the context requires a specific {@link AuthSession}, this method should return a new + * {@link AuthSession} instance, otherwise it should return the parent session. + * + *

This method cannot return null. By default, it returns the parent session. + * + *

Implementors should cache contextual sessions internally, as the catalog will not cache + * them. Also, the owning catalog never closes contextual sessions; implementations should manage + * their lifecycle themselves and close them when they are no longer needed. + */ + default AuthSession contextualSession(SessionCatalog.SessionContext context, AuthSession parent) { + return parent; + } + + /** + * Returns a new session targeting a specific table or view. The properties are the ones returned + * by the table/view endpoint. + * + *

If the table or view requires a specific {@link AuthSession}, this method should return a + * new {@link AuthSession} instance, otherwise it should return the parent session. + * + *

This method cannot return null. By default, it returns the parent session. + * + *

Implementors should cache table sessions internally, as the catalog will not cache them. + * Also, the owning catalog never closes table sessions; implementations should manage their + * lifecycle themselves and close them when they are no longer needed. + */ + default AuthSession tableSession( + TableIdentifier table, Map properties, AuthSession parent) { + return parent; + } + + /** + * Closes the manager and releases any resources. + * + *

This method is called when the owning catalog is closed. + */ + @Override + default void close() { + // Do nothing + } +} diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java new file mode 100644 index 000000000000..7f1d1573d297 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +import java.util.Locale; +import java.util.Map; +import org.apache.iceberg.common.DynConstructors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class AuthManagers { + + private static final Logger LOG = LoggerFactory.getLogger(AuthManagers.class); + + private AuthManagers() {} + + public static AuthManager loadAuthManager(String name, Map properties) { + + String authType = + properties.getOrDefault(AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_NONE); + + String impl; + switch (authType.toLowerCase(Locale.ROOT)) { + case AuthProperties.AUTH_TYPE_NONE: + impl = AuthProperties.AUTH_MANAGER_IMPL_NONE; + break; + case AuthProperties.AUTH_TYPE_BASIC: + impl = AuthProperties.AUTH_MANAGER_IMPL_BASIC; + break; + default: + impl = authType; + } + + LOG.info("Loading AuthManager implementation: {}", impl); + DynConstructors.Ctor ctor; + try { + ctor = + DynConstructors.builder(AuthManager.class) + .loader(AuthManagers.class.getClassLoader()) + .impl(impl, String.class) // with name + .impl(impl) // without name + .buildChecked(); + } catch (NoSuchMethodException e) { + throw new IllegalArgumentException( + String.format( + "Cannot initialize AuthManager implementation %s: %s", impl, e.getMessage()), + e); + } + + AuthManager authManager; + try { + authManager = ctor.newInstance(name); + } catch (ClassCastException e) { + throw new IllegalArgumentException( + String.format("Cannot initialize AuthManager, %s does not implement AuthManager", impl), + e); + } + + return authManager; + } +} diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/AuthProperties.java b/core/src/main/java/org/apache/iceberg/rest/auth/AuthProperties.java new file mode 100644 index 000000000000..bf94311d5578 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/auth/AuthProperties.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +public final class AuthProperties { + + private AuthProperties() {} + + public static final String AUTH_TYPE = "rest.auth.type"; + + public static final String AUTH_TYPE_NONE = "none"; + public static final String AUTH_TYPE_BASIC = "basic"; + + public static final String AUTH_MANAGER_IMPL_NONE = + "org.apache.iceberg.rest.auth.NoopAuthManager"; + public static final String AUTH_MANAGER_IMPL_BASIC = + "org.apache.iceberg.rest.auth.BasicAuthManager"; + + public static final String BASIC_USERNAME = "rest.auth.basic.username"; + public static final String BASIC_PASSWORD = "rest.auth.basic.password"; +} diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/AuthSession.java b/core/src/main/java/org/apache/iceberg/rest/auth/AuthSession.java new file mode 100644 index 000000000000..169a53d7a8f2 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/auth/AuthSession.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +import org.apache.iceberg.rest.HTTPRequest; + +/** + * An authentication session that can be used to authenticate outgoing HTTP requests. + * + *

Authentication sessions are usually immutable, but may hold resources that need to be released + * when the session is no longer needed. Implementations should override {@link #close()} to release + * any resources. + */ +public interface AuthSession extends AutoCloseable { + + /** An empty session that does nothing. */ + AuthSession EMPTY = request -> request; + + /** + * Authenticates the given request and returns a new request with the necessary authentication. + */ + HTTPRequest authenticate(HTTPRequest request); + + /** + * Closes the session and releases any resources. This method is called when the session is no + * longer needed. Note that since sessions may be cached, this method may not be called + * immediately after the session is no longer needed, but rather when the session is evicted from + * the cache, or the cache itself is closed. + */ + @Override + default void close() { + // Do nothing + } +} diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java new file mode 100644 index 000000000000..007941fc9ee7 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +import java.util.Map; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.rest.HTTPHeaders; +import org.apache.iceberg.rest.RESTClient; + +/** An auth manager that adds static BASIC authentication data to outgoing HTTP requests. */ +public final class BasicAuthManager implements AuthManager { + + @Override + public AuthSession mainSession(RESTClient sharedClient, Map properties) { + Preconditions.checkArgument( + properties.containsKey(AuthProperties.BASIC_USERNAME), + "Invalid username: missing required property %s", + AuthProperties.BASIC_USERNAME); + Preconditions.checkArgument( + properties.containsKey(AuthProperties.BASIC_PASSWORD), + "Invalid password: missing required property %s", + AuthProperties.BASIC_PASSWORD); + String username = properties.get(AuthProperties.BASIC_USERNAME); + String password = properties.get(AuthProperties.BASIC_PASSWORD); + String credentials = username + ":" + password; + return DefaultAuthSession.of(HTTPHeaders.of(OAuth2Util.basicAuthHeaders(credentials))); + } +} diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/DefaultAuthSession.java b/core/src/main/java/org/apache/iceberg/rest/auth/DefaultAuthSession.java new file mode 100644 index 000000000000..f1d033704a6a --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/auth/DefaultAuthSession.java @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +import org.apache.iceberg.rest.HTTPHeaders; +import org.apache.iceberg.rest.HTTPRequest; +import org.apache.iceberg.rest.ImmutableHTTPRequest; +import org.immutables.value.Value; + +/** + * Default implementation of {@link AuthSession}. It authenticates requests by setting the provided + * headers on the request. + * + *

Most {@link AuthManager} implementations should make use of this class, unless they need to + * retain state when creating sessions, or if they need to modify the request in a different way. + */ +@Value.Style(redactedMask = "****") +@Value.Immutable +@SuppressWarnings({"ImmutablesStyle", "SafeLoggingPropagation"}) +public interface DefaultAuthSession extends AuthSession { + + /** Headers containing authentication data to set on the request. */ + HTTPHeaders headers(); + + @Override + default HTTPRequest authenticate(HTTPRequest request) { + HTTPHeaders headers = request.headers().putIfAbsent(headers()); + return headers.equals(request.headers()) + ? request + : ImmutableHTTPRequest.builder().from(request).headers(headers).build(); + } + + static DefaultAuthSession of(HTTPHeaders headers) { + return ImmutableDefaultAuthSession.builder().headers(headers).build(); + } +} diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java new file mode 100644 index 000000000000..589d372faa80 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +import java.util.Map; +import org.apache.iceberg.rest.RESTClient; + +/** An auth manager that does not add any authentication data to outgoing HTTP requests. */ +public class NoopAuthManager implements AuthManager { + + @Override + public AuthSession mainSession(RESTClient sharedClient, Map properties) { + return AuthSession.EMPTY; + } +} diff --git a/core/src/test/java/org/apache/iceberg/rest/TestHTTPHeaders.java b/core/src/test/java/org/apache/iceberg/rest/TestHTTPHeaders.java index 9380073f7643..a8531e6ff510 100644 --- a/core/src/test/java/org/apache/iceberg/rest/TestHTTPHeaders.java +++ b/core/src/test/java/org/apache/iceberg/rest/TestHTTPHeaders.java @@ -21,6 +21,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.rest.HTTPHeaders.HTTPHeader; import org.junit.jupiter.api.Test; @@ -119,6 +120,17 @@ void putIfAbsentHTTPHeaders() { .hasMessage("headers"); } + @Test + void ofMap() { + HTTPHeaders actual = + HTTPHeaders.of( + ImmutableMap.of( + "header1", "value1a", + "HEADER1", "value1b", + "header2", "value2")); + assertThat(actual).isEqualTo(headers); + } + @Test void invalidHeader() { // invalid input (null name or value) diff --git a/core/src/test/java/org/apache/iceberg/rest/auth/TestAuthManagers.java b/core/src/test/java/org/apache/iceberg/rest/auth/TestAuthManagers.java new file mode 100644 index 000000000000..21bd8c1b2963 --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/rest/auth/TestAuthManagers.java @@ -0,0 +1,88 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class TestAuthManagers { + + private final PrintStream standardErr = System.err; + private final ByteArrayOutputStream streamCaptor = new ByteArrayOutputStream(); + + @BeforeEach + public void before() { + System.setErr(new PrintStream(streamCaptor)); + } + + @AfterEach + public void after() { + System.setErr(standardErr); + } + + @Test + void noop() { + try (AuthManager manager = AuthManagers.loadAuthManager("test", Map.of())) { + assertThat(manager).isInstanceOf(NoopAuthManager.class); + } + assertThat(streamCaptor.toString()) + .contains( + "Loading AuthManager implementation: org.apache.iceberg.rest.auth.NoopAuthManager"); + } + + @Test + void noopExplicit() { + try (AuthManager manager = + AuthManagers.loadAuthManager( + "test", Map.of(AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_NONE))) { + assertThat(manager).isInstanceOf(NoopAuthManager.class); + } + assertThat(streamCaptor.toString()) + .contains( + "Loading AuthManager implementation: org.apache.iceberg.rest.auth.NoopAuthManager"); + } + + @Test + void basicExplicit() { + try (AuthManager manager = + AuthManagers.loadAuthManager( + "test", Map.of(AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_BASIC))) { + assertThat(manager).isInstanceOf(BasicAuthManager.class); + } + assertThat(streamCaptor.toString()) + .contains( + "Loading AuthManager implementation: org.apache.iceberg.rest.auth.BasicAuthManager"); + } + + @Test + @SuppressWarnings("resource") + void nonExistentAuthManager() { + assertThatThrownBy( + () -> AuthManagers.loadAuthManager("test", Map.of(AuthProperties.AUTH_TYPE, "unknown"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot initialize AuthManager implementation unknown"); + } +} diff --git a/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java b/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java new file mode 100644 index 000000000000..4f6b517f16bc --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.Map; +import org.apache.iceberg.rest.HTTPHeaders; +import org.junit.jupiter.api.Test; + +class TestBasicAuthManager { + + @Test + void missingUsername() { + try (AuthManager authManager = new BasicAuthManager()) { + assertThatThrownBy(() -> authManager.mainSession(null, Map.of())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Invalid username: missing required property %s", AuthProperties.BASIC_USERNAME); + } + } + + @Test + void missingPassword() { + try (AuthManager authManager = new BasicAuthManager()) { + Map properties = Map.of(AuthProperties.BASIC_USERNAME, "alice"); + assertThatThrownBy(() -> authManager.mainSession(null, properties)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Invalid password: missing required property %s", AuthProperties.BASIC_PASSWORD); + } + } + + @Test + void success() { + Map properties = + Map.of(AuthProperties.BASIC_USERNAME, "alice", AuthProperties.BASIC_PASSWORD, "secret"); + try (AuthManager authManager = new BasicAuthManager(); + AuthSession session = authManager.mainSession(null, properties)) { + assertThat(session) + .isEqualTo( + DefaultAuthSession.of(HTTPHeaders.of(OAuth2Util.basicAuthHeaders("alice:secret")))); + } + } +} diff --git a/core/src/test/java/org/apache/iceberg/rest/auth/TestDefaultAuthSession.java b/core/src/test/java/org/apache/iceberg/rest/auth/TestDefaultAuthSession.java new file mode 100644 index 000000000000..9a534913102d --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/rest/auth/TestDefaultAuthSession.java @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.auth; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; +import org.apache.iceberg.rest.HTTPHeaders; +import org.apache.iceberg.rest.HTTPHeaders.HTTPHeader; +import org.apache.iceberg.rest.HTTPRequest; +import org.apache.iceberg.rest.HTTPRequest.HTTPMethod; +import org.apache.iceberg.rest.ImmutableHTTPRequest; +import org.junit.jupiter.api.Test; + +class TestDefaultAuthSession { + + @Test + void authenticate() { + + try (DefaultAuthSession session = + DefaultAuthSession.of(HTTPHeaders.of(HTTPHeader.of("Authorization", "s3cr3t")))) { + + HTTPRequest original = + ImmutableHTTPRequest.builder() + .method(HTTPMethod.GET) + .baseUri(URI.create("https://localhost")) + .path("path") + .build(); + + HTTPRequest authenticated = session.authenticate(original); + + assertThat(authenticated.headers().entries()) + .singleElement() + .extracting(HTTPHeader::name, HTTPHeader::value) + .containsExactly("Authorization", "s3cr3t"); + } + } + + @Test + void authenticateWithConflictingHeader() { + + try (DefaultAuthSession session = + DefaultAuthSession.of(HTTPHeaders.of(HTTPHeader.of("Authorization", "s3cr3t")))) { + + HTTPRequest original = + ImmutableHTTPRequest.builder() + .method(HTTPMethod.GET) + .baseUri(URI.create("https://localhost")) + .path("path") + .headers(HTTPHeaders.of(HTTPHeader.of("Authorization", "other"))) + .build(); + + HTTPRequest authenticated = session.authenticate(original); + + assertThat(authenticated).isSameAs(original); + } + } +} From 28db2396aedd305d3aaaf622e6f968573ac081ad Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Wed, 18 Dec 2024 17:06:49 +0100 Subject: [PATCH 2/6] [review] close() --- .../main/java/org/apache/iceberg/rest/auth/AuthManager.java | 4 +--- .../java/org/apache/iceberg/rest/auth/BasicAuthManager.java | 5 +++++ .../java/org/apache/iceberg/rest/auth/NoopAuthManager.java | 5 +++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java index 4e321f619723..191fde3a269e 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java @@ -102,7 +102,5 @@ default AuthSession tableSession( *

This method is called when the owning catalog is closed. */ @Override - default void close() { - // Do nothing - } + void close(); } diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java index 007941fc9ee7..8e08890bb1de 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java @@ -41,4 +41,9 @@ public AuthSession mainSession(RESTClient sharedClient, Map prop String credentials = username + ":" + password; return DefaultAuthSession.of(HTTPHeaders.of(OAuth2Util.basicAuthHeaders(credentials))); } + + @Override + public void close() { + // no resources to close + } } diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java index 589d372faa80..bb7fce914775 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java @@ -28,4 +28,9 @@ public class NoopAuthManager implements AuthManager { public AuthSession mainSession(RESTClient sharedClient, Map properties) { return AuthSession.EMPTY; } + + @Override + public void close() { + // no resources to close + } } From df4243ba2815287d624aa4d109e344e58392fd36 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Wed, 18 Dec 2024 17:08:09 +0100 Subject: [PATCH 3/6] [review] nits --- .../main/java/org/apache/iceberg/rest/auth/AuthManagers.java | 1 - .../org/apache/iceberg/rest/auth/TestDefaultAuthSession.java | 2 -- 2 files changed, 3 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java index 7f1d1573d297..948f549e02da 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java @@ -31,7 +31,6 @@ public class AuthManagers { private AuthManagers() {} public static AuthManager loadAuthManager(String name, Map properties) { - String authType = properties.getOrDefault(AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_NONE); diff --git a/core/src/test/java/org/apache/iceberg/rest/auth/TestDefaultAuthSession.java b/core/src/test/java/org/apache/iceberg/rest/auth/TestDefaultAuthSession.java index 9a534913102d..f6fee42e0d52 100644 --- a/core/src/test/java/org/apache/iceberg/rest/auth/TestDefaultAuthSession.java +++ b/core/src/test/java/org/apache/iceberg/rest/auth/TestDefaultAuthSession.java @@ -32,7 +32,6 @@ class TestDefaultAuthSession { @Test void authenticate() { - try (DefaultAuthSession session = DefaultAuthSession.of(HTTPHeaders.of(HTTPHeader.of("Authorization", "s3cr3t")))) { @@ -54,7 +53,6 @@ void authenticate() { @Test void authenticateWithConflictingHeader() { - try (DefaultAuthSession session = DefaultAuthSession.of(HTTPHeaders.of(HTTPHeader.of("Authorization", "s3cr3t")))) { From e3c4b311de21194ac82e4c450cb72d9f393beba3 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Wed, 18 Dec 2024 17:47:17 +0100 Subject: [PATCH 4/6] HTTPAuthSession close() --- .../org/apache/iceberg/rest/auth/AuthSession.java | 15 +++++++++++---- .../iceberg/rest/auth/DefaultAuthSession.java | 5 +++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/AuthSession.java b/core/src/main/java/org/apache/iceberg/rest/auth/AuthSession.java index 169a53d7a8f2..eed7caf84572 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/AuthSession.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/AuthSession.java @@ -30,7 +30,16 @@ public interface AuthSession extends AutoCloseable { /** An empty session that does nothing. */ - AuthSession EMPTY = request -> request; + AuthSession EMPTY = + new AuthSession() { + @Override + public HTTPRequest authenticate(HTTPRequest request) { + return request; + } + + @Override + public void close() {} + }; /** * Authenticates the given request and returns a new request with the necessary authentication. @@ -44,7 +53,5 @@ public interface AuthSession extends AutoCloseable { * the cache, or the cache itself is closed. */ @Override - default void close() { - // Do nothing - } + void close(); } diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/DefaultAuthSession.java b/core/src/main/java/org/apache/iceberg/rest/auth/DefaultAuthSession.java index f1d033704a6a..002f47459dd7 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/DefaultAuthSession.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/DefaultAuthSession.java @@ -46,6 +46,11 @@ default HTTPRequest authenticate(HTTPRequest request) { : ImmutableHTTPRequest.builder().from(request).headers(headers).build(); } + @Override + default void close() { + // no resources to close + } + static DefaultAuthSession of(HTTPHeaders headers) { return ImmutableDefaultAuthSession.builder().headers(headers).build(); } From 52f4423858a7e217fba223d06bda0e25c1a440a5 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Thu, 19 Dec 2024 21:37:47 +0100 Subject: [PATCH 5/6] mainSession -> catalogSession --- .../main/java/org/apache/iceberg/rest/auth/AuthManager.java | 4 ++-- .../java/org/apache/iceberg/rest/auth/BasicAuthManager.java | 2 +- .../java/org/apache/iceberg/rest/auth/NoopAuthManager.java | 2 +- .../org/apache/iceberg/rest/auth/TestBasicAuthManager.java | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java index 191fde3a269e..8f6f16f925e3 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManager.java @@ -43,7 +43,7 @@ public interface AuthManager extends AutoCloseable { *

This method cannot return null. By default, it returns the catalog session. */ default AuthSession initSession(RESTClient initClient, Map properties) { - return mainSession(initClient, properties); + return catalogSession(initClient, properties); } /** @@ -60,7 +60,7 @@ default AuthSession initSession(RESTClient initClient, Map prope *

It is not required to cache the returned session internally, as the catalog will keep it * alive for the lifetime of the catalog. */ - AuthSession mainSession(RESTClient sharedClient, Map properties); + AuthSession catalogSession(RESTClient sharedClient, Map properties); /** * Returns a session for a specific context. diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java index 8e08890bb1de..d19fc7302885 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java @@ -27,7 +27,7 @@ public final class BasicAuthManager implements AuthManager { @Override - public AuthSession mainSession(RESTClient sharedClient, Map properties) { + public AuthSession catalogSession(RESTClient sharedClient, Map properties) { Preconditions.checkArgument( properties.containsKey(AuthProperties.BASIC_USERNAME), "Invalid username: missing required property %s", diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java index bb7fce914775..0dc617ecc12c 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java @@ -25,7 +25,7 @@ public class NoopAuthManager implements AuthManager { @Override - public AuthSession mainSession(RESTClient sharedClient, Map properties) { + public AuthSession catalogSession(RESTClient sharedClient, Map properties) { return AuthSession.EMPTY; } diff --git a/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java b/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java index 4f6b517f16bc..6c4de441ae2a 100644 --- a/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java +++ b/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java @@ -30,7 +30,7 @@ class TestBasicAuthManager { @Test void missingUsername() { try (AuthManager authManager = new BasicAuthManager()) { - assertThatThrownBy(() -> authManager.mainSession(null, Map.of())) + assertThatThrownBy(() -> authManager.catalogSession(null, Map.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessage( "Invalid username: missing required property %s", AuthProperties.BASIC_USERNAME); @@ -41,7 +41,7 @@ void missingUsername() { void missingPassword() { try (AuthManager authManager = new BasicAuthManager()) { Map properties = Map.of(AuthProperties.BASIC_USERNAME, "alice"); - assertThatThrownBy(() -> authManager.mainSession(null, properties)) + assertThatThrownBy(() -> authManager.catalogSession(null, properties)) .isInstanceOf(IllegalArgumentException.class) .hasMessage( "Invalid password: missing required property %s", AuthProperties.BASIC_PASSWORD); @@ -53,7 +53,7 @@ void success() { Map properties = Map.of(AuthProperties.BASIC_USERNAME, "alice", AuthProperties.BASIC_PASSWORD, "secret"); try (AuthManager authManager = new BasicAuthManager(); - AuthSession session = authManager.mainSession(null, properties)) { + AuthSession session = authManager.catalogSession(null, properties)) { assertThat(session) .isEqualTo( DefaultAuthSession.of(HTTPHeaders.of(OAuth2Util.basicAuthHeaders("alice:secret")))); From 2a3f4afd4b124c3999dd80d2423d2a9132fd0309 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Thu, 19 Dec 2024 21:41:17 +0100 Subject: [PATCH 6/6] AuthManager constructor --- .../java/org/apache/iceberg/rest/auth/AuthManagers.java | 1 - .../java/org/apache/iceberg/rest/auth/BasicAuthManager.java | 4 ++++ .../java/org/apache/iceberg/rest/auth/NoopAuthManager.java | 4 ++++ .../org/apache/iceberg/rest/auth/TestBasicAuthManager.java | 6 +++--- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java index 948f549e02da..42c2b1eeba83 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/AuthManagers.java @@ -53,7 +53,6 @@ public static AuthManager loadAuthManager(String name, Map prope DynConstructors.builder(AuthManager.class) .loader(AuthManagers.class.getClassLoader()) .impl(impl, String.class) // with name - .impl(impl) // without name .buildChecked(); } catch (NoSuchMethodException e) { throw new IllegalArgumentException( diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java index d19fc7302885..d0d56d3d3794 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/BasicAuthManager.java @@ -26,6 +26,10 @@ /** An auth manager that adds static BASIC authentication data to outgoing HTTP requests. */ public final class BasicAuthManager implements AuthManager { + public BasicAuthManager(String ignored) { + // no-op + } + @Override public AuthSession catalogSession(RESTClient sharedClient, Map properties) { Preconditions.checkArgument( diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java b/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java index 0dc617ecc12c..d706d78ef3ae 100644 --- a/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java +++ b/core/src/main/java/org/apache/iceberg/rest/auth/NoopAuthManager.java @@ -24,6 +24,10 @@ /** An auth manager that does not add any authentication data to outgoing HTTP requests. */ public class NoopAuthManager implements AuthManager { + public NoopAuthManager(String ignored) { + // no-op + } + @Override public AuthSession catalogSession(RESTClient sharedClient, Map properties) { return AuthSession.EMPTY; diff --git a/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java b/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java index 6c4de441ae2a..c34654cdeff5 100644 --- a/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java +++ b/core/src/test/java/org/apache/iceberg/rest/auth/TestBasicAuthManager.java @@ -29,7 +29,7 @@ class TestBasicAuthManager { @Test void missingUsername() { - try (AuthManager authManager = new BasicAuthManager()) { + try (AuthManager authManager = new BasicAuthManager("test")) { assertThatThrownBy(() -> authManager.catalogSession(null, Map.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessage( @@ -39,7 +39,7 @@ void missingUsername() { @Test void missingPassword() { - try (AuthManager authManager = new BasicAuthManager()) { + try (AuthManager authManager = new BasicAuthManager("test")) { Map properties = Map.of(AuthProperties.BASIC_USERNAME, "alice"); assertThatThrownBy(() -> authManager.catalogSession(null, properties)) .isInstanceOf(IllegalArgumentException.class) @@ -52,7 +52,7 @@ void missingPassword() { void success() { Map properties = Map.of(AuthProperties.BASIC_USERNAME, "alice", AuthProperties.BASIC_PASSWORD, "secret"); - try (AuthManager authManager = new BasicAuthManager(); + try (AuthManager authManager = new BasicAuthManager("test"); AuthSession session = authManager.catalogSession(null, properties)) { assertThat(session) .isEqualTo(