make audience check safer

Signed-off-by: Julien Veyssier <[email protected]>
nextcloud · Jun 3, 2024 · 6984473 · 6984473
1 parent 0262e48
commit 6984473
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 4 deletions.
diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php
@@ -455,7 +455,12 @@ 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]);

diff --git a/lib/User/Validator/SelfEncodedValidator.php b/lib/User/Validator/SelfEncodedValidator.php
@@ -89,16 +89,21 @@ public function isValidBearerToken(Provider $provider, string $bearerToken): ?st
 		$checkAudience = !isset($oidcSystemConfig['selfencoded_bearer_validation_audience_check'])
 			|| !in_array($oidcSystemConfig['selfencoded_bearer_validation_audience_check'], [false, 'false', 0, '0'], true);
 		if ($checkAudience) {
-			if (!($payload->aud === $provider->getClientId() || in_array($provider->getClientId(), $payload->aud, true))) {
+			$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;
 			}
 
 			// 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($payload->aud) && count($payload->aud) > 1) {
+			if (is_array($tokenAudience) && count($tokenAudience) > 1) {
 				if (isset($payload->azp)) {
-					if ($payload->azp !== $provider->getClientId()) {
+					if ($payload->azp !== $providerClientId) {
 						$this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID');
 						return null;
 					}