Merge pull request #1272 from nextcloud/enh/noid/allow-disabling-jwk-strength-check

Allow disabling JWK strength validation

Author: julien-nc

README.md

@@ -70,6 +70,17 @@ is not set in the discovery. In such case, you can set the default token endpoin
 ]
 ```
 
+## `user_oidc.validate_jwk_strength`
+
+By default, user_oidc validates the strength of the JWK keys received from the discovery endpoint.
+It will check that RSA keys are long enough and that EC/OKP keys have the correct curve.
+This can be disabled with:
+
+```php
+'user_oidc' => [
+  'validate_jwk_strength' => false
+]
+```
 
 ---
 

lib/Service/DiscoveryService.php

@@ -15,6 +15,7 @@
 use OCA\UserOIDC\Vendor\Firebase\JWT\JWT;
 use OCP\ICache;
 use OCP\ICacheFactory;
+use OCP\IConfig;
 use Psr\Log\LoggerInterface;
 
 class DiscoveryService {
@@ -42,6 +43,7 @@ public function __construct(
 		private LoggerInterface $logger,
 		private HttpClientHelper $clientService,
 		private ProviderService $providerService,
+		private IConfig $config,
 		ICacheFactory $cacheFactory,
 	) {
 		$this->cache = $cacheFactory->createDistributed('user_oidc');
@@ -208,8 +210,12 @@ private function fixJwksAlg(array $jwks, string $jwt): array {
 				continue;
 			}
 
-			// Validate key strength
-			$this->validateKeyStrength($key, $alg);
+			$oidcSystemConfig = $this->config->getSystemValue('user_oidc', []);
+			if (!isset($oidcSystemConfig['validate_jwk_strength'])
+				|| !in_array($oidcSystemConfig['validate_jwk_strength'], [false, 'false', 0, '0'], true)) {
+				// Validate key strength
+				$this->validateKeyStrength($key, $alg);
+			}
 
 			// If JWT has a kid, match strictly
 			if ($kid !== null) {

tests/unit/Service/DiscoveryServiceTest.php

@@ -12,41 +12,35 @@
 use OCA\UserOIDC\Service\DiscoveryService;
 use OCA\UserOIDC\Service\ProviderService;
 use OCP\ICacheFactory;
+use OCP\IConfig;
 use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
 
 class DiscoveryServiceTest extends TestCase {
 
-	/**
-	 * @var MockObject|LoggerInterface
-	 */
+	/** @var MockObject|LoggerInterface */
 	private $logger;
-	/**
-	 * @var HttpClientHelper|MockObject
-	 */
+	/** @var HttpClientHelper|MockObject */
 	private $clientHelper;
-	/**
-	 * @var ProviderService|MockObject
-	 */
+	/** @var ProviderService|MockObject */
 	private $providerService;
-	/**
-	 * @var ICacheFactory|MockObject
-	 */
+	/** @var IConfig|MockObject */
+	private $config;
+	/** @var ICacheFactory|MockObject */
 	private $cacheFactory;
-	/**
-	 * @var DiscoveryService
-	 */
+	/** @var DiscoveryService */
 	private $discoveryService;
 
 	public function setUp(): void {
 		parent::setUp();
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->clientHelper = $this->createMock(HttpClientHelper::class);
 		$this->providerService = $this->createMock(ProviderService::class);
+		$this->config = $this->createMock(IConfig::class);
 		$this->cacheFactory = $this->createMock(ICacheFactory::class);
-		$this->discoveryService = new DiscoveryService($this->logger, $this->clientHelper, $this->providerService, $this->cacheFactory);
+		$this->discoveryService = new DiscoveryService($this->logger, $this->clientHelper, $this->providerService, $this->config, $this->cacheFactory);
 	}
 
 	public function testBuildAuthorizationUrl() {