-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[5.6] Allowing IAM role SESSION_TOKEN to be used with SES #23766
Conversation
Omg +1 this is needed, i was recently doing some work with this and took me ages to figure out why it was breaking. Absolutely need this! +1+1+1 |
@@ -91,7 +91,7 @@ protected function createSesDriver() | |||
protected function addSesCredentials(array $config) | |||
{ | |||
if ($config['key'] && $config['secret']) { | |||
$config['credentials'] = Arr::only($config, ['key', 'secret']); | |||
$config['credentials'] = Arr::only($config, ['key', 'secret', 'token']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if token
is not defined in the array at this point? Does Arr::only()
handle that gracefully?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I've been away for the last couple of days for Easter.
From looking at the Arr::only function, all it does is an intersection:
return array_intersect_key($array, array_flip((array) $keys));
So, running that through PHP with all 3 gives:
Just having 'key' and 'secret' in the config gives:
So it handles the lack of the token gracefully based off the source code.
I can check with the actual code tonight once I've finished work, however based off the check now, it's handled gracefully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can confirm it has no effect on existing functionality. Our current local development environment for the given project only contains key and secret, yet our live servers with the above change contain key, secret and token, and both send emails as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any progress on this guys?
Ping. |
By default, the SES driver allows for the AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY to be passed to the SES driver - however for instances using an IAM role, rather than these credentials in the .env file, sending an email will fail with an invalid Session Token error.
This change adds the token attribute in to allow for the AWS_SESSION_TOKEN to be passed in. Config would look something like:
If the token does not exist, or if the request does not rely on one, this option is safely ignored as the AWS SDK gracefully ignores it (and uses null as default).