From 7c7a1222d2caaf57601ba3e318cadab111b38748 Mon Sep 17 00:00:00 2001 From: Benjamin Brabant Date: Tue, 8 Dec 2020 10:08:40 +0100 Subject: [PATCH] Fix saml-assertion parsing in assume-role-with-saml (#3523) * Retrieve SAML Attribute by Name instead of relying on order which is too fragile * Handle case when SAML Attribute SessionDuration is not provided, as it is not a required attribute from SAML response When session duration not provided, AWS consider by default a duration of one hour as cited in the following documentation: "If this attribute is not present, then the credential last for one hour (the default value of the DurationSeconds parameter of the AssumeRoleWithSAML API)." https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_saml_assertions.html#saml_role-session-duration Traceback was: [...] File "/Users/benjamin.brabant/Projects/PERSO/moto/moto/sts/responses.py", line 79, in assume_role_with_saml role = sts_backend.assume_role_with_saml( File "/Users/benjamin.brabant/Projects/PERSO/moto/moto/sts/models.py", line 99, in assume_role_with_saml role = AssumedRole(**kwargs) TypeError: __init__() missing 1 required positional argument: 'duration' * Process saml xml namespaces properly instead of relying on textual prefix that can vary between identity providers * Handle when SAML response AttributeValue xml tag contains attributes that force xmltodict to build a dictionary as for complex types instead of directly returning string value Leverage force_cdata option of xmltodict parser that always return a complex dictionary even if xml tag contains only text and no attributes. * Improve existing test_assume_role_with_saml to be coherent with other assume_role_with_saml tests and remove dead code at the same time --- moto/sts/models.py | 38 +++- moto/sts/utils.py | 1 + tests/test_sts/test_sts.py | 422 +++++++++++++++++++++++++++++++++++-- 3 files changed, 431 insertions(+), 30 deletions(-) diff --git a/moto/sts/models.py b/moto/sts/models.py index b274b1acd..c76e5148a 100644 --- a/moto/sts/models.py +++ b/moto/sts/models.py @@ -10,6 +10,7 @@ from moto.sts.utils import ( random_secret_access_key, random_session_token, random_assumed_role_id, + DEFAULT_STS_SESSION_DURATION, ) @@ -85,15 +86,36 @@ class STSBackend(BaseBackend): del kwargs["principal_arn"] saml_assertion_encoded = kwargs.pop("saml_assertion") saml_assertion_decoded = b64decode(saml_assertion_encoded) - saml_assertion = xmltodict.parse(saml_assertion_decoded.decode("utf-8")) - kwargs["duration"] = int( - saml_assertion["samlp:Response"]["Assertion"]["AttributeStatement"][ - "Attribute" - ][2]["AttributeValue"] + + namespaces = { + "urn:oasis:names:tc:SAML:2.0:protocol": "samlp", + "urn:oasis:names:tc:SAML:2.0:assertion": "saml", + } + saml_assertion = xmltodict.parse( + saml_assertion_decoded.decode("utf-8"), + force_cdata=True, + process_namespaces=True, + namespaces=namespaces, ) - kwargs["role_session_name"] = saml_assertion["samlp:Response"]["Assertion"][ - "AttributeStatement" - ]["Attribute"][0]["AttributeValue"] + + saml_assertion_attributes = saml_assertion["samlp:Response"]["saml:Assertion"][ + "saml:AttributeStatement" + ]["saml:Attribute"] + for attribute in saml_assertion_attributes: + if ( + attribute["@Name"] + == "https://aws.amazon.com/SAML/Attributes/RoleSessionName" + ): + kwargs["role_session_name"] = attribute["saml:AttributeValue"]["#text"] + if ( + attribute["@Name"] + == "https://aws.amazon.com/SAML/Attributes/SessionDuration" + ): + kwargs["duration"] = int(attribute["saml:AttributeValue"]["#text"]) + + if "duration" not in kwargs: + kwargs["duration"] = DEFAULT_STS_SESSION_DURATION + kwargs["external_id"] = None kwargs["policy"] = None role = AssumedRole(**kwargs) diff --git a/moto/sts/utils.py b/moto/sts/utils.py index 1e8a13569..e71dc60a9 100644 --- a/moto/sts/utils.py +++ b/moto/sts/utils.py @@ -8,6 +8,7 @@ import six ACCOUNT_SPECIFIC_ACCESS_KEY_PREFIX = "8NWMTLYQ" ACCOUNT_SPECIFIC_ASSUMED_ROLE_ID_PREFIX = "3X42LBCD" SESSION_TOKEN_PREFIX = "FQoGZXIvYXdzEBYaD" +DEFAULT_STS_SESSION_DURATION = 3600 def random_access_key_id(): diff --git a/tests/test_sts/test_sts.py b/tests/test_sts/test_sts.py index 098da5881..c0324d9f4 100644 --- a/tests/test_sts/test_sts.py +++ b/tests/test_sts/test_sts.py @@ -108,23 +108,10 @@ def test_assume_role(): @mock_sts def test_assume_role_with_saml(): client = boto3.client("sts", region_name="us-east-1") - - session_name = "session-name" - policy = json.dumps( - { - "Statement": [ - { - "Sid": "Stmt13690092345534", - "Action": ["S3:ListBucket"], - "Effect": "Allow", - "Resource": ["arn:aws:s3:::foobar-tester"], - } - ] - } - ) role_name = "test-role" provider_name = "TestProvFed" - user_name = "testuser" + fed_identifier = "7ca82df9-1bad-4dd3-9b2b-adb68b554282" + fed_name = "testuser" role_input = "arn:aws:iam::{account_id}:role/{role_name}".format( account_id=ACCOUNT_ID, role_name=role_name ) @@ -161,7 +148,7 @@ def test_assume_role_with_saml(): - {username} + {fed_identifier} @@ -173,7 +160,7 @@ def test_assume_role_with_saml(): - {username}@localhost + {fed_name} arn:aws:iam::{account_id}:saml-provider/{provider_name},arn:aws:iam::{account_id}:role/{role_name} @@ -192,7 +179,8 @@ def test_assume_role_with_saml(): account_id=ACCOUNT_ID, role_name=role_name, provider_name=provider_name, - username=user_name, + fed_identifier=fed_identifier, + fed_name=fed_name, ).replace( "\n", "" ) @@ -213,19 +201,409 @@ def test_assume_role_with_saml(): credentials["SecretAccessKey"].should.have.length_of(40) assume_role_response["AssumedRoleUser"]["Arn"].should.equal( - "arn:aws:sts::{account_id}:assumed-role/{role_name}/{fed_name}@localhost".format( - account_id=ACCOUNT_ID, role_name=role_name, fed_name=user_name + "arn:aws:sts::{account_id}:assumed-role/{role_name}/{fed_name}".format( + account_id=ACCOUNT_ID, role_name=role_name, fed_name=fed_name ) ) assert assume_role_response["AssumedRoleUser"]["AssumedRoleId"].startswith("AROA") assert assume_role_response["AssumedRoleUser"]["AssumedRoleId"].endswith( - ":{fed_name}@localhost".format(fed_name=user_name) + ":{fed_name}".format(fed_name=fed_name) ) assume_role_response["AssumedRoleUser"]["AssumedRoleId"].should.have.length_of( - 21 + 1 + len("{fed_name}@localhost".format(fed_name=user_name)) + 21 + 1 + len("{fed_name}".format(fed_name=fed_name)) ) +@freeze_time("2012-01-01 12:00:00") +@mock_sts +def test_assume_role_with_saml_should_not_rely_on_attribute_order(): + client = boto3.client("sts", region_name="us-east-1") + role_name = "test-role" + provider_name = "TestProvFed" + fed_identifier = "7ca82df9-1bad-4dd3-9b2b-adb68b554282" + fed_name = "testuser" + role_input = "arn:aws:iam::{account_id}:role/{role_name}".format( + account_id=ACCOUNT_ID, role_name=role_name + ) + principal_role = "arn:aws:iam:{account_id}:saml-provider/{provider_name}".format( + account_id=ACCOUNT_ID, provider_name=provider_name + ) + saml_assertion = """ + + + http://localhost/ + + + + + http://localhost:3000/ + + + + + + + + + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + + + {fed_identifier} + + + + + + + urn:amazon:webservices + + + + + arn:aws:iam::{account_id}:saml-provider/{provider_name},arn:aws:iam::{account_id}:role/{role_name} + + + 900 + + + {fed_name} + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + +""".format( + account_id=ACCOUNT_ID, + role_name=role_name, + provider_name=provider_name, + fed_identifier=fed_identifier, + fed_name=fed_name, + ).replace( + "\n", "" + ) + + assume_role_response = client.assume_role_with_saml( + RoleArn=role_input, + PrincipalArn=principal_role, + SAMLAssertion=b64encode(saml_assertion.encode("utf-8")).decode("utf-8"), + ) + + credentials = assume_role_response["Credentials"] + if not settings.TEST_SERVER_MODE: + credentials["Expiration"].isoformat().should.equal("2012-01-01T12:15:00+00:00") + + assume_role_response["AssumedRoleUser"]["Arn"].should.equal( + "arn:aws:sts::{account_id}:assumed-role/{role_name}/{fed_name}".format( + account_id=ACCOUNT_ID, role_name=role_name, fed_name=fed_name + ) + ) + + +@freeze_time("2012-01-01 12:00:00") +@mock_sts +def test_assume_role_with_saml_should_respect_xml_namespaces(): + client = boto3.client("sts", region_name="us-east-1") + role_name = "test-role" + provider_name = "TestProvFed" + fed_identifier = "7ca82df9-1bad-4dd3-9b2b-adb68b554282" + fed_name = "testuser" + role_input = "arn:aws:iam::{account_id}:role/{role_name}".format( + account_id=ACCOUNT_ID, role_name=role_name + ) + principal_role = "arn:aws:iam:{account_id}:saml-provider/{provider_name}".format( + account_id=ACCOUNT_ID, provider_name=provider_name + ) + saml_assertion = """ + + + http://localhost/ + + + + + http://localhost:3000/ + + + + + + + + + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + + + {fed_identifier} + + + + + + + urn:amazon:webservices + + + + + {fed_name} + + + arn:aws:iam::{account_id}:saml-provider/{provider_name},arn:aws:iam::{account_id}:role/{role_name} + + + 900 + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + +""".format( + account_id=ACCOUNT_ID, + role_name=role_name, + provider_name=provider_name, + fed_identifier=fed_identifier, + fed_name=fed_name, + ).replace( + "\n", "" + ) + + assume_role_response = client.assume_role_with_saml( + RoleArn=role_input, + PrincipalArn=principal_role, + SAMLAssertion=b64encode(saml_assertion.encode("utf-8")).decode("utf-8"), + ) + + credentials = assume_role_response["Credentials"] + if not settings.TEST_SERVER_MODE: + credentials["Expiration"].isoformat().should.equal("2012-01-01T12:15:00+00:00") + + assume_role_response["AssumedRoleUser"]["Arn"].should.equal( + "arn:aws:sts::{account_id}:assumed-role/{role_name}/{fed_name}".format( + account_id=ACCOUNT_ID, role_name=role_name, fed_name=fed_name + ) + ) + + +@freeze_time("2012-01-01 12:00:00") +@mock_sts +def test_assume_role_with_saml_should_retrieve_attribute_value_from_text_when_xml_tag_contains_xmlns_attributes(): + client = boto3.client("sts", region_name="us-east-1") + role_name = "test-role" + provider_name = "TestProvFed" + fed_identifier = "7ca82df9-1bad-4dd3-9b2b-adb68b554282" + fed_name = "testuser" + role_input = "arn:aws:iam::{account_id}:role/{role_name}".format( + account_id=ACCOUNT_ID, role_name=role_name + ) + principal_role = "arn:aws:iam:{account_id}:saml-provider/{provider_name}".format( + account_id=ACCOUNT_ID, provider_name=provider_name + ) + saml_assertion = """ + + + http://localhost/ + + + + + http://localhost:3000/ + + + + + + + + + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + + + {fed_identifier} + + + + + + + urn:amazon:webservices + + + + + {fed_name} + + + arn:aws:iam::{account_id}:saml-provider/{provider_name},arn:aws:iam::{account_id}:role/{role_name} + + + 900 + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + +""".format( + account_id=ACCOUNT_ID, + role_name=role_name, + provider_name=provider_name, + fed_identifier=fed_identifier, + fed_name=fed_name, + ).replace( + "\n", "" + ) + + assume_role_response = client.assume_role_with_saml( + RoleArn=role_input, + PrincipalArn=principal_role, + SAMLAssertion=b64encode(saml_assertion.encode("utf-8")).decode("utf-8"), + ) + + credentials = assume_role_response["Credentials"] + if not settings.TEST_SERVER_MODE: + credentials["Expiration"].isoformat().should.equal("2012-01-01T12:15:00+00:00") + + assume_role_response["AssumedRoleUser"]["Arn"].should.equal( + "arn:aws:sts::{account_id}:assumed-role/{role_name}/{fed_name}".format( + account_id=ACCOUNT_ID, role_name=role_name, fed_name=fed_name + ) + ) + + +@freeze_time("2012-01-01 12:00:00") +@mock_sts +def test_assume_role_with_saml_should_default_session_duration_to_3600_seconds_when_saml_attribute_not_provided(): + client = boto3.client("sts", region_name="us-east-1") + role_name = "test-role" + provider_name = "TestProvFed" + fed_identifier = "7ca82df9-1bad-4dd3-9b2b-adb68b554282" + fed_name = "testuser" + role_input = "arn:aws:iam::{account_id}:role/{role_name}".format( + account_id=ACCOUNT_ID, role_name=role_name + ) + principal_role = "arn:aws:iam:{account_id}:saml-provider/{provider_name}".format( + account_id=ACCOUNT_ID, provider_name=provider_name + ) + saml_assertion = """ + + + http://localhost/ + + + + + http://localhost:3000/ + + + + + + + + + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + NTIyMzk0ZGI4MjI0ZjI5ZGNhYjkyOGQyZGQ1NTZjODViZjk5YTY4ODFjOWRjNjkyYzZmODY2ZDQ4NjlkZjY3YSAgLQo= + + + + + {fed_identifier} + + + + + + + urn:amazon:webservices + + + + + arn:aws:iam::{account_id}:saml-provider/{provider_name},arn:aws:iam::{account_id}:role/{role_name} + + + {fed_name} + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + +""".format( + account_id=ACCOUNT_ID, + role_name=role_name, + provider_name=provider_name, + fed_identifier=fed_identifier, + fed_name=fed_name, + ).replace( + "\n", "" + ) + + assume_role_response = client.assume_role_with_saml( + RoleArn=role_input, + PrincipalArn=principal_role, + SAMLAssertion=b64encode(saml_assertion.encode("utf-8")).decode("utf-8"), + ) + + credentials = assume_role_response["Credentials"] + credentials.should.have.key("Expiration") + if not settings.TEST_SERVER_MODE: + credentials["Expiration"].isoformat().should.equal("2012-01-01T13:00:00+00:00") + + @freeze_time("2012-01-01 12:00:00") @mock_sts_deprecated def test_assume_role_with_web_identity():