Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AccessTokenEntityInterface::getNewToken() and scopes parameter. #715

Open
Danack opened this issue Mar 17, 2017 · 4 comments
Open

AccessTokenEntityInterface::getNewToken() and scopes parameter. #715

Danack opened this issue Mar 17, 2017 · 4 comments
Labels
Milestone

Comments

@Danack
Copy link

Danack commented Mar 17, 2017

Hi,

Please could you clarify for me what an implementation of AccessTokenEntityInterface should do with the scopes parameter in getNewToken() ? or any of the parameters actually.

The getNewToken method is called, and then immediately scopes are added to the token again, which lead to the scopes being double entered on my implementation.

$accessToken = $this->accessTokenRepository->getNewToken($client, $scopes, $userIdentifier);
$accessToken->setClient($client);
$accessToken->setUserIdentifier($userIdentifier);
$accessToken->setExpiryDateTime((new \DateTime())->add($accessTokenTTL));
foreach ($scopes as $scope) {
$accessToken->addScope($scope);
}

cheers
Dan

@Sephster Sephster added the Bug label Apr 3, 2018
@Sephster
Copy link
Member

Sephster commented Apr 3, 2018

Hi @Danack - Apologies for the delayed response. This looks like a bug to me as you are correct in that we are likely double entering scopes. This shouldn't created duplicates against the access token though as these are indexed on the scope ID:

public function addScope(ScopeEntityInterface $scope)
{
$this->scopes[$scope->getIdentifier()] = $scope;
}

The easiest fix would be to remove the duplicate code from the Abstract Grant but I will need to do a thorough check to make sure this won't break anything. Thanks for reporting this

@lordrhodos
Copy link

I hit the same issue as well and was wondering why the AbstractGrant adds the scopes again after they had been passed to the getNewToken method of the repository. I was not using indexed entries in the scopes collection (common doctrine ArrayCollection). Changed that for now, but would love to see a fix in the future.

@Sephster
Copy link
Member

This issue was originally tackled in #553 but reverted as it introduced a breaking change. I think it should be reinstated for version 8.

@Sephster Sephster added this to the 8.00 milestone Jan 22, 2019
@crtl
Copy link

crtl commented Jan 29, 2019

I wanted to ask the same question.
Either AccessTokenEntityRepository:getNewToken or AbstractGrant should setup the token.
Why call a factory when I do everything myself?

If it is to execute additional logic, why dont you use an event system?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants