From 8473fe33c15aea92efe0d847b21cc0b5e17e457d Mon Sep 17 00:00:00 2001
From: Slavek Kabrda <bkabrda@redhat.com>
Date: Jun 05 2018 12:32:40 +0000
Subject: Fix race condition in OpenIDConnect api


Cherrypy seems to reuse objects if running in a multithreaded environment
(e.g. mod_wsgi on httpd). This can result in multiple threads resetting
attributes of OpenIDConnect api objects which then leads to various 500
errors. A fresh cherrypy.request object is generated for every request,
therefore it can be used to store these attributes safely.

Related: #302

Signed-off-by: Slavek Kabrda <bkabrda@redhat.com>

---

diff --git a/ipsilon/providers/openidc/api.py b/ipsilon/providers/openidc/api.py
index c1cb07d..f665256 100644
--- a/ipsilon/providers/openidc/api.py
+++ b/ipsilon/providers/openidc/api.py
@@ -40,15 +40,25 @@ class APIRequest(ProviderPageBase):
 
     def __init__(self, site, provider, *args, **kwargs):
         super(APIRequest, self).__init__(site, provider)
-        self.api_client_id = None
-        self.api_valid_token = False
-        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
+        self._set_apistore_key('api_client_id', None)
+        self._set_apistore_key('api_valid_token', False)
+        self._set_apistore_key('api_username', None)
+        self._set_apistore_key('api_scopes', [])
+        self._set_apistore_key('api_client_authenticated', False)
+        self._set_apistore_key('api_client_authenticated_no_secret', False)
+        self._set_apistore_key('api_client_id', None)
+        self._set_apistore_key('api_token', None)
+        self._set_apistore_key('api_client', None)
+
+    def _get_apistore_key(self, key):
+        return cherrypy.request.apistore[self][key]
+
+    def _set_apistore_key(self, key, value):
+        if not hasattr(cherrypy.request, 'apistore'):
+            cherrypy.request.apistore = {}
+        if self not in cherrypy.request.apistore:
+            cherrypy.request.apistore[self] = {}
+        cherrypy.request.apistore[self][key] = value
 
     def pre_GET(self, *args, **kwargs):
         # Note that we explicitly do NOT support URI Query parameter posting
@@ -63,18 +73,18 @@ class APIRequest(ProviderPageBase):
             'Content-Type': 'application/json'
         })
 
-        self.api_client_id = None
-        self.api_valid_token = False
-        self.api_username = None
-        self.api_scopes = []
-        self.api_client_authenticated = False
-        self.api_client_id = None
-        self.api_token = None
-        self.api_client = None
+        self._set_apistore_key('api_client_id', None)
+        self._set_apistore_key('api_valid_token', False)
+        self._set_apistore_key('api_username', None)
+        self._set_apistore_key('api_scopes', [])
+        self._set_apistore_key('api_client_authenticated', False)
+        self._set_apistore_key('api_client_id', None)
+        self._set_apistore_key('api_token', None)
+        self._set_apistore_key('api_client', None)
 
         if self.authenticate_client:
             self._authenticate_client(kwargs)
-            if self.requires_client_auth and not self.api_client_id:
+            if self.requires_client_auth and not self._get_apistore_key('api_client_id'):
                 raise APIError(400, 'invalid_client',
                                'client authentication required')
 
@@ -116,7 +126,7 @@ class APIRequest(ProviderPageBase):
                            'client authentication error')
 
         if auth_method == 'none':
-            self.api_client_authenticated_no_secret = True
+            self._set_apistore_key('api_client_authenticated_no_secret', True)
 
         elif not constant_time_string_comparison(client['client_secret'],
                                                  client_secret):
@@ -125,9 +135,9 @@ class APIRequest(ProviderPageBase):
             raise APIError(400, 'invalid_client',
                            'client authentication error')
 
-        self.api_client_authenticated = True
-        self.api_client_id = client_id
-        self.api_client = client
+        self._set_apistore_key('api_client_authenticated', True)
+        self._set_apistore_key('api_client_id', client_id)
+        self._set_apistore_key('api_client', client)
 
     def _authenticate_client(self, post_args):
         request = cherrypy.serving.request
@@ -180,14 +190,17 @@ class APIRequest(ProviderPageBase):
             self.error('Unknown token provided')
             raise APIError(400, 'invalid_token')
 
-        if self.api_client_id:
+        if self._get_apistore_key('api_client_id'):
             if token['client_id'] != self.api_client_id:
                 self.error('Authenticated client is not token owner: %s != %s'
-                           % (token['client_id'], self.api_client_id))
+                           % (token['client_id'], self._get_apistore_key('api_client_id')))
                 raise APIError(400, 'invalid_request')
         else:
-            self.api_client = self.cfg.datastore.getClient(token['client_id'])
-            if not self.api_client:
+            self._set_apistore_key(
+                'api_client',
+                self.cfg.datastore.getClient(token['client_id'])
+            )
+            if not self._get_apistore_key('api_client'):
                 self.error('Token authentication with invalid client ID')
                 raise APIError(400, 'invalid_client',
                                'client authentication error')
@@ -198,11 +211,11 @@ class APIRequest(ProviderPageBase):
                        % (self.required_scope, token['scope']))
             raise APIError(403, 'insufficient_scope')
 
-        self.api_client_id = token['client_id']
-        self.api_valid_token = True
-        self.api_username = token['username']
-        self.api_scopes = token['scope']
-        self.api_token = token
+        self._set_apistore_key('api_client_id', token['client_id'])
+        self._set_apistore_key('api_valid_token', True)
+        self._set_apistore_key('api_username', token['username'])
+        self._set_apistore_key('api_scopes', token['scope'])
+        self._set_apistore_key('api_token', token)
 
     def _authenticate_token(self, post_args):
         request = cherrypy.serving.request
@@ -217,12 +230,12 @@ class APIRequest(ProviderPageBase):
             # Bearer token
             token = post_args['access_token']
             self._handle_token_authentication(token)
-        if self.requires_valid_token and not self.api_token:
+        if self.requires_valid_token and not self._get_apistore_key('api_token'):
             self.error('No token provided in call that requires one')
             raise APIError(403, 'no_token_provided')
 
     def require_scope(self, scope):
-        if scope not in self.api_scopes:
+        if scope not in self._get_apistore_key('api_scopes'):
             raise APIError(403, 'insufficient_scope')
 
 
@@ -244,10 +257,10 @@ class Token(APIRequest):
                 return self._respond_error('invalid_request',
                                            'invalid token')
 
-            if token['client_id'] != self.api_client_id:
+            if token['client_id'] != self._get_apistore_key('api_client_id'):
                 self.error('Authz code owner does not match authenticated ' +
                            'client: %s != %s' % (token['client_id'],
-                                                 self.api_client_id))
+                                                 self._get_apistore_key('api_client_id')))
                 return self._respond_error('invalid_request',
                                            'invalid token for client ID')
 
@@ -272,7 +285,7 @@ class Token(APIRequest):
 
             refresh_result = self.cfg.datastore.refreshToken(
                 refresh_token,
-                self.api_client_id)
+                self._get_apistore_key('api_client_id'))
 
             if not refresh_result:
                 return self._respond_error('invalid_grant',
@@ -328,17 +341,17 @@ class UserInfo(APIRequest):
     required_scope = 'openid'
 
     def _get_userinfo(self, *args, **kwargs):
-        info = self.cfg.datastore.getUserInfo(self.api_token['userinfocode'])
+        info = self.cfg.datastore.getUserInfo(self._get_apistore_key('api_token')['userinfocode'])
         if not info:
             return self._respond_error('invalid_request',
                                        'No userinfo for token')
 
-        if self.api_client.get('userinfo_signed_response_alg'):
+        if self._get_apistore_key('api_client').get('userinfo_signed_response_alg'):
             cherrypy.response.headers.update({
                 'Content-Type': 'application/jwt'
             })
 
-            if self.api_client.get('userinfo_signed_response_alg') == 'RS256':
+            if self._get_apistore_key('api_client').get('userinfo_signed_response_alg') == 'RS256':
                 sig = JWT(header={'alg': 'RS256',
                                   'kid': self.cfg.idp_sig_key_id},
                           claims=info)

