Closed itsnagaraj closed 4 years ago
@itsnagaraj - I think this is a pretty large feature. It involves adding one or more new configurable fields on the base object, and changing the documentation, and gating the login based on one (or more) new XPaths. There are also going to be a lot of tests with this feature.
That being said, I think this is an important feature to add. I think it's obviously an oversight on my part to not have included this in the first version of the project, and I think that it's a good thing that people are calling out for a better user experience when using this software.
I think because of the reasons listed above, this should probably go into some sort of 0.1.0 version - this is probably not a bugfix-type operation, but it's important.
Since I'm not getting a lot of time to work on this project at work, this might be slow, but I'll definitely add this to the v0.1.0 milestone.
Thanks for your response. I am thinking along these lines
2 configurable fields required to allow for role check
xpath_role_location = Unicode(
default_value='//saml:Role/text()',
allow_none=True,
config=True,
help='''
This is an XPath that specifies where the user's allowed roles is located in the
SAML Assertion. This is to restrict users with certain roles granted by the administrator
to have access to jupyterhub:
{
'ds' : 'http://www.w3.org/2000/09/xmldsig#',
'md' : 'urn:oasis:names:tc:SAML:2.0:metadata',
'saml' : 'urn:oasis:names:tc:SAML:2.0:assertion',
'samlp': 'urn:oasis:names:tc:SAML:2.0:protocol'
}
'''
)
allowed_roles = List(
default_value=None,
allow_none=True,
config=True,
help='''
This values specifies what roles are allowed to have access to jupyterhub.
'''
)
Rules for check:
allowed_roles
are specifiedallowed_roles
are specified then read the roles
form SAML response using the xpathroles
is empty then fail authenticationroles
is not empty and does not have any of the specified allowed_roles
then fail authentication if self.allowed_roles and self.allowed_roles.len() > 0:
user_roles = self._get_roles_from_saml_doc(signed_xml, saml_doc_etree)
if user_roles.len() == 0:
return None
if user_roles.len() > 0:
role_check = any(elem in self.allowed_roles for elem in user_roles)
if not role_check:
return None
@distortedsignal I have added configurable role access but need advice on updating the test constants. It looks to be using responses from a cloud IAM provider. So changes can't be made that require the signature to be recalculated.
Any recommendations on the best way forward?
@0nebody if you want to open the PR, I'd be willing to work on getting the unit tests set up for you.
I think that after @0nebody's PR, this can be closed. I'll work on getting this out over the weekend.
With the existing implementation of SAML authenticator is it possible to restrict access by roles / groups that are returned in the SAML response? If not any plans to add in the near future?