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

Fix Events and add TODOs for the next major release #1467

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
## [Unreleased]
### Changed
- Key permission checks ignored on Windows regardless of userland choice as cannot be run successfully on this OS (PR #1447)
- Fixed bug on setting interval visibility of device authorization grant (PR #1410)
- Fix a bug where the new poll date were not persisted when `slow_down` error happens, because the exception is thrown before calling `persistDeviceCode`. (PR #1410)
- Fix a bug where `slow_down` error response may have been returned even after the user has completed the auth flow (already approved / denied the request). (PR #1410)
- Emit `RequestAccessTokenEvent` and `RequestRefreshTokenEvent` events instead of the general `RequestEvent` event when an access / refresh token is issued using device authorization grant. (PR #1467)

## [9.1.0] - released 2024-11-21
### Added
Expand All @@ -16,9 +20,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- In the Auth Code grant, when requesting an access token with an invalid auth code, we now respond with an invalid_grant error instead of invalid_request (PR #1433)
- Fixed spec compliance issue where device access token request was mistakenly expecting to receive scopes in the request (PR #1412)
- Refresh tokens pre version 9 might have had user IDs set as ints which meant they were incorrectly rejected. We now cast these values to strings to allow old refresh tokens (PR #1436)
- Fixed bug on setting interval visibility of device authorization grant (PR #1410)
- Fix a bug where the new poll date were not persisted when `slow_down` error happens, because the exception is thrown before calling `persistDeviceCode`. (PR #1410)
- Fix a bug where `slow_down` error response may have been returned even after the user has completed the auth flow (already approved / denied the request). (PR #1410)

## [9.0.1] - released 2024-10-14
### Fixed
Expand Down
6 changes: 4 additions & 2 deletions src/Grant/DeviceCodeGrant.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
use League\OAuth2\Server\Exception\UniqueTokenIdentifierConstraintViolationException;
use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface;
use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
use League\OAuth2\Server\RequestAccessTokenEvent;
use League\OAuth2\Server\RequestEvent;
use League\OAuth2\Server\RequestRefreshTokenEvent;
use League\OAuth2\Server\ResponseTypes\DeviceCodeResponse;
use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface;
use Psr\Http\Message\ServerRequestInterface;
Expand Down Expand Up @@ -166,14 +168,14 @@ public function respondToAccessTokenRequest(

// Issue and persist new access token
$accessToken = $this->issueAccessToken($accessTokenTTL, $client, $deviceCodeEntity->getUserIdentifier(), $finalizedScopes);
$this->getEmitter()->emit(new RequestEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request));
$this->getEmitter()->emit(new RequestAccessTokenEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request, $accessToken));
$responseType->setAccessToken($accessToken);

// Issue and persist new refresh token if given
$refreshToken = $this->issueRefreshToken($accessToken);

if ($refreshToken !== null) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request));
$this->getEmitter()->emit(new RequestRefreshTokenEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request, $refreshToken));
$responseType->setRefreshToken($refreshToken);
}

Expand Down
3 changes: 3 additions & 0 deletions src/Grant/ImplicitGrant.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth
$finalizedScopes
);

// TODO: next major release: this method needs `ServerRequestInterface` as an argument
// $this->getEmitter()->emit(new RequestAccessTokenEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request, $accessToken));

$response = new RedirectResponse();
$response->setRedirectUri(
$this->makeRedirectUri(
Expand Down
2 changes: 1 addition & 1 deletion src/Repositories/DeviceCodeRepositoryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function persistDeviceCode(DeviceCodeEntityInterface $deviceCodeEntity):
* Get a device code entity.
*/
public function getDeviceCodeEntityByDeviceCode(
string $deviceCodeEntity
string $deviceCodeEntity // TODO: next major release: rename to `$deviceCode`
): ?DeviceCodeEntityInterface;

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/AuthorizationServerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public function testMultipleRequestsGetDifferentResponseTypeInstances(): void
$privateKey = 'file://' . __DIR__ . '/Stubs/private.key';
$encryptionKey = 'file://' . __DIR__ . '/Stubs/public.key';

$responseTypePrototype = new class() extends BearerTokenResponse {
$responseTypePrototype = new class () extends BearerTokenResponse {
protected CryptKeyInterface $privateKey;
protected Key|string|null $encryptionKey = null;

Expand Down
32 changes: 32 additions & 0 deletions tests/Grant/AuthCodeGrantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
use League\OAuth2\Server\Repositories\ClientRepositoryInterface;
use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
use League\OAuth2\Server\Repositories\ScopeRepositoryInterface;
use League\OAuth2\Server\RequestAccessTokenEvent;
use League\OAuth2\Server\RequestEvent;
use League\OAuth2\Server\RequestRefreshTokenEvent;
use League\OAuth2\Server\RequestTypes\AuthorizationRequest;
use League\OAuth2\Server\ResponseTypes\RedirectResponse;
use LeagueTests\Stubs\AccessTokenEntity;
Expand Down Expand Up @@ -635,6 +638,27 @@ public function testRespondToAccessTokenRequest(): void
$grant->setEncryptionKey($this->cryptStub->getKey());
$grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));

$accessTokenEventEmitted = false;
$refreshTokenEventEmitted = false;

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::ACCESS_TOKEN_ISSUED,
function ($event) use (&$accessTokenEventEmitted): void {
self::assertInstanceOf(RequestAccessTokenEvent::class, $event);

$accessTokenEventEmitted = true;
}
);

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::REFRESH_TOKEN_ISSUED,
function ($event) use (&$refreshTokenEventEmitted): void {
self::assertInstanceOf(RequestRefreshTokenEvent::class, $event);

$refreshTokenEventEmitted = true;
}
);

$request = new ServerRequest(
[],
[],
Expand Down Expand Up @@ -665,6 +689,14 @@ public function testRespondToAccessTokenRequest(): void
$response = $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M'));

self::assertInstanceOf(RefreshTokenEntityInterface::class, $response->getRefreshToken());

if (!$accessTokenEventEmitted) {
self::fail('Access token issued event is not emitted.');
}

if (!$refreshTokenEventEmitted) {
self::fail('Refresh token issued event is not emitted.');
}
}

public function testRespondToAccessTokenRequestWithDefaultRedirectUri(): void
Expand Down
17 changes: 17 additions & 0 deletions tests/Grant/ClientCredentialsGrantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface;
use League\OAuth2\Server\Repositories\ClientRepositoryInterface;
use League\OAuth2\Server\Repositories\ScopeRepositoryInterface;
use League\OAuth2\Server\RequestAccessTokenEvent;
use League\OAuth2\Server\RequestEvent;
use LeagueTests\Stubs\AccessTokenEntity;
use LeagueTests\Stubs\ClientEntity;
use LeagueTests\Stubs\ScopeEntity;
Expand Down Expand Up @@ -53,6 +55,17 @@ public function testRespondToRequest(): void
$grant->setDefaultScope(self::DEFAULT_SCOPE);
$grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));

$accessTokenEventEmitted = false;

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::ACCESS_TOKEN_ISSUED,
function ($event) use (&$accessTokenEventEmitted): void {
self::assertInstanceOf(RequestAccessTokenEvent::class, $event);

$accessTokenEventEmitted = true;
}
);

$serverRequest = (new ServerRequest())->withParsedBody([
'client_id' => 'foo',
'client_secret' => 'bar',
Expand All @@ -64,5 +77,9 @@ public function testRespondToRequest(): void
$response = $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M'));

self::assertNotEmpty($response->getAccessToken()->getIdentifier());

if (!$accessTokenEventEmitted) {
self::fail('Access token issued event is not emitted.');
}
}
}
32 changes: 32 additions & 0 deletions tests/Grant/DeviceCodeGrantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface;
use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
use League\OAuth2\Server\Repositories\ScopeRepositoryInterface;
use League\OAuth2\Server\RequestAccessTokenEvent;
use League\OAuth2\Server\RequestEvent;
use League\OAuth2\Server\RequestRefreshTokenEvent;
use LeagueTests\Stubs\AccessTokenEntity;
use LeagueTests\Stubs\ClientEntity;
use LeagueTests\Stubs\DeviceCodeEntity;
Expand Down Expand Up @@ -380,6 +383,27 @@ public function testRespondToAccessTokenRequest(): void

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

$accessTokenEventEmitted = false;
$refreshTokenEventEmitted = false;

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::ACCESS_TOKEN_ISSUED,
function ($event) use (&$accessTokenEventEmitted): void {
self::assertInstanceOf(RequestAccessTokenEvent::class, $event);

$accessTokenEventEmitted = true;
}
);

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::REFRESH_TOKEN_ISSUED,
function ($event) use (&$refreshTokenEventEmitted): void {
self::assertInstanceOf(RequestRefreshTokenEvent::class, $event);

$refreshTokenEventEmitted = true;
}
);

$serverRequest = (new ServerRequest())->withParsedBody([
'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code',
'device_code' => $deviceCodeEntity->getIdentifier(),
Expand All @@ -391,6 +415,14 @@ public function testRespondToAccessTokenRequest(): void

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

if (!$accessTokenEventEmitted) {
self::fail('Access token issued event is not emitted.');
}

if (!$refreshTokenEventEmitted) {
self::fail('Refresh token issued event is not emitted.');
}
}

public function testRespondToRequestMissingClient(): void
Expand Down
17 changes: 17 additions & 0 deletions tests/Grant/ImplicitGrantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
use League\OAuth2\Server\Repositories\ClientRepositoryInterface;
use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
use League\OAuth2\Server\Repositories\ScopeRepositoryInterface;
use League\OAuth2\Server\RequestAccessTokenEvent;
use League\OAuth2\Server\RequestEvent;
use League\OAuth2\Server\RequestTypes\AuthorizationRequest;
use League\OAuth2\Server\ResponseTypes\RedirectResponse;
use LeagueTests\Stubs\AccessTokenEntity;
Expand Down Expand Up @@ -272,7 +274,22 @@ public function testCompleteAuthorizationRequest(): void
$grant->setAccessTokenRepository($accessTokenRepositoryMock);
$grant->setScopeRepository($scopeRepositoryMock);

$accessTokenEventEmitted = false;

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::ACCESS_TOKEN_ISSUED,
function ($event) use (&$accessTokenEventEmitted): void {
self::assertInstanceOf(RequestAccessTokenEvent::class, $event);

$accessTokenEventEmitted = true;
}
);

self::assertInstanceOf(RedirectResponse::class, $grant->completeAuthorizationRequest($authRequest));

if (!$accessTokenEventEmitted) {
// self::fail('Access token issued event is not emitted.'); // TODO: next major release
}
}

public function testCompleteAuthorizationRequestDenied(): void
Expand Down
32 changes: 32 additions & 0 deletions tests/Grant/PasswordGrantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
use League\OAuth2\Server\Repositories\ScopeRepositoryInterface;
use League\OAuth2\Server\Repositories\UserRepositoryInterface;
use League\OAuth2\Server\RequestAccessTokenEvent;
use League\OAuth2\Server\RequestEvent;
use League\OAuth2\Server\RequestRefreshTokenEvent;
use LeagueTests\Stubs\AccessTokenEntity;
use LeagueTests\Stubs\ClientEntity;
use LeagueTests\Stubs\RefreshTokenEntity;
Expand Down Expand Up @@ -69,6 +72,27 @@ public function testRespondToRequest(): void
$grant->setDefaultScope(self::DEFAULT_SCOPE);
$grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));

$accessTokenEventEmitted = false;
$refreshTokenEventEmitted = false;

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::ACCESS_TOKEN_ISSUED,
function ($event) use (&$accessTokenEventEmitted): void {
self::assertInstanceOf(RequestAccessTokenEvent::class, $event);

$accessTokenEventEmitted = true;
}
);

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::REFRESH_TOKEN_ISSUED,
function ($event) use (&$refreshTokenEventEmitted): void {
self::assertInstanceOf(RequestRefreshTokenEvent::class, $event);

$refreshTokenEventEmitted = true;
}
);

$serverRequest = (new ServerRequest())->withParsedBody([
'client_id' => 'foo',
'client_secret' => 'bar',
Expand All @@ -80,6 +104,14 @@ public function testRespondToRequest(): void
$grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M'));

self::assertInstanceOf(RefreshTokenEntityInterface::class, $responseType->getRefreshToken());

if (!$accessTokenEventEmitted) {
self::fail('Access token issued event is not emitted.');
}

if (!$refreshTokenEventEmitted) {
self::fail('Refresh token issued event is not emitted.');
}
}

public function testRespondToRequestNullRefreshToken(): void
Expand Down
32 changes: 32 additions & 0 deletions tests/Grant/RefreshTokenGrantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
use League\OAuth2\Server\Repositories\ClientRepositoryInterface;
use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
use League\OAuth2\Server\Repositories\ScopeRepositoryInterface;
use League\OAuth2\Server\RequestAccessTokenEvent;
use League\OAuth2\Server\RequestEvent;
use League\OAuth2\Server\RequestRefreshTokenEvent;
use League\OAuth2\Server\ResponseTypes\BearerTokenResponse;
use LeagueTests\Stubs\AccessTokenEntity;
use LeagueTests\Stubs\ClientEntity;
Expand Down Expand Up @@ -76,6 +79,27 @@ public function testRespondToRequest(): void
$grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));
$grant->revokeRefreshTokens(true);

$accessTokenEventEmitted = false;
$refreshTokenEventEmitted = false;

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::ACCESS_TOKEN_ISSUED,
function ($event) use (&$accessTokenEventEmitted): void {
self::assertInstanceOf(RequestAccessTokenEvent::class, $event);

$accessTokenEventEmitted = true;
}
);

$grant->getListenerRegistry()->subscribeTo(
RequestEvent::REFRESH_TOKEN_ISSUED,
function ($event) use (&$refreshTokenEventEmitted): void {
self::assertInstanceOf(RequestRefreshTokenEvent::class, $event);

$refreshTokenEventEmitted = true;
}
);

$oldRefreshToken = json_encode(
[
'client_id' => 'foo',
Expand Down Expand Up @@ -106,6 +130,14 @@ public function testRespondToRequest(): void
$grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M'));

self::assertInstanceOf(RefreshTokenEntityInterface::class, $responseType->getRefreshToken());

if (!$accessTokenEventEmitted) {
self::fail('Access token issued event is not emitted.');
}

if (!$refreshTokenEventEmitted) {
self::fail('Refresh token issued event is not emitted.');
}
}

public function testRespondToRequestNullRefreshToken(): void
Expand Down
Loading