Reduce logging severity/don't log expired tokens/401s

From what I understand, these error logs come from SimpleOauthAuthenticationProvider's authenticate() function.

  public function authenticate(Request $request) {
    // Update the request with the OAuth information.
    try {
      $auth_request = $this->resourceServer->validateAuthenticatedRequest($request);
    }
    catch (OAuthServerException $exception) {
      // Procedural code here is hard to avoid.
      watchdog_exception('simple_oauth', $exception);

      return NULL;
    }

The validateAuthenticatedRequest() function from League's Oauth2 package, at least on version 7.1, which is the one this module is using, will always throw an OAuthServerException::accessDenied() exception:

public function validateAuthenticatedRequest(ServerRequestInterface $request)
    {
        return $this->getAuthorizationValidator()->validateAuthorization($request);
    }

And validateAuthorization():

    public function validateAuthorization(ServerRequestInterface $request)
    {
        if ($request->hasHeader('authorization') === false) {
            throw OAuthServerException::accessDenied('Missing "Authorization" header');
        }

        $header = $request->getHeader('authorization');
        $jwt = trim((string) preg_replace('/^(?:\s+)?Bearer\s/', '', $header[0]));

        try {
            // Attempt to parse and validate the JWT
            $token = (new Parser())->parse($jwt);
            try {
                if ($token->verify(new Sha256(), $this->publicKey->getKeyPath()) === false) {
                    throw OAuthServerException::accessDenied('Access token could not be verified');
                }
            } catch (BadMethodCallException $exception) {
                throw OAuthServerException::accessDenied('Access token is not signed', null, $exception);
            }

            // Ensure access token hasn't expired
            $data = new ValidationData();
            $data->setCurrentTime(time());

            if ($token->validate($data) === false) {
                throw OAuthServerException::accessDenied('Access token is invalid');
            }

            // Check if token has been revoked
            if ($this->accessTokenRepository->isAccessTokenRevoked($token->getClaim('jti'))) {
                throw OAuthServerException::accessDenied('Access token has been revoked');
            }

            // Return the request with additional attributes
            return $request
                ->withAttribute('oauth_access_token_id', $token->getClaim('jti'))
                ->withAttribute('oauth_client_id', $token->getClaim('aud'))
                ->withAttribute('oauth_user_id', $token->getClaim('sub'))
                ->withAttribute('oauth_scopes', $token->getClaim('scopes'));
        } catch (InvalidArgumentException $exception) {
            // JWT couldn't be parsed so return the request as is
            throw OAuthServerException::accessDenied($exception->getMessage(), null, $exception);
        } catch (RuntimeException $exception) {
            //JWR couldn't be parsed so return the request as is
            throw OAuthServerException::accessDenied('Error while decoding to JSON', null, $exception);
        }
    }

With that being the case, it seems that these can be reduced to a notice log instead? Some of those messages could still prove useful for debugging, so I don't think the logging should be outright removed.

Let me know if my assumptions are correct here. I'm providing a patch with the change to a notice level log for now. It should apply on both the 8.x-3.x and 8.x-4.x branches, as the class was unaffected by the sub-module merge.

retagging

Adding tangentially related issue ...
https://www.drupal.org/project/simple_oauth/issues/3143809

My solution is much more simple maybe isn't enough but we are using in our project, maybe it would be useful to someone.

The technique used by @segi is interesting and something I think we should consider/evaluate. One thing that was nice about the other patch by @gueguerreiro was that it removed the procedural code logging as well, which might be advantageous?

The following patch does:
- Injects the (new) simple_oauth logger channel.
- Logs a notice with the message and hint from the OAuthServerException.
- Throws a UnauthorizedHttpException, which provides a WWW-Authenticate challenge for the requester (e.g. frontend).
- Catches the exception and forwards the response with the (new) event subscriber UnauthorizedExceptionSubscriber before Drupal logging is triggered.
- Adjust the unit test for the SimpleOauthAuthenticationProvider

Out of the scope of this issue:
- Replaces watchdog_exception() calls throughout the module with the injections of the simple_oauth logger channel.

Some thoughts:
Intercepting the authentication with an exception is actually a smart way to provide some information to the user, but we should decide where to direct this information. In my opinion it is more important to let the frontend know, that the authentication failed. Hence, I introduced the new header in the response. Further, the Drupal log should not be flooded with exceptions, if we are already passing on the notice. Introducing an event subscriber might be overkill - but we throw the exception in the first place and should also handle it.
Further, we should really consider, if logging a notice in the backend is really necessary. I can see that it might has some benefit when debugging, but in production it does not really hold any value. Maybe, we could even introduce a setting.

After further inspection, I think it would also be good to handle exceptions thrown by the OAuthToken controller (see line 83).

But, by now, I am convinced, it would be nice to have an adapter for the OAuthServerException to handle the errors comprehensively. I will give it a shot, when there is some time.

I re-rolled my patch if somebody use it. I have check the MR and it looks promising and it covers my little "fix" as well.

Update: This patch contains a bug. I forgot to include the RfcLogLevel class. I don't upload the new version because it's just confusing who see the issue first time.

@bradjones1 I don't seem to be able to review/test that patch from #12. I cant apply the patch to 5.2.x either as the code seems to be outdated. I'm seeing the merge request and actually needing it on one of my projects, I'd guess it's about to be merged and released?

Thanks for your comment @bradjones1 - Even if I kinda know after I've been around on d.o for such a long time, I'm just never sure about the way to proceed. I'm re-rolling the patch against 5.2.x as I need an online link to be able to test on my platform (which already has an online API from which I need to test that code).
Thanks! I'll keep you informed of my findings.

Missed including new files. Re-renerating the patch.

Adding a request to also look at this issue for the new 6.x branch.

Would love to have a little more control over the >3,000 daily simple_oauth errors in the logs on our high volume site.

I created a MR based on 6.0.x.

Please address the phpcs issues on the MR.

Done!