diff --git a/app/Auth/Access/OpenIdConnect/JwtSigningKey.php b/app/Auth/Access/OpenIdConnect/JwtSigningKey.php index c835c04c3..1687069a9 100644 --- a/app/Auth/Access/OpenIdConnect/JwtSigningKey.php +++ b/app/Auth/Access/OpenIdConnect/JwtSigningKey.php @@ -26,6 +26,28 @@ class JwtSigningKey { if (is_array($jwkOrKeyPath)) { $this->loadFromJwkArray($jwkOrKeyPath); + } else if (is_string($jwkOrKeyPath) && strpos($jwkOrKeyPath, 'file://') === 0) { + $this->loadFromPath($jwkOrKeyPath); + } else { + throw new InvalidKeyException('Unexpected type of key value provided'); + } + } + + /** + * @throws InvalidKeyException + */ + protected function loadFromPath(string $path) + { + try { + $this->key = PublicKeyLoader::load( + file_get_contents($path) + )->withPadding(RSA::SIGNATURE_PKCS1); + } catch (\Exception $exception) { + throw new InvalidKeyException("Failed to load key from file path with error: {$exception->getMessage()}"); + } + + if (!($this->key instanceof RSA)) { + throw new InvalidKeyException("Key loaded from file path is not an RSA key as expected"); } } @@ -54,12 +76,10 @@ class JwtSigningKey try { /** @var RSA $key */ - $key = PublicKeyLoader::load([ + $this->key = PublicKeyLoader::load([ 'e' => new BigInteger(base64_decode($jwk['e']), 256), 'n' => new BigInteger(base64_decode($n), 256), ])->withPadding(RSA::SIGNATURE_PKCS1); - - $this->key = $key; } catch (\Exception $exception) { throw new InvalidKeyException("Failed to load key from JWK parameters with error: {$exception->getMessage()}"); } @@ -73,4 +93,12 @@ class JwtSigningKey return $this->key->verify($content, $signature); } + /** + * Convert the key to a PEM encoded key string. + */ + public function toPem(): string + { + return $this->key->toString('PKCS8'); + } + } \ No newline at end of file diff --git a/app/Auth/Access/OpenIdConnect/OpenIdConnectIdToken.php b/app/Auth/Access/OpenIdConnect/OpenIdConnectIdToken.php index 09527c3ed..b5cef1772 100644 --- a/app/Auth/Access/OpenIdConnect/OpenIdConnectIdToken.php +++ b/app/Auth/Access/OpenIdConnect/OpenIdConnectIdToken.php @@ -76,11 +76,29 @@ class OpenIdConnectIdToken * Validate all possible parts of the id token. * @throws InvalidTokenException */ - public function validate() + public function validate(string $clientId) { $this->validateTokenStructure(); $this->validateTokenSignature(); - $this->validateTokenClaims(); + $this->validateTokenClaims($clientId); + } + + /** + * Fetch a specific claim from this token. + * Returns null if it is null or does not exist. + * @return mixed|null + */ + public function getClaim(string $claim) + { + return $this->payload[$claim] ?? null; + } + + /** + * Get all returned claims within the token. + */ + public function claims(): array + { + return $this->payload; } /** @@ -122,22 +140,92 @@ class OpenIdConnectIdToken $parsedKeys = array_filter($parsedKeys); $contentToSign = $this->tokenParts[0] . '.' . $this->tokenParts[1]; + /** @var JwtSigningKey $parsedKey */ foreach ($parsedKeys as $parsedKey) { if ($parsedKey->verify($contentToSign, $this->signature)) { return; } } - throw new InvalidTokenException('Token signature could not be validated using the provided keys.'); + throw new InvalidTokenException('Token signature could not be validated using the provided keys'); } /** * Validate the claims of the token. * As per https://openid.net/specs/openid-connect-basic-1_0.html#IDTokenValidation + * @throws InvalidTokenException */ - protected function validateTokenClaims(): void + protected function validateTokenClaims(string $clientId): void { - // TODO + // 1. The Issuer Identifier for the OpenID Provider (which is typically obtained during Discovery) + // MUST exactly match the value of the iss (issuer) Claim. + if (empty($this->payload['iss']) || $this->issuer !== $this->payload['iss']) { + throw new InvalidTokenException('Missing or non-matching token issuer value'); + } + + // 2. The Client MUST validate that the aud (audience) Claim contains its client_id value registered + // at the Issuer identified by the iss (issuer) Claim as an audience. The ID Token MUST be rejected + // if the ID Token does not list the Client as a valid audience, or if it contains additional + // audiences not trusted by the Client. + if (empty($this->payload['aud'])) { + throw new InvalidTokenException('Missing token audience value'); + } + + $aud = is_string($this->payload['aud']) ? [$this->payload['aud']] : $this->payload['aud']; + if (count($aud) !== 1) { + throw new InvalidTokenException('Token audience value has ' . count($aud) . ' values. Expected 1.'); + } + + if ($aud[0] !== $clientId) { + throw new InvalidTokenException('Token audience value did not match the expected client_id'); + } + + // 3. If the ID Token contains multiple audiences, the Client SHOULD verify that an azp Claim is present. + // NOTE: Addressed by enforcing a count of 1 above. + + // 4. If an azp (authorized party) Claim is present, the Client SHOULD verify that its client_id + // is the Claim Value. + if (isset($this->payload['azp']) && $this->payload['azp'] !== $clientId) { + throw new InvalidTokenException('Token authorized party exists but does not match the expected client_id'); + } + + // 5. The current time MUST be before the time represented by the exp Claim + // (possibly allowing for some small leeway to account for clock skew). + if (empty($this->payload['exp'])) { + throw new InvalidTokenException('Missing token expiration time value'); + } + + $skewSeconds = 120; + $now = time(); + if ($now >= (intval($this->payload['exp']) + $skewSeconds)) { + throw new InvalidTokenException('Token has expired'); + } + + // 6. The iat Claim can be used to reject tokens that were issued too far away from the current time, + // limiting the amount of time that nonces need to be stored to prevent attacks. + // The acceptable range is Client specific. + if (empty($this->payload['iat'])) { + throw new InvalidTokenException('Missing token issued at time value'); + } + + $dayAgo = time() - 86400; + $iat = intval($this->payload['iat']); + if ($iat > ($now + $skewSeconds) || $iat < $dayAgo) { + throw new InvalidTokenException('Token issue at time is not recent or is invalid'); + } + + // 7. If the acr Claim was requested, the Client SHOULD check that the asserted Claim Value is appropriate. + // The meaning and processing of acr Claim Values is out of scope for this document. + // NOTE: Not used for our case here. acr is not requested. + + // 8. When a max_age request is made, the Client SHOULD check the auth_time Claim value and request + // re-authentication if it determines too much time has elapsed since the last End-User authentication. + // NOTE: Not used for our case here. A max_age request is not made. + + // Custom: Ensure the "sub" (Subject) Claim exists and has a value. + if (empty($this->payload['sub'])) { + throw new InvalidTokenException('Missing token subject value'); + } } } \ No newline at end of file diff --git a/app/Auth/Access/OpenIdConnect/OpenIdConnectService.php b/app/Auth/Access/OpenIdConnect/OpenIdConnectService.php index 40369dee7..0f9fed006 100644 --- a/app/Auth/Access/OpenIdConnect/OpenIdConnectService.php +++ b/app/Auth/Access/OpenIdConnect/OpenIdConnectService.php @@ -1,7 +1,6 @@ config['display_name_claims']; $displayName = []; foreach ($displayNameAttr as $dnAttr) { - $dnComponent = $token->claims()->get($dnAttr, ''); + $dnComponent = $token->getClaim($dnAttr) ?? ''; if ($dnComponent !== '') { $displayName[] = $dnComponent; } @@ -112,12 +108,12 @@ class OpenIdConnectService * Extract the details of a user from an ID token. * @return array{name: string, email: string, external_id: string} */ - protected function getUserDetails(Token $token): array + protected function getUserDetails(OpenIdConnectIdToken $token): array { - $id = $token->claims()->get('sub'); + $id = $token->getClaim('sub'); return [ 'external_id' => $id, - 'email' => $token->claims()->get('email'), + 'email' => $token->getClaim('email'), 'name' => $this->getUserDisplayName($token, $id), ]; } @@ -139,20 +135,19 @@ class OpenIdConnectService [$this->config['jwt_public_key']] ); - // TODO - Create a class to manage token parsing and validation on this - // Ensure ID token validation is done: - // https://openid.net/specs/openid-connect-basic-1_0.html#IDTokenValidation - // To full affect and tested - // JWT signature algorthims: - // https://datatracker.ietf.org/doc/html/rfc7518#section-3 - - $userDetails = $this->getUserDetails($accessToken->getIdToken()); - $isLoggedIn = auth()->check(); - if ($this->config['dump_user_details']) { - throw new JsonDebugException($accessToken->jsonSerialize()); + throw new JsonDebugException($idToken->claims()); } + try { + $idToken->validate($this->config['client_id']); + } catch (InvalidTokenException $exception) { + throw new OpenIdConnectException("ID token validate failed with error: {$exception->getMessage()}"); + } + + $userDetails = $this->getUserDetails($idToken); + $isLoggedIn = auth()->check(); + if ($userDetails['email'] === null) { throw new OpenIdConnectException(trans('errors.oidc_no_email_address')); } diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 7c8eb2c86..d12d7c9bc 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -43,7 +43,8 @@ class LoginController extends Controller public function __construct(SocialAuthService $socialAuthService, LoginService $loginService) { $this->middleware('guest', ['only' => ['getLogin', 'login']]); - $this->middleware('guard:standard,ldap', ['only' => ['login', 'logout']]); + $this->middleware('guard:standard,ldap', ['only' => ['login']]); + $this->middleware('guard:standard,ldap,oidc', ['only' => ['logout']]); $this->socialAuthService = $socialAuthService; $this->loginService = $loginService; diff --git a/composer.json b/composer.json index e53d9d25a..066e67c4f 100644 --- a/composer.json +++ b/composer.json @@ -36,8 +36,7 @@ "socialiteproviders/okta": "^4.1", "socialiteproviders/slack": "^4.1", "socialiteproviders/twitch": "^5.3", - "ssddanbrown/htmldiff": "^v1.0.1", - "steverhoades/oauth2-openid-connect-client": "^0.3.0" + "ssddanbrown/htmldiff": "^v1.0.1" }, "require-dev": { "barryvdh/laravel-debugbar": "^3.5.1", diff --git a/composer.lock b/composer.lock index 62b2aa621..d64d8d640 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "5cbbf417bd19cd2164f91b9b2d38600c", + "content-hash": "ef6a8bb7bc6e99c70eeabc7695fc56eb", "packages": [ { "name": "aws/aws-crt-php", @@ -2069,83 +2069,6 @@ }, "time": "2021-08-31T15:16:26+00:00" }, - { - "name": "lcobucci/jwt", - "version": "3.4.6", - "source": { - "type": "git", - "url": "https://github.com/lcobucci/jwt.git", - "reference": "3ef8657a78278dfeae7707d51747251db4176240" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/lcobucci/jwt/zipball/3ef8657a78278dfeae7707d51747251db4176240", - "reference": "3ef8657a78278dfeae7707d51747251db4176240", - "shasum": "" - }, - "require": { - "ext-mbstring": "*", - "ext-openssl": "*", - "php": "^5.6 || ^7.0" - }, - "require-dev": { - "mikey179/vfsstream": "~1.5", - "phpmd/phpmd": "~2.2", - "phpunit/php-invoker": "~1.1", - "phpunit/phpunit": "^5.7 || ^7.3", - "squizlabs/php_codesniffer": "~2.3" - }, - "suggest": { - "lcobucci/clock": "*" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "3.1-dev" - } - }, - "autoload": { - "psr-4": { - "Lcobucci\\JWT\\": "src" - }, - "files": [ - "compat/class-aliases.php", - "compat/json-exception-polyfill.php", - "compat/lcobucci-clock-polyfill.php" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "BSD-3-Clause" - ], - "authors": [ - { - "name": "Luís Otávio Cobucci Oblonczyk", - "email": "lcobucci@gmail.com", - "role": "Developer" - } - ], - "description": "A simple library to work with JSON Web Token and JSON Web Signature", - "keywords": [ - "JWS", - "jwt" - ], - "support": { - "issues": "https://github.com/lcobucci/jwt/issues", - "source": "https://github.com/lcobucci/jwt/tree/3.4.6" - }, - "funding": [ - { - "url": "https://github.com/lcobucci", - "type": "github" - }, - { - "url": "https://www.patreon.com/lcobucci", - "type": "patreon" - } - ], - "time": "2021-09-28T19:18:28+00:00" - }, { "name": "league/commonmark", "version": "1.6.6", @@ -2613,76 +2536,6 @@ }, "time": "2021-08-15T23:05:49+00:00" }, - { - "name": "league/oauth2-client", - "version": "2.6.0", - "source": { - "type": "git", - "url": "https://github.com/thephpleague/oauth2-client.git", - "reference": "badb01e62383430706433191b82506b6df24ad98" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/thephpleague/oauth2-client/zipball/badb01e62383430706433191b82506b6df24ad98", - "reference": "badb01e62383430706433191b82506b6df24ad98", - "shasum": "" - }, - "require": { - "guzzlehttp/guzzle": "^6.0 || ^7.0", - "paragonie/random_compat": "^1 || ^2 || ^9.99", - "php": "^5.6 || ^7.0 || ^8.0" - }, - "require-dev": { - "mockery/mockery": "^1.3", - "php-parallel-lint/php-parallel-lint": "^1.2", - "phpunit/phpunit": "^5.7 || ^6.0 || ^9.3", - "squizlabs/php_codesniffer": "^2.3 || ^3.0" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-2.x": "2.0.x-dev" - } - }, - "autoload": { - "psr-4": { - "League\\OAuth2\\Client\\": "src/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Alex Bilbie", - "email": "hello@alexbilbie.com", - "homepage": "http://www.alexbilbie.com", - "role": "Developer" - }, - { - "name": "Woody Gilk", - "homepage": "https://github.com/shadowhand", - "role": "Contributor" - } - ], - "description": "OAuth 2.0 Client Library", - "keywords": [ - "Authentication", - "SSO", - "authorization", - "identity", - "idp", - "oauth", - "oauth2", - "single sign on" - ], - "support": { - "issues": "https://github.com/thephpleague/oauth2-client/issues", - "source": "https://github.com/thephpleague/oauth2-client/tree/2.6.0" - }, - "time": "2020-10-28T02:03:40+00:00" - }, { "name": "monolog/monolog", "version": "2.3.5", @@ -4675,53 +4528,6 @@ ], "time": "2021-01-24T18:51:30+00:00" }, - { - "name": "steverhoades/oauth2-openid-connect-client", - "version": "v0.3.0", - "source": { - "type": "git", - "url": "https://github.com/steverhoades/oauth2-openid-connect-client.git", - "reference": "0159471487540a4620b8d0b693f5f215503a8d75" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/steverhoades/oauth2-openid-connect-client/zipball/0159471487540a4620b8d0b693f5f215503a8d75", - "reference": "0159471487540a4620b8d0b693f5f215503a8d75", - "shasum": "" - }, - "require": { - "lcobucci/jwt": "^3.2", - "league/oauth2-client": "^2.0" - }, - "require-dev": { - "phpmd/phpmd": "~2.2", - "phpunit/php-invoker": "~1.1", - "phpunit/phpunit": "~4.5", - "squizlabs/php_codesniffer": "~2.3" - }, - "type": "library", - "autoload": { - "psr-4": { - "OpenIDConnectClient\\": "src/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Steve Rhoades", - "email": "sedonami@gmail.com" - } - ], - "description": "OAuth2 OpenID Connect Client that utilizes the PHP Leagues OAuth2 Client", - "support": { - "issues": "https://github.com/steverhoades/oauth2-openid-connect-client/issues", - "source": "https://github.com/steverhoades/oauth2-openid-connect-client/tree/master" - }, - "time": "2020-05-19T23:06:36+00:00" - }, { "name": "swiftmailer/swiftmailer", "version": "v6.2.7",