From 7abc45795f938325b7256f9810b0598df0533bf4 Mon Sep 17 00:00:00 2001 From: Howard Johnson Date: May 21 2016 13:22:34 +0000 Subject: Don't try to use SAML error strings as HTTPError codes in a ProviderException ProviderException and the exceptions that inherit from it have an optional code parameter, which we either don't set, set to a HTTP status code, or pass a lasso error code to. We then try to use it as an HTTP status code for the cherrypy HTTPError constructor. In the case of the lasso errors (which are strings) that is very much wrong. Clear up the confusion by having each exception class have an explicit HTTP status code to use with the HTTPError constructor, and store the code parameter seperately. Add a new UnauthorizedRequest exception that returns an HTTP 401. Change the openid code to use that and InvalidRequest instead of raising AuthenticationError with an HTTP status code. Signed-off-by: Howard Johnson --- diff --git a/ipsilon/providers/common.py b/ipsilon/providers/common.py index bfe5f2d..b48c301 100644 --- a/ipsilon/providers/common.py +++ b/ipsilon/providers/common.py @@ -11,13 +11,14 @@ import cherrypy class ProviderException(cherrypy.HTTPError, Log): - code = 500 + statuscode = 500 message = None def __init__(self, message, code=None): - super(ProviderException, self).__init__(code or self.code, - self.message) + super(ProviderException, self).__init__(self.statuscode, + message or self.message) self.message = message + self.code = code self.debug('%s [%s]' % (self.message, self.code)) def __str__(self): @@ -25,19 +26,26 @@ class ProviderException(cherrypy.HTTPError, Log): class AuthenticationError(ProviderException): - code = 403 + statuscode = 403 def __init__(self, message, code=None): super(AuthenticationError, self).__init__(message, code) class InvalidRequest(ProviderException): - code = 400 + statuscode = 400 def __init__(self, message, code=None): super(InvalidRequest, self).__init__(message, code) +class UnauthorizedRequest(ProviderException): + statuscode = 401 + + def __init__(self, message, code=None): + super(UnauthorizedRequest, self).__init__(message, code) + + class ProviderBase(ConfigHelper, PluginObject): def __init__(self, name, path, *pargs): diff --git a/ipsilon/providers/openid/auth.py b/ipsilon/providers/openid/auth.py index 1e9b1fc..16fa5fc 100644 --- a/ipsilon/providers/openid/auth.py +++ b/ipsilon/providers/openid/auth.py @@ -1,7 +1,7 @@ # Copyright (C) 2014 Ipsilon project Contributors, for license see COPYING from ipsilon.providers.common import ProviderPageBase -from ipsilon.providers.common import AuthenticationError, InvalidRequest +from ipsilon.providers.common import InvalidRequest, UnauthorizedRequest from ipsilon.providers.openid.meta import XRDSHandler, UserXRDSHandler from ipsilon.providers.openid.meta import IDHandler from ipsilon.util.policy import Policy @@ -57,7 +57,7 @@ class AuthenticateRequest(ProviderPageBase): return self._openid_checks(request, form, **kwargs) except InvalidRequest, e: raise cherrypy.HTTPError(e.code, e.message) - except AuthenticationError, e: + except UnauthorizedRequest, e: if request is None: raise cherrypy.HTTPError(e.code, e.message) return self._respond(request.answer(False)) @@ -111,7 +111,7 @@ class AuthenticateRequest(ProviderPageBase): self.debug('Redirecting: %s' % redirect) raise cherrypy.HTTPRedirect(redirect) else: - raise AuthenticationError("unknown user", 401) + raise UnauthorizedRequest("unknown user") elif kwargs.get('openid.mode', None) == 'checkid_immediate': # This is immediate, so we need to assert or fail @@ -127,11 +127,11 @@ class AuthenticateRequest(ProviderPageBase): if not request.idSelect(): idurl = self.cfg.identity_url_template % {'username': user.name} if request.identity != idurl: - raise AuthenticationError("User ID mismatch!", 401) + raise UnauthorizedRequest("User ID mismatch!") # check if the relying party is trusted if request.trust_root in self.cfg.untrusted_roots: - raise AuthenticationError("Untrusted Relying party", 401) + raise UnauthorizedRequest("Untrusted Relying party") # if the party is explicitly whitelisted just respond if request.trust_root in self.cfg.trusted_roots: @@ -150,14 +150,14 @@ class AuthenticateRequest(ProviderPageBase): return self._respond(self._response(request, us)) if immediate: - raise AuthenticationError("No consent for immediate", 401) + raise UnauthorizedRequest("No consent for immediate") if self.stage == 'consent': if form is None: - raise AuthenticationError("Unintelligible consent", 401) + raise UnauthorizedRequest("Unintelligible consent") allow = form.get('decided_allow', False) if not allow: - raise AuthenticationError("User declined", 401) + raise UnauthorizedRequest("User declined") try: days = int(form.get('remember_for_days', '0')) if days < 0 or days > 7: @@ -236,7 +236,7 @@ class Continue(AuthenticateRequest): self.stage = transdata.get('openid_stage', None) openid_request = transdata.get('openid_request', None) if self.stage is None or openid_request is None: - raise AuthenticationError("unknown state", 400) + raise InvalidRequest("unknown state") kwargs = json.loads(openid_request) return self.auth(**kwargs) @@ -249,7 +249,7 @@ class Consent(AuthenticateRequest): self.stage = transdata.get('openid_stage', None) openid_request = transdata.get('openid_request', None) if self.stage is None or openid_request is None: - raise AuthenticationError("unknown state", 400) + raise InvalidRequest("unknown state") args = ({'form': kwargs},) kwargs = json.loads(openid_request) diff --git a/ipsilon/providers/saml2/auth.py b/ipsilon/providers/saml2/auth.py index d3eb3f5..84b5a4e 100644 --- a/ipsilon/providers/saml2/auth.py +++ b/ipsilon/providers/saml2/auth.py @@ -17,6 +17,7 @@ import hashlib class UnknownProvider(ProviderException): + statuscode = 400 def __init__(self, message): super(UnknownProvider, self).__init__(message) diff --git a/ipsilon/providers/saml2/provider.py b/ipsilon/providers/saml2/provider.py index 383bc6a..8635b00 100644 --- a/ipsilon/providers/saml2/provider.py +++ b/ipsilon/providers/saml2/provider.py @@ -14,6 +14,7 @@ VALID_IN_NAME = r'[^\ a-zA-Z0-9\-\.]' class InvalidProviderId(ProviderException): + statuscode = 400 def __init__(self, msg): message = 'Invalid Provider ID: %s' % msg @@ -22,6 +23,7 @@ class InvalidProviderId(ProviderException): class InvalidProviderMetadata(ProviderException): + statuscode = 400 def __init__(self, msg): message = 'Invalid Provider Metadata: %s' % msg