Fix explicit redirect uri not being optional in authorization code grant

addresses #1418 Also fixed event emitter trait not storing automatically created instance (addresses #1422)
thephpleague · Jul 22, 2024 · f328e5d · f328e5d
1 parent 2ed9e5f
commit f328e5d
Show file tree

Hide file tree

Showing 4 changed files with 182 additions and 4 deletions.
diff --git a/src/EventEmitting/EmitterAwarePolyfill.php b/src/EventEmitting/EmitterAwarePolyfill.php
@@ -13,7 +13,7 @@ trait EmitterAwarePolyfill
 
     public function getEmitter(): EventEmitter
     {
-        return $this->emitter ?? new EventEmitter();
+        return $this->emitter ??= new EventEmitter();
     }
 
     public function setEmitter(EventEmitter $emitter): self

diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php
@@ -217,13 +217,15 @@ private function validateAuthorizationCode(
             throw OAuthServerException::invalidRequest('code', 'Authorization code was not issued to this client');
         }
 
-        // The redirect URI is required in this request
+        // The redirect URI is required in this request if it was specified
+        // in the authorization request
         $redirectUri = $this->getRequestParameter('redirect_uri', $request);
-        if ($authCodePayload->redirect_uri !== '' && $redirectUri === null) {
+        if ($authCodePayload->redirect_uri !== null && $redirectUri === null) {
             throw OAuthServerException::invalidRequest('redirect_uri');
         }
 
-        if ($authCodePayload->redirect_uri !== $redirectUri) {
+        // If a redirect URI has been provided ensure it matches the stored redirect URI
+        if ($redirectUri !== null && $authCodePayload->redirect_uri !== $redirectUri) {
             throw OAuthServerException::invalidRequest('redirect_uri', 'Invalid redirect URI');
         }
     }

diff --git a/tests/EventEmitting/EmitterAwarePolyfillTest.php b/tests/EventEmitting/EmitterAwarePolyfillTest.php
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace LeagueTests\EventEmitting;
+
+use League\OAuth2\Server\EventEmitting\EmitterAwarePolyfill;
+use League\OAuth2\Server\EventEmitting\EventEmitter;
+use PHPUnit\Framework\TestCase;
+
+class EmitterAwarePolyfillTest extends TestCase
+{
+    public function testGetEmitter(): void
+    {
+        $emitterAwarePolyfill = new class {
+            use EmitterAwarePolyfill;
+        };
+
+        // automatically generated
+        $emitter = $emitterAwarePolyfill->getEmitter();
+        self::assertSame(
+            $emitter,
+            $emitterAwarePolyfill->getEmitter(),
+            'The emitter should be the same instance'
+        );
+        self::assertSame(
+            $emitter,
+            $emitterAwarePolyfill->getEventDispatcher(),
+            'The event dispatcher should be the same instance'
+        );
+        self::assertSame(
+            $emitter,
+            $emitterAwarePolyfill->getListenerRegistry(),
+            'The listener registry should be the same instance'
+        );
+
+        // manually set
+        $emitter = new EventEmitter();
+        $emitterAwarePolyfill->setEmitter($emitter);
+        self::assertSame(
+            $emitter,
+            $emitterAwarePolyfill->getEmitter(),
+            'The emitter should be the same instance'
+        );
+        self::assertSame(
+            $emitter,
+            $emitterAwarePolyfill->getEventDispatcher(),
+            'The event dispatcher should be the same instance'
+        );
+        self::assertSame(
+            $emitter,
+            $emitterAwarePolyfill->getListenerRegistry(),
+            'The listener registry should be the same instance'
+        );
+    }
+}
diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php
@@ -616,6 +616,75 @@ public function testRespondToAccessTokenRequest(): void
         self::assertInstanceOf(RefreshTokenEntityInterface::class, $response->getRefreshToken());
     }
 
+    public function testRespondToAccessTokenRequestWithDefaultRedirectUri(): void
+    {
+        $client = new ClientEntity();
+
+        $client->setIdentifier('foo');
+        $client->setRedirectUri(self::REDIRECT_URI);
+        $client->setConfidential();
+
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+        $clientRepositoryMock->method('validateClient')->willReturn(true);
+
+        $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
+        $scopeEntity = new ScopeEntity();
+        $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity);
+        $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
+
+        $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());
+
+        $grant = new AuthCodeGrant(
+            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new DateInterval('PT10M')
+        );
+        $grant->setClientRepository($clientRepositoryMock);
+        $grant->setScopeRepository($scopeRepositoryMock);
+        $grant->setAccessTokenRepository($accessTokenRepositoryMock);
+        $grant->setRefreshTokenRepository($refreshTokenRepositoryMock);
+        $grant->setEncryptionKey($this->cryptStub->getKey());
+        $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));
+
+        $request = new ServerRequest(
+            [],
+            [],
+            null,
+            'POST',
+            'php://input',
+            [],
+            [],
+            [],
+            [
+                'grant_type'   => 'authorization_code',
+                'client_id'    => 'foo',
+                'code'         => $this->cryptStub->doEncrypt(
+                    json_encode([
+                        'auth_code_id' => uniqid(),
+                        'expire_time'  => time() + 3600,
+                        'client_id'    => 'foo',
+                        'user_id'      => '123',
+                        'scopes'       => ['foo'],
+                        'redirect_uri' => null,
+                    ], JSON_THROW_ON_ERROR)
+                ),
+            ]
+        );
+
+        /** @var StubResponseType $response */
+        $response = $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M'));
+
+        self::assertInstanceOf(RefreshTokenEntityInterface::class, $response->getRefreshToken());
+    }
+
     public function testRespondToAccessTokenRequestUsingHttpBasicAuth(): void
     {
         $client = new ClientEntity();
@@ -1131,6 +1200,57 @@ public function testRespondToAccessTokenRequestRedirectUriMismatch(): void
         $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M'));
     }
 
+    public function testRespondToAccessTokenRequestUnecessaryRedirectUri(): void
+    {
+        $client = new ClientEntity();
+
+        $client->setIdentifier('foo');
+        $client->setConfidential();
+        $client->setRedirectUri('http://bar/foo');
+
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+        $clientRepositoryMock->method('validateClient')->willReturn(true);
+
+        $grant = new AuthCodeGrant(
+            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new DateInterval('PT10M')
+        );
+        $grant->setClientRepository($clientRepositoryMock);
+        $grant->setEncryptionKey($this->cryptStub->getKey());
+
+        $request = new ServerRequest(
+            [],
+            [],
+            null,
+            'POST',
+            'php://input',
+            [],
+            [],
+            [],
+            [
+                'client_id'  => 'foo',
+                'grant_type' => 'authorization_code',
+                'redirect_uri' => 'http://bar/foo',
+                'code'       => $this->cryptStub->doEncrypt(
+                    json_encode([
+                        'auth_code_id'          => uniqid(),
+                        'expire_time'           => time() + 3600,
+                        'client_id'             => 'foo',
+                        'redirect_uri'          => null,
+                    ], JSON_THROW_ON_ERROR)
+                ),
+            ]
+        );
+
+        $this->expectException(OAuthServerException::class);
+        $this->expectExceptionCode(3);
+
+        $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M'));
+    }
+
     public function testRespondToAccessTokenRequestMissingCode(): void
     {
         $client = new ClientEntity();