From 0ef93ffa2b9baa6cb079b0e1ef62b1eb9d561097 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 23 2016 12:29:49 +0000 Subject: [PATCH 1/4] Update consent revoke test helper to explicitly revoke all consent Signed-off-by: Patrick Uiterwijk Reviewed-by: Howard Johnson --- diff --git a/tests/helpers/http.py b/tests/helpers/http.py index 210ecfe..587dc93 100755 --- a/tests/helpers/http.py +++ b/tests/helpers/http.py @@ -589,9 +589,9 @@ class HttpSessions(object): if client_id in r.text: raise ValueError('Client was not gone after deletion') - def revoke_oidc_consent(self, idp): + def revoke_all_consent(self, idp): """ - Revoke user's consent for all OpenIDC clients. + Revoke user's consent for all clients. """ idpsrv = self.servers[idp] idpuri = idpsrv['baseuri'] diff --git a/tests/openidc.py b/tests/openidc.py index 87a3381..e4215ba 100755 --- a/tests/openidc.py +++ b/tests/openidc.py @@ -277,7 +277,7 @@ if __name__ == '__main__': print "openidc: Revoking SP consent ...", try: - page = sess.revoke_oidc_consent(idpname) + page = sess.revoke_all_consent(idpname) except ValueError, e: print >> sys.stderr, "" % repr(e) sys.exit(1) From 997c143d00ef3050222f4ca652dcb14e27520a96 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 23 2016 12:29:52 +0000 Subject: [PATCH 2/4] Do not mark the OpenID submission form as a consent page Signed-off-by: Patrick Uiterwijk Reviewed-by: Howard Johnson --- diff --git a/tests/helpers/http.py b/tests/helpers/http.py index 587dc93..161479b 100755 --- a/tests/helpers/http.py +++ b/tests/helpers/http.py @@ -330,7 +330,6 @@ class HttpSessions(object): try: (action, url, args) = self.handle_openid_form(page) - seen_consent = True continue except WrongPage: pass From 60189a8714183d7cea93837d5166f077c489c104 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 23 2016 12:29:54 +0000 Subject: [PATCH 3/4] Plumb the Consent system into OpenID Signed-off-by: Patrick Uiterwijk Reviewed-by: Howard Johnson --- diff --git a/ipsilon/providers/openid/auth.py b/ipsilon/providers/openid/auth.py index 0a8672c..2521785 100644 --- a/ipsilon/providers/openid/auth.py +++ b/ipsilon/providers/openid/auth.py @@ -10,8 +10,8 @@ from ipsilon.util.user import UserSession from openid.server.server import ProtocolError, EncodingError +from base64 import b64encode import cherrypy -import time import json @@ -147,17 +147,29 @@ class AuthenticateRequest(ProviderPageBase): if request.trust_root in self.cfg.trusted_roots: return self._respond(self._response(request, us)) - allowroot = 'allow-%s' % request.trust_root - - try: - userdata = user.load_plugin_data(self.cfg.name) - expiry = int(userdata[allowroot]) - except Exception, e: # pylint: disable=broad-except - self.debug(e) - expiry = 0 - if expiry > int(time.time()): - self.debug("User has unexpired previous authorization") - return self._respond(self._response(request, us)) + # Add extension data to this dictionary + ad = { + "Trust Root": request.trust_root, + } + userattrs = self._source_attributes(us) + for n, e in self.cfg.extensions.available().items(): + data = e.get_display_data(request, userattrs) + self.debug('%s returned %s' % (n, repr(data))) + for key, value in data.items(): + ad[self.cfg.mapping.display_name(key)] = value + + # We base64 encode the trust_root when looking up consent data to + # ensure the client ID is safe for the cherrypy url routing + consentdata = user.get_consent('openid', b64encode(request.trust_root)) + if consentdata is not None: + # Consent has already been granted + self.debug('Consent already granted') + + attrlist = set(ad.keys()) + consattrs = set(consentdata['attributes']) + + if attrlist.issubset(consattrs): + return self._respond(self._response(request, us)) if immediate: raise UnauthorizedRequest("No consent for immediate") @@ -168,16 +180,13 @@ class AuthenticateRequest(ProviderPageBase): allow = form.get('decided_allow', False) if not allow: raise UnauthorizedRequest("User declined") - try: - days = int(form.get('remember_for_days', '0')) - if days < 0 or days > 7: - raise InvalidRequest('Invalid number of days to ' + - 'remember specified') - userdata = {allowroot: str(int(time.time()) + (days*86400))} - user.save_plugin_data(self.cfg.name, userdata) - except Exception, e: # pylint: disable=broad-except - self.debug(e) - days = 0 + + # Store new consent + consentdata = { + 'attributes': ad.keys() + } + user.grant_consent('openid', b64encode(request.trust_root), + consentdata) # all done we consent! return self._respond(self._response(request, us)) @@ -187,17 +196,6 @@ class AuthenticateRequest(ProviderPageBase): 'openid_request': json.dumps(kwargs)} self.trans.store(data) - # Add extension data to this dictionary - ad = { - "Trust Root": request.trust_root, - } - userattrs = self._source_attributes(us) - for n, e in self.cfg.extensions.available().items(): - data = e.get_display_data(request, userattrs) - self.debug('%s returned %s' % (n, repr(data))) - for key, value in data.items(): - ad[self.cfg.mapping.display_name(key)] = value - context = { "title": 'Consent', "action": '%s/openid/Consent' % (self.basepath), diff --git a/ipsilon/providers/openidp.py b/ipsilon/providers/openidp.py index 3016656..0b07170 100644 --- a/ipsilon/providers/openidp.py +++ b/ipsilon/providers/openidp.py @@ -10,6 +10,7 @@ from ipsilon.util.plugin import PluginObject from ipsilon.util import config as pconfig from ipsilon.info.common import InfoMapping +from base64 import b64decode from openid.server.server import Server @@ -136,10 +137,12 @@ Provides OpenID 2.0 authentication infrastructure. """ self.extensions.enable(self._config['enabled extensions'].get_value()) def get_client_display_name(self, clientid): - return clientid + # We store this base64-encoded to get around limitations in the url + # routing of cherrypy + return b64decode(clientid) def consent_to_display(self, consentdata): - return [] + return consentdata['attributes'] class Installer(ProviderInstaller): diff --git a/templates/openid/consent_form.html b/templates/openid/consent_form.html index e8ead1d..3ce40e7 100644 --- a/templates/openid/consent_form.html +++ b/templates/openid/consent_form.html @@ -19,18 +19,6 @@ {%- endfor %}
- -
- -
-
-
From 231deb8bfec17e4688861c490508a9a2eee9b8ea Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 23 2016 12:29:56 +0000 Subject: [PATCH 4/4] Implement test suite for OpenID consent Signed-off-by: Patrick Uiterwijk Reviewed-by: Howard Johnson --- diff --git a/tests/openid.py b/tests/openid.py index 31dd005..c1ee6bc 100755 --- a/tests/openid.py +++ b/tests/openid.py @@ -104,7 +104,38 @@ if __name__ == '__main__': print "openid: Run OpenID Protocol ...", try: page = sess.fetch_page(idpname, - 'https://127.0.0.11:45081/?extensions=NO') + 'https://127.0.0.11:45081/?extensions=NO', + require_consent=True) + page.expected_value('text()', 'SUCCESS, WITHOUT EXTENSIONS') + except ValueError as e: + print >> sys.stderr, " ERROR: %s" % repr(e) + sys.exit(1) + print " SUCCESS" + + print "openid: Run OpenID Protocol without consent ...", + try: + page = sess.fetch_page(idpname, + 'https://127.0.0.11:45081/?extensions=NO', + require_consent=False) + page.expected_value('text()', 'SUCCESS, WITHOUT EXTENSIONS') + except ValueError as e: + print >> sys.stderr, " ERROR: %s" % repr(e) + sys.exit(1) + print " SUCCESS" + + print "openid: Revoking SP consent ...", + try: + page = sess.revoke_all_consent(idpname) + except ValueError, e: + print >> sys.stderr, "" % repr(e) + sys.exit(1) + print " SUCCESS" + + print "openid: Run OpenID Protocol without consent ...", + try: + page = sess.fetch_page(idpname, + 'https://127.0.0.11:45081/?extensions=NO', + require_consent=True) page.expected_value('text()', 'SUCCESS, WITHOUT EXTENSIONS') except ValueError as e: print >> sys.stderr, " ERROR: %s" % repr(e) @@ -113,8 +144,10 @@ if __name__ == '__main__': print "openid: Run OpenID Protocol with extensions ...", try: + # We expect consent again because we added more attributes page = sess.fetch_page(idpname, - 'https://127.0.0.11:45081/?extensions=YES') + 'https://127.0.0.11:45081/?extensions=YES', + require_consent=True) page.expected_value('text()', 'SUCCESS, WITH EXTENSIONS') except ValueError as e: print >> sys.stderr, " ERROR: %s" % repr(e)