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.

Sign up for GitHub

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

Fix scope on device code grant #1412

Merged
merged 13 commits into from
Nov 14, 2024
Merged

Fix scope on device code grant #1412

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8503e58
fix scope on device code grant
hafezdivandari May 23, 2024
b3d24d0
formatting
hafezdivandari Aug 24, 2024
584dea6
Merge branch 'master' into master-fix-scope
hafezdivandari Oct 11, 2024
a94e67c
add tests and changelog entry
hafezdivandari Oct 11, 2024
37f8537
fix static analyse
hafezdivandari Oct 11, 2024
735ff5a
Merge branch 'master' into master-fix-scope
hafezdivandari Oct 14, 2024
7570854
formatting
hafezdivandari Oct 14, 2024
55dd7c7
fix a typo
hafezdivandari Oct 27, 2024
8d4d98f
Merge branch 'master' into master-fix-scope
hafezdivandari Oct 28, 2024
7522a46
no encryption on this grant
hafezdivandari Oct 28, 2024
c9639be
encrypt / decrypt the device code
hafezdivandari Oct 28, 2024
3ff445b
Revert "encrypt / decrypt the device code"
hafezdivandari Nov 12, 2024
ee3d0b6
Update chagelog message
Sephster Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Fixed bug where scopes where not set on access token when using device authorization grant (PR #1412)

## [9.0.1] - released 2024-10-14
### Fixed
Expand Down
9 changes: 9 additions & 0 deletions examples/src/Repositories/DeviceCodeRepository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface;
use OAuth2ServerExamples\Entities\ClientEntity;
use OAuth2ServerExamples\Entities\DeviceCodeEntity;
use OAuth2ServerExamples\Entities\ScopeEntity;

class DeviceCodeRepository implements DeviceCodeRepositoryInterface
{
Expand Down Expand Up @@ -49,6 +50,14 @@ public function getDeviceCodeEntityByDeviceCode($deviceCode): ?DeviceCodeEntityI
$deviceCodeEntity->setIdentifier($deviceCode);
$deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('now +1 hour'));
$deviceCodeEntity->setClient($clientEntity);
$deviceCodeEntity->setLastPolledAt(new DateTimeImmutable());

$scopes = [];
foreach ($scopes as $scope) {
$scopeEntity = new ScopeEntity();
$scopeEntity->setIdentifier($scope);
$deviceCodeEntity->addScope($scopeEntity);
}

// The user identifier should be set when the user authenticates on the
// OAuth server, along with whether they approved the request
Expand Down
3 changes: 1 addition & 2 deletions src/Grant/DeviceCodeGrant.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ public function respondToAccessTokenRequest(
): ResponseTypeInterface {
// Validate request
$client = $this->validateClient($request);
$scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope));
$deviceCodeEntity = $this->validateDeviceCode($request, $client);

$deviceCodeEntity->setLastPolledAt(new DateTimeImmutable());
Expand All @@ -153,7 +152,7 @@ public function respondToAccessTokenRequest(
}

// Finalize the requested scopes
$finalizedScopes = $this->scopeRepository->finalizeScopes($scopes, $this->getIdentifier(), $client, $deviceCodeEntity->getUserIdentifier());
$finalizedScopes = $this->scopeRepository->finalizeScopes($deviceCodeEntity->getScopes(), $this->getIdentifier(), $client, $deviceCodeEntity->getUserIdentifier());

// Issue and persist new access token
$accessToken = $this->issueAccessToken($accessTokenTTL, $client, $deviceCodeEntity->getUserIdentifier(), $finalizedScopes);
Expand Down
140 changes: 37 additions & 103 deletions tests/Grant/DeviceCodeGrantTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@
use PHPUnit\Framework\TestCase;

use function base64_encode;
use function json_encode;
use function random_bytes;
use function time;
use function uniqid;

class DeviceCodeGrantTest extends TestCase
Expand Down Expand Up @@ -261,6 +259,7 @@ public function testValidateDeviceAuthorizationRequestClientMismatch(): void
public function testCompleteDeviceAuthorizationRequest(): void
{
$deviceCode = new DeviceCodeEntity();
$deviceCode->setIdentifier('deviceCodeEntityIdentifier');
$deviceCode->setUserCode('foo');

$deviceCodeRepository = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock();
Expand All @@ -275,7 +274,7 @@ public function testCompleteDeviceAuthorizationRequest(): void

$grant->setEncryptionKey($this->cryptStub->getKey());

$grant->completeDeviceAuthorizationRequest($deviceCode->getUserCode(), 'userId', true);
$grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), 'userId', true);

$this::assertEquals('userId', $deviceCode->getUserIdentifier());
}
Expand Down Expand Up @@ -346,13 +345,8 @@ public function testRespondToAccessTokenRequest(): void
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
$clientRepositoryMock->method('validateClient')->willReturn(true);

$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
$accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();

$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
$refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf();
$refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity());
$scope = new ScopeEntity();
$scope->setIdentifier('foo');

$deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock();
$deviceCodeEntity = new DeviceCodeEntity();
Expand All @@ -362,12 +356,27 @@ public function testRespondToAccessTokenRequest(): void
$deviceCodeEntity->setUserCode('123456');
$deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('+1 hour'));
$deviceCodeEntity->setClient($client);
$deviceCodeEntity->addScope($scope);

$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity);
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')
->with($deviceCodeEntity->getIdentifier())
->willReturn($deviceCodeEntity);

$accessTokenEntity = new AccessTokenEntity();
$accessTokenEntity->addScope($scope);

$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
$accessTokenRepositoryMock->method('getNewToken')
->with($client, $deviceCodeEntity->getScopes(), $deviceCodeEntity->getUserIdentifier())
->willReturn($accessTokenEntity);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();

$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
$refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf();
$refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity());

$scope = new ScopeEntity();
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
$scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier');
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);

$grant = new DeviceCodeGrant(
Expand All @@ -384,30 +393,19 @@ public function testRespondToAccessTokenRequest(): void
$grant->setEncryptionKey($this->cryptStub->getKey());
$grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));

$grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getUserCode(), '1', true);
$grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getIdentifier(), 'baz', true);

$serverRequest = (new ServerRequest())->withParsedBody([
'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code',
'device_code' => $this->cryptStub->doEncrypt(
json_encode(
[
'device_code_id' => uniqid(),
'expire_time' => time() + 3600,
'client_id' => 'foo',
'user_code' => '12345678',
'scopes' => ['foo'],
'verification_uri' => 'http://foo/bar',
],
JSON_THROW_ON_ERROR
)
),
'device_code' => $deviceCodeEntity->getIdentifier(),
'client_id' => 'foo',
]);

$responseType = new StubResponseType();
$grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M'));

$this::assertInstanceOf(RefreshTokenEntityInterface::class, $responseType->getRefreshToken());
$this::assertSame([$scope], $responseType->getAccessToken()->getScopes());
}

public function testRespondToRequestMissingClient(): void
Expand All @@ -430,19 +428,7 @@ public function testRespondToRequestMissingClient(): void
$grant->setAccessTokenRepository($accessTokenRepositoryMock);

$serverRequest = (new ServerRequest())->withQueryParams([
'device_code' => $this->cryptStub->doEncrypt(
json_encode(
[
'device_code_id' => uniqid(),
'expire_time' => time() + 3600,
'client_id' => 'foo',
'user_code' => '12345678',
'scopes' => ['foo'],
'verification_uri' => 'http://foo/bar',
],
JSON_THROW_ON_ERROR
)
),
'device_code' => uniqid(),
]);

$responseType = new StubResponseType();
Expand All @@ -469,9 +455,8 @@ public function testRespondToRequestMissingDeviceCode(): void
$deviceCodeEntity->setUserIdentifier('baz');
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity);

$scope = new ScopeEntity();
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
$scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier');
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);

$grant = new DeviceCodeGrant(
Expand Down Expand Up @@ -518,9 +503,8 @@ public function testIssueSlowDownError(): void
$deviceCodeEntity->setClient($client);
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity);

$scope = new ScopeEntity();
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
$scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier');
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);

$grant = new DeviceCodeGrant(
Expand All @@ -538,19 +522,7 @@ public function testIssueSlowDownError(): void

$serverRequest = (new ServerRequest())->withParsedBody([
'client_id' => 'foo',
'device_code' => $this->cryptStub->doEncrypt(
json_encode(
[
'device_code_id' => uniqid(),
'expire_time' => time() + 3600,
'client_id' => 'foo',
'user_code' => '12345678',
'scopes' => ['foo'],
'verification_uri' => 'http://foo/bar',
],
JSON_THROW_ON_ERROR
)
),
'device_code' => uniqid(),
]);

$responseType = new StubResponseType();
Expand Down Expand Up @@ -579,9 +551,8 @@ public function testIssueAuthorizationPendingError(): void
$deviceCodeEntity->setClient($client);
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity);

$scope = new ScopeEntity();
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
$scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier');
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);

$grant = new DeviceCodeGrant(
Expand All @@ -599,19 +570,7 @@ public function testIssueAuthorizationPendingError(): void

$serverRequest = (new ServerRequest())->withParsedBody([
'client_id' => 'foo',
'device_code' => $this->cryptStub->doEncrypt(
json_encode(
[
'device_code_id' => uniqid(),
'expire_time' => time() + 3600,
'client_id' => 'foo',
'user_code' => '12345678',
'scopes' => ['foo'],
'verification_uri' => 'http://foo/bar',
],
JSON_THROW_ON_ERROR
)
),
'device_code' => uniqid(),
]);

$responseType = new StubResponseType();
Expand Down Expand Up @@ -640,9 +599,8 @@ public function testIssueExpiredTokenError(): void
$deviceCodeEntity->setClient($client);
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity);

$scope = new ScopeEntity();
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
$scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier');
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);

$grant = new DeviceCodeGrant(
Expand All @@ -660,19 +618,7 @@ public function testIssueExpiredTokenError(): void

$serverRequest = (new ServerRequest())->withParsedBody([
'client_id' => 'foo',
'device_code' => $this->cryptStub->doEncrypt(
json_encode(
[
'device_code_id' => uniqid(),
'expire_time' => time() - 3600,
'client_id' => 'foo',
'user_code' => '12345678',
'scopes' => ['foo'],
'verification_uri' => 'http://foo/bar',
],
JSON_THROW_ON_ERROR
)
),
'device_code' => uniqid(),
]);

$responseType = new StubResponseType();
Expand Down Expand Up @@ -746,14 +692,14 @@ public function testIssueAccessDeniedError(): void

$deviceCode = new DeviceCodeEntity();

$deviceCode->setIdentifier('deviceCodeEntityIdentifier');
$deviceCode->setExpiryDateTime(new DateTimeImmutable('+1 hour'));
$deviceCode->setClient($client);
$deviceCode->setUserCode('12345678');
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCode);

$scope = new ScopeEntity();
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
$scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier');
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);

$grant = new DeviceCodeGrant(
Expand All @@ -769,23 +715,11 @@ public function testIssueAccessDeniedError(): void
$grant->setEncryptionKey($this->cryptStub->getKey());
$grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));

$grant->completeDeviceAuthorizationRequest($deviceCode->getUserCode(), '1', false);
$grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), '1', false);

$serverRequest = (new ServerRequest())->withParsedBody([
'client_id' => 'foo',
'device_code' => $this->cryptStub->doEncrypt(
json_encode(
[
'device_code_id' => uniqid(),
'expire_time' => time() + 3600,
'client_id' => 'foo',
'user_code' => '12345678',
'scopes' => ['foo'],
'verification_uri' => 'http://foo/bar',
],
JSON_THROW_ON_ERROR
),
),
'device_code' => $deviceCode->getIdentifier(),
]);

$responseType = new StubResponseType();
Expand Down