techservicesillinois / awscli-login

​awscli-login is an AWS CLI plugin that manages retrieving and rotating Amazon STS temporary credentials using SAML ECP for authentication with optional support for Duo.
https://pypi.org/project/awscli-login/
Other
54 stars 25 forks source link

configure should clear ~/.aws/credentials #195

Closed ddriddle closed 2 months ago

ddriddle commented 4 months ago

Release critical bug: $ aws login configure does not properly clear a profile in ~/.aws/credentials when configuring it. It will leave the security token unchanged and the other values are set to nothing. Leaving the security token causes aws s3 ls to fail:

$ cat ~/.aws/credentials 
[default]
aws_access_key_id = FAKE_ACCESS_KEY_ID
aws_secret_access_key = FAKE_SECRET_ACCESS_KEY
aws_session_token = 
FAKE_SESSION_TOKEN
aws_security_token = 
FAKE_SECURITY_TOKEN
$ aws login configure
Overwrite to enable login? y
ECP Endpoint URL [https://shibboleth.illinois.edu/idp/profile/SAML2/SOAP/ECP]: 
Username [ddriddle]: 
Enable Keyring [False]: 
Duo Factor [push]: 
Role ARN [arn:aws:iam::199050005105:role/Admins]:
$ $ cat ~/.aws/credentials 
[default]
aws_access_key_id = 
aws_secret_access_key = 
aws_session_token = 
aws_security_token = 
FAKE_SECURITY_TOKEN
credential_process = aws-login --profile default
$ aws s3 ls

An error occurred (AuthorizationHeaderMalformed) when calling the ListBuckets operation: The authorization header is malformed; a non-empty Access Key (AKID) must be provided in the credential.

Manually fixing ~/.aws/credentials resolves the issue:

$ cat ~/.aws/credentials 
[default]
credential_process = aws-login --profile default
$ aws s3 ls
2020-02-20 10:27:26 somebucket

Tasks

Reference

https://github.com/techservicesillinois/awscli-login/blob/2069b89e853c5b4771ec156656cd72863e53dfca/src/awscli_login/util.py#L208-L227

ddriddle commented 2 months ago

Note that credentianls can be stored in both ~/.aws/config and ~/.aws/credentials! The ones in the credentials file take precedence:

You can keep all of your profile settings in a single file as the AWS CLI can read credentials from the config file. If there are credentials in both files for a profile sharing the same name, the keys in the credentials file take precedence. We suggest keeping credentials in the credentials files. These files are also used by the various language software development kits (SDKs). If you use one of the SDKs in addition to the AWS CLI, confirm if the credentials should be stored in their own file. https://docs.aws.amazon.com/cli/v1/userguide/cli-configure-files.html#cli-configure-files-format

Not only that but it looks like anything you put in ~/.aws/credentials will override anything in ~/.aws/config!!! See the following example:

$ cat ~/.aws/config 
[plugins]
login = awscli_login

[default]
output=json
$ cat ~/.aws/credentials 
[default]
output=text
credential_process = aws-login --profile default
$ aws sts get-caller-identity
199050005105    arn:aws:sts::199050005105:assumed-role/Admins/ddriddle@illinois.edu AROAS4WCOBJY6I5J7QUED:ddriddle@illinois.edu

If we swap text in the credentials file to json we get:

$ cat ~/.aws/config 
[plugins]
login = awscli_login

[default]
output=text
$ cat ~/.aws/credentials 
[default]
output=json
credential_process = aws-login --profile default aws sts get-caller-identity
{
    "UserId": "AROAS4WCOBJY6I5J7QUED:ddriddle@illinois.edu",
    "Account": "199050005105",
    "Arn": "arn:aws:sts::199050005105:assumed-role/Admins/ddriddle@illinois.edu"
}
ddriddle commented 2 months ago

To implement this fix we need to refactor update_credential_file found in util.py. The functions _aws_set and _aws_get should be removed completely. The refactored function update_credential_file will instead use ConfigFileWriter as seen in config.py:

375     def update(self) -> None:
376         """ Interactively update the profile. """
377         new_values = {}
378         writer = ConfigFileWriter()
379     
380         for attr, string in self._config_options.items():
381             value = getattr(self, attr, self._optional.get(attr))
382 
383             prompt = "%s [%s]: " % (string, value)
384             value = input(prompt)
385 
386             if value:
387                 new_values[attr] = value
388 
389         if new_values:
390             if self.name != 'default':
391                 new_values['__section__'] = self.name
392 
393             secure_touch(self.config_file)
394             writer.update_config(new_values, self.config_file)

The entire section being updated should be cleared, and replaced with a key value pair. Comments should be preserved. Inline comments are not preserved and this is a limitation of Amazon's code.