PR#41: Ticket #191, populate user attributes when authentication is external to Ipsilon - ipsilon

ipsilon

#41 Ticket #191, populate user attributes when authentication is external to Ipsilon

Merged 8 years ago by puiterwijk. Opened 8 years ago by jdennis.

jdennis/ipsilon apache-login into master

Handle user session data for both internal and external authentication

John Dennis • 8 years ago

8e01bae

ipsilon/login/common.py

file modified

+98 -17

		`@@ -7,35 +7,93 @@`
		`from ipsilon.util.config import ConfigHelper`
		`from ipsilon.info.common import Info`
		`from ipsilon.util.cookies import SecureCookie`
		`+ from ipsilon.util.log import Log`
		`import cherrypy`


		`USERNAME_COOKIE = 'ipsilon_default_username'`


		`- class LoginManagerBase(ConfigHelper, PluginObject):`
		`+ class LoginHelper(Log):`

		`- def __init__(self, *args):`
		`- ConfigHelper.__init__(self)`
		`- PluginObject.__init__(self, *args)`
		`- self._root = None`
		`- self._site = None`
		`- self.path = '/'`
		`- self.info = None`
		`+ """Common code supporing login operations.`

		`- def redirect_to_path(self, path, trans=None):`
		`- base = cherrypy.config.get('base.mount', "")`
		`- url = '%s/login/%s' % (base, path)`
		`- if trans:`
		`- url += '?%s' % trans.get_GET_arg()`
		`- raise cherrypy.HTTPRedirect(url)`
		`+ Ipsilon can authtenticate a user by itself via it's own login`
		+ handlers (classes derived from `LoginManager`) or it can
		`+ capitalize on the authentication provided by the container Ipsilon`
		`+ is running in (currently WSGI inside Apache). We refer to the`
		`+ later as "external authentication" because it occurs outside of`
		`+ Ipsilon. However in both cases there is a common need to execute`
		`+ the same code irregardless of where the authntication`
		`+ occurred. This class serves that purpose.`
		`+ """`
		`+`
		`+ def get_external_auth_info(self):`
		`+ """Return the username and auth type for external authentication.`
		`+`
		`+ If the container Ipsilon is running inside of has already`
		`+ authenticated the user prior to reaching one of our endpoints`
		`+ return the username and the name of authenticaion method`
		`+ used. In Apache this will be REMOTE_USER and AUTH_TYPE.`
		`+`
		`+ The returned auth_type will be prefixed with the string`
		`+ "external:" to clearly distinguish between the same method`
		`+ being used internally by Ipsilon from the same method used by`
		`+ the container hosting Ipsilon. The returned auth_type string`
		`+ will be lower case.`
		`+`
		`+ If there was no external authentication both username and`
		`+ auth_type will be None. It is possible for a username to be`
		`+ returned without knowing the auth_type.`
		`+`
		`+ :return: tuple of (username, auth_type)`
		`+ """`
		`+`
		`+ auth_type = None`
		`+ username = cherrypy.request.login`
		`+ if username:`
		`+ auth_type = cherrypy.request.wsgi_environ.get('AUTH_TYPE')`
		`+ if auth_type:`
		`+ auth_type = 'external:%s' % (auth_type.lower())`
		`+`
		`+ self.debug("get_external_auth_info: username=%s auth_type=%s" % (`
		`+ username, auth_type))`
		`+`
		`+ return username, auth_type`
		`+`
		`+ def initialize_login_session(self, username, info=None,`
		`+ auth_type=None, userdata=None):`
		`+ """Establish a login session for a user.`
		`+`
		+ Builds a `UserSession` object and bind attributes associated
		`+ with the user to the session.`
		`+`
		+ User attributes derive from two sources, the `Info` object
		+ passed as the info parameter and the userdata dict. The `Info`
		`+ object encapsulates the info plugins run by Ipsilon. The`
		`+ userdata dict is additional information typically derived`
		`+ during authentication.`
		`+`
		+ The `Info` derived attributes are merged with the userdata
		`+ attributes to form one set of user attributes. The user`
		`+ attributes are checked for consistenccy. Additional attrbutes`
		`+ may be synthesized and added to the user attributes. The final`
		`+ set of user attributes is then bound to the returned`
		+ `UserSession` object.
		`+`
		`+ :param username: The username bound to the identity principal`
		+ :param info: A `Info` object providing user attributes
		`+ :param auth_type: Authenication method name`
		`+ :param userdata: Dict of additional user attributes`
		`+`
		+ :return: `UserSession` object
		`+ """`

		`- def auth_successful(self, trans, username, auth_type=None, userdata=None):`
		`session = UserSession()`

		`# merge attributes from login plugin and info plugin`
		`- if self.info:`
		`- infoattrs = self.info.get_user_attrs(username)`
		`+ if info:`
		`+ infoattrs = info.get_user_attrs(username)`
		`else:`
		`infoattrs = dict()`

		`@@ -65,6 +123,29 @@`
		`# create session login including all the userdata just gathered`
		`session.login(username, userdata)`

		`+ return session`
alert("boo! (puiterwijk was here)"); Uiterwijk (puiterwijk)">puiterwijk commented 8 years ago Why do you return this, if you never use the return value?
jdennis commented 8 years ago Because this is where the session object is created, if you need access to it then where do you get it from unless it's returned from this method? There is no harm in returning a value even if the return value is not currently utilized.
alert("boo! (puiterwijk was here)"); Uiterwijk (puiterwijk)">puiterwijk commented 8 years ago Fair enough, I was just being curious.
		`+`
		`+`
		`+ class LoginManagerBase(ConfigHelper, PluginObject, LoginHelper):`
		`+`
		`+ def __init__(self, *args):`
		`+ ConfigHelper.__init__(self)`
		`+ PluginObject.__init__(self, *args)`
		`+ self._root = None`
		`+ self._site = None`
		`+ self.path = '/'`
		`+ self.info = None`
		`+`
		`+ def redirect_to_path(self, path, trans=None):`
		`+ base = cherrypy.config.get('base.mount', "")`
		`+ url = '%s/login/%s' % (base, path)`
		`+ if trans:`
		`+ url += '?%s' % trans.get_GET_arg()`
		`+ raise cherrypy.HTTPRedirect(url)`
		`+`
		`+ def auth_successful(self, trans, username, auth_type=None, userdata=None):`
		`+ self.initialize_login_session(username, self.info, auth_type, userdata)`
		`+`
		`# save username into a cookie if parent was form base auth`
		`if auth_type == 'password':`
		`cookie = SecureCookie(USERNAME_COOKIE, username)`

ipsilon/providers/openid/auth.py

file modified

+2 -2

		`@@ -17,8 +17,8 @@`

		`class AuthenticateRequest(ProviderPageBase):`

		`- def __init__(self, args, *kwargs):`
		`- super(AuthenticateRequest, self).__init__(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(AuthenticateRequest, self).__init__(site, provider)`
		`self.stage = 'init'`
		`self.trans = None`

ipsilon/providers/openid/meta.py

file modified

+2 -2

		`@@ -7,8 +7,8 @@`

		`class MetaHandler(ProviderPageBase):`

		`- def __init__(self, args, *kwargs):`
		`- super(MetaHandler, self).__init__(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(MetaHandler, self).__init__(site, provider)`
		`self._template_name = None`
		`self._take_args = False`

ipsilon/providers/persona/auth.py

file modified

+2 -2

		`@@ -13,8 +13,8 @@`

		`class AuthenticateRequest(ProviderPageBase):`

		`- def __init__(self, args, *kwargs):`
		`- super(AuthenticateRequest, self).__init__(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(AuthenticateRequest, self).__init__(site, provider)`
		`self.trans = None`

		`def _preop(self, args, *kwargs):`

ipsilon/providers/saml2/auth.py

file modified

+2 -2

		`@@ -25,8 +25,8 @@`

		`class AuthenticateRequest(ProviderPageBase):`

		`- def __init__(self, args, *kwargs):`
		`- super(AuthenticateRequest, self).__init__(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(AuthenticateRequest, self).__init__(site, provider)`
		`self.stage = 'init'`
		`self.trans = None`
		`self.binding = None`

ipsilon/providers/saml2/logout.py

file modified

+2 -2

		`@@ -28,8 +28,8 @@`
		`deleted.`
		`"""`

		`- def __init__(self, args, *kwargs):`
		`- super(LogoutRequest, self).__init__(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(LogoutRequest, self).__init__(site, provider)`

		`def _handle_logout_request(self, us, logout, saml_sessions, message):`
		`self.debug('Logout request')`

ipsilon/providers/saml2idp.py

file modified

+28 -27

		`@@ -1,5 +1,6 @@`
		`# Copyright (C) 2014 Ipsilon project Contributors, for license see COPYING`

		`+ from ipsilon.login.common import LoginHelper`
		`from ipsilon.providers.common import ProviderBase, ProviderPageBase, \`
		`ProviderInstaller`
		`from ipsilon.providers.saml2.auth import AuthenticateRequest`
		`@@ -32,10 +33,12 @@`
		`return 'ECP_ERROR_MISSING_AUTHN_REQUEST' in dir(lasso)`


		`- class SSO_SOAP(AuthenticateRequest):`
		`+ class SSO_SOAP(AuthenticateRequest, LoginHelper):`

		`- def __init__(self, args, *kwargs):`
		`- super(SSO_SOAP, self).__init__(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(SSO_SOAP, self).__init__(site, provider, args, *kwargs)`
		`+ # pylint: disable=protected-access`
		`+ self.info = provider._root.login.info`
		`self.binding = metadata.SAML2_SERVICE_MAP['sso-soap'][1]`

		`@cherrypy.tools.require_content_type(`
		`@@ -49,13 +52,11 @@`
		`self.debug("SSO_SOAP transaction provider=%s id=%s" %`
		`(self.trans.provider, self.trans.transaction_id))`

		`- us = UserSession()`
		`- us.remote_login()`
		`- user = us.get_user()`
		`- self.debug("SSO_SOAP user=%s" % (user.name))`
		`-`
		`- if not user:`
		`+ username, auth_type = self.get_external_auth_info()`
		`+ if not username:`
		`raise cherrypy.HTTPError(403, 'No user specified for SSO_SOAP')`
		`+ self.debug("SSO_SOAP user=%s auth_type=%s" % (username, auth_type))`
		`+ self.initialize_login_session(username, self.info, auth_type)`

		`soap_xml_doc = cherrypy.request.rfile.read()`
		`soap_xml_doc = soap_xml_doc.strip()`
		`@@ -67,8 +68,8 @@`

		`class Redirect(AuthenticateRequest):`

		`- def __init__(self, args, *kwargs):`
		`- super(Redirect, self).__init__(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(Redirect, self).__init__(site, provider, args, *kwargs)`
		`self.binding = metadata.SAML2_SERVICE_MAP['sso-redirect'][1]`

		`def GET(self, args, *kwargs):`
		`@@ -81,8 +82,8 @@`

		`class POSTAuth(AuthenticateRequest):`

		`- def __init__(self, args, *kwargs):`
		`- super(POSTAuth, self).__init__(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(POSTAuth, self).__init__(site, provider, args, *kwargs)`
		`self.binding = metadata.SAML2_SERVICE_MAP['sso-post'][1]`

		`def POST(self, args, *kwargs):`
		`@@ -145,20 +146,20 @@`

		`class SSO(ProviderPageBase):`

		`- def __init__(self, args, *kwargs):`
		`- super(SSO, self).__init__(args, *kwargs)`
		`- self.Redirect = Redirect(args, *kwargs)`
		`- self.POST = POSTAuth(args, *kwargs)`
		`- self.Continue = Continue(args, *kwargs)`
		`- self.SOAP = SSO_SOAP(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(SSO, self).__init__(site, provider)`
		`+ self.Redirect = Redirect(site, provider, args, *kwargs)`
		`+ self.POST = POSTAuth(site, provider, args, *kwargs)`
		`+ self.Continue = Continue(site, provider, args, *kwargs)`
		`+ self.SOAP = SSO_SOAP(site, provider, args, *kwargs)`


		`class SLO(ProviderPageBase):`

		`- def __init__(self, args, *kwargs):`
		`- super(SLO, self).__init__(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(SLO, self).__init__(site, provider)`
		`self.debug('SLO init')`
		`- self.Redirect = Logout(args, *kwargs)`
		`+ self.Redirect = Logout(site, provider, args, *kwargs)`


		`# one week`
		`@@ -199,11 +200,11 @@`

		`class SAML2(ProviderPageBase):`

		`- def __init__(self, args, *kwargs):`
		`- super(SAML2, self).__init__(args, *kwargs)`
		`- self.metadata = Metadata(args, *kwargs)`
		`- self.SSO = SSO(args, *kwargs)`
		`- self.SLO = SLO(args, *kwargs)`
		`+ def __init__(self, site, provider, args, *kwargs):`
		`+ super(SAML2, self).__init__(site, provider)`
		`+ self.metadata = Metadata(site, provider, args, *kwargs)`
		`+ self.SSO = SSO(site, provider, args, *kwargs)`
		`+ self.SLO = SLO(site, provider, args, *kwargs)`


		`class IdpProvider(ProviderBase):`

no initial comment

jdennis commented 8 years ago

FYI: all unit tests continue to pass with this patch. This is needed for Openstack ECP, Nathan would like to get the patches in place so we can create a patch set for RHEL 7.2 z stream.

puiterwijk commented 8 years ago

Thanks for the info.
Could you please rebase your patch against the current master and push that?
As soon as you have done that I'll review it.

puiterwijk commented on line 18 of ipsilon/providers/saml2idp.py 8 years ago

This will re-initialize all the info plugins all over again.
It would be better if you just grab the info from the root.login.info.

puiterwijk commented on line 116 of ipsilon/login/common.py 8 years ago

Why do you return this, if you never use the return value?

puiterwijk commented 8 years ago

Okay, the rebase is a clear rebase, and other than my two comments it looks good.
Please answer on my comments and fix at least the second one.

jdennis commented 8 years ago

How do I know root.login.info has been initialized? Maybe the Info class needs a factory method.

jdennis commented on line 18 of ipsilon/providers/saml2idp.py 8 years ago

How do I know root.login.info has been initialized already? Maybe the Info class needs a factory method.

puiterwijk commented 8 years ago

If you set self.info inside init_idp, you are certain that: 1. it gets set before any requests come in, and 2. the framework itself has finished initializing at least the basic structures and pointers.

jdennis commented on line 116 of ipsilon/login/common.py 8 years ago

Because this is where the session object is created, if you need access to it then where do you get it from unless it's returned from this method? There is no harm in returning a value even if the return value is not currently utilized.

puiterwijk commented on line 116 of ipsilon/login/common.py 8 years ago

Fair enough, I was just being curious.

puiterwijk commented on line 18 of ipsilon/providers/saml2idp.py 8 years ago

jdennis commented on line 18 of ipsilon/providers/saml2idp.py 8 years ago

I don't think your suggestion is going to work Patrick. I tried to implement it but the class hierarchy doesn't allow for it, or at least as far as I can tell. Here is the problem, SSO_SOAP is derived from AuthenticateRequest which is derived from ProviderPageBase. init_idp is a method of the class IdpProvider which is derived from ProviderBase. ProviderBase. ProviderBase has the member _root which is the root that has login hung off it. But where I need access to the Info object is in a class (SSO_SOAP) that is derived from ProviderPageBase and there is no access to the root there.

Unless you can think of another way to access root.info from withing SSO_SOAP (I couldn't find a way) then I think making Info a singleton and getting it via a factory is the best solution, or I'm open to another idea. Thoughts?

jdennis commented 8 years ago

Patch has been reworked to obtain a reference to login.info instead of instantiating a new Info object.

puiterwijk commented 8 years ago

Looks good to me.