java-slack-sdk icon indicating copy to clipboard operation
java-slack-sdk copied to clipboard

AmazonS3InstallationService should accept aws credential in constructor

Open smallufo opened this issue 2 years ago • 3 comments

https://slack.dev/java-slack-sdk/guides/app-distribution

// The standard AWS env variables are expected
// export AWS_REGION=us-east-1
// export AWS_ACCESS_KEY_ID=AAAA*************
// export AWS_SECRET_ACCESS_KEY=4o7***********************

// Please be careful about the security policies on this bucket.
String awsS3BucketName = "YOUR_OWN_BUCKET_NAME_HERE";

InstallationService installationService = new AmazonS3InstallationService(awsS3BucketName);

I realize you utilize DefaultAWSCredentialsProviderChain.getInstance().getCredentials() to retrieve credentials from env. But it truly sucks (lacks flexibility)

You should provide a constructor that accepts these 3 parameters in constructor : awsAccessKey , awsSecretKey , region , and build AWS credential in the constructor.

smallufo avatar Jun 08 '23 18:06 smallufo

Thanks for the feature suggestion! We would love a pull request for this addition if you think it is something you can contribute 😄

filmaj avatar Jun 09 '23 18:06 filmaj

BTW , it seems AWS_REGION , AWS_ACCESS_KEY_ID , AWS_SECRET_ACCESS_KEY is not sufficient. I set 3 values , and I see error message :

2023-06-11 18:22:52.407 [http-nio-8080-exec-1] WARN  c.a.i.InstanceMetadataServiceResourceFetcher - Fail to retrieve token 
com.amazonaws.SdkClientException: Failed to connect to service endpoint: 
	at com.amazonaws.internal.EC2ResourceFetcher.doReadResource(EC2ResourceFetcher.java:112)
	at com.amazonaws.internal.InstanceMetadataServiceResourceFetcher.getToken(InstanceMetadataServiceResourceFetcher.java:91)
	at com.amazonaws.internal.InstanceMetadataServiceResourceFetcher.readResource(InstanceMetadataServiceResourceFetcher.java:69)
	at com.amazonaws.internal.EC2ResourceFetcher.readResource(EC2ResourceFetcher.java:66)
	at com.amazonaws.util.EC2MetadataUtils.getItems(EC2MetadataUtils.java:407)
	at com.amazonaws.util.EC2MetadataUtils.getData(EC2MetadataUtils.java:376)
	at com.amazonaws.util.EC2MetadataUtils.getData(EC2MetadataUtils.java:372)
	at com.amazonaws.util.EC2MetadataUtils.getEC2InstanceRegion(EC2MetadataUtils.java:287)
	at com.amazonaws.regions.InstanceMetadataRegionProvider.tryDetectRegion(InstanceMetadataRegionProvider.java:59)
	at com.amazonaws.regions.InstanceMetadataRegionProvider.getRegion(InstanceMetadataRegionProvider.java:50)
	at com.amazonaws.regions.AwsRegionProviderChain.getRegion(AwsRegionProviderChain.java:46)
	at com.amazonaws.client.builder.AwsClientBuilder.determineRegionFromRegionProvider(AwsClientBuilder.java:475)
	at com.amazonaws.client.builder.AwsClientBuilder.setRegion(AwsClientBuilder.java:458)
	at com.amazonaws.client.builder.AwsClientBuilder.configureMutableProperties(AwsClientBuilder.java:424)
	at com.amazonaws.client.builder.AwsSyncClientBuilder.build(AwsSyncClientBuilder.java:46)
	at com.amazonaws.services.s3.AmazonS3ClientBuilder.defaultClient(AmazonS3ClientBuilder.java:54)
	at com.slack.api.bolt.service.builtin.AmazonS3OAuthStateService.createS3Client(AmazonS3OAuthStateService.java:91)
	at com.slack.api.bolt.service.builtin.AmazonS3OAuthStateService.lambda$initializer$0(AmazonS3OAuthStateService.java:43)
	at com.slack.api.bolt.App.initialize(App.java:544)
	at com.slack.api.bolt.App.start(App.java:557)
	at com.slack.api.bolt.App.run(App.java:588)

After digging into s3's code , it seems it needs additional 3 system properties : aws.region , aws.accessKeyId , aws.secretKey ( see SystemPropertiesCredentialsProvider : https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-core/src/main/java/com/amazonaws/auth/SystemPropertiesCredentialsProvider.java )

Anyway , I don't suggest using DefaultAWSCredentialsProviderChain as it assumes your server is running on AWS's image.

After I set the 3 values : aws.region , aws.accessKeyId , aws.secretKey . When event comes , bolt output :

INFO  c.s.a.b.m.b.MultiTeamsAuthorization - auth.test result: null, io error: null, api error: null

No stacktrace :( And I see no folder created under my s3 bucket .

I think maybe the AWS storage didn't run test outside AWS's image ?

While using FileInstallationService(config, "/tmp") , it correctly creates folders under /tmp/xxx

/tmp/xxx
total 0
drwxr-xr-x   4 smallufo  wheel  128  6 11 00:29 .
drwxrwxrwt  15 root      wheel  480  6 11 17:02 ..
drwxr-xr-x   4 smallufo  wheel  128  6 10 20:26 installation
drwxr-xr-x   6 smallufo  wheel  192  6 11 17:29 state

smallufo avatar Jun 11 '23 10:06 smallufo

Hey @smallufo

BTW , it seems AWS_REGION , AWS_ACCESS_KEY_ID , AWS_SECRET_ACCESS_KEY is not sufficient.

I don't think so. At least, this is not always true. From my past experience, the default resolver works very well with those env variables. As long as you provide sufficient env variables, Java AWS SDK works for you without any issues.

I don't have time to discuss details right now, but I recommend checking https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/auth/DefaultAWSCredentialsProviderChain.html The default credential resolver tries to read env variables first, then Java system properties, and so on. Also, I believe that it's the best recommended way to resolve credentials on AWS infra (and even on your local machine).

No stacktrace :( And I see no folder created under my s3 bucket .

What about enabling slf4j debug-level logging? The debug logs by this SDK and AWS SDK may provide more detailed information, which can be helpful for you.

Lastly, as for your PR to add a new constructor to add a new way to initialize the credentials, I am open to add such in addition to the current default one. But I still don't agree the current approach is not recommended. I will check your PR next week. Thanks again for your inputs and contribution.

seratch avatar Jun 11 '23 11:06 seratch