From 692189e7f2ddcab447a8632db47e78c7b484cd95 Mon Sep 17 00:00:00 2001 From: Groboclown Date: Mon, 5 Aug 2019 11:25:51 -0500 Subject: [PATCH 1/2] Fix for #11 by allowing multiple field names to choose from. --- awsprocesscreds/saml.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/awsprocesscreds/saml.py b/awsprocesscreds/saml.py index 16ed432..4cb6151 100644 --- a/awsprocesscreds/saml.py +++ b/awsprocesscreds/saml.py @@ -57,8 +57,8 @@ def retrieve_saml_assertion(self, config): class GenericFormsBasedAuthenticator(SAMLAuthenticator): - USERNAME_FIELD = 'username' - PASSWORD_FIELD = 'password' + USERNAME_FIELDS = ('username',) + PASSWORD_FIELDS = ('password',) _ERROR_BAD_RESPONSE = ( 'Received a non-200 response (%s) when making a request to: %s' @@ -175,13 +175,15 @@ 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 = set(self.USERNAME_FIELDS).intersection(form_data.keys()) + if not username_field: raise SAMLError( - self._ERROR_MISSING_FORM_FIELD % self.USERNAME_FIELD) + self._ERROR_MISSING_FORM_FIELD % self.USERNAME_FIELDS) 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.pop()] = username + password_field = set(self.PASSWORD_FIELDS).intersection(form_data.keys()) + if password_field: + form_data[password_field.pop()] = self._password_prompter( "Password: ") def _send_form_post(self, login_url, form_data): @@ -255,8 +257,8 @@ def is_suitable(self, config): class ADFSFormsBasedAuthenticator(GenericFormsBasedAuthenticator): - USERNAME_FIELD = 'ctl00$ContentPlaceHolder1$UsernameTextBox' - PASSWORD_FIELD = 'ctl00$ContentPlaceHolder1$PasswordTextBox' + USERNAME_FIELDS = ('ctl00$ContentPlaceHolder1$UsernameTextBox', 'UserName',) + PASSWORD_FIELDS = ('ctl00$ContentPlaceHolder1$PasswordTextBox', 'Password',) def is_suitable(self, config): return (config.get('saml_authentication_type') == 'form' and From 820cd4219b50d32cca19844c696ef8681d7c11c3 Mon Sep 17 00:00:00 2001 From: Groboclown Date: Mon, 5 Aug 2019 11:37:07 -0500 Subject: [PATCH 2/2] Added unit test for new form check, and updated minor stylings based on prcheck results. --- awsprocesscreds/saml.py | 34 ++++++++++++++++++++++++---------- tests/unit/test_saml.py | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/awsprocesscreds/saml.py b/awsprocesscreds/saml.py index 4cb6151..4a04fdc 100644 --- a/awsprocesscreds/saml.py +++ b/awsprocesscreds/saml.py @@ -175,13 +175,17 @@ def _parse_form_from_html(self, html): def _fill_in_form_values(self, config, form_data): username = config['saml_username'] - username_field = set(self.USERNAME_FIELDS).intersection(form_data.keys()) + username_field = set(self.USERNAME_FIELDS).intersection( + form_data.keys() + ) if not username_field: raise SAMLError( self._ERROR_MISSING_FORM_FIELD % self.USERNAME_FIELDS) - else: - form_data[username_field.pop()] = username - password_field = set(self.PASSWORD_FIELDS).intersection(form_data.keys()) + form_data[username_field.pop()] = username + + password_field = set(self.PASSWORD_FIELDS).intersection( + form_data.keys() + ) if password_field: form_data[password_field.pop()] = self._password_prompter( "Password: ") @@ -252,17 +256,27 @@ def retrieve_saml_assertion(self, config): return r def is_suitable(self, config): - return (config.get('saml_authentication_type') == 'form' and - config.get('saml_provider') == 'okta') + return ( + config.get('saml_authentication_type') == 'form' + and config.get('saml_provider') == 'okta' + ) class ADFSFormsBasedAuthenticator(GenericFormsBasedAuthenticator): - USERNAME_FIELDS = ('ctl00$ContentPlaceHolder1$UsernameTextBox', 'UserName',) - PASSWORD_FIELDS = ('ctl00$ContentPlaceHolder1$PasswordTextBox', 'Password',) + USERNAME_FIELDS = ( + 'ctl00$ContentPlaceHolder1$UsernameTextBox', + 'UserName', + ) + PASSWORD_FIELDS = ( + 'ctl00$ContentPlaceHolder1$PasswordTextBox', + 'Password', + ) def is_suitable(self, config): - return (config.get('saml_authentication_type') == 'form' and - config.get('saml_provider') == 'adfs') + return ( + config.get('saml_authentication_type') == 'form' + and config.get('saml_provider') == 'adfs' + ) class FormParser(six.moves.html_parser.HTMLParser): diff --git a/tests/unit/test_saml.py b/tests/unit/test_saml.py index db2e218..a588c3f 100644 --- a/tests/unit/test_saml.py +++ b/tests/unit/test_saml.py @@ -423,8 +423,8 @@ def test_non_adfs_not_suitable(self, adfs_auth): } assert not adfs_auth.is_suitable(config) - def test_uses_adfs_fields(self, adfs_auth, mock_requests_session, - adfs_config): + def test_uses_adfs_fields_newer(self, adfs_auth, mock_requests_session, + adfs_config): adfs_login_form = ( '' '
' @@ -454,6 +454,37 @@ def test_uses_adfs_fields(self, adfs_auth, mock_requests_session, } ) + def test_uses_adfs_fields_older(self, adfs_auth, mock_requests_session, + adfs_config): + adfs_login_form = ( + '' + '' + '' + '' + '
' + '' + ) + mock_requests_session.get.return_value = mock.Mock( + spec=requests.Response, status_code=200, text=adfs_login_form + ) + mock_requests_session.post.return_value = mock.Mock( + spec=requests.Response, status_code=200, text=( + '
' + ) + ) + + saml_assertion = adfs_auth.retrieve_saml_assertion(adfs_config) + assert saml_assertion == 'fakeassertion' + + mock_requests_session.post.assert_called_with( + "https://example.com/login", verify=True, + data={ + 'UserName': 'monty', + 'Password': 'mypassword' + } + ) + class TestFormParser(object): def test_parse_form(self, basic_form):