klapaudius / FOSOAuthServerBundle

A server side OAuth2 Bundle for Symfony 5.0 or higher
7 stars 22 forks source link

UserBadge should use "user" instead of "client" followup #29

Closed AppdevOWA closed 1 month ago

AppdevOWA commented 1 month ago

I'm reopening issue 22 as this still happens where it always gets the client id rather than user.

The possible solution doesn't work. The only way I could get it to work was to make a copy Custom Oauth2Authenticator and return: return new SelfValidatingPassport(new UserBadge($user->getUserIdentifier()), [$accessTokenBadge]);

so it returns the user rather than the client ID.

Is there any update to getting this fixed?

klapaudius commented 1 month ago

Hi, regarding the RFC 6749, the client is not the same as the end-user. Client is in fact a third party app that can authenticate to get a granted access to protected ressources.

An AccessToken can have a standalone client with the grant type "client_credentials" which normally authenticates the third party app or a client and a end-user with grant type "password"

AppdevOWA commented 1 month ago

Thanks for the explanation @klapaudius

However I've got a similar scenario to @SanderVerkuil where it's the user who is authenticating and I need to retrieve the user account object in loadUserByIdentifier based off the AccessToken. Currently loadUserByIdentifier is returning the Client ID, not the User account. I looked at @jcrombez solution and it doesn't work.

How would I go about getting the user who authenticated?

valentine-morozov commented 1 month ago

@klapaudius The same question. I am migrating from original FOS bundle and it was working ok. We had a user from access token in controllers. Now the API returns "invalid credentials" for all requests because Authenticator passes randomId from the Client into UserProvider that accepts email of User. So your fork is not working for the case where we authenticate users, not only clients. I've spend two days with debugger to understand where it fails. I think you can add an option to the config so if it's enabled then SelfValidatingPassport will get User's identifier instead of Client's one. Or maybe you know how to implement it better. The proposed solution from the previous issue is not working.

valentine-morozov commented 1 month ago

Thanks for the explanation @klapaudius

However I've got a similar scenario to @SanderVerkuil where it's the user who is authenticating and I need to retrieve the user account object in loadUserByIdentifier based off the AccessToken. Currently loadUserByIdentifier is returning the Client ID, not the User account. I looked at @jcrombez solution and it doesn't work.

How would I go about getting the user who authenticated?

Did you solve the issue?

valentine-morozov commented 1 month ago

This also doesn't solve the problem at least for branch v3 return new SelfValidatingPassport(new UserBadge($user->getUserIdentifier()), [$accessTokenBadge]); In Controller we have user === null and we can't check any roles

valentine-morozov commented 1 month ago

What helped me: I removed $client by $user like you did in this line of Oauth2Authenticator::authenticate

return new SelfValidatingPassport(new UserBadge($user->getUserIdentifier()), [$accessTokenBadge]);

Then I changed method Oauth2Authenticator::createAuthenticatedToken so it is setting user from passport and sets it as authenticated.

public function createAuthenticatedToken(PassportInterface $passport, string $firewallName): TokenInterface
{
    /** @var AccessTokenBadge $accessTokenBadge */
    $accessTokenBadge = $passport->getBadge( AccessTokenBadge::class );
    $token = new OAuthToken( $accessTokenBadge->getRoles() );
    $token->setToken( $accessTokenBadge->getAccessToken()->getToken() );
    $token->setUser( $passport->getUser() );
    $token->setAuthenticated(true);
    return $token;
}

Not sure if it can cause any issues but it works for now. Checking of roles is working correctly and user exists in the controller