From b4bcb99e3217e658c1277cd5d484fa0c62c7aa0c Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Wed, 10 Sep 2014 17:20:02 -0400 Subject: [PATCH 1/1] Use transactions throughout the code Signed-off-by: Simo Sorce Reviewed-by: Patrick Uiterwijk --- ipsilon/login/authfas.py | 3 +- ipsilon/login/authform.py | 4 +-- ipsilon/login/authkrb.py | 9 +++-- ipsilon/login/authpam.py | 2 +- ipsilon/login/authtest.py | 3 +- ipsilon/login/common.py | 62 +++++++++++++++++++++++---------- ipsilon/providers/common.py | 8 ++++- ipsilon/providers/saml2/auth.py | 50 ++++++++++++++++++-------- ipsilon/providers/saml2idp.py | 8 ++--- templates/login/fas.html | 1 + templates/login/form.html | 1 + 11 files changed, 106 insertions(+), 45 deletions(-) diff --git a/ipsilon/login/authfas.py b/ipsilon/login/authfas.py index c688f3e..a571dd4 100755 --- a/ipsilon/login/authfas.py +++ b/ipsilon/login/authfas.py @@ -28,7 +28,8 @@ class FAS(LoginFormBase): except Exception, e: # pylint: disable=broad-except cherrypy.log.error("Unknown Error [%s]" % str(e)) if data and data.user: - return self.lm.auth_successful(data.user['username'], + return self.lm.auth_successful(self.trans, + data.user['username'], userdata={'fas': data.user}) else: error = "Authentication failed" diff --git a/ipsilon/login/authform.py b/ipsilon/login/authform.py index d372eff..9c20cf5 100755 --- a/ipsilon/login/authform.py +++ b/ipsilon/login/authform.py @@ -33,14 +33,14 @@ class Form(LoginFormBase): us.remote_login() user = us.get_user() if not user.is_anonymous: - return self.lm.auth_successful(user.name, 'password') + return self.lm.auth_successful(self.trans, user.name, 'password') else: try: error = cherrypy.request.headers['EXTERNAL_AUTH_ERROR'] except KeyError: error = "Unknown error using external authentication" cherrypy.log.error("Error: %s" % error) - return self.lm.auth_failed() + return self.lm.auth_failed(self.trans) class LoginManager(LoginManagerBase): diff --git a/ipsilon/login/authkrb.py b/ipsilon/login/authkrb.py index d5ceaf3..5f2d682 100755 --- a/ipsilon/login/authkrb.py +++ b/ipsilon/login/authkrb.py @@ -20,6 +20,7 @@ from ipsilon.login.common import LoginPageBase, LoginManagerBase from ipsilon.login.common import FACILITY from ipsilon.util.plugin import PluginObject +from ipsilon.util.trans import Transaction from string import Template import cherrypy import os @@ -36,13 +37,15 @@ class Krb(LoginPageBase): class KrbAuth(LoginPageBase): def root(self, *args, **kwargs): + trans = Transaction('login', **kwargs) # If we can get here, we must be authenticated and remote_user # was set. Check the session has a user set already or error. if self.user and self.user.name: userdata = {'krb_principal_name': self.user.name} - return self.lm.auth_successful(self.user.name, 'krb', userdata) + return self.lm.auth_successful(trans, self.user.name, + 'krb', userdata) else: - return self.lm.auth_failed() + return self.lm.auth_failed(trans) class KrbError(LoginPageBase): @@ -64,7 +67,7 @@ class KrbError(LoginPageBase): cont=conturl) # If we get here, negotiate failed - return self.lm.auth_failed() + return self.lm.auth_failed(Transaction('login', **kwargs)) class LoginManager(LoginManagerBase): diff --git a/ipsilon/login/authpam.py b/ipsilon/login/authpam.py index f322e14..fca44ba 100755 --- a/ipsilon/login/authpam.py +++ b/ipsilon/login/authpam.py @@ -49,7 +49,7 @@ class Pam(LoginFormBase): if username and password: user = self._authenticate(username, password) if user: - return self.lm.auth_successful(user, 'password') + return self.lm.auth_successful(self.trans, user, 'password') else: error = "Authentication failed" cherrypy.log.error(error) diff --git a/ipsilon/login/authtest.py b/ipsilon/login/authtest.py index 8eae0b6..55b30a4 100755 --- a/ipsilon/login/authtest.py +++ b/ipsilon/login/authtest.py @@ -33,7 +33,8 @@ class TestAuth(LoginFormBase): if username and password: if password == 'ipsilon': cherrypy.log("User %s successfully authenticated." % username) - return self.lm.auth_successful(username, 'password') + return self.lm.auth_successful(self.trans, + username, 'password') else: cherrypy.log("User %s failed authentication." % username) error = "Authentication failed" diff --git a/ipsilon/login/common.py b/ipsilon/login/common.py index f2254c9..2b3ac19 100755 --- a/ipsilon/login/common.py +++ b/ipsilon/login/common.py @@ -24,6 +24,7 @@ from ipsilon.util.plugin import PluginLoader, PluginObject from ipsilon.util.plugin import PluginInstaller from ipsilon.info.common import Info from ipsilon.util.cookies import SecureCookie +from ipsilon.util.trans import Transaction import cherrypy @@ -42,13 +43,9 @@ class LoginManagerBase(PluginObject, Log): base = cherrypy.config.get('base.mount', "") raise cherrypy.HTTPRedirect('%s/login/%s' % (base, path)) - def auth_successful(self, username, auth_type=None, userdata=None): - # save ref before calling UserSession login() as it - # may regenerate the session + def auth_successful(self, trans, username, auth_type=None, userdata=None): session = UserSession() - ref = session.get_data('login', 'Return') - if not ref: - ref = cherrypy.config.get('base.mount', "") + '/' + session.login(username, userdata) if self.info: userattrs = self.info.get_user_attrs(username) @@ -64,8 +61,6 @@ class LoginManagerBase(PluginObject, Log): else: userdata = {'auth_type': auth_type} - session.login(username, userdata) - # save username into a cookie if parent was form base auth if auth_type == 'password': cookie = SecureCookie(USERNAME_COOKIE, username) @@ -73,23 +68,39 @@ class LoginManagerBase(PluginObject, Log): cookie.maxage = 1296000 cookie.send() - raise cherrypy.HTTPRedirect(ref) + transdata = trans.retrieve() + self.debug(transdata) + redirect = transdata.get('login_return', + cherrypy.config.get('base.mount', "") + '/') + self.debug('Redirecting back to: %s' % redirect) + + # on direct login the UI (ie not redirected by a provider) we ned to + # remove the transaction cookie as it won't be needed anymore + if trans.provider == 'login': + trans.wipe() + raise cherrypy.HTTPRedirect(redirect) - def auth_failed(self): + def auth_failed(self, trans): # try with next module if self.next_login: return self.redirect_to_path(self.next_login.path) # return to the caller if any session = UserSession() - ref = session.get_data('login', 'Return') - # otherwise destroy session and return error - if not ref: + transdata = trans.retrieve() + + # on direct login the UI (ie not redirected by a provider) we ned to + # remove the transaction cookie as it won't be needed anymore + if trans.provider == 'login': + trans.wipe() + + # destroy session and return error + if 'login_return' not in transdata: session.logout(None) raise cherrypy.HTTPError(401) - raise cherrypy.HTTPRedirect(ref) + raise cherrypy.HTTPRedirect(transdata['login_return']) def get_tree(self, site): raise NotImplementedError @@ -151,6 +162,7 @@ class LoginPageBase(Page): def __init__(self, site, mgr): super(LoginPageBase, self).__init__(site) self.lm = mgr + self._Transaction = None def root(self, *args, **kwargs): raise cherrypy.HTTPError(500) @@ -162,6 +174,7 @@ class LoginFormBase(LoginPageBase): super(LoginFormBase, self).__init__(site, mgr) self.formpage = page self.formtemplate = template or 'login/form.html' + self.trans = None def GET(self, *args, **kwargs): context = self.create_tmpl_context() @@ -169,6 +182,7 @@ class LoginFormBase(LoginPageBase): return self._template(self.formtemplate, **context) def root(self, *args, **kwargs): + self.trans = Transaction('login', **kwargs) op = getattr(self, cherrypy.request.method, self.GET) if callable(op): return op(*args, **kwargs) @@ -176,7 +190,8 @@ class LoginFormBase(LoginPageBase): def create_tmpl_context(self, **kwargs): next_url = None if self.lm.next_login is not None: - next_url = self.lm.next_login.path + next_url = '%s?%s' % (self.lm.next_login.path, + self.trans.get_GET_arg()) cookie = SecureCookie(USERNAME_COOKIE) cookie.receive() @@ -184,6 +199,11 @@ class LoginFormBase(LoginPageBase): if username is None: username = '' + if self.trans is not None: + tid = self.trans.transaction_id + if tid is None: + tid = '' + context = { "title": 'Login', "action": '%s/%s' % (self.basepath, self.formpage), @@ -195,6 +215,10 @@ class LoginFormBase(LoginPageBase): "username": username, } context.update(kwargs) + if self.trans is not None: + t = self.trans.get_POST_tuple() + context.update({t[0]: t[1]}) + return context @@ -227,9 +251,11 @@ class Login(Page): def root(self, *args, **kwargs): if self.first_login: - raise cherrypy.HTTPRedirect('%s/login/%s' % - (self.basepath, - self.first_login.path)) + trans = Transaction('login', **kwargs) + redirect = '%s/login/%s?%s' % (self.basepath, + self.first_login.path, + trans.get_GET_arg()) + raise cherrypy.HTTPRedirect(redirect) return self._template('login/index.html', title='Login') diff --git a/ipsilon/providers/common.py b/ipsilon/providers/common.py index 6bcfef8..b1968f4 100755 --- a/ipsilon/providers/common.py +++ b/ipsilon/providers/common.py @@ -118,7 +118,13 @@ class ProviderPageBase(Page): raise cherrypy.HTTPError(501) def root(self, *args, **kwargs): - op = getattr(self, cherrypy.request.method, self.GET) + method = cherrypy.request.method + + preop = getattr(self, 'pre_%s' % method, None) + if preop and callable(preop): + preop(*args, **kwargs) + + op = getattr(self, method, self.GET) if callable(op): return op(*args, **kwargs) else: diff --git a/ipsilon/providers/saml2/auth.py b/ipsilon/providers/saml2/auth.py index e35ff13..861ef96 100755 --- a/ipsilon/providers/saml2/auth.py +++ b/ipsilon/providers/saml2/auth.py @@ -22,6 +22,7 @@ from ipsilon.providers.saml2.provider import ServiceProvider from ipsilon.providers.saml2.provider import InvalidProviderId from ipsilon.providers.saml2.provider import NameIdNotAllowed from ipsilon.util.user import UserSession +from ipsilon.util.trans import Transaction import cherrypy import datetime import lasso @@ -53,9 +54,25 @@ class AuthenticateRequest(ProviderPageBase): def __init__(self, *args, **kwargs): super(AuthenticateRequest, self).__init__(*args, **kwargs) - self.STAGE_INIT = 0 - self.STAGE_AUTH = 1 - self.stage = self.STAGE_INIT + self.stage = 'init' + self.trans = None + + def _preop(self, *args, **kwargs): + try: + # generate a new id or get current one + self.trans = Transaction('saml2', **kwargs) + if self.trans.cookie.value != self.trans.provider: + self.debug('Invalid transaction, %s != %s' % ( + self.trans.cookie.value, self.trans.provider)) + except Exception, e: # pylint: disable=broad-except + self.debug('Transaction initialization failed: %s' % repr(e)) + raise cherrypy.HTTPError(400, 'Invalid transaction id') + + def pre_GET(self, *args, **kwargs): + self._preop(*args, **kwargs) + + def pre_POST(self, *args, **kwargs): + self._preop(*args, **kwargs) def auth(self, login): try: @@ -116,21 +133,28 @@ class AuthenticateRequest(ProviderPageBase): def saml2checks(self, login): - session = UserSession() - user = session.get_user() + us = UserSession() + user = us.get_user() if user.is_anonymous: - if self.stage < self.STAGE_AUTH: - session.save_data('saml2', 'stage', self.STAGE_AUTH) - session.save_data('saml2', 'Request', login.dump()) - session.save_data('login', 'Return', - '%s/saml2/SSO/Continue' % self.basepath) - raise cherrypy.HTTPRedirect('%s/login' % self.basepath) + if self.stage == 'init': + returl = '%s/saml2/SSO/Continue?%s' % ( + self.basepath, self.trans.get_GET_arg()) + data = {'saml2_stage': 'auth', + 'saml2_request': login.dump(), + 'login_return': returl} + self.trans.store(data) + redirect = '%s/login?%s' % (self.basepath, + self.trans.get_GET_arg()) + raise cherrypy.HTTPRedirect(redirect) else: raise AuthenticationError( "Unknown user", lasso.SAML2_STATUS_CODE_AUTHN_FAILED) self._audit("Logged in user: %s [%s]" % (user.name, user.fullname)) + # We can wipe the transaction now, as this is the last step + self.trans.wipe() + # TODO: check if this is the first time this user access this SP # If required by user prefs, ask user for consent once and then # record it @@ -157,9 +181,6 @@ class AuthenticateRequest(ProviderPageBase): authtime_notbefore = authtime - skew authtime_notafter = authtime + skew - us = UserSession() - user = us.get_user() - # TODO: get authentication type fnd name format from session # need to save which login manager authenticated and map it to a # saml2 authentication context @@ -190,6 +211,7 @@ class AuthenticateRequest(ProviderPageBase): login.assertion.subject.nameId.format = nameidfmt login.assertion.subject.nameId.content = nameid else: + self.trans.wipe() raise AuthenticationError("Unavailable Name ID type", lasso.SAML2_STATUS_CODE_AUTHN_FAILED) diff --git a/ipsilon/providers/saml2idp.py b/ipsilon/providers/saml2idp.py index a19899c..e30e4a1 100755 --- a/ipsilon/providers/saml2idp.py +++ b/ipsilon/providers/saml2idp.py @@ -60,8 +60,8 @@ class Continue(AuthenticateRequest): session = UserSession() user = session.get_user() - session.nuke_data('login', 'Return') - self.stage = session.get_data('saml2', 'stage') + transdata = self.trans.retrieve() + self.stage = transdata['saml2_stage'] if user.is_anonymous: self._debug("User is marked anonymous?!") @@ -70,11 +70,11 @@ class Continue(AuthenticateRequest): self._debug('Continue auth for %s' % user.name) - dump = session.get_data('saml2', 'Request') - if not dump: + if 'saml2_request' not in transdata: self._debug("Couldn't find Request dump?!") # TODO: Return to SP with auth failed error raise cherrypy.HTTPError(400) + dump = transdata['saml2_request'] try: login = self.cfg.idp.get_login_handler(dump) diff --git a/templates/login/fas.html b/templates/login/fas.html index b856731..4188ead 100644 --- a/templates/login/fas.html +++ b/templates/login/fas.html @@ -12,6 +12,7 @@