Skip to content

Align with backchannel logout OIDC spec#1432

Open
Spitfireap wants to merge 7 commits intonextcloud:mainfrom
Spitfireap:fix/1430-backchannel-oidc-spec
Open

Align with backchannel logout OIDC spec#1432
Spitfireap wants to merge 7 commits intonextcloud:mainfrom
Spitfireap:fix/1430-backchannel-oidc-spec

Conversation

@Spitfireap
Copy link
Copy Markdown

@Spitfireap Spitfireap commented Apr 28, 2026

fixes #1430

Current behaviour

Backchannel logout yields a HTTP/400 error when the session is not retrieved.
The spec states that it should be considered as a success and thus should return HTTP/200 :

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.

Furthermore the response seems to be missing the Cache-Control header (spec).

The BC logout validation is also missing exp token validation (not expired) and iss validation (equals to issuer in discovery endpoint) per Backchannel logout token validation spec and ID Token validation spec

Changes

  • Yield a success (HTTP/200) when the session is not found instead of an error (HTTP/400)
  • Removed an unused variable from getBackchannelLogoutErrorResponse ($throttleMetadata)
  • Increased the severity of the logging when a Backchannel logout fails
  • Added the Cache-Control header in the backchannel logout response
  • Added a verification for the exp and iss claim

@Spitfireap Spitfireap force-pushed the fix/1430-backchannel-oidc-spec branch from da506df to 149ded7 Compare April 28, 2026 18:33
@Spitfireap
Copy link
Copy Markdown
Author

@julien-nc not entirely sure about the change for the error logging.
I think the severity should be increased since it might be a security issue. The thing is : could these be triggered by a random idp and in that case we should perhaps be cautious about using error or it requires the idp to be verified so error would be well suited ?

Copy link
Copy Markdown
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks. Let's get #1431 in first, then you can rebase and adjust this one.

Comment thread lib/Controller/LoginController.php Outdated
Comment thread lib/Controller/LoginController.php
@Spitfireap Spitfireap force-pushed the fix/1430-backchannel-oidc-spec branch from 302aee6 to 0478e3e Compare April 29, 2026 11:56
see nextcloud#1430

Signed-off-by: Spitap <dev@asdrip.fr>
not used since 9b5d6c6

Signed-off-by: Spitap <dev@asdrip.fr>
Admins should be aware that a BCLO attempt failed since it might be a security issue.
On the other side we shouldn't spam them if an unknown IdP is attempting a logout

Signed-off-by: Spitap <dev@asdrip.fr>
Signed-off-by: Spitap <dev@asdrip.fr>
Signed-off-by: Spitap <dev@asdrip.fr>
@Spitfireap Spitfireap force-pushed the fix/1430-backchannel-oidc-spec branch from 0478e3e to 05b982d Compare April 29, 2026 12:07
Signed-off-by: Spitap <dev@asdrip.fr>
Signed-off-by: Spitap <dev@asdrip.fr>
@Spitfireap
Copy link
Copy Markdown
Author

Spitfireap commented Apr 29, 2026

Added some more alignment with the spec (exp + 1 iss validation step).

Was wondering if we should verify that jti and iat claims are there as well (since they are required) or am I overdoing it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants