Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't use non-access key credentials to send SMS messages via SNS #10

Open
alex-ng-wesoft opened this issue May 5, 2022 · 1 comment

Comments

@alex-ng-wesoft
Copy link
Contributor

Issue

There are various ways to provide credentials to the AWS SDK, with varying appropriateness based on the context the library is used.
For example, when running a PHP app that includes this library in an EC2 instance it makes sense to use the instance's instance profile / IAM role as the credentials. This is more secure and less of a hassle than setting up a dedicated IAM user.

However, this library currently forces an access key and secret to be used, so users aren't given the flexibility to pick the most appropriate way of providing credentials.
It would be nice to provide a mechanism to allow this to be possible.

Existing solutions

An example of doing this is how Laravel handles SES - it takes the services.ses config and merges it with some defaults, but otherwise passes through the config to the AWS SDK SesClient class.
This makes it much easier to change what gets passed to SesClient without having to modify Laravel itself.

Proposal 1

Directly pass through the otp.aws.sns config to the SnsClient class.
Since the structure of the config (at least in the default otp.php config file) matches what the class expects, this will allow users to simply change the otp.aws.sns array as needed.
Supporting something like instance profiles can be as easy as just deleting the credentials array.

This change should mostly be backwards compatible, but it will pass through values such as otp.aws.sns.profile that were previously ignored.
Such custom values can end up changing how the SDK behaves in an unexpected way.

Proposal 2

Add fallback support for services.sns config (if it exists), and just pass it through to the SnsClient class directly.
Only respect the config if the otp.aws.sns config is not set.

This works for the same reason proposal 1 works, but is guaranteed to be backwards compatible:

  1. How the otp.aws.sns config is handled remains unchanged.
  2. All existing users of the library should have the otp.aws.sns config set, so even if they also happened to have services.sns set the otp.aws.sns config would override it.

This proposal has the extra advantage of allowing users of the library to specify their SNS config in a single location if they use something else that also respects services.sns.
An example of this is the laravel-notification-channels/aws-sns library.

@ferdousulhaque
Copy link
Owner

I got your point, right, most of the people who use AWS, do it over the profile rather than access key and secret now a days. Same in my last employer, we also used to use the AWS profile. Let me fix it shortly. Thanks for your suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants