[Security] Validate aud and iss claims on OidcTokenHandler

symfony · May 30, 2023 · 7f35dd7 · 7f35dd7
1 parent 4419c6d
commit 7f35dd7
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 20 deletions.
diff --git a/AccessToken/Oidc/OidcTokenHandler.php b/AccessToken/Oidc/OidcTokenHandler.php
@@ -41,10 +41,11 @@ final class OidcTokenHandler implements AccessTokenHandlerInterface
     public function __construct(
         private Algorithm $signatureAlgorithm,
         private JWK $jwk,
-        private ?LoggerInterface $logger = null,
-        private ClockInterface $clock = new Clock(),
+        private string $audience,
+        private array $issuers,
         private string $claim = 'sub',
-        private ?string $audience = null
+        private ?LoggerInterface $logger = null,
+        private ClockInterface $clock = new Clock()
     ) {
     }
 
@@ -80,10 +81,9 @@ public function getUserBadgeFrom(string $accessToken): UserBadge
                 new Checker\IssuedAtChecker(0, false, $this->clock),
                 new Checker\NotBeforeChecker(0, false, $this->clock),
                 new Checker\ExpirationTimeChecker(0, false, $this->clock),
+                new Checker\AudienceChecker($this->audience),
+                new Checker\IssuerChecker($this->issuers),
             ];
-            if ($this->audience) {
-                $checkers[] = new Checker\AudienceChecker($this->audience);
-            }
             $claimCheckerManager = new ClaimCheckerManager($checkers);
             // if this check fails, an InvalidClaimException is thrown
             $claimCheckerManager->check($claims);

diff --git a/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php b/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php
@@ -18,7 +18,6 @@
 use Jose\Component\Signature\Serializer\CompactSerializer;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
-use Symfony\Component\Clock\Clock;
 use Symfony\Component\Security\Core\Exception\BadCredentialsException;
 use Symfony\Component\Security\Core\User\OidcUser;
 use Symfony\Component\Security\Http\AccessToken\Oidc\OidcTokenHandler;
@@ -41,7 +40,7 @@ public function testGetsUserIdentifierFromSignedToken(string $claim, string $exp
             'iat' => $time,
             'nbf' => $time,
             'exp' => $time + 3600,
-            'iss' => 'https://www.example.com/',
+            'iss' => 'https://www.example.com',
             'aud' => self::AUDIENCE,
             'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
             'email' => 'foo@example.com',
@@ -55,10 +54,10 @@ public function testGetsUserIdentifierFromSignedToken(string $claim, string $exp
         $userBadge = (new OidcTokenHandler(
             new ES256(),
             $this->getJWK(),
-            $loggerMock,
-            new Clock(),
+            self::AUDIENCE,
+            ['https://www.example.com'],
             $claim,
-            self::AUDIENCE
+            $loggerMock,
         ))->getUserBadgeFrom($token);
         $actualUser = $userBadge->getUserLoader()();
 
@@ -89,10 +88,10 @@ public function testThrowsAnErrorIfTokenIsInvalid(string $token)
         (new OidcTokenHandler(
             new ES256(),
             $this->getJWK(),
-            $loggerMock,
-            new Clock(),
+            self::AUDIENCE,
+            ['https://www.example.com'],
             'sub',
-            self::AUDIENCE
+            $loggerMock,
         ))->getUserBadgeFrom($token);
     }
 
@@ -106,7 +105,7 @@ public static function getInvalidTokens(): iterable
                 'iat' => time() - 3600,
                 'nbf' => time() - 3600,
                 'exp' => time() - 3590,
-                'iss' => 'https://www.example.com/',
+                'iss' => 'https://www.example.com',
                 'aud' => self::AUDIENCE,
                 'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
                 'email' => 'foo@example.com',
@@ -118,7 +117,7 @@ public static function getInvalidTokens(): iterable
                 'iat' => time(),
                 'nbf' => time(),
                 'exp' => time() + 3590,
-                'iss' => 'https://www.example.com/',
+                'iss' => 'https://www.example.com',
                 'aud' => 'invalid',
                 'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
                 'email' => 'foo@example.com',
@@ -139,7 +138,7 @@ public function testThrowsAnErrorIfUserPropertyIsMissing()
             'iat' => $time,
             'nbf' => $time,
             'exp' => $time + 3600,
-            'iss' => 'https://www.example.com/',
+            'iss' => 'https://www.example.com',
             'aud' => self::AUDIENCE,
             'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
         ];
@@ -148,10 +147,10 @@ public function testThrowsAnErrorIfUserPropertyIsMissing()
         (new OidcTokenHandler(
             new ES256(),
             self::getJWK(),
-            $loggerMock,
-            new Clock(),
+            self::AUDIENCE,
+            ['https://www.example.com'],
             'email',
-            self::AUDIENCE
+            $loggerMock,
         ))->getUserBadgeFrom($token);
     }