Skip to content

Commit

Permalink
fix: cache MFA based credentials (#1744)
Browse files Browse the repository at this point in the history
* Revert "Revert "fix: botocore session cache across samcli (#1693)" (#1717)"

This reverts commit d72e1ea.

* fix: do not rely on boto private methods for session

* chore: run black autoformatter

* tests(context): assert botocore session

Co-authored-by: Alex Wood <awood45@gmail.com>
  • Loading branch information
sriram-mv and awood45 committed Jan 28, 2020
1 parent d807f78 commit 125f0b0
Show file tree
Hide file tree
Showing 8 changed files with 144 additions and 26 deletions.
117 changes: 117 additions & 0 deletions designs/botocore_sessions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
CLI: Botocore Sessions
====================================

What is the problem?
--------------------

SAM CLI does not have first class support for MFA based authentication, which leads to multiple requests for such tokens degrading the overall CLI experience.

What will be changed?
---------------------
* SAM CLI will support default sessions with caching of temporary credentials to a json cache.

Success criteria for the change
-------------------------------
* SAM CLI only ever asks for a MFA token once and caches it under the default boto location. This allows for other application that uses default location for caching credentials.

Out-of-Scope
------------

User Experience Walkthrough
---------------------------
* SAM CLI on the first interaction with any AWS resource using AWS credentials that support MFA credentials creates a prompt which says `Enter MFA Code: xxxxxxxxxxxxxx/xxxx`

Note: This MFA code is entered via a U2f/virtual mfa device/gemalto token.

Once the code is entered and successfully validated, this results in temporary caching of the credentials with an `sts` call.

Implementation
==============

CLI Changes
-----------

The only changes are during the setting up the boto3 session in the cli context, this propogates the boto3 session context all the way down across all sub commands of SAM CLI.


### Breaking Change

There are no breaking changes.

samconfig.toml Changes
----------------

There are no samconfig.toml changes.

Security
--------

**What new dependencies (libraries/cli) does this change require?**

N/A

**What other Docker container images are you using?**

N/A

**Are you creating a new HTTP endpoint? If so explain how it will be
created & used**

N/A

**Are you connecting to a remote API? If so explain how is this
connection secured**

* Connect to all aws endpoints, secured with aws credentials.

**Are you reading/writing to a temporary folder? If so, what is this
used for and when do you clean up?**

N/A

**How do you validate new .samrc configuration?**

N/A


What is your Testing Plan (QA)?
===============================

Goal
----

* All unit tests and integration tests pass.

Pre-requesites
--------------

* IAM user with MFA enabled.

Test Scenarios/Cases
--------------------

* Specific integration test case with a MFA prompt.

Expected Results
----------------
* No regressions

Documentation Changes
=====================

Open Issues
============
[1682](https://github.com/awslabs/aws-sam-cli/issues/1682)
[1623](https://github.com/awslabs/aws-sam-cli/issues/1623)

Task Breakdown
==============

- \[x\] Send a Pull Request with this design document
- \[ \] Build the command line interface
- \[ \] Build the underlying library
- \[ \] Unit tests
- \[ \] Functional Tests
- \[ \] Integration tests
- \[ \] Run all tests on Windows
- \[ \] Update documentation
15 changes: 13 additions & 2 deletions samcli/cli/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
Context information passed to each CLI command
"""

import uuid
import logging
import uuid

import boto3
import botocore
import botocore.session
from botocore import credentials
import click

from samcli.commands.exceptions import CredentialsError
Expand Down Expand Up @@ -143,7 +146,15 @@ def _refresh_session(self):
region & profile), it will call this method to create a new session with latest values for these properties.
"""
try:
boto3.setup_default_session(region_name=self._aws_region, profile_name=self._aws_profile)
botocore_session = botocore.session.get_session()
boto3.setup_default_session(
botocore_session=botocore_session, region_name=self._aws_region, profile_name=self._aws_profile
)
# get botocore session and setup caching for MFA based credentials
botocore_session.get_component("credential_provider").get_provider(
"assume-role"
).cache = credentials.JSONFileCache()

except botocore.exceptions.ProfileNotFound as ex:
raise CredentialsError(str(ex))

Expand Down
6 changes: 2 additions & 4 deletions samcli/commands/deploy/deploy_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,11 @@ def run(self):
template_size = os.path.getsize(self.template_file)
if template_size > 51200 and not self.s3_bucket:
raise deploy_exceptions.DeployBucketRequiredError()

session = boto3.Session(profile_name=self.profile if self.profile else None)
cloudformation_client = session.client("cloudformation", region_name=self.region if self.region else None)
cloudformation_client = boto3.client("cloudformation", region_name=self.region if self.region else None)

s3_client = None
if self.s3_bucket:
s3_client = session.client("s3", region_name=self.region if self.region else None)
s3_client = boto3.client("s3", region_name=self.region if self.region else None)

self.s3_uploader = S3Uploader(s3_client, self.s3_bucket, self.s3_prefix, self.kms_key_id, self.force_upload)

Expand Down
3 changes: 1 addition & 2 deletions samcli/commands/package/package_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ def __exit__(self, *args):

def run(self):

session = boto3.Session(profile_name=self.profile if self.profile else None)
s3_client = session.client(
s3_client = boto3.client(
"s3", config=Config(signature_version="s3v4", region_name=self.region if self.region else None)
)

Expand Down
4 changes: 1 addition & 3 deletions samcli/lib/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@
from samcli.cli.global_config import GlobalConfig
from samcli.commands.exceptions import UserException, CredentialsError, RegionError


SAM_CLI_STACK_NAME = "aws-sam-cli-managed-default"
LOG = logging.getLogger(__name__)


def manage_stack(profile, region):
try:
session = boto3.Session(profile_name=profile if profile else None)
cloudformation_client = session.client("cloudformation", config=Config(region_name=region if region else None))
cloudformation_client = boto3.client("cloudformation", config=Config(region_name=region if region else None))
except NoCredentialsError:
raise CredentialsError(
"Error Setting Up Managed Stack Client: Unable to resolve credentials for the AWS SDK for Python client. Please see their documentation for options to pass in credentials: https://boto3.amazonaws.com/v1/documentation/api/latest/guide/configuration.html"
Expand Down
8 changes: 5 additions & 3 deletions tests/unit/cli/test_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import logging

from unittest import TestCase
from unittest.mock import patch
from unittest.mock import patch, ANY

from samcli.cli.context import Context

Expand Down Expand Up @@ -47,7 +47,7 @@ def test_must_set_aws_profile_in_boto_session(self, boto_mock):

ctx.profile = profile
self.assertEqual(ctx.profile, profile)
boto_mock.setup_default_session.assert_called_with(region_name=None, profile_name=profile)
boto_mock.setup_default_session.assert_called_with(region_name=None, profile_name=profile, botocore_session=ANY)

@patch("samcli.cli.context.boto3")
def test_must_set_all_aws_session_properties(self, boto_mock):
Expand All @@ -57,7 +57,9 @@ def test_must_set_all_aws_session_properties(self, boto_mock):

ctx.profile = profile
ctx.region = region
boto_mock.setup_default_session.assert_called_with(region_name=region, profile_name=profile)
boto_mock.setup_default_session.assert_called_with(
region_name=region, profile_name=profile, botocore_session=ANY
)

@patch("samcli.cli.context.uuid")
def test_must_set_session_id_to_uuid(self, uuid_mock):
Expand Down
12 changes: 4 additions & 8 deletions tests/unit/lib/bootstrap/test_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,15 @@ def _stubbed_cf_client(self):
cf = botocore.session.get_session().create_client("cloudformation", region_name="us-west-2")
return [cf, Stubber(cf)]

@patch("boto3.Session")
@patch("boto3.client")
def test_client_missing_credentials(self, boto_mock):
session_mock = Mock()
session_mock.client.side_effect = NoCredentialsError()
boto_mock.return_value = session_mock
boto_mock.side_effect = NoCredentialsError()
with self.assertRaises(CredentialsError):
manage_stack("testprofile", "fake-region")

@patch("boto3.Session")
@patch("boto3.client")
def test_client_missing_region(self, boto_mock):
session_mock = Mock()
session_mock.client.side_effect = NoRegionError()
boto_mock.return_value = session_mock
boto_mock.side_effect = NoRegionError()
with self.assertRaises(RegionError):
manage_stack("testprofile", "fake-region")

Expand Down
5 changes: 1 addition & 4 deletions tests/unit/lib/build_module/test_app_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,7 @@ def setUp(self):

def test_must_write_relative_build_artifacts_path(self):
original_template_path = "/path/to/tempate.txt"
built_artifacts = {
"MyFunction1": "/path/to/build/MyFunction1",
"MyFunction2": "/path/to/build/MyFunction2",
}
built_artifacts = {"MyFunction1": "/path/to/build/MyFunction1", "MyFunction2": "/path/to/build/MyFunction2"}

expected_result = {
"Resources": {
Expand Down

0 comments on commit 125f0b0

Please sign in to comment.