From b3ba2a0250880ebf6f5dc30b57905968aa756232 Mon Sep 17 00:00:00 2001 From: Mateusz Boryn Date: Tue, 2 Jul 2019 08:24:18 +0200 Subject: [PATCH 1/4] better error reporting for missing form fields --- awsprocesscreds/saml.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/awsprocesscreds/saml.py b/awsprocesscreds/saml.py index 16ed432..4522aca 100644 --- a/awsprocesscreds/saml.py +++ b/awsprocesscreds/saml.py @@ -67,7 +67,8 @@ class GenericFormsBasedAuthenticator(SAMLAuthenticator): 'Could not find login form from: %s' ) _ERROR_MISSING_FORM_FIELD = ( - 'Error parsing HTML form, could not find the form field: "%s"' + 'Error parsing HTML form, could not find the form field: "%s" ' + 'Available fields: %s.' ) _ERROR_LOGIN_FAILED_NON_200 = ( 'Login failed, received non 200 response: %s' @@ -177,7 +178,7 @@ def _fill_in_form_values(self, config, form_data): username = config['saml_username'] if self.USERNAME_FIELD not in form_data: raise SAMLError( - self._ERROR_MISSING_FORM_FIELD % self.USERNAME_FIELD) + self._ERROR_MISSING_FORM_FIELD % (self.USERNAME_FIELD, ", ".join(form_data.keys()))) else: form_data[self.USERNAME_FIELD] = username if self.PASSWORD_FIELD in form_data: From e65f951e7eecfa7ffd1f6ff4fd9fef73ce3a8bdc Mon Sep 17 00:00:00 2001 From: Mateusz Boryn Date: Tue, 2 Jul 2019 09:51:21 +0200 Subject: [PATCH 2/4] custom form field names for user and password --- README.rst | 12 ++++++++++++ awsprocesscreds/__init__.py | 2 +- awsprocesscreds/cli.py | 16 +++++++++++++++- awsprocesscreds/saml.py | 21 ++++++++++---------- tests/functional/test_saml.py | 4 +++- tests/unit/test_saml.py | 36 +++++++++++++++++++++++++++++++++-- 6 files changed, 76 insertions(+), 15 deletions(-) diff --git a/README.rst b/README.rst index 1af65d6..619408c 100644 --- a/README.rst +++ b/README.rst @@ -53,6 +53,18 @@ disable the cache by specifying ``--no-cache``. Additionally, you can show logs by specifying ``-v`` or ``--verbose``. +Forms-based authentication can be customized: names of fields in form in ADFS +tend to change. You can specify field names that are present in HTML form: + +* ``--form-username-field`` - name of field for username. +* ``--form-password-field`` - name of field for password. + +For older Active Directory those fields are +``ctl00$ContentPlaceHolder1$UsernameTextBox`` +and ``ctl00$ContentPlaceHolder1$PasswordTextBox``. +In newer AD, you can change it to ``UserName`` and ``Password`` using above +options. + To configure this provider, you need create a profile using the ``credential_process`` config variable. See the `AWS CLI Config docs`_ for more details on this config option. diff --git a/awsprocesscreds/__init__.py b/awsprocesscreds/__init__.py index 13df3e0..c5d869b 100644 --- a/awsprocesscreds/__init__.py +++ b/awsprocesscreds/__init__.py @@ -1,6 +1,6 @@ import logging -__version__ = '0.0.2' +__version__ = '0.0.3' class NullHandler(logging.Handler): diff --git a/awsprocesscreds/cli.py b/awsprocesscreds/cli.py index ca6c98d..77c9119 100644 --- a/awsprocesscreds/cli.py +++ b/awsprocesscreds/cli.py @@ -45,6 +45,18 @@ def saml(argv=None, prompter=getpass.getpass, client_creator=None, 'local file cache.' ) ) + parser.add_argument( + '--form-username-field', default="ctl00$ContentPlaceHolder1$UsernameTextBox", help=( + '(ADFS only) - name of the form field for user name supplied by ADFS. ' + 'Adjust according to your ADFS version.' + ) + ) + parser.add_argument( + '--form-password-field', default="ctl00$ContentPlaceHolder1$PasswordTextBox", help=( + '(ADFS only) - name of the form field for password supplied by ADFS. ' + 'Adjust according to your ADFS version.' + ) + ) parser.add_argument( '-v', '--verbose', action='store_true', help=('Enables verbose mode.') ) @@ -73,7 +85,9 @@ def saml(argv=None, prompter=getpass.getpass, client_creator=None, 'saml_endpoint': args.endpoint, 'saml_authentication_type': 'form', 'saml_username': args.username, - 'role_arn': args.role_arn + 'role_arn': args.role_arn, + 'form_username_field': args.form_username_field, + 'form_password_field': args.form_password_field }, password_prompter=prompter, cache=cache diff --git a/awsprocesscreds/saml.py b/awsprocesscreds/saml.py index 4522aca..1734f3f 100644 --- a/awsprocesscreds/saml.py +++ b/awsprocesscreds/saml.py @@ -57,8 +57,6 @@ def retrieve_saml_assertion(self, config): class GenericFormsBasedAuthenticator(SAMLAuthenticator): - USERNAME_FIELD = 'username' - PASSWORD_FIELD = 'password' _ERROR_BAD_RESPONSE = ( 'Received a non-200 response (%s) when making a request to: %s' @@ -121,6 +119,8 @@ def retrieve_saml_assertion(self, config): * saml_endpoint * saml_username + * form_username_field + * form_password_field :raises SAMLError: Raised when we are unable to retrieve a SAML assertion. @@ -143,7 +143,7 @@ def retrieve_saml_assertion(self, config): return self._extract_saml_assertion_from_response(response) def _validate_config_values(self, config): - for required in ['saml_endpoint', 'saml_username']: + for required in ['saml_endpoint', 'saml_username', 'form_username_field', 'form_password_field']: if required not in config: raise SAMLError(self._ERROR_MISSING_CONFIG % required) @@ -176,13 +176,16 @@ def _parse_form_from_html(self, html): def _fill_in_form_values(self, config, form_data): username = config['saml_username'] - if self.USERNAME_FIELD not in form_data: + username_field = config['form_username_field'] + password_field = config['form_password_field'] + + if username_field not in form_data: raise SAMLError( - self._ERROR_MISSING_FORM_FIELD % (self.USERNAME_FIELD, ", ".join(form_data.keys()))) + self._ERROR_MISSING_FORM_FIELD % (username_field, ", ".join(form_data.keys()))) else: - form_data[self.USERNAME_FIELD] = username - if self.PASSWORD_FIELD in form_data: - form_data[self.PASSWORD_FIELD] = self._password_prompter( + form_data[username_field] = username + if password_field in form_data: + form_data[password_field] = self._password_prompter( "Password: ") def _send_form_post(self, login_url, form_data): @@ -256,8 +259,6 @@ def is_suitable(self, config): class ADFSFormsBasedAuthenticator(GenericFormsBasedAuthenticator): - USERNAME_FIELD = 'ctl00$ContentPlaceHolder1$UsernameTextBox' - PASSWORD_FIELD = 'ctl00$ContentPlaceHolder1$PasswordTextBox' def is_suitable(self, config): return (config.get('saml_authentication_type') == 'form' and diff --git a/tests/functional/test_saml.py b/tests/functional/test_saml.py index dcaf836..2588eea 100644 --- a/tests/functional/test_saml.py +++ b/tests/functional/test_saml.py @@ -255,7 +255,9 @@ def test_prompter_only_called_once(client_creator, prompter, assertion, 'saml_provider': 'okta', 'saml_endpoint': 'https://example.com/', 'saml_username': 'monty', - 'role_arn': 'arn:aws:iam::123456789012:role/monty' + 'role_arn': 'arn:aws:iam::123456789012:role/monty', + 'form_username_field': 'username', + 'form_password_field': 'password', } fetcher = SAMLCredentialFetcher( client_creator=client_creator, diff --git a/tests/unit/test_saml.py b/tests/unit/test_saml.py index db2e218..c02a370 100644 --- a/tests/unit/test_saml.py +++ b/tests/unit/test_saml.py @@ -45,6 +45,8 @@ def generic_config(): 'saml_authentication_type': 'form', 'saml_username': 'monty', 'role_arn': 'arn:aws:iam::123456789012:role/monty', + 'form_username_field': 'username', + 'form_password_field': 'password', } @@ -55,6 +57,8 @@ def okta_config(): 'saml_authentication_type': 'form', 'saml_username': 'monty', 'saml_provider': 'okta', + 'form_username_field': 'username', + 'form_password_field': 'password', } @@ -65,6 +69,8 @@ def adfs_config(): 'saml_authentication_type': 'form', 'saml_username': 'monty', 'saml_provider': 'adfs', + 'form_username_field': 'ctl00$ContentPlaceHolder1$UsernameTextBox', + 'form_password_field': 'ctl00$ContentPlaceHolder1$PasswordTextBox', } @@ -124,6 +130,8 @@ def test_config_missing_username(self, generic_auth): config = { 'saml_endpoint': 'https://example.com', 'saml_authentication_type': 'form', + 'form_username_field': 'username', + 'form_password_field': 'password', } with pytest.raises(SAMLError, match='Missing required'): generic_auth.retrieve_saml_assertion(config) @@ -132,6 +140,28 @@ def test_config_missing_endpoint(self, generic_auth): config = { 'saml_username': 'monty', 'saml_authentication_type': 'form', + 'form_username_field': 'username', + 'form_password_field': 'password', + } + with pytest.raises(SAMLError, match='Missing required'): + generic_auth.retrieve_saml_assertion(config) + + def test_config_missing_form_username_field(self, generic_auth): + config = { + 'saml_username': 'monty', + 'saml_authentication_type': 'form', + 'saml_endpoint': 'https://example.com', + 'form_password_field': 'password', + } + with pytest.raises(SAMLError, match='Missing required'): + generic_auth.retrieve_saml_assertion(config) + + def test_config_missing_form_password_field(self, generic_auth): + config = { + 'saml_username': 'monty', + 'saml_authentication_type': 'form', + 'saml_endpoint': 'https://example.com', + 'form_username_field': 'username', } with pytest.raises(SAMLError, match='Missing required'): generic_auth.retrieve_saml_assertion(config) @@ -150,6 +180,8 @@ def test_non_https_url(self, generic_auth, mock_requests_session, 'saml_endpoint': 'http://example.com', 'saml_authentication_type': 'form', 'saml_username': 'monty', + 'form_username_field': 'username', + 'form_password_field': 'password', } # The error is raised after the call to get the form, but before the # call to submit it. @@ -590,7 +622,7 @@ def test_cache_key_is_windows_safe(self, fetcher, cache, retrieve.return_value = saml_assertion fetcher.fetch_credentials() - cache_key = '0cebd512540a4f5fe2edce26319cf1cf3138684f' + cache_key = 'af7a32316c966f76d660f9610c0ec56d91bb2f03' assert cache_key in cache def test_datetime_cache_is_always_serialized(self, fetcher, cache, @@ -615,7 +647,7 @@ def test_datetime_cache_is_always_serialized(self, fetcher, cache, retrieve.return_value = saml_assertion fetcher.fetch_credentials() - cache_key = '0cebd512540a4f5fe2edce26319cf1cf3138684f' + cache_key = 'af7a32316c966f76d660f9610c0ec56d91bb2f03' cache_expiration = cache[cache_key]['Credentials']['Expiration'] assert not isinstance(cache_expiration, datetime) assert cache_expiration == expiration.isoformat() From 36793be74ae42231aa5c900bcab425c033e2ab49 Mon Sep 17 00:00:00 2001 From: Mateusz Boryn Date: Tue, 2 Jul 2019 10:22:52 +0200 Subject: [PATCH 3/4] code formatting --- awsprocesscreds/cli.py | 14 ++++++++------ awsprocesscreds/saml.py | 7 ++++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/awsprocesscreds/cli.py b/awsprocesscreds/cli.py index 77c9119..7b3d52a 100644 --- a/awsprocesscreds/cli.py +++ b/awsprocesscreds/cli.py @@ -46,15 +46,17 @@ def saml(argv=None, prompter=getpass.getpass, client_creator=None, ) ) parser.add_argument( - '--form-username-field', default="ctl00$ContentPlaceHolder1$UsernameTextBox", help=( - '(ADFS only) - name of the form field for user name supplied by ADFS. ' - 'Adjust according to your ADFS version.' + '--form-username-field', + default="ctl00$ContentPlaceHolder1$UsernameTextBox", help=( + '(ADFS only) - name of the form field for user name supplied ' + 'by ADFS. Adjust according to your ADFS version.' ) ) parser.add_argument( - '--form-password-field', default="ctl00$ContentPlaceHolder1$PasswordTextBox", help=( - '(ADFS only) - name of the form field for password supplied by ADFS. ' - 'Adjust according to your ADFS version.' + '--form-password-field', + default="ctl00$ContentPlaceHolder1$PasswordTextBox", help=( + '(ADFS only) - name of the form field for password supplied ' + 'by ADFS. Adjust according to your ADFS version.' ) ) parser.add_argument( diff --git a/awsprocesscreds/saml.py b/awsprocesscreds/saml.py index 1734f3f..b3e56da 100644 --- a/awsprocesscreds/saml.py +++ b/awsprocesscreds/saml.py @@ -143,7 +143,8 @@ def retrieve_saml_assertion(self, config): return self._extract_saml_assertion_from_response(response) def _validate_config_values(self, config): - for required in ['saml_endpoint', 'saml_username', 'form_username_field', 'form_password_field']: + for required in ['saml_endpoint', 'saml_username', + 'form_username_field', 'form_password_field']: if required not in config: raise SAMLError(self._ERROR_MISSING_CONFIG % required) @@ -180,8 +181,8 @@ def _fill_in_form_values(self, config, form_data): password_field = config['form_password_field'] if username_field not in form_data: - raise SAMLError( - self._ERROR_MISSING_FORM_FIELD % (username_field, ", ".join(form_data.keys()))) + raise SAMLError(self._ERROR_MISSING_FORM_FIELD % + (username_field, ", ".join(form_data.keys()))) else: form_data[username_field] = username if password_field in form_data: From 810c9d5fa3f16e2266818abb7c006bc6be8d347c Mon Sep 17 00:00:00 2001 From: Mateusz Boryn Date: Tue, 30 Jul 2019 14:48:38 +0200 Subject: [PATCH 4/4] comment for missing passord field check, print username when prompting for password --- awsprocesscreds/saml.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/awsprocesscreds/saml.py b/awsprocesscreds/saml.py index b3e56da..49f2935 100644 --- a/awsprocesscreds/saml.py +++ b/awsprocesscreds/saml.py @@ -79,6 +79,10 @@ class GenericFormsBasedAuthenticator(SAMLAuthenticator): 'Missing required config value for SAML: "%s"' ) + _PASSWORD_PROMPT = ( + 'Password for %s: ' + ) + def __init__(self, password_prompter, requests_session=None): """Retrieve SAML assertion using form based auth. @@ -185,9 +189,12 @@ def _fill_in_form_values(self, config, form_data): (username_field, ", ".join(form_data.keys()))) else: form_data[username_field] = username + + # In special cases, password_field is not present in the form + # for example when user is remembered. if password_field in form_data: form_data[password_field] = self._password_prompter( - "Password: ") + self._PASSWORD_PROMPT % username) def _send_form_post(self, login_url, form_data): response = self._requests_session.post(