#266 Implement OpenID Connect unauthenticated clients
Merged 4 years ago by puiterwijk. Opened 4 years ago by puiterwijk.
puiterwijk/ipsilon oidc-secretless  into  master

@@ -34,6 +34,7 @@ 

      authenticate_client = False

      authenticate_token = False

      requires_client_auth = False

+     allow_none_client_auth = False

      requires_valid_token = False

      required_scope = None

  
@@ -44,6 +45,7 @@ 

          self.api_username = None

          self.api_scopes = []

          self.api_client_authenticated = False

+         self.api_client_authenticated_no_secret = False

          self.api_client_id = None

          self.api_token = None

          self.api_client = None
@@ -113,8 +115,11 @@ 

              raise APIError(400, 'invalid_client',

                             'client authentication error')

  

-         if not constant_time_string_comparison(client['client_secret'],

-                                                client_secret):

+         if auth_method == 'none':

+             self.api_client_authenticated_no_secret = True

+ 

+         elif not constant_time_string_comparison(client['client_secret'],

+                                                  client_secret):

              self.error('Client authentication with invalid secret: %s'

                         % client_secret)

              raise APIError(400, 'invalid_client',
@@ -152,10 +157,15 @@ 

          elif 'client_id' in post_args:

              self.debug('Client id found in post args: %s'

                         % post_args['client_id'])

-             self._handle_client_authentication('client_secret_post',

-                                                post_args['client_id'],

-                                                post_args.get('client_secret',

-                                                              ''))

+             authmethod = 'client_secret_post'

+             clientid = post_args['client_id']

+             clientsecret = post_args.get('client_secret')

+             if clientsecret is None and self.allow_none_client_auth:

+                 authmethod = 'none'

+ 

+             self._handle_client_authentication(authmethod,

+                                                clientid,

+                                                clientsecret)

          else:

              self.error('No authorization presented')

              response = cherrypy.serving.response
@@ -218,6 +228,7 @@ 

  

  class Token(APIRequest):

      authenticate_client = True

+     allow_none_client_auth = True

  

      def POST(self, *args, **kwargs):

          grant_type = kwargs.get('grant_type', None)

file modified
+10 -1
@@ -269,7 +269,7 @@ 

          return ['post', url, {'data': params}]

  

      def fetch_page(self, idp, target_url, follow_redirect=True, krb=False,

-                    require_consent=None):

+                    require_consent=None, return_prefix=None):

          """

          Fetch a page and parse the response code to determine what to do

          next.
@@ -281,13 +281,22 @@ 

          require_consent indicates whether consent should or should not be asked

          or if that's not in this test. None means not tested, False means must

          not be asked, True means must be asked.

+ 

+         If the url we would be requesting starts with return_prefix, instead of

+         requesting the next page, we return the previous page.

          """

          url = target_url

          action = 'get'

          args = {}

          seen_consent = False

+         r = None

  

          while True:

+             if return_prefix and url.startswith(return_prefix):

+                 if r:

+                     return PageTree(r)

+                 else:

+                     return None

              r = self.access(action, url, krb=krb, **args)

              if r.status_code == 303 or r.status_code == 302:

                  if not follow_redirect:

file modified
+82
@@ -243,6 +243,26 @@ 

          sys.exit(1)

      print " SUCCESS"

  

+     print "openidc: Registering test client with none auth ...",

+     try:

+         client_info = {

+             'redirect_uris': ['https://invalid/'],

+             'response_types': ['code'],

+             'grant_types': ['authorization_code'],

+             'application_type': 'web',

+             'client_name': 'Test suite client',

+             'client_uri': 'https://invalid/',

+             'token_endpoint_auth_method': 'none'

+         }

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

+                           json=client_info)

+         r.raise_for_status()

+         reg_resp_none = r.json()

+     except Exception, e:  # pylint: disable=broad-except

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

+         sys.exit(1)

+     print " SUCCESS"

+ 

      print "openidc: Access first SP Protected Area ...",

      try:

          page = sess.fetch_page(idpname, 'https://127.0.0.11:45081/sp/',
@@ -398,6 +418,68 @@ 

          sys.exit(1)

      print " SUCCESS"

  

+     print "openidc: Using none-authenticated client ...",

+     try:

+         # Test that none-authed clients don't have access to token info

+         r = requests.post(

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

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

+                   'client_id': reg_resp_none['client_id'],

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

+         if r.status_code != 400:

+             raise Exception('None-authed client accepted')

+ 

+         # Try the authorization flow

+         page = sess.fetch_page(idpname,

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

+                                'Authorization?scope=openid&response_type=code&'

+                                'response_mode=query&redirect_uri='

+                                'https://invalid/&client_id=' +

+                                reg_resp_none['client_id'],

+                                return_prefix='https://invalid/')

+         target = page.result.headers['Location']

+         code = target.replace('https://invalid/?code=', '')

+         token_resp = requests.post(

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

+             data={'client_id': reg_resp_none['client_id'],

+                   'grant_type': 'authorization_code',

+                   'redirect_uri': 'https://invalid/',

+                   'code': code})

+         if token_resp.status_code != 200:

+             raise Exception('Unable to get token from code')

+         anon_token = token_resp.json()

+         if not anon_token.get('token_type') == 'Bearer':

+             raise Exception('Invalid token type returned')

+         if 'access_token' not in anon_token:

+             raise Exception('Did not get access token')

+ 

+         # Test that none-authed clients also can't get their own token info

+         r = requests.post(

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

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

+                   'client_id': reg_resp_none['client_id'],

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

+         if r.status_code != 400:

+             raise Exception('None-authed client accepted')

+ 

+         # Test it does have tokeninfo access after setting to authed

+         sess.update_options(

+             idpname,

+             'providers/openidc/admin/client/%s' % reg_resp_none['client_id'],

+             {'Token Endpoint Auth Method': 'client_secret_post'})

+ 

+         r = requests.post(

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

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

+                   'client_id': reg_resp_none['client_id'],

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

+         if r.status_code != 200:

+             raise Exception('Authed client not accepted')

+     except ValueError, e:

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

+         sys.exit(1)

+     print " SUCCESS"

+ 

      print "openidc: Checking user info ...",

      try:

          # Testing user info without token

Unauthenticated clients mean that they have auth_method set to None.
This means that they do not have a way to reasonably store a secret, for e.g. client-side utilities.

These clients do not have access to any of the Ipsilon APIs with their client credentials.

The second commit message has a typo: s/sspecify/specify/.

This variable name is confusing given the variable name above it. I suggest adding a comment above it explaining what it is and how it is different from api_client_authenticated, or picking a more differentiated name.

rebased

4 years ago

As far as enablement go this code LGTM.

Commit 5a12165 fixes this pull-request

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

4 years ago