Add AWS session token support #23

br3ndonland · 2021-10-30T22:13:52Z

Description

Resolves #16

Thanks for this project. It’s refreshing to see an alternative to the other "bloated, opaque" AWS tools out there.

This PR will add basic AWS session token support to aioaws. AWS session tokens are used when an AWS resource obtains temporary security credentials.

The authorization flow for temporary security credentials commonly works like this:

IAM roles, such as service-linked roles or Lambda execution roles, are set up and linked to infrastructure resources. These roles can have two kinds of IAM policies attached: resource-based and identity-based policies.
- Identity-based policies define interactions with other resources on AWS.
- A resource-based policy called a "role trust policy" defines how the role can be assumed.
The AWS runtime (Fargate, Lambda, etc) requests authorization to use the role by calling the STS AssumeRole API.
If the requesting entity has permissions to assume the role, STS responds with temporary security credentials that have permissions based on the identity-based policies associated with the IAM role.
The AWS runtime stores the temporary security credentials, typically by setting environment variables:
- AWS_ACCESS_KEY_ID
- AWS_SECRET_ACCESS_KEY
- AWS_SESSION_TOKEN
AWS API calls with temporary credentials must include the session token, as mentioned in AWS_SESSION_TOKEN support #16.
The AWS runtime will typically rotate the temporary security credentials before they expire.

Changes

Source code

Add aws_session_token: str model fields
- aioaws._types.S3ConfigProtocol
- aioaws.s3.S3Config
Add aioaws.core.AwsClient.aws_session_token attribute
Update aioaws.core.AwsClient._auth_headers to add session token into signature
For S3 downloads, add X-Amz-Security-Token param to aioaws.core.AwsClient.add_signed_download_params
For S3 uploads, allow X-Amz-Security-Token as an extra condition in aioaws.core.AwsClient.upload_extra_conditions, then add to upload signature using aioaws.core.AwsClient.signed_upload_fields

Tests

Make S3 bucket name configurable in tests: S3 bucket names are globally unique, so other contributors will need to create buckets with different names. They can then set ${{ secrets.TEST_AWS_S3_BUCKET_NAME }} in GitHub Secrets to use the bucket name in GitHub Actions. Defaults to the previous hard-coded value (aioaws-testing).
Make SES email address configurable in tests: As with S3 bucket names, enabling configuration of the email address allows contributors to run tests on their forks and in their AWS accounts, by setting ${{ secrets.TEST_AWS_SES_ADDRESS }} in GitHub Secrets. Defaults to the previous hard-coded value (testing@scolvin.com).
Add workflow_dispatch trigger to GitHub Actions workflow: Enables workflow to be run manually, from any branch. Useful for testing code changes on feature branches.

IAM policies

Add role trust policy document aws_test_role_permissions.json
- sts:AssumeRole allows GitHub Actions to obtain temporary security credentials
- sts:TagSession allows GitHub Actions to tag the temporary security credentials with metadata as described here and here

Usage

AWS_SESSION_TOKEN can be added with the S3Config.aws_session_token attribute.

S3Config('<access key>', '<secret key>', '<region>', 'my_bucket_name.com', os.getenv('AWS_SESSION_TOKEN', ''))

Maintainer TODOs

There are a few steps needed to enable AssumeRole for testing. This may be confusing, so I'm happy to help with setup as needed. We can also do this with OpenID Connect if you prefer.

Update the new role trust policy file aws_test_role_permissions.json with:
- <AWS_ACCOUNT_NUMBER>: Your account number
- <IAM_USERNAME>: The name of the IAM user that owns the access key used for testing
Create an IAM role for the user to assume (same user that owns the access key used for testing), using aws_test_role_permissions.json as the role trust policy
Create a customer-managed identity-based policy using aws_test_key_permissions.json, if you haven't already
Attach the managed policy from aws_test_key_permissions.json to the role

AWS CLI steps look like this:

cd aioaws

# copy your account number (replace fx with jq or other JSON parser as needed)
AWS_ACCOUNT_NUMBER=$(aws sts get-caller-identity | fx .Account)

# add account number and username to role trust policy (this is GNU sed syntax)
gsed -Ei "s|<AWS_ACCOUNT_NUMBER>|$AWS_ACCOUNT_NUMBER|g" aws_test_role_permissions.json
gsed -Ei "s|<IAM_USERNAME>|aioaws|g" aws_test_role_permissions.json

# create IAM role using the role trust policy document
aws iam create-role --role-name aioaws-testing-role --assume-role-policy-document file://aws_test_role_permissions.json

# create a customer-managed identity-based policy if you haven't already
POLICY_ARN=$(aws iam create-policy --policy-name aioaws-testing --policy-document file://aws_test_key_permissions.json | fx .Policy.Arn)

# attach the policy to the role
aws iam attach-role-policy --policy-arn $POLICY_ARN --role-name aioaws-testing-role

The end result is that the AWS access key can be used to assume the role and obtain temporary credentials. The temporary credentials have the same IAM policy as the regular access key, so tests can be parametrized accordingly.

Notes

Session token expiration

aioaws clients do not automatically rotate temporary credentials. Developers are responsible for updating client attributes or instantiating new clients when temporary credentials expire.

Token expiration should be taken into account when generating S3 presigned URLs. As explained in the docs, "If you created a presigned URL using a temporary token, then the URL expires when the token expires, even if the URL was created with a later expiration time."

Other credential sources

There are several other ways to source credentials (see the AWS IAM docs, AWS CLI docs, and Boto3 docs). This project only handles AWS access keys and session tokens.

For CI/CD, GitHub Actions supports OpenID Connect ("OIDC"), which allows workflows to authenticate with AWS without having to store access keys in GitHub Secrets.

https://docs.github.com/en/actions/learn-github-actions/events-that-trigger-workflows

This commit will add a pytest fixture to configure the S3 bucket name. A configurable S3 bucket name enables contributors to run tests on their forks and in their AWS accounts, by setting a GitHub Secrets variable `${{ secrets.TEST_AWS_S3_BUCKET_NAME }}`. Defaults to `aioaws-testing`. The function is written with an if expression instead of with `os.getenv('TEST_AWS_S3_BUCKET_NAME', 'aioaws-testing')` because of how the environment variable value is passed in from GitHub Actions. If `${{ secrets.TEST_AWS_S3_BUCKET_NAME }}` is not set, then the value of the environment variable `TEST_AWS_S3_BUCKET_NAME` will be an empty string, and `os.getenv('TEST_AWS_S3_BUCKET_NAME', 'aioaws-testing')` will return an empty string.

As with the S3 bucket name, enabling configuration of the email address allowscontributors to run tests on their forks and in their AWS accounts by setting `${{ secrets.TEST_AWS_SES_ADDRESS }}` in GitHub Secrets. Defaults to `testing@scolvin.com`.

This is a "resource-based policy" that allows an IAM role to be assumed. https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_identity-vs-resource.html

- Use aws-actions/configure-aws-credentials to obtain temporary security credentials - Pass both the static and temporary credentials to pytest

Parametrize the pytest fixture so that it is generated for each item in params, and each test that uses the fixture runs for each param https://docs.pytest.org/en/latest/how-to/fixtures.html

Session tokens are used when AWS resources obtain temporary credentials. - Add `aws_session_token: str` model fields - `aioaws._types.S3ConfigProtocol` - `aioaws.s3.S3Config` - Add `aioaws.core.AwsClient.aws_session_token` attribute - Update `aioaws.core.AwsClient._auth_headers` to add session token into signature - For S3 downloads, add `X-Amz-Security-Token` param to `aioaws.core.AwsClient.add_signed_download_params` - For S3 uploads, allow `X-Amz-Security-Token` as an extra condition in `aioaws.core.AwsClient.upload_extra_conditions`, then add to upload signature using `aioaws.core.AwsClient.signed_upload_fields`

samuelcolvin

looking good, but we need to get tests to work before I can review properly.

samuelcolvin · 2021-12-06T14:58:32Z

.github/workflows/ci.yml

@@ -7,6 +7,7 @@ on:
    tags:
      - '**'
  pull_request: {}
+  workflow_dispatch:


this is making the yml invalid and meaning tests can't be run.

I think it can be removed.

The workflow YAML is valid. See here for an example run on my fork.

I'm okay with removing it though, if you prefer.

You'll need to grant approval for the workflow to run - is this what you're referring to?

samuelcolvin · 2021-12-06T15:02:26Z

aioaws/core.py

@@ -32,6 +32,7 @@ def __init__(self, client: AsyncClient, config: 'BaseConfigProtocol', service: L
        self.client = client
        self.aws_access_key = get_config_attr(config, 'aws_access_key')
        self.aws_secret_key = get_config_attr(config, 'aws_secret_key')
+        self.aws_session_token = getattr(config, 'aws_session_token') if hasattr(config, 'aws_session_token') else ''


Suggested change

self.aws_session_token = getattr(config, 'aws_session_token') if hasattr(config, 'aws_session_token') else ''

self.aws_session_token = getattr(config, 'aws_session_token', None)

Was there a reason to use an empty string as a default instead of None?

The syntax in your suggestion is clearer, thank you. -> 7b7ba5f

It is possible to use None as a default here, but I used an empty string to keep the type signature consistent with S3Config.aws_session_token and S3ConfigProtocol.aws_session_token.

samuelcolvin · 2021-12-06T15:04:05Z

aioaws/core.py

+            params.update({'X-Amz-Security-Token': self.aws_session_token})
+        params.update({'X-Amz-SignedHeaders': 'host'})


Suggested change

params.update({'X-Amz-Security-Token': self.aws_session_token})

params.update({'X-Amz-SignedHeaders': 'host'})

params['X-Amz-Security-Token'] = self.aws_session_token

params['X-Amz-SignedHeaders'] = 'host'

Is there any reason to put host after X-Amz-Security-Token?

Same question to the cases below.

Yes, to keep the fields in alphabetical order. I get errors when the fields are out of order. You had a similar note for headers here:

aioaws/aioaws/core.py

Line 146 in 9e93843

# WARNING! order is important here, headers need to be in alphabetical order

aioaws/core.py

samuelcolvin · 2021-12-06T15:16:54Z

aioaws/s3.py

    aws_region: str
    aws_s3_bucket: str
+    aws_session_token: str = field(repr=False, default='')


again, is there a reason to use '' instead of None as default?

Again, I used an empty string as a default to keep the type signature consistent with S3ConfigProtocol.aws_session_token. Otherwise the type signature would be Optional[str].

A default of None could also impact use of _utils.get_config_attr, because this method expects all config attributes to be strings.

aioaws/aioaws/_utils.py

Line 25 in 9e93843

raise TypeError(f'config.{name} must be a string not {s.__class__.__name__}')

tests/conftest.py

samuelcolvin · 2021-12-06T15:19:08Z

tests/conftest.py

+    will be an empty string, and `os.getenv('TEST_AWS_S3_BUCKET_NAME', 'aioaws-testing')`
+    will return an empty string.
+    """
+    return value if (value := os.getenv('TEST_AWS_S3_BUCKET_NAME')) else 'aioaws-testing'


Suggested change

return value if (value := os.getenv('TEST_AWS_S3_BUCKET_NAME')) else 'aioaws-testing'

return os.getenv('TEST_AWS_S3_BUCKET_NAME', 'aioaws-testing')

This syntax is a simple way to differentiate between:

an environment variable set to an empty string

an environment variable not set at all

This is mostly for GitHub Actions. If ${{ secrets.TEST_AWS_S3_BUCKET_NAME }} is not set:

GitHub Actions will pass in TEST_AWS_S3_BUCKET_NAME=''

os.getenv('TEST_AWS_S3_BUCKET_NAME', 'aioaws-testing') will return '', rather than 'aioaws-testing'.

>>> import os >>> os.getenv('TEST_AWS_S3_BUCKET_NAME') >>> os.environ['TEST_AWS_S3_BUCKET_NAME'] = '' >>> os.getenv('TEST_AWS_S3_BUCKET_NAME') '' >>> os.getenv('TEST_AWS_S3_BUCKET_NAME', 'aioaws-testing') '' >>> value if (value := os.getenv('TEST_AWS_S3_BUCKET_NAME')) else 'aioaws-testing' 'aioaws-testing'

tests/conftest.py

samuelcolvin · 2021-12-06T15:19:42Z

tests/conftest.py

+    contributors to run tests on their forks and in their AWS accounts, by setting
+    `${{ secrets.TEST_AWS_SES_ADDRESS }}` on GitHub. Defaults to `testing@scolvin.com`.
+    """
+    return value if (value := os.getenv('TEST_AWS_SES_ADDRESS')) else 'testing@scolvin.com'


Suggested change

return value if (value := os.getenv('TEST_AWS_SES_ADDRESS')) else 'testing@scolvin.com'

return os.getenv('TEST_AWS_SES_ADDRESS', 'testing@scolvin.com')

Same comment as for TEST_AWS_S3_BUCKET_NAME: #23 (comment)

#23 (comment) #23 (comment)

#23 (comment)

#23 (comment) #23 (comment)

br3ndonland · 2021-12-06T22:55:17Z

looking good, but we need to get tests to work before I can review properly.

Thanks for your review! I would welcome any further comments or suggestions.

samuelcolvin · 2022-01-21T16:16:56Z

really not sure why tests didn't run for your two pull requests, I just got a friend to try and PRs from external contributors are working #26

br3ndonland · 2022-01-22T19:55:30Z

really not sure why tests didn't run for your two pull requests, I just got a friend to try and PRs from external contributors are working #26

Tests appear to be running after merging the master branch.

You'll need to set up a role in order to test session tokens. See the "Maintainer TODOs" section above. Once you get it set up, runs should succeed like this.

AWS CLI steps would look like this:

cd aioaws

# copy your account number (replace fx with jq or other JSON parser as needed)
AWS_ACCOUNT_NUMBER=$(aws sts get-caller-identity | fx .Account)

# add account number and username to role trust policy (this is GNU sed syntax)
gsed -Ei "s|<AWS_ACCOUNT_NUMBER>|$AWS_ACCOUNT_NUMBER|g" aws_test_role_permissions.json
gsed -Ei "s|<IAM_USERNAME>|aioaws|g" aws_test_role_permissions.json

# create IAM role using the role trust policy document
aws iam create-role --role-name aioaws-testing-role --assume-role-policy-document file://aws_test_role_permissions.json

# create a customer-managed identity-based policy if you haven't already
POLICY_ARN=$(aws iam create-policy --policy-name aioaws-testing --policy-document file://aws_test_key_permissions.json | fx .Policy.Arn)

# attach the policy to the role
aws iam attach-role-policy --policy-arn $POLICY_ARN --role-name aioaws-testing-role

samuelcolvin · 2022-01-22T19:57:28Z

It's not pull_request: {} since #26 ran tests.

I'll look into the role changes next week.

This reverts commit 4f0142d.

samuelcolvin · 2022-02-21T12:17:02Z

can you remove the changes to .github/workflows/ci.yml they seem to be causing the PR to fail.

br3ndonland · 2022-02-22T22:03:34Z

can you remove the changes to .github/workflows/ci.yml they seem to be causing the PR to fail.

Again, you need to create a role in your AWS account for the updated tests to work.

samuelcolvin · 2022-02-22T22:07:55Z

Sorry, I'll do that.

br3ndonland · 2023-01-14T20:59:39Z

Closing off this PR because it hasn't moved forward in about a year, and I no longer have a use case for this package.

samuelcolvin · 2023-01-15T00:32:28Z

Sorry I didn't get around to completing this, and thanks for the contribution. 🙏

br3ndonland added 8 commits October 22, 2021 17:37

Add workflow_dispatch to run workflow manually

c2952f5

https://docs.github.com/en/actions/learn-github-actions/events-that-trigger-workflows

Add IAM role trust policy

db3a754

This is a "resource-based policy" that allows an IAM role to be assumed. https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_identity-vs-resource.html

Assume IAM role for testing with GitHub Actions

291c18a

- Use aws-actions/configure-aws-credentials to obtain temporary security credentials - Pass both the static and temporary credentials to pytest

Parametrize AWS credentials with pytest

49a713b

Parametrize the pytest fixture so that it is generated for each item in params, and each test that uses the fixture runs for each param https://docs.pytest.org/en/latest/how-to/fixtures.html

Document AWS session token support

dadbeb3

br3ndonland mentioned this pull request Dec 5, 2021

Fix handling of bucket names #24

Closed

samuelcolvin reviewed Dec 6, 2021

View reviewed changes

br3ndonland added 3 commits December 6, 2021 17:15

Use suggested docstring formatting in conftest.py

db63694

#23 (comment) #23 (comment)

Set getattr default for aws_session_token

7b7ba5f

#23 (comment)

Add single values to dictionary directly

fb8f459

#23 (comment) #23 (comment)

br3ndonland added 2 commits January 22, 2022 14:40

Merge branch 'master' into session-token

af657fa

Remove empty mapping from pull_request trigger

4f0142d

Revert "Remove empty mapping from pull_request trigger"

8839b21

This reverts commit 4f0142d.

br3ndonland closed this Jan 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AWS session token support #23

Add AWS session token support #23

br3ndonland commented Oct 30, 2021

samuelcolvin left a comment

samuelcolvin Dec 6, 2021

samuelcolvin Dec 6, 2021

br3ndonland Dec 6, 2021

samuelcolvin Dec 6, 2021

br3ndonland Dec 6, 2021

samuelcolvin Dec 6, 2021

samuelcolvin Dec 6, 2021

br3ndonland Dec 6, 2021

samuelcolvin Dec 6, 2021

br3ndonland Dec 6, 2021

samuelcolvin Dec 6, 2021

br3ndonland Dec 6, 2021

samuelcolvin Dec 6, 2021

br3ndonland Dec 6, 2021

br3ndonland commented Dec 6, 2021

samuelcolvin commented Jan 21, 2022

br3ndonland commented Jan 22, 2022 •

edited

samuelcolvin commented Jan 22, 2022

samuelcolvin commented Feb 21, 2022

br3ndonland commented Feb 22, 2022

samuelcolvin commented Feb 22, 2022

br3ndonland commented Jan 14, 2023

samuelcolvin commented Jan 15, 2023

	self.aws_session_token = getattr(config, 'aws_session_token') if hasattr(config, 'aws_session_token') else ''
	self.aws_session_token = getattr(config, 'aws_session_token', None)

		params.update({'X-Amz-Security-Token': self.aws_session_token})
		params.update({'X-Amz-SignedHeaders': 'host'})

	return value if (value := os.getenv('TEST_AWS_S3_BUCKET_NAME')) else 'aioaws-testing'
	return os.getenv('TEST_AWS_S3_BUCKET_NAME', 'aioaws-testing')

	return value if (value := os.getenv('TEST_AWS_SES_ADDRESS')) else 'testing@scolvin.com'
	return os.getenv('TEST_AWS_SES_ADDRESS', 'testing@scolvin.com')

Add AWS session token support #23

Add AWS session token support #23

Conversation

br3ndonland commented Oct 30, 2021

Description

Changes

Source code

Tests

IAM policies

Usage

Maintainer TODOs

Notes

Session token expiration

Other credential sources

samuelcolvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

br3ndonland commented Dec 6, 2021

samuelcolvin commented Jan 21, 2022

br3ndonland commented Jan 22, 2022 • edited

samuelcolvin commented Jan 22, 2022

samuelcolvin commented Feb 21, 2022

br3ndonland commented Feb 22, 2022

samuelcolvin commented Feb 22, 2022

br3ndonland commented Jan 14, 2023

samuelcolvin commented Jan 15, 2023

br3ndonland commented Jan 22, 2022 •

edited