From e81fe02c57e73a050400019baf450dab19baed22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Thu, 18 Jan 2024 18:52:35 +0100 Subject: [PATCH] Adding support for a nonce According to the OpenID connect spec: > nonce > String value used to associate a Client session with an ID Token, and to mitigate replay attacks. The value is passed through unmodified from the Authentication Request to the ID Token. If present in the ID Token, Clients MUST verify that the nonce Claim Value is equal to the value of the nonce parameter sent in the Authentication Request. If present in the Authentication Request, Authorization Servers MUST include a nonce Claim in the ID Token with the Claim Value being the nonce value sent in the Authentication Request. Authorization Servers SHOULD perform no other processing on nonce values used. The nonce value is a case-sensitive string. Right now, if a client passes a "nounce", we don't give it back and the client fails. This is happening to me right now with the client from Matrix Synapse. Here, I'm creating a new service (`CurrentRequestService`). With this new service, I can get the current PSR-7 request. I extend the AuthCodeGrant and inject this service into the extended class. With this, I can: - read the "nonce" from the request - encode the "nonce" in the "code" Then, in the `IdTokenResponse`, I read the "code" (if it is present), extract the "nounce" and inject it in the ID token as a new claim. The whole process is inspired by this comment: https://github.com/steverhoades/oauth2-openid-connect-server/issues/47#issuecomment-1228370632 With those changes, nounce is correctly handled and I've successfully tested a connection with the OpenID client from Matrix Synapse. --- README.md | 16 ++++ src/Grant/AuthCodeGrant.php | 91 +++++++++++++++++++ src/IdTokenResponse.php | 36 +++++++- .../CurrentRequestServiceInterface.php | 19 ++++ src/Laravel/LaravelCurrentRequestService.php | 22 +++++ src/Laravel/PassportServiceProvider.php | 25 ++++- src/Services/CurrentRequestService.php | 24 +++++ 7 files changed, 227 insertions(+), 6 deletions(-) create mode 100644 src/Grant/AuthCodeGrant.php create mode 100644 src/Interfaces/CurrentRequestServiceInterface.php create mode 100644 src/Laravel/LaravelCurrentRequestService.php create mode 100644 src/Services/CurrentRequestService.php diff --git a/README.md b/README.md index 9e480ea..f7d8838 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,9 @@ To enable OpenID Connect, follow these simple steps ```php $privateKeyPath = 'tmp/private.key'; +$currentRequestService = new CurrentRequestService(); +$currentRequestService->setRequest(ServerRequestFactory::fromGlobals()); + // create the response_type $responseType = new IdTokenResponse( new IdentityRepository(), @@ -44,6 +47,8 @@ $responseType = new IdTokenResponse( new Sha256(), InMemory::file($privateKeyPath), ), + $currentRequestService, + $encryptionKey, ); $server = new \League\OAuth2\Server\AuthorizationServer( @@ -62,6 +67,17 @@ Provide more scopes (e.g. `openid profile email`) to receive additional claims i For a complete implementation, visit [the OAuth2 Server example](https://github.com/ronvanderheijden/openid-connect/tree/main/example). +## Nonce support + +To prevent replay attacks, some clients can provide a "nonce" in the authorization request. If a client does so, the +server MUST include back a `nonce` claim in the `id_token`. + +To enable this feature, when registering an AuthCodeGrant, you need to use the `\OpenIDConnect\Grant\AuthCodeGrant` +instead of `\League\OAuth2\Server\Grant\AuthCodeGrant`. + +> ![NOTE] +> If you are using Laravel, the `AuthCodeGrant` is already registered for you by the service provider. + ## Laravel Passport You can use this package with Laravel Passport in 2 simple steps. diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php new file mode 100644 index 0000000..b8c029a --- /dev/null +++ b/src/Grant/AuthCodeGrant.php @@ -0,0 +1,91 @@ +psr7Response = $psr7Response; + $this->currentRequestService = $currentRequestService; + } + + /** + * {@inheritdoc} + */ + public function completeAuthorizationRequest(AuthorizationRequest $authorizationRequest) + { + // See https://github.com/steverhoades/oauth2-openid-connect-server/issues/47#issuecomment-1228370632 + + /** @var RedirectResponse $response */ + $response = parent::completeAuthorizationRequest($authorizationRequest); + + $queryParams = $this->currentRequestService->getRequest()->getQueryParams(); + + if (isset($queryParams['nonce'])) { + // The only way to get the redirect URI is to generate the PSR7 response (The RedirectResponse class does not have a getter for the redirect URI) + $httpResponse = $response->generateHttpResponse($this->psr7Response); + $redirectUri = $httpResponse->getHeader('Location')[0]; + $parsed = parse_url($redirectUri); + + parse_str($parsed['query'], $query); + + $authCodePayload = json_decode($this->decrypt($query['code']), true, 512, JSON_THROW_ON_ERROR); + + $authCodePayload['nonce'] = $queryParams['nonce']; + + $query['code'] = $this->encrypt(json_encode($authCodePayload, JSON_THROW_ON_ERROR)); + + $parsed['query'] = http_build_query($query); + + $response->setRedirectUri($this->unparse_url($parsed)); + } + + return $response; + } + + /** + * Inverse of parse_url + * + * @param mixed $parsed_url + * @return string + */ + private function unparse_url($parsed_url) + { + $scheme = isset($parsed_url['scheme']) ? $parsed_url['scheme'] . '://' : ''; + $host = isset($parsed_url['host']) ? $parsed_url['host'] : ''; + $port = isset($parsed_url['port']) ? ':' . $parsed_url['port'] : ''; + $user = isset($parsed_url['user']) ? $parsed_url['user'] : ''; + $pass = isset($parsed_url['pass']) ? ':' . $parsed_url['pass'] : ''; + $pass = ($user || $pass) ? "$pass@" : ''; + $path = isset($parsed_url['path']) ? $parsed_url['path'] : ''; + $query = isset($parsed_url['query']) ? '?' . $parsed_url['query'] : ''; + $fragment = isset($parsed_url['fragment']) ? '#' . $parsed_url['fragment'] : ''; + return "$scheme$user$pass$host$port$path$query$fragment"; + } +} diff --git a/src/IdTokenResponse.php b/src/IdTokenResponse.php index 4f7a15a..6904d7f 100644 --- a/src/IdTokenResponse.php +++ b/src/IdTokenResponse.php @@ -6,33 +6,43 @@ use DateInterval; use DateTimeImmutable; +use Defuse\Crypto\Key; use Lcobucci\JWT\Builder; use Lcobucci\JWT\Configuration; +use League\OAuth2\Server\CryptTrait; use League\OAuth2\Server\Entities\AccessTokenEntityInterface; use League\OAuth2\Server\Entities\ScopeEntityInterface; use League\OAuth2\Server\ResponseTypes\BearerTokenResponse; +use OpenIDConnect\Interfaces\CurrentRequestServiceInterface; use OpenIDConnect\Interfaces\IdentityEntityInterface; use OpenIDConnect\Interfaces\IdentityRepositoryInterface; class IdTokenResponse extends BearerTokenResponse { + use CryptTrait; + protected IdentityRepositoryInterface $identityRepository; protected ClaimExtractor $claimExtractor; private Configuration $config; - private ?string $issuer; + private ?CurrentRequestServiceInterface $currentRequestService; + /** + * @param string|Key|null $encryptionKey + */ public function __construct( IdentityRepositoryInterface $identityRepository, ClaimExtractor $claimExtractor, Configuration $config, - string $issuer = null, + CurrentRequestServiceInterface $currentRequestService = null, + $encryptionKey = null, ) { $this->identityRepository = $identityRepository; $this->claimExtractor = $claimExtractor; $this->config = $config; - $this->issuer = $issuer; + $this->currentRequestService = $currentRequestService; + $this->encryptionKey = $encryptionKey; } protected function getBuilder( @@ -41,10 +51,17 @@ protected function getBuilder( ): Builder { $dateTimeImmutableObject = new DateTimeImmutable(); + if ($this->currentRequestService) { + $uri = $this->currentRequestService->getRequest()->getUri(); + $issuer = $uri->getScheme() . '://' . $uri->getHost() . ($uri->getPort() ? ':' . $uri->getPort() : ''); + } else { + $issuer = 'https://' . $_SERVER['HTTP_HOST']; + } + return $this->config ->builder() ->permittedFor($accessToken->getClient()->getIdentifier()) - ->issuedBy($this->issuer ?? 'https://' . $_SERVER['HTTP_HOST']) + ->issuedBy($issuer) ->issuedAt($dateTimeImmutableObject) ->expiresAt($dateTimeImmutableObject->add(new DateInterval('PT1H'))) ->relatedTo($userEntity->getIdentifier()); @@ -71,6 +88,17 @@ protected function getExtraParams(AccessTokenEntityInterface $accessToken): arra $builder = $builder->withClaim($claimName, $claimValue); } + if ($this->currentRequestService) { + // If the request contains a code, we look into the code to find the nonce. + $body = $this->currentRequestService->getRequest()->getParsedBody(); + if (isset($body['code'])) { + $authCodePayload = json_decode($this->decrypt($body['code']), true, 512, JSON_THROW_ON_ERROR); + if (isset($authCodePayload['nonce'])) { + $builder = $builder->withClaim('nonce', $authCodePayload['nonce']); + } + } + } + $token = $builder->getToken( $this->config->signer(), $this->config->signingKey(), diff --git a/src/Interfaces/CurrentRequestServiceInterface.php b/src/Interfaces/CurrentRequestServiceInterface.php new file mode 100644 index 0000000..d57ae59 --- /dev/null +++ b/src/Interfaces/CurrentRequestServiceInterface.php @@ -0,0 +1,19 @@ +createRequest(request()); + } +} diff --git a/src/Laravel/PassportServiceProvider.php b/src/Laravel/PassportServiceProvider.php index 85258d6..4fbda80 100644 --- a/src/Laravel/PassportServiceProvider.php +++ b/src/Laravel/PassportServiceProvider.php @@ -11,8 +11,11 @@ use Lcobucci\JWT\Configuration; use Lcobucci\JWT\Signer\Key\InMemory; use League\OAuth2\Server\AuthorizationServer; +use League\OAuth2\Server\CryptTrait; +use Nyholm\Psr7\Response; use OpenIDConnect\ClaimExtractor; use OpenIDConnect\Claims\ClaimSet; +use OpenIDConnect\Grant\AuthCodeGrant; use OpenIDConnect\IdTokenResponse; class PassportServiceProvider extends Passport\PassportServiceProvider @@ -45,6 +48,7 @@ public function boot() public function makeAuthorizationServer(): AuthorizationServer { $cryptKey = $this->makeCryptKey('private'); + $encryptionKey = app(Encrypter::class)->getKey(); $responseType = new IdTokenResponse( app(config('openid.repositories.identity')), @@ -53,7 +57,8 @@ public function makeAuthorizationServer(): AuthorizationServer app(config('openid.signer')), InMemory::file($cryptKey->getKeyPath()), ), - app('request')->getSchemeAndHttpHost(), + app(LaravelCurrentRequestService::class), + $encryptionKey, ); return new AuthorizationServer( @@ -61,11 +66,27 @@ public function makeAuthorizationServer(): AuthorizationServer app(AccessTokenRepository::class), app(config('openid.repositories.scope')), $cryptKey, - app(Encrypter::class)->getKey(), + $encryptionKey, $responseType, ); } + /** + * Build the Auth Code grant instance. + * + * @return AuthCodeGrant + */ + protected function buildAuthCodeGrant() + { + return new AuthCodeGrant( + $this->app->make(Passport\Bridge\AuthCodeRepository::class), + $this->app->make(Passport\Bridge\RefreshTokenRepository::class), + new \DateInterval('PT10M'), + new Response(), + $this->app->make(LaravelCurrentRequestService::class), + ); + } + public function registerClaimExtractor() { $this->app->singleton(ClaimExtractor::class, function () { $customClaimSets = config('openid.custom_claim_sets'); diff --git a/src/Services/CurrentRequestService.php b/src/Services/CurrentRequestService.php new file mode 100644 index 0000000..827c8b8 --- /dev/null +++ b/src/Services/CurrentRequestService.php @@ -0,0 +1,24 @@ +request === null) { + throw new \RuntimeException('Request not set in CurrentRequestService'); + } + return $this->request; + } + + public function setRequest(ServerRequestInterface $request): void + { + $this->request = $request; + } +}