Oauth2-server: The encrypted code payload should contain scope identifiers and not Scope Entity Objects

[iliepandia]

At this line:
https://github.com/thephpleague/oauth2-server/blob/8bbd21885635e132aaeca900e8221d8ca87cb6b9/src/Grant/AuthCodeGrant.php#L323
the scopes are added to the payload but they are added as an array of ScopeEntities. getScopes() returns ScopeEntityInterface[]

However, when you decrypt the payload later on you iterate through it as if it were an array of scopeId instead of ScopeEntities:

https://github.com/thephpleague/oauth2-server/blob/8bbd21885635e132aaeca900e8221d8ca87cb6b9/src/Grant/AuthCodeGrant.php#L107

And then getScopeEntityByIdentifier is expecting a string but gets an object instead.
https://github.com/thephpleague/oauth2-server/blob/8bbd21885635e132aaeca900e8221d8ca87cb6b9/src/Grant/AuthCodeGrant.php#L108

Am I getting something wrong? Cause my code fails now in the function getScopeEntityByIdentifier because instead of a string it gets called with an object.

[jimi985]

I added a pull request to address this, but it was never responded to.

https://github.com/thephpleague/oauth2-server/pull/711

[Sephster]

Apologies for this. I've been working through the old pull requests to progress them but haven't got to yours yet. I am planning on releasing version 7 this week so will try and get this included in time. Very sorry for the delay and thanks for both of your help with this

[Sephster]

Hi @iliepandia - for your ScopeEntity implementation, how are you defining the abstract function jsonSerialize()? If you get it to return $this->getIdentifier(), does this fix your issue?

When we JSON encode the ScopeEntity's, we should use jsonSerialize() to pass back the identifiers that we want to have encoded. If you could let me know if you are doing this or not and I will look into this further if not. Many thanks

[iliepandia]

Hi @Sephster ,

Eventually after looking at your example in closer detail, because I could not understand how your tests are passing, I realized that jsonSeralize() was how you made it work. In my case jsonSeralize was returning much more than the identifier.

So yes, implemented jsonSerialize, as you suggest, does fix my issue, but in my opinion it hides the problem, not really fix it.

I would suggest that here:

https://github.com/thephpleague/oauth2-server/blob/8bbd21885635e132aaeca900e8221d8ca87cb6b9/src/Grant/AuthCodeGrant.php#L323

$authCode->getScopes() be replaced with an array where for each scope you specifically call getIdentifier(). Maybe something like

<?php
$scopes = array_map( $authCode->getScopes(), function(scope){return $scope->getIdentifier() });

This way scopes will always have an array of identifiers regardless of how people implement the jsonSerialize().

Another solution could be to make it clear in the docs that jsonSerialize() needs a very specific implementation and will not work otherwise.

[Sephster]

Yeah I see how this could be very confusing. When implementing a scope entity, you are forced to define jsonSerialize() but I can't see anything in the docs to point you in the right direction with this.

Because your implementations issue has been resolved, I will close this issue but I will keep @jimi985's PR open because this probably does need to be addressed so others don't hit the same problem. At the moment I'm leaning towards improved docs but I will need to look at the code more closely.

Thanks very much for raising this and for your suggested fixes.