Merge pull request #60 from aws/bug_fix

BUG FIX - adding support to operations that do not take bucket as an input

src/it/java/software/amazon/awssdk/s3accessgrants/plugin/S3AccessGrantsIntegrationTests.java

@@ -23,6 +23,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
 import java.util.stream.Collectors;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -57,6 +58,7 @@
 import software.amazon.awssdk.services.s3control.model.GetDataAccessResponse;
 import software.amazon.awssdk.services.s3control.model.Credentials;
 import software.amazon.awssdk.services.s3control.model.GetAccessGrantsInstanceForPrefixRequest;
+import software.amazon.awssdk.services.sts.model.StsException;
 
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
@@ -749,7 +751,8 @@ public void call_s3_with_plugin_invalid_default_credentials_provider_request_fai
                     S3AccessGrantsIntegrationTestsUtils.TEST_OBJECT1);
 
             Assert.fail("Expected an exception to occur as as a valid credentials is not provided to talk to access grants!");
-        } catch (IllegalArgumentException e) {
+        } catch (CompletionException e) {
+            Assertions.assertThat(e.getCause()).isInstanceOf(StsException.class);
             verify(accessGrantsPlugin, times(1)).configureClient(any());
         }
     }

src/main/java/software/amazon/awssdk/s3accessgrants/plugin/S3AccessGrantsAuthSchemeProvider.java

@@ -52,7 +52,7 @@ public class S3AccessGrantsAuthSchemeProvider implements S3AuthSchemeProvider {
     public List<AuthSchemeOption> resolveAuthScheme(@NotNull S3AuthSchemeParams authSchemeParams) {
         S3AccessGrantsUtils.argumentNotNull(authSchemeParams,
                 "An internal exception has occurred. Valid auth scheme params were not passed to the Auth Scheme Provider. Please contact the S3 Access Grants plugin team!");
-        S3AccessGrantsUtils.argumentNotNull(authSchemeParams.bucket(), "An internal exception has occurred. expecting bucket name to be specified for the request. Please contact the S3 Access Grants plugin team!");
+
         List<AuthSchemeOption> availableAuthSchemes = authSchemeProvider.resolveAuthScheme(authSchemeParams);
         String S3Prefix = "s3://"+authSchemeParams.bucket()+"/"+getKeyIfExists(authSchemeParams);
 

src/main/java/software/amazon/awssdk/s3accessgrants/plugin/S3AccessGrantsIdentityProvider.java

@@ -140,6 +140,8 @@ public CompletableFuture<? extends AwsCredentialsIdentity> resolveIdentity(Resol
 
             Permission permission = permissionMapper.getPermission(operation);
 
+            verifyIfValidBucket(S3Prefix);
+
             logger.debug(() -> " permission : " + permission);
 
             return isCacheEnabled ? getCredentialsFromCache(userCredentials.join(), permission, S3Prefix, accountId) : getCredentialsFromAccessGrants(createDataAccessRequest(accountId, S3Prefix, permission, privilege));
@@ -152,6 +154,15 @@ public CompletableFuture<? extends AwsCredentialsIdentity> resolveIdentity(Resol
         }
     }
 
+    /**
+     * This method verifies if the user has specified a valid bucket for the operations supported by S3 Access Grants.
+     */
+    protected void verifyIfValidBucket(String prefix) {
+        if(prefix.split("/")[2].equals("null")) {
+            throw new IllegalArgumentException("Please specify a valid bucket name for the operation!");
+        }
+    }
+
     /**
      * This method will create a request to talk to access grants.
      * @param accountId the accountId that contains the access grant instance with the desired bucket location registered.

src/test/java/software/amazon/awssdk/s3accessgrants/plugin/S3AccessGrantsAuthSchemeProviderTests.java

@@ -72,19 +72,6 @@ public void call_authSchemeProvider_with_null_params() {
         verify(authSchemeProvider,never()).resolveAuthScheme(authSchemeParams);
     }
 
-    @Test
-    public void call_authSchemeProvider_with_invalid_params_null_bucket() {
-        S3AuthSchemeProvider authSchemeProvider = mock(S3AuthSchemeProvider.class);
-        S3AccessGrantsAuthSchemeProvider accessGrantsAuthSchemeProvider = new S3AccessGrantsAuthSchemeProvider(authSchemeProvider);
-        S3AuthSchemeParams authSchemeParams = S3AuthSchemeParams.builder().bucket(null).key(KEY).operation(OPERATION).build();
-
-        when(authSchemeProvider.resolveAuthScheme(authSchemeParams)).thenReturn(authSchemeResolverResult);
-
-        Assertions.assertThatThrownBy(()->accessGrantsAuthSchemeProvider.resolveAuthScheme(authSchemeParams)).isInstanceOf(IllegalArgumentException.class)
-                .hasMessage("An internal exception has occurred. expecting bucket name to be specified for the request. Please contact the S3 Access Grants plugin team!");
-        verify(authSchemeProvider, never()).resolveAuthScheme(authSchemeParams);
-    }
-
     @Test
     public void call_authSchemeProvider_with_valid_params_valid_bucket() {
         S3AuthSchemeProvider authSchemeProvider = mock(S3AuthSchemeProvider.class);

src/test/java/software/amazon/awssdk/s3accessgrants/plugin/S3AccessGrantsIdentityProviderTests.java

@@ -263,6 +263,14 @@ public void call_resolve_identity_with_invalid_request_params_operation() {
         Assertions.assertThatThrownBy(() -> accessGrantsIdentityProvider.resolveIdentity(localResolveIdentityRequest)).isInstanceOf(IllegalArgumentException.class);
     }
 
+    @Test
+    public void call_resolve_identity_with_invalid_request_params_bucket_in_prefix() {
+
+        S3AccessGrantsIdentityProvider accessGrantsIdentityProvider = new S3AccessGrantsIdentityProvider(credentialsProvider, TEST_REGION, stsAsyncClient, TEST_PRIVILEGE, TEST_CACHE_ENABLED, s3ControlClient, cache, TEST_FALLBACK_ENABLED, metricsPublisher);
+        Assertions.assertThatThrownBy(() -> accessGrantsIdentityProvider.verifyIfValidBucket("s3://null/*"))
+                .isInstanceOf(IllegalArgumentException.class);
+    }
+
     @Test
     public void call_get_data_access_with_invalid_response_without_cache() {
         IdentityProvider credentialsProvider = mock(IdentityProvider.class);

src/test/java/software/amazon/awssdk/s3accessgrants/plugin/S3AccessGrantsPluginTests.java

@@ -15,6 +15,7 @@
 
 package software.amazon.awssdk.s3accessgrants.plugin;
 
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.assertj.core.api.Assertions;
 import software.amazon.awssdk.core.SdkServiceClientConfiguration;
@@ -28,6 +29,11 @@ public class S3AccessGrantsPluginTests {
 
     private final String TEST_ACCOUNT = "123450013912";
 
+    @BeforeClass
+    public static void setUp() {
+        System.setProperty("aws.region", "us-east-2");
+    }
+
     @Test
     public void create_access_grants_plugin() {
        Assertions.assertThatNoException().isThrownBy(() -> S3AccessGrantsPlugin.builder().build());
@@ -121,9 +127,7 @@ public void call_configure_client_with_invalid_region_in_config() {
 
 
         Assertions.assertThatThrownBy(() -> accessGrantsPlugin.configureClient(sdkServiceClientConfiguration))
-                .isInstanceOf(SdkClientException.class);
-        // SDK supports default values for the plugin as well, which bypasses the custom validation.
-        // SDK will throw SdkClientException when it attempts to look for the credentials in the environment config and does not find any.
+                .isInstanceOf(IllegalArgumentException.class);
 
     }