Apache Allura™

• revoke_token should handle deleting a refresh token too. Not sure if we necessarily need it or not, but seems like the right thing to do according to https://oauthlib.readthedocs.io/en/latest/oauth2/validator.html?highlight=validate_refresh_token#oauthlib.oauth2.RequestValidator.revoke_token That says there's a token_type_hint to indicate which type of token it is
• I noticed in save_bearer_token you changed a c.user._id to a user_id determined by the request params. That seems good. Can you double check if save_authorization_code should do that too? Or maybe c.user is correct if it is only called when a user is logged in and approving a client app?
• for the docs I found https://github.com/raml-org/raml-spec/blob/master/versions/raml-08/raml-08.md#oauth-20 which specifies different settings: fields than what you have here.
• in wiki-copy.py it mentions /auth/oauth2/ should update that to /auth/oauth/ to match my merge request changes.
• using a client app that has no redirect URI gave an error. I thought maybe that should be allowed since this situation doesn't use a redirect? But maybe we need a redirect URL anyway.

  File "/var/local/env-allura/lib/python3.11/site-packages/oauthlib/oauth2/rfc6749/endpoints/authorization.py", line 114, in validate_authorization_request
    return response_type_handler.validate_authorization_request(request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/local/env-allura/lib/python3.11/site-packages/oauthlib/oauth2/rfc6749/grant_types/authorization_code.py", line 367, in validate_authorization_request
    self._handle_redirects(request)
  File "/var/local/env-allura/lib/python3.11/site-packages/oauthlib/oauth2/rfc6749/grant_types/base.py", line 249, in _handle_redirects
    raise errors.MissingRedirectURIError(request=request)
oauthlib.oauth2.rfc6749.errors.MissingRedirectURIError: (invalid_request) Missing redirect URI. <oauthlib.Request SANITIZED>

• then using one that did have a redirect, after I approved it I got redirected. I see the code in the URL, but that's probably not the best. On oauth1 there was a mode that showed the code on the page instead of redirecting. Can we do something like that? Not sure how to know when to redirect or not, especially in relationship to the above error? Maybe if the current request doesn't ask for a redirect?
• test_run_precommit has these errors:

E   scripts/wiki-copy.py:131:        token_expires = datetime.now() + timedelta(seconds=token.get('expires_in'))
E   scripts/wiki-copy.py:159:        token_expires = datetime.now() + timedelta(seconds=response.get('expires_in'))
E   scripts/wiki-copy.py:174:        date_diff = datetime.fromtimestamp(int(expires_in)) - datetime.utcnow()

• Removed the revoke_token method since we already have our internal mechanism to revoke tokens
• save_bearer_token uses user_id because it's a public endpoint that doesn't require authentication so we attempt to fetch the user id depending on the grant_type instead than from the current context
• Added the correct OAuth2 settings for the raml file
• Updated /auth/oauth2/ to /auth/oauth in wiki-copy.py
• For the redirect issues, we agreed that upon registering a new client the user must add at least one redirect URI to simplify workflow validations
• Fixed ruff errors
• While looking at the redirect issues I found that we also needed to implement the confirm_redirect_url in the OAuth2 validator to make sure that the same redirect uri the authorization code was created with, is also used when requesting an access token

There already is a confirm_redirect_uri and it has the same code :D

in the api docs, you included client_credentials which does match what we have in validate_grant_type, but I'm wondering if we need it at all. Maybe in next merge request can decide if that is what we use for personal bearer tokens? Or if we don't need it, we should remove it entirely.