Skip to content

Commit

Permalink
Simplify device code response - remove payload
Browse files Browse the repository at this point in the history
  • Loading branch information
@Sephster
Sephster committed Mar 21, 2024
1 parent 0636d25 commit f8f15ef
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 92 deletions.
3 changes: 2 additions & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"defuse/php-encryption": "^2.4",
"ext-json": "*",
"lcobucci/clock": "^2.3 || ^3.0",
"psr/http-server-middleware": "^1.0"
"psr/http-server-middleware": "^1.0",
"ramsey/uuid": "^4.7"
},
"require-dev": {
"phpunit/phpunit": "^9.6.15",
Expand Down
81 changes: 21 additions & 60 deletions src/Grant/DeviceCodeGrant.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,11 @@
use League\OAuth2\Server\RequestEvent;
use League\OAuth2\Server\ResponseTypes\DeviceCodeResponse;
use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface;
use LogicException;
use Psr\Http\Message\ServerRequestInterface;
use stdClass;
use Ramsey\Uuid\Uuid;
use TypeError;

use function is_null;
use function json_decode;
use function json_encode;
use function property_exists;
use function random_int;
use function strlen;
use function time;
Expand Down Expand Up @@ -93,35 +89,20 @@ public function respondToDeviceAuthorizationRequest(ServerRequestInterface $requ

$scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope));

$deviceCode = $this->issueDeviceCode(
$deviceCodeEntity = $this->issueDeviceCode(
$this->deviceCodeTTL,
$client,
$this->verificationUri,
$scopes
);

// TODO: Check payload generation. Is this the best way to handle things?
$payload = [
'device_code_id' => $deviceCode->getIdentifier(),
'user_code' => $deviceCode->getUserCode(),
'verification_uri' => $deviceCode->getVerificationUri(),
'expire_time' => $deviceCode->getExpiryDateTime()->getTimestamp(),
'client_id' => $deviceCode->getClient()->getIdentifier(),
'scopes' => $deviceCode->getScopes(),
'interval' => $deviceCode->getInterval(),
];

$response = new DeviceCodeResponse();

if ($this->includeVerificationUriComplete === true) {
$response->includeVerificationUriComplete();
$payload['verification_uri_complete'] = $deviceCode->getVerificationUriComplete();
}

$jsonPayload = json_encode($payload, JSON_THROW_ON_ERROR);

$response->setDeviceCode($deviceCode);
$response->setPayload($this->encrypt($jsonPayload));
$response->setDeviceCodeEntity($deviceCodeEntity);

return $response;
}
Expand Down Expand Up @@ -198,64 +179,46 @@ public function respondToAccessTokenRequest(
*/
protected function validateDeviceCode(ServerRequestInterface $request, ClientEntityInterface $client): DeviceCodeEntityInterface
{
$encryptedDeviceCode = $this->getRequestParameter('device_code', $request);
$deviceCode = $this->getRequestParameter('device_code', $request);

if (is_null($encryptedDeviceCode)) {
if (is_null($deviceCode)) {
throw OAuthServerException::invalidRequest('device_code');
}

$deviceCodePayload = $this->decodeDeviceCode($encryptedDeviceCode);
$deviceCodeEntity = $this->deviceCodeRepository->getDeviceCodeEntityByDeviceCode(
$deviceCode
);

if (!property_exists($deviceCodePayload, 'device_code_id')) {
throw OAuthServerException::invalidRequest('device_code', 'Device code malformed');
if ($deviceCodeEntity instanceof DeviceCodeEntityInterface === false) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request));

throw OAuthServerException::invalidGrant();
}

if (time() > $deviceCodePayload->expire_time) {
if (time() > $deviceCodeEntity->getExpiryDateTime()->getTimestamp()) {
throw OAuthServerException::expiredToken('device_code');
}

if ($this->deviceCodeRepository->isDeviceCodeRevoked($deviceCodePayload->device_code_id) === true) {
if ($this->deviceCodeRepository->isDeviceCodeRevoked($deviceCode) === true) {
throw OAuthServerException::invalidRequest('device_code', 'Device code has been revoked');
}

if ($deviceCodePayload->client_id !== $client->getIdentifier()) {
if ($deviceCodeEntity->getClient()->getIdentifier() !== $client->getIdentifier()) {
throw OAuthServerException::invalidRequest('device_code', 'Device code was not issued to this client');
}

$deviceCode = $this->deviceCodeRepository->getDeviceCodeEntityByDeviceCode(
$deviceCodePayload->device_code_id
);

if ($deviceCode instanceof DeviceCodeEntityInterface === false) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request));

throw OAuthServerException::invalidGrant();
}

if ($this->deviceCodePolledTooSoon($deviceCode->getLastPolledAt()) === true) {
if ($this->deviceCodePolledTooSoon($deviceCodeEntity->getLastPolledAt()) === true) {
throw OAuthServerException::slowDown();
}

return $deviceCode;
return $deviceCodeEntity;
}

private function deviceCodePolledTooSoon(?DateTimeImmutable $lastPoll): bool
{
return $lastPoll !== null && $lastPoll->getTimestamp() + $this->retryInterval > time();
}

/**
* @throws OAuthServerException
*/
protected function decodeDeviceCode(string $encryptedDeviceCode): stdClass
{
try {
return json_decode($this->decrypt($encryptedDeviceCode));
} catch (LogicException $e) {
throw OAuthServerException::invalidRequest('device_code', 'Cannot decrypt the device code', $e);
}
}

/**
* Set the verification uri
*/
Expand Down Expand Up @@ -309,7 +272,8 @@ protected function issueDeviceCode(

while ($maxGenerationAttempts-- > 0) {
$deviceCode->setIdentifier($this->generateUniqueIdentifier());
$deviceCode->setUserCode($this->generateUniqueUserCode());
$deviceCode->setUserCode($this->generateUserCode());

try {
$this->deviceCodeRepository->persistDeviceCode($deviceCode);

Expand All @@ -321,19 +285,16 @@ protected function issueDeviceCode(
}
}


// This should never be hit. It is here to work around a PHPStan false error
return $deviceCode;
}

/**
* Generate a new unique user code.
*
*
* Generate a new user code.
*
* @throws OAuthServerException
*/
protected function generateUniqueUserCode(int $length = 8): string
protected function generateUserCode(int $length = 8): string
{
try {
$userCode = '';
Expand Down
10 changes: 10 additions & 0 deletions src/Repositories/DeviceCodeRepositoryInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,14 @@ public function revokeDeviceCode(string $codeId): void;
* @return bool Return true if this code has been revoked
*/
public function isDeviceCodeRevoked(string $codeId): bool;

/**
* Get the expiry time of the device code.
*/
public function getDeviceCodeExpiryTime(string $codeId): int;

/**
* Get the client ID of the device code.
*/
public function getDeviceCodeClientId(string $codeId): string;
}
26 changes: 10 additions & 16 deletions src/ResponseTypes/DeviceCodeResponse.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@

class DeviceCodeResponse extends AbstractResponseType
{
protected DeviceCodeEntityInterface $deviceCode;
protected string $payload;
protected DeviceCodeEntityInterface $deviceCodeEntity;
private bool $includeVerificationUriComplete = false;
private const DEFAULT_INTERVAL = 5;

Expand All @@ -33,21 +32,21 @@ class DeviceCodeResponse extends AbstractResponseType
*/
public function generateHttpResponse(ResponseInterface $response): ResponseInterface
{
$expireDateTime = $this->deviceCode->getExpiryDateTime()->getTimestamp();
$expireDateTime = $this->deviceCodeEntity->getExpiryDateTime()->getTimestamp();

$responseParams = [
'device_code' => $this->payload,
'user_code' => $this->deviceCode->getUserCode(),
'verification_uri' => $this->deviceCode->getVerificationUri(),
'device_code' => $this->deviceCodeEntity->getIdentifier(),
'user_code' => $this->deviceCodeEntity->getUserCode(),
'verification_uri' => $this->deviceCodeEntity->getVerificationUri(),
'expires_in' => $expireDateTime - time(),
];

if ($this->includeVerificationUriComplete === true) {
$responseParams['verification_uri_complete'] = $this->deviceCode->getVerificationUriComplete();
$responseParams['verification_uri_complete'] = $this->deviceCodeEntity->getVerificationUriComplete();
}

if ($this->deviceCode->getInterval() !== self::DEFAULT_INTERVAL) {
$responseParams['interval'] = $this->deviceCode->getInterval();
if ($this->deviceCodeEntity->getInterval() !== self::DEFAULT_INTERVAL) {
$responseParams['interval'] = $this->deviceCodeEntity->getInterval();
}

$responseParams = json_encode($responseParams);
Expand All @@ -67,17 +66,12 @@ public function generateHttpResponse(ResponseInterface $response): ResponseInter
return $response;
}

public function setPayload(string $payload): void
{
$this->payload = $payload;
}

/**
* {@inheritdoc}
*/
public function setDeviceCode(DeviceCodeEntityInterface $deviceCode): void
public function setDeviceCodeEntity(DeviceCodeEntityInterface $deviceCodeEntity): void
{
$this->deviceCode = $deviceCode;
$this->deviceCodeEntity = $deviceCodeEntity;
}

public function includeVerificationUriComplete(): void
Expand Down
30 changes: 20 additions & 10 deletions tests/Grant/DeviceCodeGrantTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,15 @@ public function testRespondToAccessTokenRequest(): void
$refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity());

$deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock();
$deviceCode = new DeviceCodeEntity();
$deviceCodeEntity = new DeviceCodeEntity();

$deviceCode->setUserIdentifier('baz');
$deviceCode->setIdentifier('deviceCodeEntityIdentifier');
$deviceCode->setUserCode('123456');
$deviceCodeEntity->setUserIdentifier('baz');
$deviceCodeEntity->setIdentifier('deviceCodeEntityIdentifier');
$deviceCodeEntity->setUserCode('123456');
$deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('+1 hour'));
$deviceCodeEntity->setClient($client);

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

$scope = new ScopeEntity();
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
Expand All @@ -382,7 +384,7 @@ public function testRespondToAccessTokenRequest(): void
$grant->setEncryptionKey($this->cryptStub->getKey());
$grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));

$grant->completeDeviceAuthorizationRequest($deviceCode->getUserCode(), "1", true);
$grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getUserCode(), "1", true);

$serverRequest = (new ServerRequest())->withParsedBody([
'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code',
Expand Down Expand Up @@ -511,6 +513,8 @@ public function testIssueSlowDownError(): void
$deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock();
$deviceCodeEntity = new DeviceCodeEntity();
$deviceCodeEntity->setLastPolledAt(new DateTimeImmutable());
$deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('+1 hour'));
$deviceCodeEntity->setClient($client);
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity);

$scope = new ScopeEntity();
Expand Down Expand Up @@ -569,8 +573,10 @@ public function testIssueAuthorizationPendingError(): void
$refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity());

$deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock();
$deviceCode = new DeviceCodeEntity();
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCode);
$deviceCodeEntity = new DeviceCodeEntity();
$deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('+1 hour'));
$deviceCodeEntity->setClient($client);
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity);

$scope = new ScopeEntity();
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
Expand Down Expand Up @@ -628,8 +634,10 @@ public function testIssueExpiredTokenError(): void
$refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity());

$deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock();
$deviceCode = new DeviceCodeEntity();
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCode);
$deviceCodeEntity = new DeviceCodeEntity();
$deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('-1 hour'));
$deviceCodeEntity->setClient($client);
$deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity);

$scope = new ScopeEntity();
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
Expand Down Expand Up @@ -736,6 +744,8 @@ public function testIssueAccessDeniedError(): void

$deviceCode = new DeviceCodeEntity();

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

Expand Down
7 changes: 2 additions & 5 deletions tests/ResponseTypes/DeviceCodeResponseTypeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
use LeagueTests\Stubs\DeviceCodeEntity;
use LeagueTests\Stubs\ScopeEntity;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\ResponseInterface;

use function base64_encode;
use function json_decode;
Expand Down Expand Up @@ -41,9 +40,7 @@ public function testGenerateHttpResponse(): void
$deviceCode->addScope($scope);
$deviceCode->setVerificationUri('https://example.com/device');


$responseType->setDeviceCode($deviceCode);
$responseType->setPayload('test');
$responseType->setDeviceCodeEntity($deviceCode);

$response = $responseType->generateHttpResponse(new Response());

Expand All @@ -56,7 +53,7 @@ public function testGenerateHttpResponse(): void
$json = json_decode($response->getBody()->getContents());
$this::assertObjectHasProperty('expires_in', $json);
$this::assertObjectHasProperty('device_code', $json);
$this::assertEquals('test', $json->device_code);
$this::assertEquals('abcdef', $json->device_code);
$this::assertObjectHasProperty('verification_uri', $json);
$this::assertObjectHasProperty('user_code', $json);
}
Expand Down

0 comments on commit f8f15ef

Please sign in to comment.