From 99ea021fa7318b6cbfe0428ab407d39e18677082 Mon Sep 17 00:00:00 2001
From: Julien Veyssier <julien-nc@posteo.net>
Date: Fri, 10 May 2024 11:03:34 +0200
Subject: [PATCH] fix: add issuer, audience and azp checks in bearer token
 validator, make it possible to disable audience check via config

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
---
 README.md                                   | 10 +++++
 lib/User/Validator/SelfEncodedValidator.php | 41 ++++++++++++++++++++-
 2 files changed, 50 insertions(+), 1 deletion(-)

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/User/Validator/SelfEncodedValidator.php b/lib/User/Validator/SelfEncodedValidator.php
index 0e462fe9..63993540 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,36 @@ 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) {
+			if (!($payload->aud === $provider->getClientId() || in_array($provider->getClientId(), $payload->aud, 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 (isset($payload->azp)) {
+					if ($payload->azp !== $provider->getClientId()) {
+						$this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID');
+						return null;
+					}
+				} else {
+					$this->logger->debug('Multiple audiences but no authorized party (azp) in the id token');
+					return null;
+				}
+			}
+		}
+
 		// find the user ID
 		if (!isset($payload->{$uidAttribute})) {
 			return null;