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.

Sign up for GitHub

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

[FEATURE] Use STS to receive a temporary credentials role session #12

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[FEATURE] Use STS to receive a temporary credentials role session #12

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43a83b7
Assume an IAM role using STS
MurraySpeight May 24, 2022
b69e213
Document the ckanext.s3filestore.aws_role
MurraySpeight May 24, 2022
a51f8d2
Fix flake8 indentation warning
MurraySpeight Jun 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ Conditional::

Optional::

# An optional role ARN that will be assumed when getting a session for S3
ckanext.s3filestore.aws_role = arn:aws:iam::123456789012:role/RoleName

# An optional path to prepend to keys
ckanext.s3filestore.aws_storage_path = my-site-name

Expand Down
14 changes: 13 additions & 1 deletion ckanext/s3filestore/uploader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def __init__(self):
self.bucket_name = config.get('ckanext.s3filestore.aws_bucket_name')
self.p_key = config.get('ckanext.s3filestore.aws_access_key_id')
self.s_key = config.get('ckanext.s3filestore.aws_secret_access_key')
self.role = config.get('ckanext.s3filestore.aws_role', None)
self.region = config.get('ckanext.s3filestore.region_name')
self.signature = config.get('ckanext.s3filestore.signature_version')
self.host_name = config.get('ckanext.s3filestore.host_name', None)
Expand All @@ -64,9 +65,20 @@ def get_directory(self, id, storage_path):
return directory

def get_s3_session(self):
return boto3.session.Session(aws_access_key_id=self.p_key,
session = boto3.session.Session(aws_access_key_id=self.p_key,
aws_secret_access_key=self.s_key,
region_name=self.region)
if self.role:
assumed_role_object = session.client('sts').assume_role(
RoleArn=self.role,
RoleSessionName="CkanExtS3Session")
credentials = assumed_role_object['Credentials']
return boto3.session.Session(
Copy link
Contributor

@ThrawnCA ThrawnCA Jun 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Perhaps assign this to session (which is about to be returned) instead of returning it directly, so there's only a single exit point?

Or change line 71 into a guard clause, "if no role specified then return immediately".

Copy link
Author

Choose a reason for hiding this comment

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

I think you've missed the part where the initial session is used to communicate with STS if there is a role configured. So in this case both sessions are needed but only the one that has the assumed role is returned to be used for communicating with S3.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I didn't miss that. I'm just suggesting that it's more readable to assign and then return, instead of having two separate return statements.

(Having only one exit from a function is more of a guideline than a hard rule, but it's not a bad guideline when it's convenient.)

aws_access_key_id=credentials['AccessKeyId'],
aws_secret_access_key=credentials['SecretAccessKey'],
aws_session_token=credentials['SessionToken'],
region_name=self.region)
return session

def get_s3_resource(self):
return \
Expand Down