fix(validation): allow RFC 6749 VSCHAR characters in client_id and client_secret

lib/BasicAuthBackend.php

@@ -84,7 +84,7 @@ public function implementsActions($actions) {
      * Check if the password is correct without logging in the user
      */
     public function checkPassword($uid, $password) {
-        if (strlen($uid) !== 64 || strlen($password) !== 64) {
+        if (strlen($uid) < 32 || strlen($uid) > 64 || strlen($password) < 32 || strlen($password) > 64) {
             return false;
         }
 
@@ -187,7 +187,7 @@ public function toggleLock($uid) {
      * @return boolean
      */
     public function userExists($uid) {
-        if (strlen($uid) !== 64) {
+        if (strlen($uid) < 32 || strlen($uid) > 64) {
             return false;
         }
 

lib/Command/Clients/OIDCCreate.php

@@ -105,14 +105,14 @@ protected function configure(): void
                 'client_id',
                 null,
                 InputOption::VALUE_OPTIONAL,
-                'The client id to be used. If not provided the client id will be generated internally. Requirements: chars A-Za-z0-9 & min length 32 & max length 64',
+                'The client id to be used. If not provided the client id will be generated internally. Requirements: printable ASCII except : and length 32-64',
                 ''
             )
             ->addOption(
                 'client_secret',
                 null,
                 InputOption::VALUE_OPTIONAL,
-                'The client secret to be used. If not provided the client secret will be generated internally. Requirements: chars A-Za-z0-9 & min length 32 & max length 64',
+                'The client secret to be used. If not provided the client secret will be generated internally. Requirements: printable ASCII except : and length 32-64',
                 ''
             )
             ->addOption(
@@ -157,15 +157,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             $clientId = $input->getOption('client_id');
             $clientSecret = $input->getOption('client_secret');
             if (isset($clientId) && trim($clientId) !== '') {
-                if (filter_var($clientId, FILTER_VALIDATE_REGEXP, array("options" => array("regexp" => "/^[A-Za-z0-9]{32,64}$/"))) === false) {
-                    throw new CliException("Your clientId must comply with the following rules: chars A-Za-z0-9 & min length 32 & max length 64");
+                if (filter_var($clientId, FILTER_VALIDATE_REGEXP, array("options" => array("regexp" => "/^[\x21-\x39\x3B-\x7E]{32,64}$/"))) === false) {
+                    throw new CliException("Your clientId must comply with the following rules: printable ASCII except : and length 32-64");
                 }
                 $client->setClientIdentifier($clientId);
             }
 
             if (isset($clientSecret) && trim($clientSecret) !== '') {
-                if (filter_var($clientSecret, FILTER_VALIDATE_REGEXP, array("options" => array("regexp" => "/^[A-Za-z0-9]{32,64}$/"))) === false) {
-                    throw new CliException("Your clientSecret must comply with the following rules: chars A-Za-z0-9 & min length 32 & max length 64");
+                if (filter_var($clientSecret, FILTER_VALIDATE_REGEXP, array("options" => array("regexp" => "/^[\x21-\x39\x3B-\x7E]{32,64}$/"))) === false) {
+                    throw new CliException("Your clientSecret must comply with the following rules: printable ASCII except : and length 32-64");
                 }
                 $client->setSecret($clientSecret);
             }

lib/Controller/SettingsController.php

@@ -134,15 +134,15 @@ public function addClient(
         );
 
         if (isset($clientId) && trim($clientId) !== '') {
-            if (filter_var($clientId, FILTER_VALIDATE_REGEXP, array("options" => array("regexp" => "/^[A-Za-z0-9]{32,64}$/"))) === false) {
-                return new JSONResponse(['message' => $this->l->t('Your client ID must comply with the following rules: chars A-Za-z0-9 & min length 32 & max length 64')], Http::STATUS_BAD_REQUEST);
+            if (filter_var($clientId, FILTER_VALIDATE_REGEXP, array("options" => array("regexp" => "/^[\x21-\x39\x3B-\x7E]{32,64}$/"))) === false) {
+                return new JSONResponse(['message' => $this->l->t('Your client ID must comply with the following rules: printable ASCII except : and length 32-64')], Http::STATUS_BAD_REQUEST);
             }
             $client->setClientIdentifier($clientId);
         }
 
         if (isset($clientSecret) && trim($clientSecret) !== '') {
-            if (filter_var($clientSecret, FILTER_VALIDATE_REGEXP, array("options" => array("regexp" => "/^[A-Za-z0-9]{32,64}$/"))) === false) {
-                return new JSONResponse(['message' => $this->l->t('Your client secret must comply with the following rules: chars A-Za-z0-9 & min length 32 & max length 64')], Http::STATUS_BAD_REQUEST);
+            if (filter_var($clientSecret, FILTER_VALIDATE_REGEXP, array("options" => array("regexp" => "/^[\x21-\x39\x3B-\x7E]{32,64}$/"))) === false) {
+                return new JSONResponse(['message' => $this->l->t('Your client secret must comply with the following rules: printable ASCII except : and length 32-64')], Http::STATUS_BAD_REQUEST);
             }
             $client->setSecret($clientSecret);
         }

tests/Unit/BasicAuthBackendTest.php

@@ -0,0 +1,242 @@
+<?php
+
+namespace OCA\OIDCIdentityProvider\Tests\Unit;
+
+use PHPUnit\Framework\TestCase;
+
+use OCP\IRequest;
+use OCP\IURLGenerator;
+use OCP\IDBConnection;
+use OCP\AppFramework\Services\IAppConfig;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\Security\ISecureRandom;
+
+use OCA\OIDCIdentityProvider\BasicAuthBackend;
+use OCA\OIDCIdentityProvider\Db\Client;
+use OCA\OIDCIdentityProvider\Db\ClientMapper;
+use OCA\OIDCIdentityProvider\Db\CustomClaimMapper;
+use OCA\OIDCIdentityProvider\Db\RedirectUriMapper;
+use OCA\OIDCIdentityProvider\Exceptions\ClientNotFoundException;
+
+use Psr\Log\LoggerInterface;
+
+class BasicAuthBackendTest extends TestCase {
+    /** @var BasicAuthBackend */
+    private $backend;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|IRequest */
+    private $request;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|IURLGenerator */
+    private $urlGenerator;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|ClientMapper */
+    private $clientMapper;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */
+    private $logger;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|IAppConfig */
+    private $appConfig;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|ITimeFactory */
+    private $time;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|IDBConnection */
+    private $db;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|ISecureRandom */
+    private $secureRandom;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|RedirectUriMapper */
+    private $redirectUriMapper;
+    /** @var \PHPUnit\Framework\MockObject\MockObject|CustomClaimMapper */
+    private $customClaimMapper;
+
+    /** 64-char test credentials (auto-generated length) */
+    private $uid64 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz012345678901';
+    private $secret64 = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01';
+
+    /** 48-char test credentials (user-provided length) */
+    private $uid48 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuv';
+    private $secret48 = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKL';
+
+    /** 32-char test credentials (minimum valid length) */
+    private $uid32 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdef';
+    private $secret32 = '0123456789abcdefghijklmnopqrstuv';
+
+    public function setUp(): void {
+        $this->request = $this->getMockBuilder(IRequest::class)->getMock();
+        $this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)->getMock();
+        $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
+        $this->appConfig = $this->getMockBuilder(IAppConfig::class)->getMock();
+        $this->secureRandom = $this->getMockBuilder(ISecureRandom::class)->getMock();
+        $this->time = $this->getMockBuilder(ITimeFactory::class)->getMock();
+        $this->db = $this->getMockBuilder(IDBConnection::class)->getMock();
+        $this->redirectUriMapper = $this->getMockBuilder(RedirectUriMapper::class)->setConstructorArgs([
+            $this->db,
+            $this->time,
+            $this->appConfig])->getMock();
+        $this->customClaimMapper = $this->getMockBuilder(CustomClaimMapper::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->clientMapper = $this->getMockBuilder(ClientMapper::class)->setConstructorArgs([
+            $this->db,
+            $this->time,
+            $this->appConfig,
+            $this->redirectUriMapper,
+            $this->customClaimMapper,
+            $this->secureRandom,
+            $this->logger])->getMock();
+
+        $this->backend = new BasicAuthBackend(
+            $this->request,
+            $this->urlGenerator,
+            $this->clientMapper,
+            $this->logger
+        );
+    }
+
+    // --- checkPassword: length validation ---
+
+    public function testCheckPasswordRejectsTooShort() {
+        $this->assertFalse($this->backend->checkPassword('short', 'short'));
+    }
+
+    public function testCheckPasswordRejectsTooLong() {
+        $uid = str_repeat('a', 65);
+        $secret = str_repeat('b', 65);
+        $this->assertFalse($this->backend->checkPassword($uid, $secret));
+    }
+
+    public function testCheckPasswordRejectsEmpty() {
+        $this->assertFalse($this->backend->checkPassword('', ''));
+    }
+
+    // --- checkPassword: accepts valid lengths on token endpoint ---
+
+    private function setupValidClient(string $uid, string $secret): void {
+        $this->request->method('getRequestUri')
+            ->willReturn('/apps/oidc/token');
+
+        $client = new Client();
+        $client->setClientIdentifier($uid);
+        $client->setSecret($secret);
+        $client->setType('confidential');
+
+        $this->clientMapper->method('getByIdentifier')
+            ->with($uid)
+            ->willReturn($client);
+    }
+
+    public function testCheckPasswordAccepts64CharCredentials() {
+        $this->setupValidClient($this->uid64, $this->secret64);
+        $this->assertTrue($this->backend->checkPassword($this->uid64, $this->secret64));
+    }
+
+    public function testCheckPasswordAccepts48CharCredentials() {
+        $this->setupValidClient($this->uid48, $this->secret48);
+        $this->assertTrue($this->backend->checkPassword($this->uid48, $this->secret48));
+    }
+
+    public function testCheckPasswordAccepts32CharCredentials() {
+        $this->setupValidClient($this->uid32, $this->secret32);
+        $this->assertTrue($this->backend->checkPassword($this->uid32, $this->secret32));
+    }
+
+    // --- checkPassword: endpoint restriction ---
+
+    public function testCheckPasswordRejectsNonTokenEndpoint() {
+        $this->request->method('getRequestUri')
+            ->willReturn('/apps/files/index.php');
+
+        $this->assertFalse($this->backend->checkPassword($this->uid64, $this->secret64));
+    }
+
+    public function testCheckPasswordAcceptsIntrospectEndpoint() {
+        $this->request->method('getRequestUri')
+            ->willReturn('/apps/oidc/introspect');
+
+        $client = new Client();
+        $client->setClientIdentifier($this->uid64);
+        $client->setSecret($this->secret64);
+        $client->setType('confidential');
+
+        $this->clientMapper->method('getByIdentifier')
+            ->with($this->uid64)
+            ->willReturn($client);
+
+        $this->assertTrue($this->backend->checkPassword($this->uid64, $this->secret64));
+    }
+
+    // --- checkPassword: wrong secret ---
+
+    public function testCheckPasswordRejectsWrongSecret() {
+        $this->request->method('getRequestUri')
+            ->willReturn('/apps/oidc/token');
+
+        $client = new Client();
+        $client->setClientIdentifier($this->uid64);
+        $client->setSecret('WrongSecretThatIsExactlySixtyFourCharactersLongForTestingPurpose1');
+        $client->setType('confidential');
+
+        $this->clientMapper->method('getByIdentifier')
+            ->with($this->uid64)
+            ->willReturn($client);
+
+        $this->assertFalse($this->backend->checkPassword($this->uid64, $this->secret64));
+    }
+
+    // --- checkPassword: unknown client ---
+
+    public function testCheckPasswordRejectsUnknownClient() {
+        $this->request->method('getRequestUri')
+            ->willReturn('/apps/oidc/token');
+
+        $this->clientMapper->method('getByIdentifier')
+            ->willThrowException(new ClientNotFoundException());
+
+        $this->assertFalse($this->backend->checkPassword($this->uid64, $this->secret64));
+    }
+
+    // --- userExists: length validation ---
+
+    public function testUserExistsRejectsTooShort() {
+        $this->assertFalse($this->backend->userExists('short'));
+    }
+
+    public function testUserExistsRejectsTooLong() {
+        $this->assertFalse($this->backend->userExists(str_repeat('a', 65)));
+    }
+
+    public function testUserExistsAccepts64Char() {
+        $client = new Client();
+        $client->setClientIdentifier($this->uid64);
+
+        $this->clientMapper->method('getByIdentifier')
+            ->with($this->uid64)
+            ->willReturn($client);
+
+        $this->assertTrue($this->backend->userExists($this->uid64));
+    }
+
+    public function testUserExistsAccepts48Char() {
+        $client = new Client();
+        $client->setClientIdentifier($this->uid48);
+
+        $this->clientMapper->method('getByIdentifier')
+            ->with($this->uid48)
+            ->willReturn($client);
+
+        $this->assertTrue($this->backend->userExists($this->uid48));
+    }
+
+    public function testUserExistsAccepts32Char() {
+        $client = new Client();
+        $client->setClientIdentifier($this->uid32);
+
+        $this->clientMapper->method('getByIdentifier')
+            ->with($this->uid32)
+            ->willReturn($client);
+
+        $this->assertTrue($this->backend->userExists($this->uid32));
+    }
+
+    public function testUserExistsReturnsFalseForUnknownClient() {
+        $this->clientMapper->method('getByIdentifier')
+            ->willThrowException(new ClientNotFoundException());
+
+        $this->assertFalse($this->backend->userExists($this->uid64));
+    }
+}