fix(login): fall through to normal login when no IdP is configured by matsaur · Pull Request #2 · IONOS-Productivity/nc-user_saml

matsaur · 2026-04-20T15:03:14Z

Summary

When the user_saml app is enabled but no IdP has been configured yet
(i.e. idp-entityId and idp-singleSignOnService.url are empty), the
login redirect in Application::boot() would still intercept all /login
requests and forward them to the SAML endpoint — making it impossible to
log in at all.

Changes

Added SAMLSettings::getListOfConfiguredIdps() which filters the IdP
list to only entries that have both idp-entityId and
idp-singleSignOnService.url set.
Application::boot() now uses this method instead of getListOfIdps()
and returns early (falling through to normal Nextcloud login) when no
fully configured IdP exists.

How to reproduce

Enable the user_saml app
Do not configure any IdP
Open the login page → previously redirected to a broken SAML flow;
now falls through to the standard login form.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Matthias Sauer <sauerm@strato.de>

Copilot

Pull request overview

Fixes a login dead-end in the user_saml Nextcloud app when SAML is enabled but no IdP has been configured yet, by allowing /login to fall through to the normal Nextcloud login instead of forcing a broken SAML redirect.

Changes:

Added SAMLSettings::getListOfConfiguredIdps() to filter IdPs to those with both idp-entityId and idp-singleSignOnService.url set.
Updated Application::boot() to use the filtered IdP list and return early when none are configured.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
lib/SAMLSettings.php	Adds a filtered IdP list helper to distinguish minimally configured IdPs.
lib/AppInfo/Application.php	Uses the filtered list to avoid redirecting `/login` into a broken SAML flow when no IdP is configured.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T15:07:24Z

+				$configuredIdps = $samlSettings->getListOfConfiguredIdps();
+				// If no IdP has the minimum required config (entityId + SSO URL), fall through to normal login
+				if (empty($configuredIdps)) {
+					return;
+				}


The new login-flow behavior (falling through to core login when SAML is enabled but no IdP is minimally configured) is security/availability sensitive and currently isn’t covered by automated tests. Please add a unit/integration test that asserts /login is redirected to the SAML endpoint when an IdP is configured, and is not redirected when no IdP has entityId+SSO URL.

@copilot apply changes based on this feedback

Added unit tests for SAMLSettings::getListOfConfiguredIdps() in tests/unit/SAMLSettingsTest.php (commit 6c8ed9c). The tests cover:

No configurations → empty array (login falls through to Nextcloud)

IdP with both idp-entityId + idp-singleSignOnService.url → returned (SAML redirect applies)

IdP with whitespace-only values → filtered out (login falls through)

Mix of fully and partially configured IdPs → only fully configured ones returned

Since Application::boot() uses header() + exit() which can't be unit-tested in isolation, the tests are placed on getListOfConfiguredIdps() — the method that drives the redirect decision.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Matthias Sauer <151215545+matsaur@users.noreply.github.com>

Agent-Logs-Url: https://github.com/IONOS-Productivity/nc-user_saml/sessions/096a3167-5eda-4bd0-bc50-582afe712801 Co-authored-by: matsaur <151215545+matsaur@users.noreply.github.com>

come-nc · 2026-04-30T10:15:33Z

Hello,

I imported and adapted this PR as nextcloud#1091 on the main repo.

matsaur · 2026-04-30T11:49:37Z

@come-nc Thank you for your feedback, I'll close this PR 👍

fix: Make sure gss.jwt.key is configured

When user_saml is enabled (type=saml) but no IdP exists in the database, navigating to the login page caused a TypeError: array_key_exists(): Argument #2 ($array) must be of type array, null given in SAMLSettings.php line 147 Root cause: Application.php beforeware calls getListOfIdps() which sets LOADED_ALL on SAMLSettings. When SAMLController::login() then calls getOneLoginSettingsArray(1), ensureConfigurationsLoaded(1) returns early due to LOADED_ALL, leaving $this->configurations[1] undefined (null). The array_key_exists() call on null then throws a TypeError. Two fixes in SAMLSettings::getOneLoginSettingsArray(): - Replace the lone array_key_exists() call (line 156) with ?? '' to be consistent with every other field access in the method. - Add an early guard that throws InvalidArgumentException when the requested IdP is not in $configurations, converting a cryptic null-pointer into a clear error message. SAMLController::login() catches InvalidArgumentException and redirects to the existing genericError page with a translatable user-readable message instead of letting the exception propagate as a 500. Tests added: - SAMLSettingsTest: reproduces the exact LOADED_ALL bug scenario, tests the simple not-in-db case, happy path, and sp-entityId fallback. - SAMLControllerTest: verifies the controller redirects to genericError when getOneLoginSettingsArray() throws InvalidArgumentException. Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>

fix(login): fall through to normal login when no IdP is configured

ec578ca

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Matthias Sauer <sauerm@strato.de>

matsaur requested a review from Copilot April 20, 2026 15:03

Copilot started reviewing on behalf of matsaur April 20, 2026 15:03 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

matsaur and others added 2 commits April 20, 2026 17:08

Update lib/SAMLSettings.php

3fe80b0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Matthias Sauer <151215545+matsaur@users.noreply.github.com>

Update lib/AppInfo/Application.php

ca0ee07

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Matthias Sauer <151215545+matsaur@users.noreply.github.com>

Copilot started work on behalf of matsaur April 20, 2026 15:10 View session

test(saml-settings): add unit tests for getListOfConfiguredIdps()

6c8ed9c

Agent-Logs-Url: https://github.com/IONOS-Productivity/nc-user_saml/sessions/096a3167-5eda-4bd0-bc50-582afe712801 Co-authored-by: matsaur <151215545+matsaur@users.noreply.github.com>

Copilot finished work on behalf of matsaur April 20, 2026 15:15

artonge mentioned this pull request Apr 21, 2026

Handle incomplete saml configuration more gracefully nextcloud/user_saml#840

Closed

artonge linked an issue Apr 21, 2026 that may be closed by this pull request

Handle incomplete saml configuration more gracefully nextcloud/user_saml#840

Closed

matsaur closed this Apr 30, 2026

printminion-co pushed a commit that referenced this pull request Jun 10, 2026

Merge pull request #2 from nextcloud-gmbh/carl/gss.jwt.key

376155b

fix: Make sure gss.jwt.key is configured

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(login): fall through to normal login when no IdP is configured#2

fix(login): fall through to normal login when no IdP is configured#2
matsaur wants to merge 4 commits into
ionos-devfrom
fix-login

matsaur commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

matsaur Apr 20, 2026

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

come-nc commented Apr 30, 2026

Uh oh!

matsaur commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

matsaur commented Apr 20, 2026

Summary

Changes

How to reproduce

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

matsaur Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

come-nc commented Apr 30, 2026

Uh oh!

matsaur commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants