diff --git a/lib/SessionManager.php b/lib/SessionManager.php index 27912f11..a0dd1e24 100644 --- a/lib/SessionManager.php +++ b/lib/SessionManager.php @@ -132,6 +132,8 @@ public static function sealSessionFromAuthResponse( * @param string $cookiePassword The encryption key. * @param string $clientId The WorkOS client ID (for JWKS URL). * @param string $baseUrl The WorkOS API base URL. Defaults to 'https://api.workos.com/'. + * @param string|array|null $issuer Expected `iss` claim (one issuer or a list of + * accepted issuers). When null, the issuer is not validated. * @return array Authentication result. */ public function authenticate( @@ -139,6 +141,7 @@ public function authenticate( string $cookiePassword, string $clientId, string $baseUrl = 'https://api.workos.com/', + string|array|null $issuer = null, ): array { if (empty($sessionData)) { return [ @@ -164,7 +167,7 @@ public function authenticate( } try { - $decoded = $this->decodeAccessToken($session['access_token'], $clientId); + $decoded = $this->decodeAccessToken($session['access_token'], $clientId, $issuer); } catch (\Exception $e) { return [ 'authenticated' => false, @@ -263,6 +266,7 @@ public function refresh( * @param string $clientId The WorkOS client ID. * @param string|null $returnTo Optional URL to redirect to after logout. * @param string $baseUrl The WorkOS API base URL. + * @param string|array|null $issuer Expected `iss` claim; see {@see authenticate()}. * @return string The logout URL. * @throws \InvalidArgumentException If the session cannot be authenticated. */ @@ -272,8 +276,9 @@ public function getLogoutUrl( string $clientId, ?string $returnTo = null, string $baseUrl = 'https://api.workos.com/', + string|array|null $issuer = null, ): string { - $authResult = $this->authenticate($sessionData, $cookiePassword, $clientId, $baseUrl); + $authResult = $this->authenticate($sessionData, $cookiePassword, $clientId, $baseUrl, $issuer); if (!$authResult['authenticated']) { throw new \InvalidArgumentException( @@ -361,18 +366,21 @@ private function getCachedJwks(string $clientId, bool $forceRefresh = false): ar * Decode and validate an access token JWT. * * Verifies the JWS signature against the JWKS published for `$clientId`, - * enforces an algorithm allow-list, and requires a numeric, unexpired exp - * claim. This is the only path used by {@see authenticate()}; callers must - * not bypass it. + * enforces an algorithm allow-list, requires a numeric, unexpired exp + * claim, and — when `$issuer` is given — requires the `iss` claim to match + * one of the accepted issuers. This is the only path used by + * {@see authenticate()}; callers must not bypass it. * * @param string $accessToken The JWT access token. * @param string $clientId The WorkOS client ID (used to fetch JWKS). + * @param string|array|null $issuer Accepted `iss` value(s), or null to skip the check. * @return array The decoded JWT claims. * @throws \InvalidArgumentException If the token cannot be decoded or fails verification. */ private function decodeAccessToken( string $accessToken, string $clientId, + string|array|null $issuer = null, ): array { $parts = explode('.', $accessToken); if (count($parts) !== 3) { @@ -448,11 +456,20 @@ private function decodeAccessToken( throw new \InvalidArgumentException('JWT has expired'); } - // TODO(security-fix-plan.md, finding #60): enforce documented WorkOS - // `iss` and `aud` values once empirically confirmed. The other WorkOS - // SDKs (Ruby, Python) currently skip `aud` verification, so the - // canonical values are not authoritatively documented in this repo. - // Track resolution under "Open questions / follow-ups" in the plan. + if ($issuer !== null) { + $accepted = is_array($issuer) ? $issuer : [$issuer]; + $iss = $decoded['iss'] ?? null; + if (!is_string($iss) || !in_array($iss, $accepted, true)) { + throw new \InvalidArgumentException('JWT issuer mismatch'); + } + } + + // TODO(security-fix-plan.md, finding #60): enforce `iss` and `aud` by + // default once the canonical WorkOS values are empirically confirmed. + // The other WorkOS SDKs (Ruby, Python) currently skip `aud` verification + // and only check `iss` when configured, so the canonical values are not + // authoritatively documented in this repo. Track resolution under + // "Open questions / follow-ups" in the plan. return $decoded; } diff --git a/tests/SessionManagerTest.php b/tests/SessionManagerTest.php index 5be91ac7..2e73e184 100644 --- a/tests/SessionManagerTest.php +++ b/tests/SessionManagerTest.php @@ -264,6 +264,77 @@ public function testAuthenticateRequiresUnexpiredNumericExp(array $claims, bool } } + /** + * @param string|array|null $issuer + * @return array + */ + private function authenticateWithIssuer(string|array|null $issuer, ?string $iss): array + { + $claims = ['sid' => 'session_iss', 'exp' => time() + 3600]; + if ($iss !== null) { + $claims['iss'] = $iss; + } + [$jwks, $jwt] = $this->buildSignedJwt($claims); + + $sealed = SessionManager::sealSessionFromAuthResponse( + accessToken: $jwt, + refreshToken: 'ref_test', + cookiePassword: $this->cookiePassword, + ); + + $client = $this->createMockClient([['status' => 200, 'body' => $jwks]]); + + return $client->sessionManager()->authenticate( + sessionData: $sealed, + cookiePassword: $this->cookiePassword, + clientId: 'client_123', + issuer: $issuer, + ); + } + + public function testAuthenticateIgnoresIssuerWhenNotConfigured(): void + { + $result = $this->authenticateWithIssuer(null, 'https://other.example.com'); + $this->assertTrue($result['authenticated']); + } + + public function testAuthenticateAcceptsMatchingIssuer(): void + { + $result = $this->authenticateWithIssuer('https://api.workos.com', 'https://api.workos.com'); + $this->assertTrue($result['authenticated']); + $this->assertSame('session_iss', $result['session_id']); + } + + public function testAuthenticateRejectsMismatchedIssuer(): void + { + $result = $this->authenticateWithIssuer('https://api.workos.com', 'https://other.example.com'); + $this->assertFalse($result['authenticated']); + $this->assertSame('invalid_jwt', $result['reason']); + } + + public function testAuthenticateRejectsMissingIssWhenIssuerConfigured(): void + { + $result = $this->authenticateWithIssuer('https://api.workos.com', null); + $this->assertFalse($result['authenticated']); + $this->assertSame('invalid_jwt', $result['reason']); + } + + public function testAuthenticateAcceptsAnyListedIssuer(): void + { + $result = $this->authenticateWithIssuer( + ['https://api.workos.com', 'https://auth.example.com'], + 'https://auth.example.com', + ); + $this->assertTrue($result['authenticated']); + } + + public function testAuthenticateRejectsAllTokensWhenIssuerListIsEmpty(): void + { + $result = $this->authenticateWithIssuer([], 'https://api.workos.com'); + $this->assertFalse($result['authenticated']); + $this->assertSame('invalid_jwt', $result['reason']); + } + public function testAuthenticateRejectsTamperedSignature(): void { [$jwks, $jwt] = $this->buildSignedJwt([