fix(login): treat already-logged-out as success in backchannel logout (#1430) by SAY-5 · Pull Request #1431 · nextcloud/user_oidc

SAY-5 · 2026-04-28T11:12:32Z

Closes #1430.

Summary

If the identified End-User is already logged out at the RP when the logout request is received, the logout is considered to have succeeded.

Today backChannelLogout() returns HTTP/400 when the (sid, iss) or (sub, iss) lookup yields no matching session, which is exactly the already-logged-out case the spec calls out as a success. The reporter (running LemonLDAP) sees a confusing error surfaced on the IdP UI even though the desired state — the RP session being gone — is already true. The same shape would happen with Keycloak, Authelia, etc.

Change

Only the two "session not found" paths flip to HTTP/200:

findSessionBySid → DoesNotExistException ⇒ return HTTP/200 OK + debug log.
findSessionsBySubAndIss returns empty ⇒ return HTTP/200 OK + debug log.

The other validation branches in the same method — invalid issuer, invalid signature, malformed token, missing claims, multiple-sessions-found — keep their HTTP/400 responses. Operators that need visibility into already-logged-out backchannel calls can flip Nextcloud's loglevel to debug and see the [BackchannelLogout] line.

Mirrors the proposed fix in the issue and the spec citation.

Test plan

No LoginControllerTest exists in tests/unit/Controller/ today, and the existing integration runner under tests/integration/ does not cover the backchannel flow. The change is two short branches that swap a getBackchannelLogoutErrorResponse() for new JSONResponse([], Http::STATUS_OK) plus a debug log; both branches are syntactically equivalent to the existing return new JSONResponse([], Http::STATUS_OK) at the end of the success path.
Ran git diff --check clean.
Verified the spec citation against the linked RFC section.

Notes

This is the spec-compliant behavior; existing IdPs will start receiving cleaner success responses for the already-logged-out case.
The optional "debug checkbox" the reporter floated is left for follow-up — the spec is unambiguous, so a per-provider opt-out adds configuration surface for no clear benefit.

Closes nextcloud#1430. OIDC Backchannel Logout 1.0 §2.6 says: 'If the identified End-User is already logged out at the RP when the logout request is received, the logout is considered to have succeeded.' Today `backChannelLogout()` returns HTTP/400 when the (sid, iss) or (sub, iss) lookup yields no matching session, even though that is exactly the already-logged-out case the spec calls out as a *success*. IdPs (e.g. LemonLDAP, Keycloak) surface this 400 to the end user and log a spurious error. The other validation branches in this method — invalid issuer, invalid signature, missing claims, malformed token — keep their HTTP/400 responses. Only the two 'session not found' paths flip to HTTP/200 + a debug log so administrators can still trace what happened. See https://openid.net/specs/openid-connect-backchannel-1_0.html#BCActions Signed-off-by: SAY-5 <say.apm35@gmail.com>

Spitfireap · 2026-04-28T14:14:44Z

Thanks !
tested on v33.0.2 without any issue. It's working as intended

julien-nc

Thanks a lot. Small adjustments and let's get this in!

julien-nc · 2026-04-29T09:44:31Z

+				// out is a success). Same rationale as the (sid,iss)
+				// branch above.
+				$this->logger->debug(
+					'[BackchannelLogout] no RP sessions for (sub,iss) — treating as already-logged-out per spec',


Suggested change

'[BackchannelLogout] no RP sessions for (sub,iss) — treating as already-logged-out per spec',

'[BackchannelLogout] no RP sessions for (sub,iss) — treating as already-logged-out',

julien-nc · 2026-04-29T09:44:41Z

+				// state — the RP session being gone — is already true.
+				// See https://openid.net/specs/openid-connect-backchannel-1_0.html#BCActions
+				$this->logger->debug(
+					'[BackchannelLogout] no RP session for (sid,iss) — treating as already-logged-out per spec',


Suggested change

'[BackchannelLogout] no RP session for (sid,iss) — treating as already-logged-out per spec',

'[BackchannelLogout] no RP session for (sid,iss) — treating as already-logged-out',

julien-nc · 2026-04-29T09:47:21Z

+				// Per OIDC Backchannel Logout 1.0 §2.6:
+				// "If the identified End-User is already logged out at the
+				// RP when the logout request is received, the logout is
+				// considered to have succeeded." Returning 400 here
+				// causes IdPs (e.g. LemonLDAP, Keycloak) to surface a
+				// confusing error to the user even though the desired
+				// state — the RP session being gone — is already true.
+				// See https://openid.net/specs/openid-connect-backchannel-1_0.html#BCActions


Can you make this a little bit shorter? The link + a very brief explanation should be enough.

Address julien-nc's review: - Shorten the long rationale comment to a one-liner with the spec link, per inline review at lib/Controller/LoginController.php:940. - Drop the redundant ' per spec' suffix from both already-logged-out debug log lines (lib/Controller/LoginController.php:942 and :972), matching the suggested edits. Behaviour is unchanged — only comment / log message wording.

SAY-5 · 2026-04-29T15:55:23Z

Thanks for the review @julien-nc — applied all three suggestions in aa87b8f: comment block shortened to a single line + spec link, and the redundant ' per spec' suffix dropped from both debug log messages.

julien-nc · 2026-04-29T15:58:04Z

Thanks for the adjustments. Could you sign your commits like indicated in the DCO check output?

Address julien-nc's review: - Shorten the long rationale comment to a one-liner with the spec link, per inline review at lib/Controller/LoginController.php:940. - Drop the redundant ' per spec' suffix from both already-logged-out debug log lines (lib/Controller/LoginController.php:942 and :972), matching the suggested edits. Behaviour is unchanged — only comment / log message wording. Signed-off-by: SAY-5 <say.apm35@gmail.com>

SAY-5 · 2026-04-30T21:16:34Z

@julien-nc the DCO check is currently passing — both commits on this branch carry Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>. Latest run: ✅ DCO (https://github.com/nextcloud/user_oidc/actions). If the maintainer-side merge UI is showing the GitHub commit-verification badge as unverified, that's a separate signing concern — happy to re-push with SSH-verified signatures if that's the blocker. Let me know which one you saw failing.

julien-nc · 2026-05-01T07:30:08Z

Thank you!

github-actions · 2026-05-13T02:21:10Z

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

julien-nc mentioned this pull request Apr 28, 2026

RP initiated logout makes backchannel logout throw an HTTP/400 error #1430

Closed

julien-nc requested changes Apr 29, 2026

View reviewed changes

julien-nc mentioned this pull request Apr 29, 2026

Align with backchannel logout OIDC spec #1432

Merged

julien-nc approved these changes Apr 29, 2026

View reviewed changes

SAY-5 force-pushed the fix/backchannel-logout-already-logged-out-1430 branch from aa87b8f to abe6772 Compare April 29, 2026 16:04

julien-nc merged commit dcc36fa into nextcloud:main May 1, 2026
45 checks passed

github-actions Bot added the feedback-requested label May 13, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(login): treat already-logged-out as success in backchannel logout (#1430)#1431

fix(login): treat already-logged-out as success in backchannel logout (#1430)#1431
julien-nc merged 2 commits into
nextcloud:mainfrom
SAY-5:fix/backchannel-logout-already-logged-out-1430

SAY-5 commented Apr 28, 2026

Uh oh!

Spitfireap commented Apr 28, 2026

Uh oh!

julien-nc left a comment

Uh oh!

julien-nc Apr 29, 2026

Uh oh!

julien-nc Apr 29, 2026

Uh oh!

julien-nc Apr 29, 2026

Uh oh!

SAY-5 commented Apr 29, 2026

Uh oh!

julien-nc commented Apr 29, 2026

Uh oh!

SAY-5 commented Apr 30, 2026

Uh oh!

Uh oh!

julien-nc commented May 1, 2026

Uh oh!

github-actions Bot commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	'[BackchannelLogout] no RP sessions for (sub,iss) — treating as already-logged-out per spec',
	'[BackchannelLogout] no RP sessions for (sub,iss) — treating as already-logged-out',

	'[BackchannelLogout] no RP session for (sid,iss) — treating as already-logged-out per spec',
	'[BackchannelLogout] no RP session for (sid,iss) — treating as already-logged-out',

Conversation

SAY-5 commented Apr 28, 2026

Summary

Change

Test plan

Notes

Uh oh!

Spitfireap commented Apr 28, 2026

Uh oh!

julien-nc left a comment

Choose a reason for hiding this comment

Uh oh!

julien-nc Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

julien-nc Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

julien-nc Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

SAY-5 commented Apr 29, 2026

Uh oh!

julien-nc commented Apr 29, 2026

Uh oh!

SAY-5 commented Apr 30, 2026

Uh oh!

Uh oh!

julien-nc commented May 1, 2026

Uh oh!

github-actions Bot commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants