Merge pull request #864 from nextcloud/enh/856/add-bearer-token-audie…

…nce-check

Add issuer, audience and azp checks in bearer token validator

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:

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)) {

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;