diff --git a/lib/Portal/PortalAssertionVerifier.php b/lib/Portal/PortalAssertionVerifier.php index 4245f02cd..87de4add3 100644 --- a/lib/Portal/PortalAssertionVerifier.php +++ b/lib/Portal/PortalAssertionVerifier.php @@ -20,6 +20,12 @@ * accepts only what the minter can produce, so this one reads the dedicated * secret only. The secret never comes from the request. * + * Portaliq stores that secret SENSITIVE (encrypted at rest), so it is read + * through `IAppConfig::getValueString()`, which decrypts it, exactly as + * portaliq reads it. The legacy `IConfig::getAppValue()` returns the stored + * ciphertext, and every HMAC check then failed (401 on every portal action, + * 13-14 Oct 2026). + * * @category Portal * @package OCA\Learniq\Portal * @@ -40,7 +46,7 @@ namespace OCA\Learniq\Portal; -use OCP\IConfig; +use OCP\IAppConfig; use Psr\Log\LoggerInterface; /** @@ -98,14 +104,14 @@ class PortalAssertionVerifier { /** * Constructor. Tests pass a plain secret instead of config. * - * @param IConfig|null $config Source of portaliq's app value. + * @param IAppConfig|null $config Source of portaliq's sensitive app value (read decrypted). * @param string|null $secretOverride Plain secret for tests. * @param LoggerInterface|null $logger Rejection reasons, debug level only. * * @return void */ public function __construct( - private readonly ?IConfig $config = null, + private readonly ?IAppConfig $config = null, private readonly ?string $secretOverride = null, private readonly ?LoggerInterface $logger = null, ) { @@ -210,14 +216,18 @@ private function timeProblem(array $claims): ?string { /** * Portaliq's dedicated signing secret, or null when it is missing or too - * short (verification then fails closed). + * short (verification then fails closed). Read through IAppConfig, + * non-lazy as portaliq stores it, so the sensitive value comes back + * decrypted. * * @return string|null + * + * @spec openspec/specs/assessment/spec.md#requirement-portal-test-requests-are-accepted-only-from-portaliqs-signed-forward */ private function secret(): ?string { $secret = $this->secretOverride; if ($secret === null && $this->config !== null) { - $secret = (string)$this->config->getAppValue(self::PORTALIQ_APP_ID, self::SECRET_KEY, ''); + $secret = $this->config->getValueString(self::PORTALIQ_APP_ID, self::SECRET_KEY, '', lazy: false); } if ($secret === null || strlen($secret) < self::MIN_SECRET_LENGTH) { diff --git a/tests/Unit/Portal/PortalAssertionVerifierTest.php b/tests/Unit/Portal/PortalAssertionVerifierTest.php index f7e47e488..16bd3a472 100644 --- a/tests/Unit/Portal/PortalAssertionVerifierTest.php +++ b/tests/Unit/Portal/PortalAssertionVerifierTest.php @@ -29,6 +29,7 @@ namespace OCA\Learniq\Tests\Unit\Portal; use OCA\Learniq\Portal\PortalAssertionVerifier; +use OCP\IAppConfig; use OCP\IConfig; use PHPUnit\Framework\TestCase; @@ -178,17 +179,35 @@ public function testMalformedTokensAreRefused(): void { * @return void */ public function testOnlyTheDedicatedSecretCounts(): void { - $config = $this->createMock(IConfig::class); - $config->method('getAppValue')->willReturnCallback( - static fn (string $app, string $key, string $default = ''): string => ($app === 'portaliq' && $key === 'jwt_signing_secret') ? self::SECRET : $default + $config = $this->createMock(IAppConfig::class); + $config->method('getValueString')->willReturnCallback( + static fn (string $app, string $key, string $default = '', bool $lazy = false): string => ($app === 'portaliq' && $key === 'jwt_signing_secret' && $lazy === false) ? self::SECRET : $default ); self::assertIsArray((new PortalAssertionVerifier(config: $config))->verify(jwt: $this->mint())); - $short = $this->createMock(IConfig::class); - $short->method('getAppValue')->willReturn('too-short'); - $short->method('getSystemValue')->willReturn(self::SECRET); + $short = $this->createMock(IAppConfig::class); + $short->method('getValueString')->willReturn('too-short'); self::assertNull((new PortalAssertionVerifier(config: $short))->verify(jwt: $this->mint())); self::assertNull((new PortalAssertionVerifier())->verify(jwt: $this->mint())); }//end testOnlyTheDedicatedSecretCounts() + + /** + * REGRESSION (13-14 Oct 2026, 401 on every portal action): portaliq stores + * the secret SENSITIVE, so the legacy `IConfig::getAppValue()` returns the + * encrypted value and only `IAppConfig::getValueString()` returns the real + * secret. The mock answers both ways, so the old legacy read fails here + * on the signature check. + * + * @return void + */ + public function testTheSensitiveSecretIsReadDecrypted(): void { + $ciphertext = '$AppConfigEncryption$' . str_repeat('a1b2c3d4e5f6', 22); + $config = $this->createMockForIntersectionOfInterfaces([IConfig::class, IAppConfig::class]); + $config->method('getAppValue')->willReturn($ciphertext); + $config->method('getValueString')->willReturn(self::SECRET); + + self::assertIsArray((new PortalAssertionVerifier(config: $config))->verify(jwt: $this->mint())); + self::assertNull((new PortalAssertionVerifier(config: $config))->verify(jwt: $this->mint(secret: $ciphertext))); + }//end testTheSensitiveSecretIsReadDecrypted() }//end class