fixup! [#7272] Implement additional security features for OAuth2 support
diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py
index d8e4366..1b9ea3d 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -1416,8 +1416,8 @@
 
     # Revokes the authorization code and access tokens for a given client and user
     def _revoke_user_tokens(self, client_id, user_id):
-        M.OAuth2AuthorizationCode.query.remove({'client_id': client_id, 'owner_id': user_id})
-        M.OAuth2AccessToken.query.remove({'client_id': client_id, 'owner_id': user_id})
+        M.OAuth2AuthorizationCode.query.remove({'client_id': client_id, 'user_id': user_id})
+        M.OAuth2AccessToken.query.remove({'client_id': client_id, 'user_id': user_id})
 
     # Revokes the authorization code and access tokens for a given client and all its users
     def _revoke_all(self, client_id):
@@ -1429,12 +1429,12 @@
     def index(self, **kw):
         c.form = F.oauth2_application_form
         provider = plugin.AuthenticationProvider.get(request)
-        clients = M.OAuth2ClientApp.for_owner(c.user)
+        clients = M.OAuth2ClientApp.for_user(c.user)
         model = []
 
         for client in clients:
-            authorization = M.OAuth2AuthorizationCode.query.get(client_id=client.client_id, owner_id=c.user._id)
-            token = M.OAuth2AccessToken.query.get(client_id=client.client_id, owner_id=c.user._id)
+            authorization = M.OAuth2AuthorizationCode.query.get(client_id=client.client_id, user_id=c.user._id)
+            token = M.OAuth2AccessToken.query.get(client_id=client.client_id, user_id=c.user._id)
             model.append(dict(client=client, authorization=authorization, token=token))
 
         return dict(
@@ -1449,7 +1449,7 @@
         M.OAuth2ClientApp(name=application_name,
                        description=application_description,
                        redirect_uris=[redirect_url],
-                       owner_id=c.user._id)
+                       user_id=c.user._id)
         flash('Oauth2 Client registered')
         redirect('.')
 
@@ -1457,7 +1457,7 @@
     @require_post()
     def do_client_action(self, _id=None, deregister=None, revoke=None):
         client = M.OAuth2ClientApp.query.get(client_id=_id)
-        if client is None or client.owner_id != c.user._id:
+        if client is None or client.user_id != c.user._id:
             flash('Invalid client ID', 'error')
             redirect('.')
 
diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py
index d072206..b1bafea 100644
--- a/Allura/allura/controllers/rest.py
+++ b/Allura/allura/controllers/rest.py
@@ -17,7 +17,6 @@
 
 """REST Controller"""
 from __future__ import annotations
-import ast
 import json
 import logging
 from datetime import datetime, timedelta
@@ -294,9 +293,7 @@
         M.OAuth2AuthorizationCode.query.remove({'client_id': client_id, 'authorization_code': code})
 
     def authenticate_client(self, request: oauthlib.common.Request, *args, **kwargs) -> bool:
-        client_id = request.body.get('client_id')
-        client_secret = request.body.get('client_secret')
-        request.client = M.OAuth2ClientApp.query.get(client_id=client_id, client_secret=client_secret)
+        request.client = M.OAuth2ClientApp.query.get(client_id=request.client_id, client_secret=request.client_secret)
         return request.client is not None
 
     def validate_code(self, client_id: str, code: str, client: oauthlib.oauth2.Client, request: oauthlib.common.Request, *args, **kwargs) -> bool:
@@ -314,11 +311,11 @@
         return authorization.redirect_uri == redirect_uri
 
     def save_authorization_code(self, client_id: str, code, request: oauthlib.common.Request, *args, **kwargs) -> None:
-        authorization = M.OAuth2AuthorizationCode.query.get(client_id=client_id, owner_id=c.user._id, authorization_code=code['code'])
+        authorization = M.OAuth2AuthorizationCode.query.get(client_id=client_id, user_id=c.user._id, authorization_code=code['code'])
 
         # Remove the existing authorization code if it exists and create a new record
         if authorization:
-            M.OAuth2AuthorizationCode.query.remove({'client_id': client_id, 'owner_id': c.user._id, 'authorization_code': code['code']})
+            M.OAuth2AuthorizationCode.query.remove({'client_id': client_id, 'user_id': c.user._id, 'authorization_code': code['code']})
 
         log.info('Saving authorization code for client: %s', client_id)
         auth_code = M.OAuth2AuthorizationCode(
@@ -326,7 +323,7 @@
             authorization_code=code['code'],
             expires_at=datetime.utcnow() + timedelta(minutes=10),
             redirect_uri=request.redirect_uri,
-            owner_id=c.user._id,
+            user_id=c.user._id,
             code_challenge=request.code_challenge,
             code_challenge_method=request.code_challenge_method
         )
@@ -335,18 +332,18 @@
 
     def save_bearer_token(self, token, request: oauthlib.common.Request, *args, **kwargs) -> object:
         authorization_code = M.OAuth2AuthorizationCode.query.get(client_id=request.client_id, authorization_code=request.code)
-        current_token = M.OAuth2AccessToken.query.get(client_id=request.client_id, owner_id=authorization_code.owner_id)
+        current_token = M.OAuth2AccessToken.query.get(client_id=request.client_id, user_id=authorization_code.user_id)
 
         if current_token:
-            M.OAuth2AccessToken.remove({'client_id': request.client_id, 'owner_id': c.user._id})
+            M.OAuth2AccessToken.query.remove({'client_id': request.client_id, 'user_id': c.user._id})
 
         bearer_token = M.OAuth2AccessToken(
             client_id = request.client_id,
             scopes = token.get('scope', []),
             access_token = token.get('access_token'),
             refresh_token = token.get('refresh_token'),
-            expires_at = datetime.utcfromtimestamp(token.get('expires_in')),
-            owner_id = authorization_code.owner_id
+            expires_at = datetime.utcnow() + timedelta(seconds=token.get('expires_in')),
+            user_id = authorization_code.user_id
         )
 
         session(bearer_token).flush()
@@ -495,11 +492,6 @@
         return oauthlib.oauth2.WebApplicationServer(Oauth2Validator())
 
     def _authenticate(self):
-        bearer_token_prefix = 'Bearer ' # noqa: S105
-        auth_header = request.headers.get('Authorization')
-        if auth_header and auth_header.startswith(bearer_token_prefix):
-            access_token = auth_header[len(bearer_token_prefix):]
-
         valid, req = self.server.verify_request(
             request.url,
             http_method=request.method,
@@ -509,9 +501,16 @@
         if not valid:
             raise exc.HTTPUnauthorized
 
-        access_token = M.OAuth2AccessToken.query.get(access_token=req.access_token)
-        access_token.last_access = datetime.utcnow()
-        return access_token
+        bearer_token_prefix = 'Bearer ' # noqa: S105
+        auth_header = req.headers.get('Authorization')
+        if auth_header and auth_header.startswith(bearer_token_prefix):
+            access_token = auth_header[len(bearer_token_prefix):]
+        else:
+            raise exc.HTTPUnauthorized
+
+        token = M.OAuth2AccessToken.query.get(access_token=access_token)
+        token.last_access = datetime.utcnow()
+        return token
 
 
     @expose('jinja:allura:templates/oauth2_authorize.html')
@@ -525,26 +524,23 @@
             decoded_body = str(request.body, 'utf-8')
             json_body = json.loads(decoded_body)
 
-        try:
-            scopes, credentials = self.server.validate_authorization_request(uri=request.url, http_method=request.method, headers=request.headers, body=json_body)
 
-            client_id = request.params.get('client_id')
-            client = M.OAuth2ClientApp.query.get(client_id=client_id)
+        scopes, credentials = self.server.validate_authorization_request(uri=request.url, http_method=request.method, headers=request.headers, body=json_body)
 
-            # The credentials object has a request object that it's too big to be serialized,
-            # so we remove it because we don't need it for the rest of the authorization workflow
-            del credentials['request']
+        client_id = request.params.get('client_id')
+        client = M.OAuth2ClientApp.query.get(client_id=client_id)
 
-            return dict(client=client, credentials=credentials)
-        except Exception as e:
-            log.exception(e)
+        # The credentials object has a request object that it's too big to be serialized,
+        # so we remove it because we don't need it for the rest of the authorization workflow
+        del credentials['request']
+
+        return dict(client=client, credentials=json.dumps(credentials))
 
     @expose('jinja:allura:templates/oauth2_authorize_ok.html')
     @require_post()
     def do_authorize(self, yes=None, no=None):
         security.require_authenticated()
 
-        credentials = ast.literal_eval(request.params['credentials'])
         client_id = request.params['client_id']
         client = M.OAuth2ClientApp.query.get(client_id=client_id)
 
@@ -552,30 +548,31 @@
             flash(f'{client.name} NOT AUTHORIZED', 'error')
             redirect('/auth/oauth2/')
 
-        try:
-            headers, body, status = self.server.create_authorization_response(
-                uri=request.url, http_method=request.method, body=request.body, headers=request.headers, scopes=[], credentials=credentials
-            )
+        credentials = json.loads(request.params['credentials'])
+        headers, body, status = self.server.create_authorization_response(
+            uri=request.url, http_method=request.method, body=request.body, headers=request.headers, scopes=[], credentials=credentials
+        )
 
-            response.status_int = status
-            for k, v in headers.items():
-                response.headers[k] = v
+        response.status_int = status
+        for k, v in headers.items():
+            response.headers[k] = v
 
-            return body
-        except Exception as ex:
-            log.exception(ex)
-
+        return body
 
     @expose('json:')
     @require_post()
     def token(self, **kwargs):
+        decoded_body = str(request.body, 'utf-8')
+
+        # We try to parse the request body as JSON, if it fails we just use the body as is
+        # so it's treated as x-www-form-urlencoded
         try:
-            decoded_body = str(request.body, 'utf-8')
-            json_body = json.loads(decoded_body)
-            headers, body, status = self.server.create_token_response(uri=request.url, http_method=request.method, body=json_body, headers=request.headers)
-            return body
-        except Exception as e:
-            log.exception(e)
+            request_body = json.loads(decoded_body)
+        except json.decoder.JSONDecodeError:
+            request_body = decoded_body
+
+        headers, body, status = self.server.create_token_response(uri=request.url, http_method=request.method, body=request_body, headers=request.headers)
+        return body
 
 
 def rest_has_access(obj, user, perm):
diff --git a/Allura/allura/model/oauth.py b/Allura/allura/model/oauth.py
index 9e9b10c..6ce0877 100644
--- a/Allura/allura/model/oauth.py
+++ b/Allura/allura/model/oauth.py
@@ -158,7 +158,7 @@
     class __mongometa__:
         session = main_orm_session
         name = 'oauth2_client_app'
-        unique_indexes = [('client_id', 'owner_id')]
+        unique_indexes = [('client_id', 'user_id')]
 
     query: 'Query[OAuth2ClientApp]'
 
@@ -168,20 +168,20 @@
     name = FieldProperty(str)
     description = FieldProperty(str, if_missing='')
     description_cache = FieldProperty(MarkdownCache)
-    owner_id: ObjectId = AlluraUserProperty(if_missing=lambda: c.user._id)
+    user_id: ObjectId = AlluraUserProperty(if_missing=lambda: c.user._id)
     grant_type = FieldProperty(str, if_missing='authorization_code')
     response_type = FieldProperty(str, if_missing='code')
     scopes = FieldProperty([str])
     redirect_uris = FieldProperty([str])
 
-    owner = RelationProperty('User')
+    user = RelationProperty('User')
 
 
     @classmethod
-    def for_owner(cls, owner=None):
-        if owner is None:
-            owner = c.user
-        return cls.query.find(dict(owner_id=owner._id)).all()
+    def for_user(cls, user=None):
+        if user is None:
+            user = c.user
+        return cls.query.find(dict(user_id=user._id)).all()
 
     @property
     def description_html(self):
@@ -192,13 +192,13 @@
     class __mongometa__:
         session = main_orm_session
         name = 'oauth2_authorization_code'
-        unique_indexes = [('authorization_code', 'client_id', 'owner_id')]
+        unique_indexes = [('authorization_code', 'client_id', 'user_id')]
 
     query: 'Query[OAuth2AuthorizationCode]'
 
     _id = FieldProperty(S.ObjectId)
     client_id = FieldProperty(str)
-    owner_id: ObjectId = AlluraUserProperty(if_missing=lambda: c.user._id)
+    user_id: ObjectId = AlluraUserProperty(if_missing=lambda: c.user._id)
     scopes = FieldProperty([str])
     redirect_uri = FieldProperty(str)
     authorization_code = FieldProperty(str)
@@ -207,27 +207,27 @@
     code_challenge = FieldProperty(str)
     code_challenge_method = FieldProperty(str)
 
-    owner = RelationProperty('User')
+    user = RelationProperty('User')
 
 
 class OAuth2AccessToken(MappedClass):
     class __mongometa__:
         session = main_orm_session
         name = 'oauth2_access_token'
-        unique_indexes = [('access_token', 'client_id', 'owner_id')]
+        unique_indexes = [('access_token', 'client_id', 'user_id')]
 
     query: 'Query[OAuth2AccessToken]'
 
     _id = FieldProperty(S.ObjectId)
     client_id = FieldProperty(str)
-    owner_id: ObjectId = AlluraUserProperty(if_missing=lambda: c.user._id)
+    user_id: ObjectId = AlluraUserProperty(if_missing=lambda: c.user._id)
     scopes = FieldProperty([str])
     access_token = FieldProperty(str)
     refresh_token = FieldProperty(str)
     expires_at = FieldProperty(S.DateTime)
     last_access = FieldProperty(datetime)
 
-    owner = RelationProperty('User')
+    user = RelationProperty('User')
 
 
 def dummy_oauths():
diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py
index 2ed7b06..e50a348 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -2090,14 +2090,14 @@
         user = M.User.by_username('test-admin')
         M.OAuth2ClientApp(
             client_id='client_12345',
-            owner_id=user._id,
+            user_id=user._id,
             name='testoauth2',
             description='test client',
             response_type='code',
             redirect_uris=['https://localhost/']
         )
         ThreadLocalODMSession.flush_all()
-        r = self.app.get('/rest/oauth2/authorize/', params={'client_id': 'client_12345', 'response_type': 'code'})
+        r = self.app.get('/rest/oauth2/authorize', params={'client_id': 'client_12345', 'response_type': 'code', 'redirect_uri': 'https://localhost/'})
         assert 'testoauth2' in r.text
         assert 'client_12345' in r.text
 
@@ -2106,7 +2106,7 @@
         user = M.User.by_username('test-admin')
         M.OAuth2ClientApp(
             client_id='client_12345',
-            owner_id=user._id,
+            user_id=user._id,
             name='testoauth2',
             description='test client',
             response_type='code',
@@ -2121,7 +2121,7 @@
         user = M.User.by_username('test-admin')
         M.OAuth2ClientApp(
             client_id='client_12345',
-            owner_id=user._id,
+            user_id=user._id,
             name='testoauth2',
             description='test client',
             response_type='code',
@@ -2133,7 +2133,10 @@
         r = self.app.get('/rest/oauth2/authorize', params={'client_id': 'client_12345', 'response_type': 'code', 'redirect_uri': 'https://localhost/'})
 
         # The submit authorization for the authorization code to be created
-        r = self.app.post('/rest/oauth2/do_authorize', params={'yes': '1', 'client_id': 'client_12345', 'response_type': 'code', 'redirect_uri': 'https://localhost/'})
+        mock_credentials = dict(client_id='client_12345', redirect_uri='https://localhost/', response_type='code', state=None)
+        r = self.app.post('/rest/oauth2/do_authorize',
+                          params={'yes': '1', 'client_id': 'client_12345', 'response_type': 'code',
+                                  'redirect_uri': 'https://localhost/', 'credentials': json.dumps(mock_credentials)})
 
         q = M.OAuth2AuthorizationCode.query.get(client_id='client_12345')
         assert q is not None
@@ -2146,7 +2149,8 @@
         user = M.User.by_username('test-admin')
         M.OAuth2ClientApp(
             client_id='client_12345',
-            owner_id=user._id,
+            client_secret='98765',
+            user_id=user._id,
             name='testoauth2',
             description='test client',
             response_type='code',
@@ -2158,7 +2162,10 @@
         r = self.app.get('/rest/oauth2/authorize', params={'client_id': 'client_12345', 'response_type': 'code', 'redirect_uri': 'https://localhost/'})
 
         # The submit authorization for the authorization code to be created
-        r = self.app.post('/rest/oauth2/do_authorize', params={'yes': '1', 'client_id': 'client_12345', 'response_type': 'code', 'redirect_uri': 'https://localhost/'})
+        mock_credentials = dict(client_id='client_12345', redirect_uri='https://localhost/', response_type='code', state=None)
+        r = self.app.post('/rest/oauth2/do_authorize',
+                            params={'yes': '1', 'client_id': 'client_12345', 'response_type': 'code',
+                                    'redirect_uri': 'https://localhost/', 'credentials': json.dumps(mock_credentials)})
 
         ac = M.OAuth2AuthorizationCode.query.get(client_id='client_12345')
         assert ac is not None
@@ -2169,8 +2176,10 @@
         # Create the authorization token
         oauth2_params = dict(
             client_id='client_12345',
+            client_secret='98765',
             code=ac.authorization_code,
-            grant_type='authorization_code'
+            grant_type='authorization_code',
+            redirect_uri='https://localhost/'
         )
         r = self.app.post_json('/rest/oauth2/token', oauth2_params)
         t = M.OAuth2AccessToken.query.get(client_id='client_12345')
@@ -2186,7 +2195,7 @@
         user = M.User.by_username('test-admin')
         M.OAuth2ClientApp(
             client_id='client_12345',
-            owner_id=user._id,
+            user_id=user._id,
             name='testoauth2',
             description='test client',
             response_type='code',
@@ -2197,7 +2206,7 @@
             client_id='client_12345',
             authotization_code='authcode_12345',
             expires_at=datetime.utcnow() + timedelta(minutes=10),
-            owner_id=user._id,
+            user_id=user._id,
         )
 
         M.OAuth2AccessToken(
@@ -2205,7 +2214,7 @@
             access_token='12345',
             refresh_token='54321',
             expires_at=datetime.utcnow() + timedelta(minutes=20),
-            owner_id=user._id,
+            user_id=user._id,
         )
 
         ThreadLocalODMSession.flush_all()