#259 Make OpenID Connect tokens revoke when consent is revoked
Merged 4 years ago by puiterwijk. Opened 4 years ago by puiterwijk.
puiterwijk/ipsilon oidc-revoke  into  master

@@ -85,6 +85,9 @@ 

      def consent_to_display(self, consentdata):

          raise NotImplementedError

  

+     def revoke_consent(self, user, clientid):

+         return True

+ 

  

  class ProviderPageBase(Page):

  

@@ -289,6 +289,18 @@ 

  

          self.del_unique_data('token', token_id)

  

+     def revokeConsent(self, username, client_id):

PEP-8 would suggest that you call this method revoke_consent.

Yep. And I opened #261 for this.
However, since this file is currently camelCase, I wanted to do all the fixing in one commit so that it doesn't become inconsistent in a file.

+         data = self.get_unique_data('token', name='username', value=username)

+ 

+         removed_token = False

+         for uuid in data.keys():

+             token = self.get_unique_data('token', uuid)[uuid]

+             if token['client_id'] == client_id:

+                 self.invalidateToken(uuid)

+                 removed_token = True

+ 

+         return removed_token

+ 

      def storeUserInfo(self, userinfo):

          to_store = {}

          for key in userinfo:

@@ -223,6 +223,9 @@ 

  

          return d

  

+     def revoke_consent(self, user, clientid):

+         return self.datastore.revokeConsent(user, clientid)

+ 

      def on_reconfigure(self):

          super(IdpProvider, self).on_reconfigure()

          self.init_idp()

file modified
+7
@@ -22,6 +22,13 @@ 

      def revoke(self, provider, clientid):

          us = UserSession()

          user = us.get_user()

+ 

+         provname = provider

+         provmod = self._site['provider_config'].available.get(provname,

+                                                               None)

+         if provmod is not None:

+             if not provmod.revoke_consent(user.name, clientid):

+                 raise Exception('Provider refused to revoke')

          user.revoke_consent(provider, clientid)

          raise cherrypy.HTTPRedirect(self._master.url)

      revoke.public_function = True

file modified
+34 -8
@@ -257,7 +257,7 @@ 

              'amr': json.dumps([]),

              'acr': '0'

          }

-         token = check_info_results(page.text, expect)

+         old_token = check_info_results(page.text, expect)

      except ValueError, e:

          print >> sys.stderr, " ERROR: %s" % repr(e)

          sys.exit(1)
@@ -279,7 +279,7 @@ 

      try:

          page = sess.revoke_all_consent(idpname)

      except ValueError, e:

-         print >> sys.stderr, "" % repr(e)

+         print >> sys.stderr, " ERROR: %s" % repr(e)

          sys.exit(1)

      print " SUCCESS"

  
@@ -289,6 +289,17 @@ 

                                 'https://127.0.0.11:45081/sp/redirect_uri?log'

                                 'out=https%3A%2F%2F127.0.0.11%3A45081%2Fsp%2F',

                                 require_consent=True)

+         h = hashlib.sha256()

+         h.update('127.0.0.11')

+         h.update(user)

+         h.update('testcase')

+         expect = {

+             'sub': h.hexdigest(),

+             'iss': 'https://127.0.0.10:45080/idp1/openidc/',

+             'amr': json.dumps([]),

+             'acr': '0'

+         }

+         new_token = check_info_results(page.text, expect)

      except ValueError, e:

          print >> sys.stderr, " ERROR: %s" % repr(e)

          sys.exit(1)
@@ -309,13 +320,13 @@ 

      try:

          # Testing token without client auth

          r = requests.post('https://127.0.0.10:45080/idp1/openidc/TokenInfo',

-                           data={'token': token['access_token']})

+                           data={'token': new_token['access_token']})

          if r.status_code != 401:

              raise Exception('No 401 provided')

  

          # Testing token where we removed part of token ID

          r = requests.post('https://127.0.0.10:45080/idp1/openidc/TokenInfo',

-                           data={'token': token['access_token'][1:],

+                           data={'token': new_token['access_token'][1:],

                                  'client_id': reg_resp['client_id'],

                                  'client_secret': reg_resp['client_secret']})

          r.raise_for_status()
@@ -325,7 +336,7 @@ 

  

          # Testing token where we rempoved part of check string

          r = requests.post('https://127.0.0.10:45080/idp1/openidc/TokenInfo',

-                           data={'token': token['access_token'][:-1],

+                           data={'token': new_token['access_token'][:-1],

                                  'client_id': reg_resp['client_id'],

                                  'client_secret': reg_resp['client_secret']})

          r.raise_for_status()
@@ -335,7 +346,7 @@ 

  

          # Testing valid token

          r = requests.post('https://127.0.0.10:45080/idp1/openidc/TokenInfo',

-                           data={'token': token['access_token'],

+                           data={'token': new_token['access_token'],

                                  'client_id': reg_resp['client_id'],

                                  'client_secret': reg_resp['client_secret']})

          r.raise_for_status()
@@ -359,10 +370,25 @@ 

          if len(info['scope']) != 0:

              raise Exception('Unexpected scopes found: %s' % info['scope'])

  

+         # Testing previously revoked token

+         r = requests.post('https://127.0.0.10:45080/idp1/openidc/TokenInfo',

+                           data={'token': old_token['access_token'],

+                                 'client_id': reg_resp['client_id'],

+                                 'client_secret': reg_resp['client_secret']})

+         r.raise_for_status()

+         info = r.json()

+         if 'error' in info:

+             raise Exception('Token introspection returned error: %s'

+                             % info['error'])

+         if info['active']:

+             raise Exception('Revoked token active')

+         if len(info) != 1:

+             raise Exception('Token contained more info then inactive')

+ 

          # Delete test client and then try to use it

          sess.delete_oidc_client(idpname, reg_resp['client_id'])

          r = requests.post('https://127.0.0.10:45080/idp1/openidc/TokenInfo',

-                           data={'token': token['access_token'],

+                           data={'token': new_token['access_token'],

                                  'client_id': reg_resp['client_id'],

                                  'client_secret': reg_resp['client_secret']})

          if r.status_code != 400:
@@ -381,7 +407,7 @@ 

  

          # Testing valid token

          r = requests.post('https://127.0.0.10:45080/idp1/openidc/UserInfo',

-                           data={'access_token': token['access_token']})

+                           data={'access_token': new_token['access_token']})

          r.raise_for_status()

          info = r.json()

          if 'sub' not in info:

no initial comment

PEP-8 would suggest that you call this method revoke_consent.

Yep. And I opened #261 for this.
However, since this file is currently camelCase, I wanted to do all the fixing in one commit so that it doesn't become inconsistent in a file.

Commit daed728 fixes this pull-request

Pull-Request has been merged by puiterwijk@redhat.com

4 years ago