[9.x] Use psr request attributes in 'check client credentials' middleware

[gdebrauwer]

The 'check client credentials' middleware currently fetches the client via the token. That means it first has to get the token and then load the client relation. The client id is already present in psr request via 'oauth_client_id' attribute. By using that attribute, the middleware has to make one less database query.
It also doesn't need to fetch the token in order to check the scopes. The psr request includes the scopes via 'oauth_scopes' attribute.

These psr attributes were used by the middleware in the previous version(s) of Passport.

[driesvints]

Isn't the whole point of the middleware to "check credentials"? If the token isn't valid but the client id is then I think this would defeat the purpose? The current middleware checks first if the token is valid and then fetches the client to check the scopes.

@Sephster do you maybe have some insights here?

[gdebrauwer]

@driesvints The middleware already checks if the token is valid by running $this->server->validateAuthenticatedRequest($psr);. That method validates the 'Authorization' header, parses the access token and sets attributes on the psr-request based on parsed data.

[driesvints]

Is it also verified that $tokenScopes = $psr->getAttribute('oauth_scopes') is the same as the ones defined on the actual client? These can't be changed later on the client itself, right?

I'm gonna wait a little bit more until @Sephster can verify that this looks okay.

[driesvints]

I think the main question we need to ask ourselves here: do we need to rely on the scopes from the token instead of the ones from the DB? Because if it's the former, do the $token->can( and $token->cant( methods still make sense?

[gdebrauwer]

I don't think it's possible (or advised) to alter the scopes of an issued access token, so I think it might not matter which ones you check.

[driesvints]

@gdebrauwer I believe so as well. But would like some further confirmation before we proceed.

Thanks for the pr btw 👍

[driesvints]

@gdebrauwer we reverted this for now until we can get to the bottom of the scopes thing. To me personally it doesn't makes sense that we keep the scopes in the database if they're already inside the token. We need to make a clear distinction and make sure that we understand properly which scopes we need to check (psr request or db ones) before we do any more modifications.

[gdebrauwer]

@driesvints no problem 🙂

I think the only reason to keep the scopes in the database is to show the user's tokens and their scopes.
The AuthorizedClients component does that by fetching the tokens via the /oauth/tokens route. That route returns all the tokens and their scopes.

[Sephster]

Hi @driesvints - apologies if I'm making some incorrect assumptions here. I will lay out my understanding just in case.

At the moment in Passport, when the resource server receives an access token, it is doing two things:

• running the authentication checks provided by the OAuth2 Server
• Checking that the scopes in the token are associated with the access token issued

I've had a look through the code and it looks like Passport is using the BearerTokenValidator provided with the OAuth2-Server. If this is the case, there should be no need to check the scopes against the access token in the DB.

The OAuth2 server uses a JWT bearer token which is signed so it can't be tampered with. If it passes all of its validation checks in stage one, we can be certain that the scopes requested are the same as the ones that were originally granted (assuming your keys are still secure).

If someone were to alter the JWT to change scopes, the validation on the JWT would fail and the request would be rejected.

Hope this helps. Happy to answer any further questions that might come up. Cheers!

[driesvints]

Hey all, thanks for replying. Still investigating this.

@themsaid shared the good remark that services like DigitalOcean allow you to change the scopes through an interface:

So if we'd always check the scopes through the psr token this would mean this isn't possible anymore?

[Sephster]

Hey @driesvints - I don't know if this helps but there is no means to change your scopes on a token in the OAuth2 specs once the token has been issued.

Well, this isn't quite true. When you request a new token using a refresh token, you can request the same scopes or fewer scopes (as long as all of the scopes you are requesting are a subset of the ones you requested originally). Other than that, you must request a new token with new scopes to my knowledge.

It looks like DO are doing something custom outside of the standard specs, or they are just invalidating the old token and issuing a completely new one. I'm not familiar with the mechanism sorry. Hope this helps somewhat.

[driesvints]

Hey @Sephster. Thanks that helps. DigitalOcean doesn't invalidates the old token.

I guess we've reached the conclusion that in order to allow for customization like DigitalOcean does, we'll have to step outside the OAuth2 spec here a bit. That's why I think it's good to let the current implementation stand and always read the scopes from the DB.

Thank you to you both @Sephster and @gdebrauwer for helping figuring this out 👍