diff --git a/openid/extensions/draft/pape5.py b/openid/extensions/draft/pape5.py index 497df154..5826f651 100644 --- a/openid/extensions/draft/pape5.py +++ b/openid/extensions/draft/pape5.py @@ -13,12 +13,12 @@ 'AUTH_PHISHING_RESISTANT', 'AUTH_MULTI_FACTOR', 'AUTH_MULTI_FACTOR_PHYSICAL', - 'AUTH_NONE', 'LEVELS_NIST', 'LEVELS_JISA', ] from openid.extension import Extension +import warnings import re ns_uri = "http://specs.openid.net/extensions/pape/1.0" @@ -271,6 +271,9 @@ def preferredTypes(self, supported_types): class Response(PAPEExtension): """A Provider Authentication Policy response, sent from a provider to a relying party + + @ivar auth_policies: List of authentication policies conformed to + by this OpenID assertion, represented as policy URIs """ ns_alias = 'pape' @@ -338,6 +341,10 @@ def addPolicyURI(self, policy_uri): authentication. @see: http://openid.net/specs/openid-provider-authentication-policy-extension-1_0-01.html#auth_policies """ + if policy_uri == AUTH_NONE: + raise RuntimeError( + 'To send no policies, do not set any on the response.') + if policy_uri not in self.auth_policies: self.auth_policies.append(policy_uri) @@ -382,8 +389,28 @@ def parseExtensionArgs(self, args, strict=False): this object. """ policies_str = args.get('auth_policies') - if policies_str and policies_str != 'none': - self.auth_policies = policies_str.split(' ') + if policies_str: + auth_policies = policies_str.split(' ') + elif strict: + raise ValueError('Missing auth_policies') + else: + auth_policies = [] + + if (len(auth_policies) > 1 and strict and AUTH_NONE in auth_policies): + raise ValueError('Got some auth policies, as well as the special ' + '"none" URI: %r' % (auth_policies,)) + + if 'none' in auth_policies: + msg = '"none" used as a policy URI (see PAPE draft < 5)' + if strict: + raise ValueError(msg) + else: + warnings.warn(msg, stacklevel=2) + + auth_policies = [u for u in auth_policies + if u not in ['none', AUTH_NONE]] + + self.auth_policies = auth_policies for (key, val) in args.iteritems(): if key.startswith('auth_level.'): @@ -418,7 +445,7 @@ def getExtensionArgs(self): """ if len(self.auth_policies) == 0: ns_args = { - 'auth_policies':'none', + 'auth_policies': AUTH_NONE, } else: ns_args = { diff --git a/openid/test/test_pape_draft5.py b/openid/test/test_pape_draft5.py index a61aa685..acd13de3 100644 --- a/openid/test/test_pape_draft5.py +++ b/openid/test/test_pape_draft5.py @@ -3,6 +3,10 @@ from openid.message import * from openid.server import server +import warnings +warnings.filterwarnings('ignore', module=__name__, + message='"none" used as a policy URI') + import unittest class PapeRequestTestCase(unittest.TestCase): @@ -246,8 +250,11 @@ def test_add_policy_uri(self): pape.AUTH_PHISHING_RESISTANT], self.resp.auth_policies) + self.failUnlessRaises(RuntimeError, self.resp.addPolicyURI, + pape.AUTH_NONE) + def test_getExtensionArgs(self): - self.failUnlessEqual({'auth_policies': 'none'}, + self.failUnlessEqual({'auth_policies': pape.AUTH_NONE}, self.resp.getExtensionArgs()) self.resp.addPolicyURI('http://uri') self.failUnlessEqual({'auth_policies': 'http://uri'}, @@ -280,11 +287,52 @@ def test_parseExtensionArgs(self): self.failUnlessEqual(['http://foo','http://bar'], self.resp.auth_policies) + def test_parseExtensionArgs_valid_none(self): + args = {'auth_policies': pape.AUTH_NONE} + self.resp.parseExtensionArgs(args) + self.failUnlessEqual([], self.resp.auth_policies) + + def test_parseExtensionArgs_old_none(self): + args = {'auth_policies': 'none'} + self.resp.parseExtensionArgs(args) + self.failUnlessEqual([], self.resp.auth_policies) + + def test_parseExtensionArgs_old_none_strict(self): + args = {'auth_policies': 'none'} + self.failUnlessRaises(ValueError, + self.resp.parseExtensionArgs, args, strict=True) + def test_parseExtensionArgs_empty(self): self.resp.parseExtensionArgs({}) self.failUnlessEqual(None, self.resp.auth_time) self.failUnlessEqual([], self.resp.auth_policies) + def test_parseExtensionArgs_empty_strict(self): + self.failUnlessRaises(ValueError, + self.resp.parseExtensionArgs, {}, strict=True) + + def test_parseExtensionArgs_ignore_superfluous_none(self): + policies = [pape.AUTH_NONE, pape.AUTH_MULTI_FACTOR_PHYSICAL] + + args = { + 'auth_policies': ' '.join(policies), + } + + self.resp.parseExtensionArgs(args, strict=False) + + self.assertEqual([pape.AUTH_MULTI_FACTOR_PHYSICAL], + self.resp.auth_policies) + + def test_parseExtensionArgs_none_strict(self): + policies = [pape.AUTH_NONE, pape.AUTH_MULTI_FACTOR_PHYSICAL] + + args = { + 'auth_policies': ' '.join(policies), + } + + self.failUnlessRaises(ValueError, self.resp.parseExtensionArgs, + args, strict=True) + def test_parseExtensionArgs_strict_bogus1(self): args = {'auth_policies': 'http://foo http://bar', 'auth_time': 'yesterday'}