klapaudius / FOSOAuthServerBundle

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

Use user username instead of client identifier (which is a random id) #15

Closed victormacko closed 1 year ago

victormacko commented 1 year ago

Feel free to enlighten me, but using the client id uses a random number for the username, vs the actual user's username.

This also addresses the problem of if a custom ClientManager is used (ie. a static list/external service/etc), and the $accessToken->getClient() doesn't actually reference any client, then an error is thrown indicating the $client var within $accessToken needs to be set before it can be accessed (php8 error).

Thanks for reviewing my PR - nice job on getting the project working with Symfony6.

Edit: I've also included an additional check so multiple authenticators can be used with the existing 'Authorization' header (ie. oAuth & an API key / user & password / etc)

klapaudius commented 1 year 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"

victormacko commented 1 year ago

Thanks @klapaudius - I think i've added in several changes into the one PR -- ie. using a custom ClientManager without a 'client' variable set in the access token, as well as not failing the authentication process entirely (ie. if multiple authenticators are used) if credentials which aren't recognised are used.

Should I add these other changes to a separate PR, or they all rejected/declined?

klapaudius commented 1 year ago

You can do one PR per topic I would be glad to merge if everything is ok regarding the RFC