Discussion: generation issue access tokens and randomness vs uniqueness

[zerkms]

At the moment the new access token is issued with the following code

    protected function issueAccessToken(
        \DateInterval $accessTokenTTL,
        ClientEntityInterface $client,
        $userIdentifier,
        array $scopes = []
    ) {
        $accessToken = $this->accessTokenRepository->getNewToken($client, $scopes, $userIdentifier);
        $accessToken->setClient($client);
        $accessToken->setUserIdentifier($userIdentifier);
        $accessToken->setIdentifier($this->generateUniqueIdentifier());
        $accessToken->setExpiryDateTime((new \DateTime())->add($accessTokenTTL));

        foreach ($scopes as $scope) {
            $accessToken->addScope($scope);
        }

        $this->accessTokenRepository->persistNewAccessToken($accessToken);

        return $accessToken;
    }

and the new access token identifier is generated with

    protected function generateUniqueIdentifier($length = 40)
    {
        try {
            return bin2hex(random_bytes($length));
            // @codeCoverageIgnoreStart
        } catch (\TypeError $e) {
            throw OAuthServerException::serverError('An unexpected error has occurred');
        } catch (\Error $e) {
            throw OAuthServerException::serverError('An unexpected error has occurred');
        } catch (\Exception $e) {
            // If you get this message, the CSPRNG failed hard.
            throw OAuthServerException::serverError('Could not generate a random string');
        }
        // @codeCoverageIgnoreEnd
    }

And that's where a problem appears: even though it is 40 bytes, one still cannot interchange the "unique" term with the "random" term.

Suggestion: the method that issues an access token might have done that in a loop with N maximum iterations (to prevent an infinite loop in case of an implementation or other issues) and catch for the specifically designed exception thrown from $this->accessTokenRepository->persistNewAccessToken($accessToken);.

An exception can be of a predefined type, eg: League\OAuth2\Server\Exception\UniqueAccessTokenIdentifierConstraintViolationException.

[alexbilbie]

I'm thinking it might be better to switch to v4 UUIDs for token ID long term but your suggestion of looping over is a good one