diff --git a/README.md b/README.md index 6cdcca34..f787c1fc 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,16 @@ it is possible to disable the classic "self-encoded" validation: ], ``` +### Disable audience check in bearer token validation + +The `audience` and `azp` token claims will be checked when validating a bearer token for authenticated API requests. +You can disable this check with this config value: +``` php +'user_oidc' => [ + 'selfencoded_bearer_validation_audience_check' => false, +], +``` + ## Building the app Requirements for building: diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index d817bb7e..c11550a5 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -455,26 +455,23 @@ public function code(string $state = '', string $code = '', string $scope = '', } // Verify audience - if (!($idTokenPayload->aud === $provider->getClientId() || in_array($provider->getClientId(), $idTokenPayload->aud, true))) { + $tokenAudience = $idTokenPayload->aud; + $providerClientId = $provider->getClientId(); + if ( + (is_string($tokenAudience) && $tokenAudience !== $providerClientId) + || (is_array($tokenAudience) && !in_array($providerClientId, $tokenAudience, true)) + ) { $this->logger->debug('This token is not for us'); $message = $this->l10n->t('The audience does not match ours'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_audience' => $idTokenPayload->aud]); } - // If the ID Token contains multiple audiences, the Client SHOULD verify that an azp Claim is present. - // If an azp (authorized party) Claim is present, the Client SHOULD verify that its client_id is the Claim Value. - if (is_array($idTokenPayload->aud) && count($idTokenPayload->aud) > 1) { - if (isset($idTokenPayload->azp)) { - if ($idTokenPayload->azp !== $provider->getClientId()) { - $this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID'); - $message = $this->l10n->t('The authorized party does not match ours'); - return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_azp' => $idTokenPayload->azp]); - } - } else { - $this->logger->debug('Multiple audiences but no authorized party (azp) in the id token'); - $message = $this->l10n->t('No authorized party'); - return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['missing_azp']); - } + // ref https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation + // If the azp claim is present, it should be the client ID + if (isset($idTokenPayload->azp) && $idTokenPayload->azp !== $provider->getClientId()) { + $this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID'); + $message = $this->l10n->t('The authorized party does not match ours'); + return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_azp' => $idTokenPayload->azp]); } if (isset($idTokenPayload->nonce) && $idTokenPayload->nonce !== $this->session->get(self::NONCE)) { diff --git a/lib/User/Validator/SelfEncodedValidator.php b/lib/User/Validator/SelfEncodedValidator.php index 0e462fe9..6e479745 100644 --- a/lib/User/Validator/SelfEncodedValidator.php +++ b/lib/User/Validator/SelfEncodedValidator.php @@ -31,6 +31,7 @@ use OCA\UserOIDC\User\Provisioning\SelfEncodedTokenProvisioning; use OCA\UserOIDC\Vendor\Firebase\JWT\JWT; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IConfig; use Psr\Log\LoggerInterface; use Throwable; @@ -42,11 +43,19 @@ class SelfEncodedValidator implements IBearerTokenValidator { private $logger; /** @var ITimeFactory */ private $timeFactory; + /** @var IConfig */ + private $config; - public function __construct(DiscoveryService $discoveryService, LoggerInterface $logger, ITimeFactory $timeFactory) { + public function __construct( + DiscoveryService $discoveryService, + LoggerInterface $logger, + ITimeFactory $timeFactory, + IConfig $config + ) { $this->discoveryService = $discoveryService; $this->logger = $logger; $this->timeFactory = $timeFactory; + $this->config = $config; } public function isValidBearerToken(Provider $provider, string $bearerToken): ?string { @@ -70,6 +79,34 @@ public function isValidBearerToken(Provider $provider, string $bearerToken): ?st return null; } + $discovery = $this->discoveryService->obtainDiscovery($provider); + if ($payload->iss !== $discovery['issuer']) { + $this->logger->debug('This token is issued by the wrong issuer, it does not match the one from the discovery endpoint'); + return null; + } + + $oidcSystemConfig = $this->config->getSystemValue('user_oidc', []); + $checkAudience = !isset($oidcSystemConfig['selfencoded_bearer_validation_audience_check']) + || !in_array($oidcSystemConfig['selfencoded_bearer_validation_audience_check'], [false, 'false', 0, '0'], true); + if ($checkAudience) { + $tokenAudience = $payload->aud; + $providerClientId = $provider->getClientId(); + if ( + (is_string($tokenAudience) && $tokenAudience !== $providerClientId) + || (is_array($tokenAudience) && !in_array($providerClientId, $tokenAudience, true)) + ) { + $this->logger->debug('This token is not for us, the audience does not match the client ID'); + return null; + } + + // ref https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation + // If the azp claim is present, it should be the client ID + if (isset($payload->azp) && $payload->azp !== $providerClientId) { + $this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID'); + return null; + } + } + // find the user ID if (!isset($payload->{$uidAttribute})) { return null;