From bfc2847495ae9d73f9d3713eb558125811d5eff8 Mon Sep 17 00:00:00 2001 From: Mitchel Haring Date: Wed, 25 Aug 2021 13:40:42 +1000 Subject: [PATCH 1/3] add auth_state --- samlauthenticator/samlauthenticator.py | 29 ++++++++++++++++++-------- tests/test_authenticator.py | 8 ++++++- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/samlauthenticator/samlauthenticator.py b/samlauthenticator/samlauthenticator.py index 05b99ed..3dca2d3 100644 --- a/samlauthenticator/samlauthenticator.py +++ b/samlauthenticator/samlauthenticator.py @@ -686,20 +686,31 @@ def _authenticate(self, handler, data): valid_saml_response, signed_xml = self._test_valid_saml_response(saml_metadata_etree, saml_doc_etree) - if valid_saml_response: - self.log.debug('Authenticated user using SAML') - username = self._get_username_from_saml_doc(signed_xml, saml_doc_etree) - username = self.normalize_username(username) + if not valid_saml_response: + self.log.error('Error validating SAML response') + return None - if self._valid_config_and_roles(signed_xml, saml_doc_etree): - self.log.debug('Optionally create and return user: ' + username) - return self._check_username_and_add_user(username) + valid_config_and_roles = self._valid_config_and_roles(signed_xml, saml_doc_etree) + if not valid_config_and_roles: self.log.error('Assertion did not have appropriate roles') return None - self.log.error('Error validating SAML response') - return None + self.log.debug('Authenticated user using SAML') + username = self._get_username_from_saml_doc(signed_xml, saml_doc_etree) + username = self.normalize_username(username) + user_roles = self._get_roles_from_saml_doc(signed_xml, saml_doc_etree) + + self.log.debug('Optionally create and return user: ' + username) + if self._check_username_and_add_user(username): + return { + 'name': username, + 'auth_state': { + 'roles': user_roles + } + } + else: + return None @gen.coroutine def authenticate(self, handler, data): diff --git a/tests/test_authenticator.py b/tests/test_authenticator.py index 554e683..4d00962 100644 --- a/tests/test_authenticator.py +++ b/tests/test_authenticator.py @@ -674,7 +674,13 @@ def _confirm_tom(self, saml_data, mock_datetime, mock_pwd): a = SAMLAuthenticator() a.metadata_content = saml_data.metadata_xml - assert 'tom' == a._authenticate(None, {a.login_post_field: saml_data.b64encoded_response}) + user_tom = { + 'name': 'tom', + 'auth_state': { + 'roles': [] + } + } + assert user_tom == a._authenticate(None, {a.login_post_field: saml_data.b64encoded_response}) mock_datetime.now.assert_called_once_with(timezone.utc) mock_pwd.getpwnam.assert_called_once_with('tom') From be372e6d5b157b573c93370e7501fd138064fee4 Mon Sep 17 00:00:00 2001 From: Mitchel Haring Date: Wed, 25 Aug 2021 15:54:31 +1000 Subject: [PATCH 2/3] admin role --- README.md | 6 +++++- samlauthenticator/samlauthenticator.py | 23 ++++++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b1e90b7..6015678 100644 --- a/README.md +++ b/README.md @@ -94,7 +94,7 @@ The default nameid format that the SAMLAuthenticator expects is defined by the S If the server administrator wants to create local users for each JupyterHub user but doesn't want to use the `useradd` utility, a user can be added with any binary on the host system Set the `create_system_user_binary` field to either a) a full path to the binary or b) the name of a binary on the host's path. Please note, if the binary exits with code 0, the Authenticator will assume that the user add succeeded, and if the binary exits with any code _other than 0_, it will be assumed that creating the user failed. -Access is given to all users who successfully authenticate regardless of their role or group membership by default. Set the `roles` field to restrict access to JupyterHub to specific roles. Users with any of the specified roles will be authorized to access JupyterHub. The `xpath_role_location` field can be configured to set the location of the users roles in the SAML response. +Access is given to all users who successfully authenticate regardless of their role or group membership by default. Set the `roles` field to restrict access to JupyterHub to specific roles. Users with any of the specified roles will be authorized to access JupyterHub. The `xpath_role_location` field can be configured to set the location of the users roles in the SAML response. If the `admin_roles` field is set administrator access will be granted to members of the specified roles. Leave this unset if you want to manage administrators with JupyterHub configuration. #### Example Configurations @@ -125,6 +125,10 @@ c.SAMLAuthenticator.xpath_role_location = '//saml:Attribute[@Name="Roles"]/saml: # Comma-separated list of authorized roles. Allows all if not specified. c.SAMLAuthenticator.roles = 'group1,group2' +# Comma-separated list of admin roles. Overrides any other admin configuration. +# Admin privileges does not grant login. +c.SAMLAuthenticator.admin_roles = 'admin-group1,admin-group2' + # The IdP is sending the SAML Response in a field named 'R' c.SAMLAuthenticator.login_post_field = 'R' diff --git a/samlauthenticator/samlauthenticator.py b/samlauthenticator/samlauthenticator.py index 3dca2d3..4d00483 100644 --- a/samlauthenticator/samlauthenticator.py +++ b/samlauthenticator/samlauthenticator.py @@ -315,6 +315,15 @@ class SAMLAuthenticator(Authenticator): jupyterhub to these roles if specified. ''' ) + admin_roles = Unicode( + default_value=None, + allow_none=True, + config=True, + help=''' + Comma-separated list of admin roles. Users matching this role will be + be a jupyterhub administrator. + ''' + ) _const_warn_explain = 'Because no user would be allowed to log in via roles, role check disabled.' _const_warn_no_role_xpath = 'Allowed roles set while role location XPath is not set.' _const_warn_no_roles = 'Allowed roles not set while role location XPath is set.' @@ -647,6 +656,11 @@ def _check_role(self, user_roles): return any(elem in allowed_roles for elem in user_roles) + def _check_admin_role(self, user_roles): + admin_roles = [x.strip() for x in self.admin_roles.split(',')] + + return any(elem in admin_roles for elem in user_roles) + def _valid_roles_in_assertion(self, signed_xml, saml_doc_etree): user_roles = self._get_roles_from_saml_doc(signed_xml, saml_doc_etree) @@ -703,12 +717,19 @@ def _authenticate(self, handler, data): self.log.debug('Optionally create and return user: ' + username) if self._check_username_and_add_user(username): - return { + user_details = { 'name': username, 'auth_state': { 'roles': user_roles } } + + if self.admin_roles: + is_admin = self._check_admin_role(user_roles) + self.log.debug('Admin roles defined. Setting admin status to ' + str(is_admin)) + user_details['admin'] = is_admin + + return user_details else: return None From c46ff343bb43e238f39d06d25089979e8da0b831 Mon Sep 17 00:00:00 2001 From: Mitchel Haring Date: Thu, 26 Aug 2021 09:20:56 +1000 Subject: [PATCH 3/3] check admin role test --- tests/test_authenticator.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_authenticator.py b/tests/test_authenticator.py index 4d00962..bf99d0b 100644 --- a/tests/test_authenticator.py +++ b/tests/test_authenticator.py @@ -486,6 +486,31 @@ def test_check_role_fails(self): assert not a._check_role(['nogroup1', 'nogroup2']) +class TestAdminRoleAccess(unittest.TestCase): + + def test_check_admin_role(self): + a = SAMLAuthenticator() + a.admin_roles = 'admin1' + + assert a._check_admin_role(['admin1']) + assert a._check_admin_role(['admin1', 'admin2']) + + def test_check_admin_roles(self): + a = SAMLAuthenticator() + a.admin_roles='admin1, admin2, admin3' + + assert a._check_admin_role(['admin2']) + assert a._check_admin_role(['admin2', 'admin3']) + assert a._check_admin_role(['admin1', 'nogroup1']) + + def test_check_admin_role_fails(self): + a = SAMLAuthenticator() + a.admin_roles='admin1,admin2,admin3' + + assert not a._check_admin_role([]) + assert not a._check_admin_role(['nogroup1']) + assert not a._check_admin_role(['nogroup1', 'nogroup2']) + class TestValidRolesConfig(unittest.TestCase): def test_no_xpath_no_roles_run_default(self):