From f492885b4cfa6bb146d4359cb0048668e4201f1b Mon Sep 17 00:00:00 2001 From: Christiaan Goossens Date: Fri, 26 Apr 2024 15:38:06 +0200 Subject: [PATCH 01/11] Add more options to OpenID Discovery --- src/Laravel/DiscoveryController.php | 90 +++++++++++++++++++++++++---- 1 file changed, 79 insertions(+), 11 deletions(-) diff --git a/src/Laravel/DiscoveryController.php b/src/Laravel/DiscoveryController.php index cd17041..c208825 100644 --- a/src/Laravel/DiscoveryController.php +++ b/src/Laravel/DiscoveryController.php @@ -4,32 +4,28 @@ use Illuminate\Http\Request; use Illuminate\Support\Facades\Route; +use Laravel\Passport\Passport; class DiscoveryController { + /** + * Compatible with https://openid.net/specs/openid-connect-discovery-1_0.html, chapter 3 + */ public function __invoke(Request $request) { $response = [ 'issuer' => url('/'), 'authorization_endpoint' => route('passport.authorizations.authorize'), 'token_endpoint' => route('passport.token'), - 'response_types_supported' => [ - 'code', - 'token', - 'id_token', - 'code token', - 'code id_token', - 'token id_token', - 'code token id_token', - 'none', - ], + 'grant_types_supported' => $this->getSupportedGrantTypes(), + 'response_types_supported' => $this->getSupportedResponseTypes(), 'subject_types_supported' => [ 'public', ], 'id_token_signing_alg_values_supported' => [ 'RS256', ], - 'scopes_supported' => config('openid.passport.tokens_can'), + 'scopes_supported' => $this->getSupportedScopes(), 'token_endpoint_auth_methods_supported' => [ 'client_secret_basic', 'client_secret_post', @@ -46,4 +42,76 @@ public function __invoke(Request $request) return response()->json($response, 200, [], JSON_PRETTY_PRINT); } + + /** + * Returns JSON array containing a list of the OAuth 2.0 [RFC6749] scope values that this server supports. + * The server MUST support the openid scope value. Servers MAY choose not to advertise some supported scope values even when this parameter is used, + * although those defined in [OpenID.Core] SHOULD be listed, if supported. + */ + private function getSupportedScopes(): array { + $scopes = array_keys(config('openid.passport.tokens_can')); + + if (!config('openid.hide_scopes', false)) { + return $scopes; + } + + /** + * Otherwise, only return scopes from the OpenID Core Spec, section 5.4 + */ + return array_intersect($scopes, [ + 'openid', + 'profile', + 'email', + 'address', + 'phone' + ]); + } + + private function getSupportedGrantTypes(): array { + // See PassportServiceProvider for grant types that cannot be disabled + $grants = [ + 'authorization_code', // Cannot be disabled in Passport + 'client_credentials', // Cannot be disabled in Passport + 'refresh_token', // Cannot be disabled in Passport + ]; + + if (Passport::$implicitGrantEnabled) { + $grants[] = "implicit"; + } + + if (Passport::$passwordGrantEnabled) { + $grants[] = "password"; + } + + return $grants; + } + + /** + * Returns JSON array containing a list of the OAuth 2.0 response_type values that this OP supports. + * Dynamic OpenID Providers MUST support the code, id_token, and the id_token token Response Type values. + */ + private function getSupportedResponseTypes(): array { + /** + * These are always possible with Auth Code Grant + */ + $response_types = [ + 'code', + 'id_token', + 'code id_token', + ]; + + if (Passport::$implicitGrantEnabled) { + /** + * Return all variants, indicating both Auth Code & implicit are allowed + */ + return array_merge($response_types, [ + 'token', + 'code token', + 'token id_token', + 'code token id_token', + ]); + } + + return $response_types; + } } From 87e49ba5adba2a7ad27834caab8f6eb1a6535396 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens <9487666+christiaangoossens@users.noreply.github.com> Date: Fri, 26 Apr 2024 15:26:08 +0200 Subject: [PATCH 02/11] Add hide_scopes option --- src/Laravel/config/openid.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Laravel/config/openid.php b/src/Laravel/config/openid.php index 47b08b8..e379732 100644 --- a/src/Laravel/config/openid.php +++ b/src/Laravel/config/openid.php @@ -70,4 +70,10 @@ * By default, microseconds are included. */ 'use_microseconds' => true, + + /** + * Hide scopes that aren't from the OpenID Core spec from the Discovery, + * default = false (all scopes are listed) + */ + 'hide_scopes' => false, ]; From 5ae0e2149850e0102151fceb7830beba61c92285 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens <9487666+christiaangoossens@users.noreply.github.com> Date: Fri, 26 Apr 2024 15:35:12 +0200 Subject: [PATCH 03/11] Disable other grant_types as Passport doesn't implement them --- src/Laravel/DiscoveryController.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Laravel/DiscoveryController.php b/src/Laravel/DiscoveryController.php index c208825..e353c5c 100644 --- a/src/Laravel/DiscoveryController.php +++ b/src/Laravel/DiscoveryController.php @@ -96,8 +96,12 @@ private function getSupportedResponseTypes(): array { */ $response_types = [ 'code', - 'id_token', - 'code id_token', + /** + * Passport does not actually support `code id_token` or `id_token` and + * we return the ID token regardless on all requests. + * + * This doesn't form a problem however, even the OIDC spec doesn't do this correctly. + */ ]; if (Passport::$implicitGrantEnabled) { @@ -105,10 +109,10 @@ private function getSupportedResponseTypes(): array { * Return all variants, indicating both Auth Code & implicit are allowed */ return array_merge($response_types, [ - 'token', - 'code token', - 'token id_token', - 'code token id_token', + 'token' + /** + * Passport doesn't support `code token` either. + */ ]); } From 5f0c2823d90eb751f4900fe2c6e349655a626c09 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens <9487666+christiaangoossens@users.noreply.github.com> Date: Fri, 26 Apr 2024 15:42:32 +0200 Subject: [PATCH 04/11] Code styling --- src/Laravel/DiscoveryController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Laravel/DiscoveryController.php b/src/Laravel/DiscoveryController.php index e353c5c..ddbf919 100644 --- a/src/Laravel/DiscoveryController.php +++ b/src/Laravel/DiscoveryController.php @@ -63,7 +63,7 @@ private function getSupportedScopes(): array { 'profile', 'email', 'address', - 'phone' + 'phone', ]); } @@ -109,7 +109,7 @@ private function getSupportedResponseTypes(): array { * Return all variants, indicating both Auth Code & implicit are allowed */ return array_merge($response_types, [ - 'token' + 'token', /** * Passport doesn't support `code token` either. */ From 2c0decea929bd6055e7bdc7ee03a24fb0bedbb60 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens <9487666+christiaangoossens@users.noreply.github.com> Date: Fri, 26 Apr 2024 15:44:25 +0200 Subject: [PATCH 05/11] Fix too long line --- src/Laravel/DiscoveryController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Laravel/DiscoveryController.php b/src/Laravel/DiscoveryController.php index ddbf919..cfc12d0 100644 --- a/src/Laravel/DiscoveryController.php +++ b/src/Laravel/DiscoveryController.php @@ -45,7 +45,8 @@ public function __invoke(Request $request) /** * Returns JSON array containing a list of the OAuth 2.0 [RFC6749] scope values that this server supports. - * The server MUST support the openid scope value. Servers MAY choose not to advertise some supported scope values even when this parameter is used, + * The server MUST support the openid scope value. + * Servers MAY choose not to advertise some supported scope values even when this parameter is used, * although those defined in [OpenID.Core] SHOULD be listed, if supported. */ private function getSupportedScopes(): array { From b3f4a512ba94f9ac15dfc616b294e7d1f62e8f4e Mon Sep 17 00:00:00 2001 From: Christiaan Goossens Date: Fri, 26 Apr 2024 15:50:30 +0200 Subject: [PATCH 06/11] Move discovery settings and add to README --- README.md | 2 ++ src/Laravel/DiscoveryController.php | 2 +- src/Laravel/config/openid.php | 17 +++++++++++------ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 3ce19a6..2c1cb41 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,8 @@ Example: 'token_headers' => ['kid' => base64_encode('public-key-added-2023-01-01')] ``` +Additionally, you can configure the JWKS url and some settings for discovery in the config file. + ## Support You can fill an issue in the github section dedicated for that. I'll try to maintain this fork. diff --git a/src/Laravel/DiscoveryController.php b/src/Laravel/DiscoveryController.php index cfc12d0..32f7165 100644 --- a/src/Laravel/DiscoveryController.php +++ b/src/Laravel/DiscoveryController.php @@ -52,7 +52,7 @@ public function __invoke(Request $request) private function getSupportedScopes(): array { $scopes = array_keys(config('openid.passport.tokens_can')); - if (!config('openid.hide_scopes', false)) { + if (!config('openid.discovery.hide_scopes', false)) { return $scopes; } diff --git a/src/Laravel/config/openid.php b/src/Laravel/config/openid.php index e379732..1119f3d 100644 --- a/src/Laravel/config/openid.php +++ b/src/Laravel/config/openid.php @@ -56,6 +56,17 @@ 'jwks_url' => '/oauth/jwks', ], + /** + * Settings for the discovery endpoint + */ + 'discovery' => [ + /** + * Hide scopes that aren't from the OpenID Core spec from the Discovery, + * default = false (all scopes are listed) + */ + 'hide_scopes' => false, + ] + /** * The signer to be used */ @@ -70,10 +81,4 @@ * By default, microseconds are included. */ 'use_microseconds' => true, - - /** - * Hide scopes that aren't from the OpenID Core spec from the Discovery, - * default = false (all scopes are listed) - */ - 'hide_scopes' => false, ]; From 87bc3ea34c8581e0ba17e191ac3608dc22e11552 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens Date: Fri, 26 Apr 2024 15:51:07 +0200 Subject: [PATCH 07/11] Comma --- src/Laravel/config/openid.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Laravel/config/openid.php b/src/Laravel/config/openid.php index 1119f3d..56bb88b 100644 --- a/src/Laravel/config/openid.php +++ b/src/Laravel/config/openid.php @@ -65,7 +65,7 @@ * default = false (all scopes are listed) */ 'hide_scopes' => false, - ] + ], /** * The signer to be used From 7e26b2e4a174e41b67b56eec2e4e4c835e1c51eb Mon Sep 17 00:00:00 2001 From: Christiaan Goossens Date: Fri, 26 Apr 2024 16:09:06 +0200 Subject: [PATCH 08/11] Improve doc to better reflect https://openid.net/specs/openid-connect-core-1_0.html\#Authentication --- src/Laravel/DiscoveryController.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Laravel/DiscoveryController.php b/src/Laravel/DiscoveryController.php index 32f7165..ea213e1 100644 --- a/src/Laravel/DiscoveryController.php +++ b/src/Laravel/DiscoveryController.php @@ -97,12 +97,6 @@ private function getSupportedResponseTypes(): array { */ $response_types = [ 'code', - /** - * Passport does not actually support `code id_token` or `id_token` and - * we return the ID token regardless on all requests. - * - * This doesn't form a problem however, even the OIDC spec doesn't do this correctly. - */ ]; if (Passport::$implicitGrantEnabled) { @@ -112,8 +106,10 @@ private function getSupportedResponseTypes(): array { return array_merge($response_types, [ 'token', /** - * Passport doesn't support `code token` either. - */ + * TODO: Allow `id_token`, `id_token token`, `code id_token`, `code token`, `code id_token token` + * if we build the Implict Flow path. + * See https://github.com/jeremy379/laravel-openid-connect/issues/6 + */ ]); } From eccdce112244c2db7c67fffdd73afad5c4c8f944 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens Date: Fri, 26 Apr 2024 16:23:41 +0200 Subject: [PATCH 09/11] Also include scope by default --- src/IdTokenResponse.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/IdTokenResponse.php b/src/IdTokenResponse.php index 178be5c..319be5a 100644 --- a/src/IdTokenResponse.php +++ b/src/IdTokenResponse.php @@ -72,8 +72,16 @@ protected function getBuilder( } protected function getExtraParams(AccessTokenEntityInterface $accessToken): array { + /** + * Include the scope return value, which according to RFC 6749, section 5.1 (and 3.3) + * is also required if the scope doesn't match the requested scope, which it might, and is optional otherwise. + * + * The value of the scope parameter is expressed as a list of space-delimited, case-sensitive strings. + */ + $params = ['scope' => implode(' ', $accessToken->getScopes())]; + if (!$this->hasOpenIDScope(...$accessToken->getScopes())) { - return []; + return $params; } $user = $this->identityRepository->getByIdentifier( @@ -111,7 +119,7 @@ protected function getExtraParams(AccessTokenEntityInterface $accessToken): arra $this->config->signingKey(), ); - return ['id_token' => $token->toString()]; + return array_merge($params, ['id_token' => $token->toString()]); } private function hasOpenIDScope(ScopeEntityInterface ...$scopes): bool { From c91a3049402e3704473df1a362014d588be67b42 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens Date: Fri, 26 Apr 2024 16:28:01 +0200 Subject: [PATCH 10/11] Use map to map scopes to identifier --- src/IdTokenResponse.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/IdTokenResponse.php b/src/IdTokenResponse.php index 319be5a..4a28d09 100644 --- a/src/IdTokenResponse.php +++ b/src/IdTokenResponse.php @@ -78,9 +78,13 @@ protected function getExtraParams(AccessTokenEntityInterface $accessToken): arra * * The value of the scope parameter is expressed as a list of space-delimited, case-sensitive strings. */ - $params = ['scope' => implode(' ', $accessToken->getScopes())]; + $scopes = $accessToken->getScopes(); - if (!$this->hasOpenIDScope(...$accessToken->getScopes())) { + $params = ['scope' => implode(' ', array_map(function ($value) { + return $value->getIdentifier(); + }, $scopes))]; + + if (!$this->hasOpenIDScope(...$scopes)) { return $params; } @@ -106,7 +110,7 @@ protected function getExtraParams(AccessTokenEntityInterface $accessToken): arra } $claims = $this->claimExtractor->extract( - $accessToken->getScopes(), + $scopes, $user->getClaims(), ); From 68486b7a257484bcf9b357fcbc0cb3e2a30b8855 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens Date: Fri, 26 Apr 2024 16:29:43 +0200 Subject: [PATCH 11/11] Code style --- src/IdTokenResponse.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/IdTokenResponse.php b/src/IdTokenResponse.php index 4a28d09..be61c98 100644 --- a/src/IdTokenResponse.php +++ b/src/IdTokenResponse.php @@ -80,9 +80,11 @@ protected function getExtraParams(AccessTokenEntityInterface $accessToken): arra */ $scopes = $accessToken->getScopes(); - $params = ['scope' => implode(' ', array_map(function ($value) { - return $value->getIdentifier(); - }, $scopes))]; + $params = [ + 'scope' => implode(' ', array_map(function ($value) { + return $value->getIdentifier(); + }, $scopes)), + ]; if (!$this->hasOpenIDScope(...$scopes)) { return $params;