From c6103b497eb21de55f5e84cbaa106eadfa94b66d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 30 Apr 2026 12:13:04 +0200 Subject: [PATCH 1/3] fix: Avoid redirecting to login with an incomplete configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/AppInfo/Application.php | 8 +- lib/SAMLSettings.php | 8 +- tests/unit/SAMLSettingsTest.php | 127 ++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 tests/unit/SAMLSettingsTest.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 0268b7747..167700f31 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -176,7 +176,13 @@ public function boot(IBootContext $context): void { } $multipleUserBackEnds = $samlSettings->allowMultipleUserBackEnds(); - $configuredIdps = $samlSettings->getListOfIdps(); + $configuredIdps = $samlSettings->getListOfIdps(true); + // If no IdP has the minimum required config (entityId + SSO URL), fall through to normal login + // only for regular SAML mode. Environment-variable mode can still redirect to SAMLController::login() + // without requiring configured IdP metadata. + if ($type === 'saml' && empty($configuredIdps)) { + return; + } $showLoginOptions = $type !== 'environment-variable' && ($multipleUserBackEnds || count($configuredIdps) > 1); if ($redirectSituation === true && $showLoginOptions) { diff --git a/lib/SAMLSettings.php b/lib/SAMLSettings.php index 8f409bfd7..53ccbe52f 100644 --- a/lib/SAMLSettings.php +++ b/lib/SAMLSettings.php @@ -88,11 +88,17 @@ public function __construct( * @return array * @throws Exception */ - public function getListOfIdps(): array { + public function getListOfIdps(bool $onlyComplete = false): array { $this->ensureConfigurationsLoaded(); $result = []; foreach ($this->configurations as $configID => $config) { + if ($onlyComplete + && (trim(($config['idp-entityId'] ?? '')) === '' + || trim(($config['idp-singleSignOnService.url'] ?? '')) === '') + ) { + continue; + } // no fancy array_* method, because there might be thousands $result[$configID] = $config['general-idp0_display_name'] ?? ''; } diff --git a/tests/unit/SAMLSettingsTest.php b/tests/unit/SAMLSettingsTest.php new file mode 100644 index 000000000..3aa00d2cb --- /dev/null +++ b/tests/unit/SAMLSettingsTest.php @@ -0,0 +1,127 @@ +urlGenerator = $this->createMock(IURLGenerator::class); + $this->config = $this->createMock(IConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->session = $this->createMock(ISession::class); + $this->mapper = $this->createMock(ConfigurationsMapper::class); + + $this->samlSettings = new SAMLSettings( + $this->urlGenerator, + $this->config, + $this->appConfig, + $this->session, + $this->mapper, + ); + } + + private static function dataGetListOfIdps(): array { + return [ + 'empty-all' => [ + false, + [], + [], + ], + 'empty-complete' => [ + true, + [], + [], + ], + 'entityId-and-ssoUrl-missing' => [ + true, + [ + 1 => [ + 'general-idp0_display_name' => 'My IdP', + // no idp-entityId, no idp-singleSignOnService.url + ], + ], + [], + ], + 'only-whitespace' => [ + true, + [ + 1 => [ + 'general-idp0_display_name' => 'My IdP', + 'idp-entityId' => ' ', + 'idp-singleSignOnService.url' => "\t", + ], + ], + [], + ], + 'configured' => [ + true, + [ + 1 => [ + 'general-idp0_display_name' => 'My IdP', + 'idp-entityId' => 'https://idp.example.com', + 'idp-singleSignOnService.url' => 'https://idp.example.com/sso', + ], + ], + [1 => 'My IdP'], + ], + 'partially-configured' => [ + true, + [ + 1 => [ + 'general-idp0_display_name' => 'Configured IdP', + 'idp-entityId' => 'https://idp.example.com', + 'idp-singleSignOnService.url' => 'https://idp.example.com/sso', + ], + 2 => [ + 'general-idp0_display_name' => 'Missing SSO URL', + 'idp-entityId' => 'https://idp2.example.com', + // missing idp-singleSignOnService.url + ], + 3 => [ + 'general-idp0_display_name' => 'Missing Entity ID', + // missing idp-entityId + 'idp-singleSignOnService.url' => 'https://idp3.example.com/sso', + ], + ], + [1 => 'Configured IdP'], + ], + ]; + } + + #[DataProvider('dataGetListOfIdps')] + public function testGetListOfIdps(bool $onlyComplete, array $configs, array $expected): void { + $this->mapper->expects($this->once()) + ->method('getAll') + ->willReturn($configs); + + $result = $this->samlSettings->getListOfIdps($onlyComplete); + + $this->assertSame($expected, $result); + } +} From 0828e3c82ff8a06f2009d59e96127e62e79aa6c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 7 May 2026 12:45:26 +0200 Subject: [PATCH 2/3] chore: Adapt test for stable-7 SAMLSettings constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/unit/SAMLSettingsTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/unit/SAMLSettingsTest.php b/tests/unit/SAMLSettingsTest.php index 3aa00d2cb..0076b9cd2 100644 --- a/tests/unit/SAMLSettingsTest.php +++ b/tests/unit/SAMLSettingsTest.php @@ -11,7 +11,6 @@ use OCA\User_SAML\Db\ConfigurationsMapper; use OCA\User_SAML\SAMLSettings; -use OCP\AppFramework\Services\IAppConfig; use OCP\IConfig; use OCP\ISession; use OCP\IURLGenerator; @@ -22,7 +21,6 @@ class SAMLSettingsTest extends TestCase { private IURLGenerator&MockObject $urlGenerator; private IConfig&MockObject $config; - private IAppConfig&MockObject $appConfig; private ISession&MockObject $session; private ConfigurationsMapper&MockObject $mapper; private SAMLSettings $samlSettings; @@ -33,14 +31,12 @@ protected function setUp(): void { $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->config = $this->createMock(IConfig::class); - $this->appConfig = $this->createMock(IAppConfig::class); $this->session = $this->createMock(ISession::class); $this->mapper = $this->createMock(ConfigurationsMapper::class); $this->samlSettings = new SAMLSettings( $this->urlGenerator, $this->config, - $this->appConfig, $this->session, $this->mapper, ); From 709342a1e45efe02fefe4fc8bc5ce4e98d026276 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 7 May 2026 14:58:19 +0200 Subject: [PATCH 3/3] chore: Use legacy syntax for data provider to support old psalm versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/unit/SAMLSettingsTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/SAMLSettingsTest.php b/tests/unit/SAMLSettingsTest.php index 0076b9cd2..263653974 100644 --- a/tests/unit/SAMLSettingsTest.php +++ b/tests/unit/SAMLSettingsTest.php @@ -14,7 +14,6 @@ use OCP\IConfig; use OCP\ISession; use OCP\IURLGenerator; -use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -110,7 +109,9 @@ private static function dataGetListOfIdps(): array { ]; } - #[DataProvider('dataGetListOfIdps')] + /** + * @dataProvider dataGetListOfIdps + */ public function testGetListOfIdps(bool $onlyComplete, array $configs, array $expected): void { $this->mapper->expects($this->once()) ->method('getAll')