Skip to content

Commit

Permalink
Map Forbidden File IO exceptions to 403 (apache#402)
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrew4699
andrew4699 authored Oct 29, 2024
1 parent 2494dd0 commit 9e8a87a
Show file tree
Hide file tree
Showing 13 changed files with 814 additions and 391 deletions.
2 changes: 2 additions & 0 deletions polaris-service/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ dependencies {
implementation("software.amazon.awssdk:sts")
implementation("software.amazon.awssdk:iam-policy-builder")
implementation("software.amazon.awssdk:s3")
implementation(platform(libs.azuresdk.bom))
implementation("com.azure:azure-core")

testImplementation("org.apache.iceberg:iceberg-api:${libs.versions.iceberg.get()}:tests")
testImplementation("org.apache.iceberg:iceberg-core:${libs.versions.iceberg.get()}:tests")
Expand Down
16 changes: 12 additions & 4 deletions polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -100,6 +101,7 @@
import org.apache.polaris.core.storage.StorageLocation;
import org.apache.polaris.core.storage.aws.PolarisS3FileIOClientFactory;
import org.apache.polaris.service.catalog.io.FileIOFactory;
import org.apache.polaris.service.exception.IcebergExceptionMapper;
import org.apache.polaris.service.task.TaskExecutor;
import org.apache.polaris.service.types.NotificationRequest;
import org.apache.polaris.service.types.NotificationType;
Expand Down Expand Up @@ -2099,12 +2101,18 @@ private static boolean isStorageProviderRetryableException(Exception ex) {
}

private static boolean isAccessDenied(String errorMsg) {
// corresponding error messages for storage providers Aws/Azure/Gcp
// Corresponding error messages for storage providers Aws/Azure/Gcp
boolean isAccessDenied =
errorMsg != null
&& (errorMsg.contains("Access Denied")
|| errorMsg.contains("This request is not authorized to perform this operation")
|| errorMsg.contains("Forbidden"));
&& (errorMsg
.toLowerCase(Locale.ENGLISH)
.contains(IcebergExceptionMapper.AWS_ACCESS_DENIED_HINT)
|| errorMsg
.toLowerCase(Locale.ENGLISH)
.contains(IcebergExceptionMapper.AZURE_ACCESS_DENIED_HINT)
|| errorMsg
.toLowerCase(Locale.ENGLISH)
.contains(IcebergExceptionMapper.GCP_ACCESS_DENIED_HINT));
if (isAccessDenied) {
LOGGER.debug("Access Denied or Forbidden error: {}", errorMsg);
return true;
Expand Down
33 changes: 33 additions & 0 deletions ...is-service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@
*/
package org.apache.polaris.service.exception;

import com.azure.core.exception.AzureException;
import com.google.cloud.storage.StorageException;
import jakarta.ws.rs.WebApplicationException;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
import java.util.Arrays;
import java.util.Locale;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.CherrypickAncestorCommitException;
import org.apache.iceberg.exceptions.CleanableFailure;
Expand All @@ -45,10 +50,20 @@
import org.apache.iceberg.rest.responses.ErrorResponse;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.awssdk.services.s3.model.S3Exception;

public class IcebergExceptionMapper implements ExceptionMapper<RuntimeException> {
private static final Logger LOGGER = LoggerFactory.getLogger(IcebergExceptionMapper.class);

// Case-insensitive parts of exception messages that a request to a cloud provider was denied due
// to lack of permissions
// We may want to consider a change to Iceberg Core to wrap cloud provider IO exceptions to
// Iceberg ForbiddenException
public static final String AWS_ACCESS_DENIED_HINT = "access denied";
public static final String AZURE_ACCESS_DENIED_HINT =
"this request is not authorized to perform this operation";
public static final String GCP_ACCESS_DENIED_HINT = "forbidden";

public IcebergExceptionMapper() {}

@Override
Expand Down Expand Up @@ -79,6 +94,14 @@ public Response toResponse(RuntimeException runtimeException) {
case RESTException e -> Response.Status.SERVICE_UNAVAILABLE.getStatusCode();
case IllegalArgumentException e -> Response.Status.BAD_REQUEST.getStatusCode();
case UnsupportedOperationException e -> Response.Status.NOT_ACCEPTABLE.getStatusCode();
case S3Exception e when doesAnyThrowableContainInsensitive(e, AWS_ACCESS_DENIED_HINT) ->
Response.Status.FORBIDDEN.getStatusCode();
case AzureException e when doesAnyThrowableContainInsensitive(
e, AZURE_ACCESS_DENIED_HINT) ->
Response.Status.FORBIDDEN.getStatusCode();
case StorageException e when doesAnyThrowableContainInsensitive(
e, GCP_ACCESS_DENIED_HINT) ->
Response.Status.FORBIDDEN.getStatusCode();
case WebApplicationException e -> e.getResponse().getStatus();
default -> Response.Status.INTERNAL_SERVER_ERROR.getStatusCode();
};
Expand All @@ -100,4 +123,14 @@ public Response toResponse(RuntimeException runtimeException) {
LOGGER.debug("Mapped exception to errorResp: {}", errorResp);
return errorResp;
}

/**
* @return whether any throwable in the exception chain case-insensitive-contains the given
* message
*/
static boolean doesAnyThrowableContainInsensitive(Exception e, String message) {
String messageLower = message.toLowerCase(Locale.ENGLISH);
return Arrays.stream(ExceptionUtils.getThrowables(e))
.anyMatch(t -> t.getMessage().toLowerCase(Locale.ENGLISH).contains(messageLower));
}
}
13 changes: 6 additions & 7 deletions polaris-service/src/test/java/org/apache/polaris/service/catalog/BasePolarisCatalogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.iceberg.types.Types.NestedField.required;
import static org.apache.polaris.service.exception.IcebergExceptionMapper.*;
import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -86,7 +87,7 @@
import org.apache.polaris.service.admin.PolarisAdminService;
import org.apache.polaris.service.catalog.io.DefaultFileIOFactory;
import org.apache.polaris.service.catalog.io.FileIOFactory;
import org.apache.polaris.service.catalog.io.MeasuredFileIOFactory;
import org.apache.polaris.service.catalog.io.TestFileIOFactory;
import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory;
import org.apache.polaris.service.task.TableCleanupTaskHandler;
import org.apache.polaris.service.task.TaskExecutor;
Expand Down Expand Up @@ -1452,11 +1453,9 @@ private void createNonExistingNamespaces(Namespace namespace) {

@Test
public void testRetriableException() {
RuntimeException s3Exception = new RuntimeException("Access Denied");
RuntimeException azureBlobStorageException =
new RuntimeException(
"This request is not authorized to perform this operation using this permission");
RuntimeException gcsException = new RuntimeException("Forbidden");
RuntimeException s3Exception = new RuntimeException(AWS_ACCESS_DENIED_HINT);
RuntimeException azureBlobStorageException = new RuntimeException(AZURE_ACCESS_DENIED_HINT);
RuntimeException gcsException = new RuntimeException(GCP_ACCESS_DENIED_HINT);
RuntimeException otherException = new RuntimeException(new IOException("Connection reset"));
Assertions.assertThat(BasePolarisCatalog.SHOULD_RETRY_REFRESH_PREDICATE.test(s3Exception))
.isFalse();
Expand All @@ -1478,7 +1477,7 @@ public void testFileIOWrapper() {
new PolarisPassthroughResolutionView(
callContext, entityManager, authenticatedRoot, CATALOG_NAME);

MeasuredFileIOFactory measured = new MeasuredFileIOFactory();
TestFileIOFactory measured = new TestFileIOFactory();
BasePolarisCatalog catalog =
new BasePolarisCatalog(
entityManager,
Expand Down
137 changes: 17 additions & 120 deletions ...e/src/test/java/org/apache/polaris/service/catalog/PolarisRestCatalogIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.iceberg.BaseTable;
import org.apache.iceberg.BaseTransaction;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.Table;
Expand All @@ -51,16 +50,13 @@
import org.apache.iceberg.UpdateSchema;
import org.apache.iceberg.catalog.CatalogTests;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.SessionCatalog;
import org.apache.iceberg.catalog.TableCommit;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.ForbiddenException;
import org.apache.iceberg.expressions.Expressions;
import org.apache.iceberg.io.ResolvingFileIO;
import org.apache.iceberg.rest.HTTPClient;
import org.apache.iceberg.rest.RESTCatalog;
import org.apache.iceberg.rest.auth.OAuth2Properties;
import org.apache.iceberg.rest.responses.ErrorResponse;
import org.apache.iceberg.types.Types;
import org.apache.polaris.core.PolarisConfiguration;
Expand All @@ -84,7 +80,6 @@
import org.apache.polaris.core.entity.CatalogEntity;
import org.apache.polaris.core.entity.PolarisEntityConstants;
import org.apache.polaris.service.PolarisApplication;
import org.apache.polaris.service.auth.BasePolarisAuthenticator;
import org.apache.polaris.service.auth.TokenUtils;
import org.apache.polaris.service.config.PolarisApplicationConfig;
import org.apache.polaris.service.test.PolarisConnectionExtension;
Expand Down Expand Up @@ -221,129 +216,31 @@ public void before(
StorageConfigInfo.StorageTypeEnum.FILE, List.of("file://"))
: awsConfigModel)
.build();
try (Response response =
EXT.client()
.target(
String.format(
"http://localhost:%d/api/management/v1/catalogs", EXT.getLocalPort()))
.request("application/json")
.header("Authorization", "Bearer " + adminToken.token())
.header(REALM_PROPERTY_KEY, realm)
.post(Entity.json(catalog))) {
assertThat(response)
.returns(Response.Status.CREATED.getStatusCode(), Response::getStatus);
}

// Create a new CatalogRole that has CATALOG_MANAGE_CONTENT and CATALOG_MANAGE_ACCESS
CatalogRole newRole = new CatalogRole("custom-admin");
try (Response response =
EXT.client()
.target(
String.format(
"http://localhost:%d/api/management/v1/catalogs/%s/catalog-roles",
EXT.getLocalPort(), currentCatalogName))
.request("application/json")
.header("Authorization", "Bearer " + adminToken.token())
.header(REALM_PROPERTY_KEY, realm)
.post(Entity.json(newRole))) {
assertThat(response)
.returns(Response.Status.CREATED.getStatusCode(), Response::getStatus);
}
CatalogGrant grantResource =
new CatalogGrant(
CatalogPrivilege.CATALOG_MANAGE_CONTENT, GrantResource.TypeEnum.CATALOG);
try (Response response =
EXT.client()
.target(
String.format(
"http://localhost:%d/api/management/v1/catalogs/%s/catalog-roles/custom-admin/grants",
EXT.getLocalPort(), currentCatalogName))
.request("application/json")
.header("Authorization", "Bearer " + adminToken.token())
.header(REALM_PROPERTY_KEY, realm)
.put(Entity.json(grantResource))) {
assertThat(response)
.returns(Response.Status.CREATED.getStatusCode(), Response::getStatus);
}
CatalogGrant grantAccessResource =
new CatalogGrant(
CatalogPrivilege.CATALOG_MANAGE_ACCESS, GrantResource.TypeEnum.CATALOG);
try (Response response =
EXT.client()
.target(
String.format(
"http://localhost:%d/api/management/v1/catalogs/%s/catalog-roles/custom-admin/grants",
EXT.getLocalPort(), currentCatalogName))
.request("application/json")
.header("Authorization", "Bearer " + adminToken.token())
.header(REALM_PROPERTY_KEY, realm)
.put(Entity.json(grantAccessResource))) {
assertThat(response)
.returns(Response.Status.CREATED.getStatusCode(), Response::getStatus);
}

// Assign this new CatalogRole to the service_admin PrincipalRole
try (Response response =
EXT.client()
.target(
String.format(
"http://localhost:%d/api/management/v1/catalogs/%s/catalog-roles/custom-admin",
EXT.getLocalPort(), currentCatalogName))
.request("application/json")
.header("Authorization", "Bearer " + adminToken.token())
.header(REALM_PROPERTY_KEY, realm)
.get()) {
assertThat(response)
.returns(Response.Status.OK.getStatusCode(), Response::getStatus);
CatalogRole catalogRole = response.readEntity(CatalogRole.class);
try (Response assignResponse =
EXT.client()
.target(
String.format(
"http://localhost:%d/api/management/v1/principal-roles/catalog-admin/catalog-roles/%s",
EXT.getLocalPort(), currentCatalogName))
.request("application/json")
.header("Authorization", "Bearer " + adminToken.token())
.header(REALM_PROPERTY_KEY, realm)
.put(Entity.json(catalogRole))) {
assertThat(assignResponse)
.returns(Response.Status.CREATED.getStatusCode(), Response::getStatus);
}
}

SessionCatalog.SessionContext context = SessionCatalog.SessionContext.createEmpty();
this.restCatalog =
new RESTCatalog(
context,
(config) ->
HTTPClient.builder(config)
.uri(config.get(CatalogProperties.URI))
.build());
Optional<RestCatalogConfig> restCatalogConfig =
Optional<PolarisRestCatalogIntegrationTest.RestCatalogConfig> restCatalogConfig =
testInfo
.getTestMethod()
.flatMap(m -> Optional.ofNullable(m.getAnnotation(RestCatalogConfig.class)));
ImmutableMap.Builder<String, String> propertiesBuilder =
ImmutableMap.<String, String>builder()
.put(
CatalogProperties.URI,
"http://localhost:" + EXT.getLocalPort() + "/api/catalog")
.put(
OAuth2Properties.CREDENTIAL,
snowmanCredentials.clientId() + ":" + snowmanCredentials.clientSecret())
.put(OAuth2Properties.SCOPE, BasePolarisAuthenticator.PRINCIPAL_ROLE_ALL)
.put(
CatalogProperties.FILE_IO_IMPL,
"org.apache.iceberg.inmemory.InMemoryFileIO")
.put("warehouse", currentCatalogName)
.put("header." + REALM_PROPERTY_KEY, realm);
.flatMap(
m ->
Optional.ofNullable(
m.getAnnotation(
PolarisRestCatalogIntegrationTest.RestCatalogConfig.class)));
ImmutableMap.Builder<String, String> extraPropertiesBuilder =
ImmutableMap.<String, String>builder();
restCatalogConfig.ifPresent(
config -> {
for (int i = 0; i < config.value().length; i += 2) {
propertiesBuilder.put(config.value()[i], config.value()[i + 1]);
extraPropertiesBuilder.put(config.value()[i], config.value()[i + 1]);
}
});
this.restCatalog.initialize("polaris", propertiesBuilder.buildKeepingLast());
restCatalog =
TestUtil.createSnowmanManagedCatalog(
EXT,
adminToken,
snowmanCredentials,
realm,
catalog,
extraPropertiesBuilder.build());
});
}

Expand Down
Loading

0 comments on commit 9e8a87a

Please sign in to comment.