thephpleague / oauth2-server-bundle

Symfony bundle for the OAuth2 Server.
MIT License
176 stars 86 forks source link

Access denied #157

Closed damc closed 3 months ago

damc commented 11 months ago

What happens:

  1. The OAuth client redirects to the "sign in" page on my website (to /authorize endpoint).
  2. My website uses oauth2-server-bundle which handles OAuth and redirects it back to the client, but it add the following query parameters at the end of the url, suggesting that something is wrong:
?state=ffa0e872-00ed-4cf8-a256-16780242ca03&error=access_denied&error_description=The resource owner or authorization server denied the request.&hint=The user denied the request&message=The resource owner or authorization server denied the request.
  1. The client (not belonging to me) displays info that it couldn't log in.

  2. I can also see the following thing in "network" tab in "webdeveloper tools" in one of the responses to a client internal request:

detail  'Error forwarding auth: {"message":"Missing access_token"}

It might be caused by the fact that I haven't set the grant type in my client. If I want to use authorization code grant, then what value should I set in grant type of the client?

UPDATE: I set grant type to "authorization_code" in my client and I still get the same error.

damc commented 11 months ago

Also, I was reading the code of the bundle to find what I could have done wrong.

It looks like the kind of error that it returns can be only returned in AuthCodeGrant.php or ImplicitGrant.php in completeAuthorizationRequest if the following condition is not met:

// The user approved the client, redirect them back with an access token
if ($authorizationRequest->isAuthorizationApproved() === true)

I've found that this depends on $event->getAuthorizationResolution() in AuthorizationController.

From there, I've found that authorizationResolution property initially is set to ACCESS_DENIED (which equals false). Now, the only place where authorizationResolution property is modified is resolveAuthorization method. However, I've found that the resolveAuthorization method is not used anywhere. So, the authorizationResolution property will be always set to ACCESS_DENIED which will result in the error that I get. So, that looks to me like a bug in the bundle.

Is that a bug in the bundle, or am I missing something?

X-Coder264 commented 11 months ago

It's not a bug. In order for the authorization code grant to work you need to to have an event listener in your code for that AuthorizationRequestResolveEvent event and have your own logic there which decides when the authorization is approved or not.

Here's a never merged PR from the previous bundle from which this one was forked that documents the auth code grant -> https://github.com/trikoder/oauth2-bundle/pull/177

The namespaces are obviously different now but generally everything written there should be still applicable for this bundle.

Here's another example of that event listener for a more advanced auth code grant use-case: https://gist.github.com/ajgarlag/1f84d29ee0e1a92c8878f44a902338cd (this is from the original auth code grant implementation PR https://github.com/trikoder/oauth2-bundle/pull/18 )

TLDR; This is not a bug, it's a documentation issue as there's no documentation at the moment for the auth code grant in this bundle.

damc commented 11 months ago

But there's a code like this in vendor/league/oauth2-server-bundle/src/Resources/config/services.php:

 // Authorization listeners
        ->set('league.oauth2_server.listener.authorization_request_user_resolving', AuthorizationRequestUserResolvingListener::class)
            ->args([
                service('security.helper'),
            ])
            ->tag('kernel.event_listener', [
                'event' => OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE,
                'method' => 'onAuthorizationRequest',
                'priority' => 1024,
            ])
        ->alias(AuthorizationRequestUserResolvingListener::class, 'league.oauth2_server.listener.authorization_request_user_resolving')

and then in other file:

public function onAuthorizationRequest(AuthorizationRequestResolveEvent $event): void
    {
        $user = $this->security->getUser();
        if ($user instanceof UserInterface) {
            $event->setUser($user);
        }
    }

It looks like the event listener is already there, it's just missing the call to resolveAuthorization, am I wrong?

X-Coder264 commented 11 months ago

That is only the bundle listener that sets the logged in user onto the event. It's up to you to add your own listener (usually one with a priority that runs after that one so you can check whether the user is set) in which you implement your own logic for deciding whether the authorization request is approved or not. That is clearly documented in the docs PR I've linked in my previous comment:

After assigning routes, listener for trikoder.oauth2.authorization_request_resolve must be configured. \Trikoder\Bundle\OAuth2Bundle\Event\AuthorizationRequestResolveEvent (whose name is trikoder.oauth2.authorization_request_resolve) consist of three important methods which have to be used

  1. setUser(?UserInterface $user) and resolveAuthorization(bool $authorizationResolution) when user is already logged in when accessing authorization endpoint
  2. setResponse(ResponseInterface $response) when user needs to log in before authorization server can issue authorization code

\Trikoder\Bundle\OAuth2Bundle\EventListener\AuthorizationRequestUserResolvingListener with priority value 1024 calls setUser(?UserInterface $user) if user is logged in, so make sure your listener has lower priority than it.

chalasr commented 3 months ago

Closing as per Antonio's comment.