From 0ef93ffa2b9baa6cb079b0e1ef62b1eb9d561097 Mon Sep 17 00:00:00 2001
From: Patrick Uiterwijk <puiterwijk@redhat.com>
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 <puiterwijk@redhat.com>
Reviewed-by: Howard Johnson <merlin@merlinthp.org>

---

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 <puiterwijk@redhat.com>
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 <puiterwijk@redhat.com>
Reviewed-by: Howard Johnson <merlin@merlinthp.org>

---

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 <puiterwijk@redhat.com>
Date: Oct 23 2016 12:29:54 +0000
Subject: [PATCH 3/4] Plumb the Consent system into OpenID


Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
Reviewed-by: Howard Johnson <merlin@merlinthp.org>

---

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 %}
     </div>
     <div class="form-group">
-      <label for="remember_for_days" class="col-sm-10 col-md-10">
-        Remember authorization for
-      </label>
-      <div class="col-sm-10 col-md-10">
-        <select name="remember_for_days">
-          <option value="0">never</option>
-          <option value="3">3 days</option>
-          <option value="7">7 days</option>
-        </select>
-      </div>
-    </div>
-    <div class="form-group">
       <div class="col-sm-offset-2 col-md-offset-2 col-xs-12 col-sm-10 col-md-10 submit">
         <button type="submit" name="decided_deny" value="Reject" class="btn btn-primary btn-lg" tabindex="3">Reject</button>
         <button type="submit" name="decided_allow" value="Allow" class="btn btn-primary btn-lg" tabindex="3">Allow</button>

From 231deb8bfec17e4688861c490508a9a2eee9b8ea Mon Sep 17 00:00:00 2001
From: Patrick Uiterwijk <puiterwijk@redhat.com>
Date: Oct 23 2016 12:29:56 +0000
Subject: [PATCH 4/4] Implement test suite for OpenID consent


Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
Reviewed-by: Howard Johnson <merlin@merlinthp.org>

---

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)

