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

Added usage of aws session token to use temporary credentials. #250

Merged
merged 5 commits into from Jun 3, 2022

Conversation

salyarka
Copy link
Contributor

@salyarka salyarka commented Jun 3, 2022

Allows to use temporary aws credentials with session token.

Copy link
Contributor

@pcraciunoiu pcraciunoiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the use case for this? I'm trying to understand how useful it would be to add it.

Comment on lines 24 to 27
settings.AWS_SESSION_TOKEN = "FwoGZXIvYXdzED8aDAILqEtZvcDCx+KsFCK1AUwcLbm4d+mAlRWYN+r1adKoIfwe/T117KNcql" \
"fbFFc6lgM1BQk9RepOZOyhNnx1ji12BMnA+Sc/9H1gi/QRt51U0EQVhcT7i9YZbipzrYMLpvxe0dwXwC7MTy7NQRkEMhpyXWgFw4Wz+" \
"pHdZTFI4DOEhjf/t1FcuV2jX0oS0Eqqck2YB6yY03FpQRFVFIKUFcyvt9kMP9F77iHkgnEWBxOVcfSxBHfgQDTCHCecMNDN02/u628o" \
"xK6elAYyLZu54kuwLAbe3hD2++FpbjCSF88DFWESks8o2PP489XCCCJrX/SnurGNfeWifA=="
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For testing, a shorter, fake value would be sufficient instead of these very long strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, made shorter string.

@salyarka
Copy link
Contributor Author

salyarka commented Jun 3, 2022

Use case for example: i want to use django-ses with temporary credenttials that i got from AssumeRole operation, and i cant do this, cause boto3.client call dont accepts session token that is required for using temporary credentials (to use temporary credentials you must provide aws_access_key_id, aws_secret_access_key and aws_session_token to boto3). This fork allows to use temporary credentials.

Copy link
Contributor

@pcraciunoiu pcraciunoiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK the use case makes sense. Could you update the README to document this usage as well?

django_ses/__init__.py Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@pcraciunoiu pcraciunoiu merged commit 63cc509 into django-ses:master Jun 3, 2022
@pcraciunoiu
Copy link
Contributor

I published this on PyPi as 3.1.0, but for some reason it's not picking it up. Maybe they have some issues. You can check later, if you want.

Thanks for your contribution!

@colehorvitz
Copy link

Correct me if I'm wrong, but this PR seems to only get halfway-there in terms of supporting temporary credentials. As the name suggests, these credentials have short lifespans and therefore need to be refreshed continually to work. This PR exposes some configs which Django reads once when the server starts. The credentials will inevitably expire, and then this package will no longer be able to open a connection.

I don't necessarily think that django-ses should be responsible for refreshing credentials. I do think there should be some way of updating SESBackend's _access_key_id, _access_key, and _session_token at runtime, so that it can open a connection with fresh credentials.

Currently, the only option I see available is to write a custom SESBackend, override send_messages() and either:

  1. Set AWS_SES_ACCESS_KEY_ID, AWS_SES_SECRET_ACCESS_KEY, and AWS_SES_SESSION_TOKEN at runtime
  2. Set _access_key_id, _access_key, and _session_token at runtime

The first is expressly warned against by Django, and the second is not ideal because we are overriding private variables (although it is the better of the two options).

Perhaps there should be some exposed way of doing this?

@pcraciunoiu
Copy link
Contributor

I'm not 100% sure that you are right, but it sounds right :) And so if you want to PR an approach that is more elegant than option 2, I'd take a look at it.

I'm not actively using this on a project now, so it's tricky for me to test it, and I would trust that you'd be able to test it in a real environment for a bit before we merge it.

@colehorvitz
Copy link

@pcraciunoiu After looking at the way django-storages handles this, it seems they expose an AWS_S3_SESSION_PROFILE config. AWS profiles support temporary credentials out of the box, so my instinct is to think that doing something similar to django-storages would be the best. That is, provide an alternative means of authenticating via AWS_SES_SESSION_PROFILE.

I'm not a boto/AWS expert in general, but I'd be happy to take a crack at this.

@pcraciunoiu
Copy link
Contributor

@colehorvitz that sounds reasonable to me, I see it in use here and should be fairly straightforward
https://github.com/jschneier/django-storages/blob/70fa8d96665f0c5e44aacd6c782f6489fa8dc11e/storages/backends/s3.py#L313

I'm not an expert either, and there seem to be many ways to specify credentials!

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

Successfully merging this pull request may close these issues.

None yet

3 participants