diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index c86be1a42..ee01d8460 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -167,7 +167,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 9219c10d9..269b8dbb1 100644 --- a/lib/SAMLSettings.php +++ b/lib/SAMLSettings.php @@ -90,11 +90,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); + } +}