Skip to content

fix: skip OIDC end-session after password login for OAuth2 users - #38439

Merged
bircni merged 5 commits into
go-gitea:mainfrom
Otto-Deviant1904:fix/password-login-skip-oidc-logout
Jul 27, 2026
Merged

fix: skip OIDC end-session after password login for OAuth2 users#38439
bircni merged 5 commits into
go-gitea:mainfrom
Otto-Deviant1904:fix/password-login-skip-oidc-logout

Conversation

@Otto-Deviant1904

@Otto-Deviant1904 Otto-Deviant1904 commented Jul 13, 2026 •
edited by bircni
Loading

Copy link
Copy Markdown
Contributor

Fixes #38209

OAuth2-linked accounts that sign in via the password form were still
redirected to the provider end_session_endpoint on logout because the
redirect was keyed off account LoginType.

Store the session sign-in method (password vs oauth2) and only use
RP-initiated OIDC logout when this session was authenticated via OAuth2.
Sessions without the new key keep the previous LoginType behavior.

AI disclosure: assisted with investigation, implementation, and tests.
I reviewed and verified the change and will handle review feedback myself.

Made with Cursor

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 13, 2026
Copilot AI review requested due to automatic review settings July 22, 2026 01:59
Copilot AI reviewed Jul 22, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an OIDC logout misredirect by tracking how the current session authenticated (password vs OAuth2) and using that session-scoped method—rather than the account’s persisted LoginType—to decide whether to perform RP-initiated OIDC logout.

Changes:

  • Add a session key (KeySignInMethod) to record whether the current session signed in via password or OAuth2.
  • Update logout redirect logic to prefer the session sign-in method, with a fallback to legacy LoginType behavior when the key is absent.
  • Extend tests to cover OAuth2-session logout redirect, legacy fallback, and password-session non-OIDC logout.

Reviewed changes

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

Show a summary per file
File Description
routers/web/auth/oauth.go Sets KeySignInMethod=oauth2 for OAuth2 sign-in (including pre-2FA session).
routers/web/auth/linkaccount.go Sets KeySignInMethod during link-account 2FA session (currently set to OAuth2; should be password for password-authenticated linking).
routers/web/auth/auth.go Preserves/sets sign-in method into the authenticated session and gates OIDC end-session redirect on it.
routers/web/auth/auth_test.go Adds subtests for OAuth2 session redirect, legacy fallback, and password session skipping OIDC logout.
modules/session/key.go Introduces KeySignInMethod and method constants.

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

Comment thread routers/web/auth/linkaccount.go Outdated
Comment thread routers/web/auth/auth_test.go Outdated
@Otto-Deviant1904

Otto-Deviant1904 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Friendly ping for review when you have a moment. Happy to rebase or adjust if anything looks off.

@wxiaoguang wxiaoguang mentioned this pull request Jul 27, 2026
Closed
wxiaoguang
wxiaoguang previously approved these changes Jul 27, 2026
View reviewed changes
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 27, 2026
@wxiaoguang wxiaoguang added the backport/v1.27 This PR should be backported to Gitea 1.27 label Jul 27, 2026
@Otto-Deviant1904
Otto-Deviant1904 force-pushed the fix/password-login-skip-oidc-logout branch from c25e837 to 4ab87bb Compare July 27, 2026 07:21
@wxiaoguang

wxiaoguang commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@Otto-Deviant1904 no, don't do that, don't force push, don't revert to your commit.

Have you reviewed my change? What's wrong?

@wxiaoguang
wxiaoguang dismissed their stale review July 27, 2026 07:26

dismiss

@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 27, 2026
Otto-Deviant1904 and others added 2 commits July 27, 2026 17:30
@Otto-Deviant1904 @cursoragent
fix: skip OIDC end-session after password login for OAuth2 users
f20ff0e 
Track the session sign-in method so logout only hits the OIDC
end_session_endpoint when this session authenticated via OAuth2.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wxiaoguang @cursoragent @Otto-Deviant1904
clean up
2358d4e 
Co-authored-by: Cursor <cursoragent@cursor.com>
@Otto-Deviant1904
Otto-Deviant1904 force-pushed the fix/password-login-skip-oidc-logout branch from 4ab87bb to 2358d4e Compare July 27, 2026 07:30
@Otto-Deviant1904

Otto-Deviant1904 commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Sorry - that force-push was a mistake. I was rebasing onto current main and overwrote your cleanup.

Restored your clean up commit on top of current main (same file contents as before). Happy to leave it alone from here unless you want something else changed.

@wxiaoguang

wxiaoguang commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Don't rebase or force-push

https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#maintaining-open-prs

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 27, 2026
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 27, 2026
@bircni
bircni enabled auto-merge (squash) July 27, 2026 14:13
@bircni bircni added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 27, 2026
@bircni
bircni merged commit e67dd4c into go-gitea:main Jul 27, 2026
23 checks passed
@GiteaBot GiteaBot added this to the 28.0.0 milestone Jul 27, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 27, 2026
@GiteaBot GiteaBot mentioned this pull request Jul 27, 2026
Merged
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Jul 27, 2026
bircni pushed a commit that referenced this pull request Jul 27, 2026
@GiteaBot @Otto-Deviant1904 @wxiaoguang
fix: skip OIDC end-session after password login for OAuth2 users (#38439
0d9ce64 
) (#38666)

Backport #38439 by @Otto-Deviant1904

Fixes #38209

OAuth2-linked accounts that sign in via the password form were still
redirected to the provider end_session_endpoint on logout because the
redirect was keyed off account LoginType.

Store the session sign-in method (password vs oauth2) and only use
RP-initiated OIDC logout when this session was authenticated via OAuth2.
Sessions without the new key keep the previous LoginType behavior.


Co-authored-by: Harsh Satyajit Thakur <f20240223@goa.bits-pilani.ac.in>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 30, 2026
@zjjhot
Merge branch 'main' of https://github.com/go-gitea/gitea
5fd11fa 
* 'main' of https://github.com/go-gitea/gitea:
  chore: tweak AGENTS.md (go-gitea#38702)
  [skip ci] Updated translations via Crowdin
  fix: avoid markup render panic (go-gitea#38698)
  fix(ui): too many participants shown in commit avatar stacks (go-gitea#38689)
  [skip ci] Updated translations via Crowdin
  chore: still keep ctx in git.Repository struct for cat-file batch command (go-gitea#38684)
  feat: Add support for dynamic matrix evaluation in Gitea Actions workflows (go-gitea#36564)
  fix: Alpine registry APKINDEX.tar.gz returns 405 for HEAD requests [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT] (go-gitea#38686)
  fix: repo home page 500 due to the timeout of "get last commit info" (go-gitea#38678)
  [skip ci] Updated translations via Crowdin
  docs: Update Changelog for release v1.27.1 (go-gitea#38670)
  refactor(git): clarify GetBranch behavior to make it only gets an existing branch (go-gitea#38662)
  chore(deps): update dependencies (go-gitea#38660)
  feat: Add max-parallel Support for Gitea Actions (go-gitea#36357)
  fix: skip OIDC end-session after password login for OAuth2 users (go-gitea#38439)
  ci: set AWS_REGION for Cloudflare R2 upload steps (go-gitea#38658)
  fix: make Actions log parser support multiple line message encoding (go-gitea#38659)
  test(pubsub): stop racing the Redis SUBSCRIBE ack (go-gitea#38661)
  chore(ci): fix renovate custom manager regex (go-gitea#38656)
  feat: Replace SSE with WebSocket for UI notifications (go-gitea#36965)
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 30, 2026
@zjjhot
Merge branch 'main' into mainzjj
d6e7915 
* main:
  chore: tweak AGENTS.md (go-gitea#38702)
  [skip ci] Updated translations via Crowdin
  fix: avoid markup render panic (go-gitea#38698)
  fix(ui): too many participants shown in commit avatar stacks (go-gitea#38689)
  [skip ci] Updated translations via Crowdin
  chore: still keep ctx in git.Repository struct for cat-file batch command (go-gitea#38684)
  feat: Add support for dynamic matrix evaluation in Gitea Actions workflows (go-gitea#36564)
  fix: Alpine registry APKINDEX.tar.gz returns 405 for HEAD requests [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT] (go-gitea#38686)
  fix: repo home page 500 due to the timeout of "get last commit info" (go-gitea#38678)
  [skip ci] Updated translations via Crowdin
  docs: Update Changelog for release v1.27.1 (go-gitea#38670)
  refactor(git): clarify GetBranch behavior to make it only gets an existing branch (go-gitea#38662)
  chore(deps): update dependencies (go-gitea#38660)
  feat: Add max-parallel Support for Gitea Actions (go-gitea#36357)
  fix: skip OIDC end-session after password login for OAuth2 users (go-gitea#38439)
  ci: set AWS_REGION for Cloudflare R2 upload steps (go-gitea#38658)
  fix: make Actions log parser support multiple line message encoding (go-gitea#38659)
  test(pubsub): stop racing the Redis SUBSCRIBE ack (go-gitea#38661)
  chore(ci): fix renovate custom manager regex (go-gitea#38656)
  feat: Replace SSE with WebSocket for UI notifications (go-gitea#36965)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@wxiaoguang wxiaoguang wxiaoguang approved these changes
@bircni bircni bircni approved these changes

Assignees

No one assigned

Labels

backport/done All backports for this PR have been created backport/v1.27 This PR should be backported to Gitea 1.27 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Milestone

28.0.0

Development

Successfully merging this pull request may close these issues.

OAuth2 user logged in via password form is redirected to OIDC logout endpoint on sign out

5 participants

@Otto-Deviant1904 @wxiaoguang @bircni @GiteaBot