Add Session Token to Amazon Mailer #42982
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
|
I don't know why the PhpUnit with PHP 8.0 fails but it seems unrelated to my PR. Maybe somebody can trigger a rerun. Error: |
|
Hey, thank you for this PR. Is it a duplicate of #40193 ? Having a That's said. I realize that the Note: In the Laravel PR you linked, the DSN is built from the Why not using Env variable like suggested in #40193 ? |
|
Yeah it is a duplicate of the PR. In Laravel the token is passed through the config right now. I am not sure why though and I am not sure when and where it is explicitly set. It was introduced in the following PR for SES and it seems like it is intended for the config: It was expanded to SQS and S3 in the following PR: (unrelated for this PR though) I would forward the question to @driesvints and @taylorotwell I think it would be possible using ENV. But I am unsure what happens if you maybe change the SESSION_TOKEN between two different SesTransports. Example you want to send an email with one configuration to the users and then send another internal email to the admins using another configuration. (Probably unlikely but possible) |
There was a problem hiding this comment.
regarding laravel/framework#23766 the simplest way is to NOT convert env variable in config, and let SDK deal with the env variable instead.
return [
'ses' => [
- 'key' => env('AWS_ACCESS_KEY_ID'),
- 'secret' => env('AWS_SECRET_ACCESS_KEY'),
- 'token' => env('AWS_SESSION_TOKEN'),
'region' => env('AWS_REGION', 'eu-west-1'), // e.g. us-east-1
],
];but still... People fetching config in another way (lets say people use an internal mechanism to fetch the session token that is not an env variable) will be stuck. I'm 👍 for this PR
|
Using the config has another advantage: Because the access_key and secret is loaded from the config in laravel. It doesn't matter how the env variables is named in the Before 2018 the variables in the default config also were named differently and were then renamed: Some people don't upgrade the default configuration for the configs. That would be an addionational upgrade step. |
93eb5e0 to
fdeec77
Compare
|
Thank you @Jubeki. |
This PR provide's support for Amazons separate Session Token feature in both the API and HTTPS transports. I've added the ability to set the session token via a query parameter in the DSN.
Right now in Laravel users have the ability to use a Session Token together with temporary Access and Secret keys for sending emails. But unfortunately Symfony Mailer, which we're switching to for the upcoming Laravel v9 release doesn't has this feature yet. That's why I decided to send in this PR so both Symfony and Laravel users can enjoy this feature from SES / Amazon. This PR is needed for laravel/framework#38481
Documentation for the Session Token with temporary credentials can be found here: https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_temp_use-resources.html
Documentation for creating
AsyncAws/Ses/SesClientcan be found here: https://async-aws.com/configuration.html#sessiontokenCurrent setup through Laravel can be found here: https://github.com/laravel/framework/blob/8.x/src/Illuminate/Mail/MailManager.php#L258-L293
(It seems like Laravel has not documented this feature yet. I will be sending a PR to the Laravel docs and then update this PR accordingly)
@driesvints maybe you can also take a look at this PR.