Skip to content

Redirect to the only OAuth2 provider when no other login methods and fix various problems#36901

Merged
wxiaoguang merged 14 commits intogo-gitea:mainfrom
navneet102:redirect-to-only-login-provider
Apr 1, 2026
Merged

Redirect to the only OAuth2 provider when no other login methods and fix various problems#36901
wxiaoguang merged 14 commits intogo-gitea:mainfrom
navneet102:redirect-to-only-login-provider

Conversation

@navneet102
Copy link
Copy Markdown
Contributor

@navneet102 navneet102 commented Mar 14, 2026

Fixes: #36846

  1. When there is only on OAuth2 login method, automatically direct to it
  2. Fix legacy problems in code, including:
    • Rename template filename and fix TODO comments
    • Fix legacy variable names
    • Add missing SSPI variable for template
    • Fix unnecessary layout, remove garbage styles
    • Only do AppUrl(ROOT_URL) check when it is needed (avoid unnecessary warnings to end users)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 14, 2026
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 14, 2026
@navneet102 navneet102 marked this pull request as draft March 14, 2026 11:32
@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Mar 14, 2026

  • Added skipAutoLogin URL param to bypass this feature by including ?skipAutoLogin=true

What will it be used for? Who will use it? Any real world use case for it?

If it is not used by our code, not clearly documented for real world use cases, then it doesn't make sense.


Update: added screenshots for the UI changes:

image

@navneet102
Copy link
Copy Markdown
Contributor Author

I included it because it was mentioned in this comment, it could be used if auth source is unavailable.

What do you suggest, should I?

  1. Document it.
  2. Remove it.

@navneet102 navneet102 marked this pull request as ready for review March 14, 2026 17:37
@navneet102
Copy link
Copy Markdown
Contributor Author

@wxiaoguang I have added some tests. Could you check this now please?

@wxiaoguang
Copy link
Copy Markdown
Contributor

  • Added skipAutoLogin URL param to bypass this feature by including ?skipAutoLogin=true

What will it be used for? Who will use it? Any real world use case for it?

If it is not used by our code, not clearly documented for real world use cases, then it doesn't make sense.

I included it because it was mentioned in this comment, it could be used if auth source is unavailable.

What do you suggest, should I?

1. Document it.

2. Remove it.

I don't understand it

@navneet102
Copy link
Copy Markdown
Contributor Author

Sorry for being vague.

  • Added skipAutoLogin URL param to bypass this feature by including ?skipAutoLogin=true

What will it be used for? Who will use it? Any real world use case for it?

If it is not used by our code, not clearly documented for real world use cases, then it doesn't make sense.

I think you asked here about why I included the skipAutoLogin URL parameter if there was no documentation for it.

In the reply, I mentioned that it was a design choice specified by @silverwind while accepting the feature proposal in this issue's (#36846 ) discussion.

What do you suggest, should I?

  1. Document it.
  2. Remove it.

Here I asked you, if I should remove the skipAutoLogin URL param or should I add documentation for it.

@wxiaoguang
Copy link
Copy Markdown
Contributor

What do you suggest, should I?

  1. Document it.
  2. Remove it.

Here I asked you, if I should remove the skipAutoLogin URL param or should I add documentation for it.

As I said, I don't understand it.

You are the author so you need to propose reasonable and feasible designs.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Mar 20, 2026

You can explain your design by answering the questions:

  1. For "Document it": why it is useful, in which case it will be used, how end users can know how to use it.
    • "useful when the auth source is temporarily unavailable": you already have !EnablePasswordSignInForm check, so users can do nothing on the login page even if the auth source is unavailable
    • I don't think such reason makes sense.
  2. For "Remove it": why the extra parameter is useless, why the suggestion from silverwind (If there is just 1 login provider configured, redirect to it #36846 (comment)) is not right.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an auto-redirect behavior on the /login page to streamline sign-in when only a single OAuth2 auth source is available and all other sign-in methods are disabled, with a skipAutoLogin=true escape hatch.

Changes:

  • Implement performAutoLoginOAuth2 and invoke it from SignIn under a “OAuth2-only” configuration.
  • Add a unit test covering the auto-redirect behavior and skipAutoLogin bypass.

Reviewed changes

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

File Description
routers/web/auth/auth.go Adds OAuth2-only auto-redirect logic when rendering /login.
routers/web/auth/auth_test.go Adds a unit test validating redirect + bypass behavior.

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

@wxiaoguang wxiaoguang marked this pull request as draft March 21, 2026 19:39
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Mar 28, 2026

I have to correct my request: skipAutoLogin only makes sense if there is more than 1 login option. If there is only 1 auto-login-able action (like only sso button), its not needed. As I understand the PR handles only the case of 1 SSO option, so we don't need it for this use case.

@navneet102
Copy link
Copy Markdown
Contributor Author

Thank you everyone for the review, and sorry for the late reply. I have removed the skipAutoLogin param. Currently there is no solution for situations when OAuth service is unavailable because after being redirected (to OAuth provider) we cannot do anything until SignInOAuthCallback is called. The only solution would be to check for the availability of the service before redirection, which would be slow.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Mar 31, 2026

  1. performAutoLoginOAuth2 reads OAuth2Providers from ctx.Data which creates a fragile implicit dependency on prepareSignInPageData having been called first. As wxiaoguang noted, these should be return values of prepareSignInPageData instead.

  2. The test only covers the happy path (single provider + redirect_to). Consider adding cases for:

    1. 0 OAuth providers → should render login page
    2. 2+ OAuth providers → should render login page
    3. No redirect_to query param → should redirect without query string
    4. Any of EnablePasswordSignInForm/EnableOpenIDSignIn/EnablePasskeyAuth/SSPI enabled → should render login page, not redirect

This comment was written by Claude Opus 4.6.

@github-actions github-actions bot added modifies/templates This PR modifies the template files modifies/frontend labels Apr 1, 2026
@wxiaoguang wxiaoguang changed the title Redirect to the only oAuth2 provider when other login methods are disabled Redirect to the only OAuth2 provider when other login methods are disabled and fix various problems Apr 1, 2026
@wxiaoguang wxiaoguang changed the title Redirect to the only OAuth2 provider when other login methods are disabled and fix various problems Redirect to the only OAuth2 provider when no other login methods and fix various problems Apr 1, 2026
@wxiaoguang wxiaoguang force-pushed the redirect-to-only-login-provider branch from 998eeef to b10e68b Compare April 1, 2026 01:29
@wxiaoguang wxiaoguang marked this pull request as ready for review April 1, 2026 01:36
@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 Apr 1, 2026
@silverwind
Copy link
Copy Markdown
Member

Would still add those test cases mentioned in #36901 (comment). If you like I can exercise AI on it, or you do it yourself.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Would still add those test cases mentioned in #36901 (comment). If you like I can exercise AI on it, or you do it yourself.

Have you read the new code by yourself?

image

@wxiaoguang wxiaoguang requested a review from silverwind April 1, 2026 11:59
@silverwind
Copy link
Copy Markdown
Member

Ok, but a test with 2 providers is still missing it seems?

@wxiaoguang
Copy link
Copy Markdown
Contributor

Ok, but a test with 2 providers is still missing it seems?

image

@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 Apr 1, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 1, 2026 12:22
@wxiaoguang wxiaoguang merged commit 3ffccb8 into go-gitea:main Apr 1, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.26.0 milestone Apr 1, 2026
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 1, 2026
* origin/main:
  Update JS dependencies and misc tweaks (go-gitea#37064)
  Redirect to the only OAuth2 provider when no other login methods and fix various problems (go-gitea#36901)
  Show workflow link (go-gitea#37070)
  Remove leftover `webpackChunkName` comments from codeeditor (go-gitea#37062)
  Update Go dependencies (go-gitea#36781)
  Add webhook name field to improve webhook identification (go-gitea#37025) (go-gitea#37040)
  Upgrade `go-git` to v5.17.2 (go-gitea#37060)
  Replace Monaco with CodeMirror (go-gitea#36764)
  Update Combine method to treat warnings as failures and adjust tests (go-gitea#37048)
  Raise minimum Node.js version to 22.18.0 (go-gitea#37058)
  Update golangci-lint to v2.11.4 (go-gitea#37059)
  Upgrade `golang.org/x/image` to v0.38.0 (go-gitea#37054)

# Conflicts:
#	web_src/css/themes/theme-gitea-dark.css
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 2, 2026
* main:
  Fix NuGet package upload error handling (go-gitea#37074)
  Desaturate dark theme background colors (go-gitea#37056)
  Update JS dependencies and misc tweaks (go-gitea#37064)
  Redirect to the only OAuth2 provider when no other login methods and fix various problems (go-gitea#36901)
  Show workflow link (go-gitea#37070)
  Remove leftover `webpackChunkName` comments from codeeditor (go-gitea#37062)
  Update Go dependencies (go-gitea#36781)
  Add webhook name field to improve webhook identification (go-gitea#37025) (go-gitea#37040)
  Upgrade `go-git` to v5.17.2 (go-gitea#37060)
  Replace Monaco with CodeMirror (go-gitea#36764)
  Update Combine method to treat warnings as failures and adjust tests (go-gitea#37048)
  Raise minimum Node.js version to 22.18.0 (go-gitea#37058)
  Update golangci-lint to v2.11.4 (go-gitea#37059)
  Upgrade `golang.org/x/image` to v0.38.0 (go-gitea#37054)
  Increase e2e test timeouts on CI to fix flaky tests (go-gitea#37053)
  Refactor "org teams" page and help new users to "add member" to an org (go-gitea#37051)
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 2, 2026
* origin/main: (192 commits)
  Fix NuGet package upload error handling (go-gitea#37074)
  Desaturate dark theme background colors (go-gitea#37056)
  Update JS dependencies and misc tweaks (go-gitea#37064)
  Redirect to the only OAuth2 provider when no other login methods and fix various problems (go-gitea#36901)
  Show workflow link (go-gitea#37070)
  Remove leftover `webpackChunkName` comments from codeeditor (go-gitea#37062)
  Update Go dependencies (go-gitea#36781)
  Add webhook name field to improve webhook identification (go-gitea#37025) (go-gitea#37040)
  Upgrade `go-git` to v5.17.2 (go-gitea#37060)
  Replace Monaco with CodeMirror (go-gitea#36764)
  Update Combine method to treat warnings as failures and adjust tests (go-gitea#37048)
  Raise minimum Node.js version to 22.18.0 (go-gitea#37058)
  Update golangci-lint to v2.11.4 (go-gitea#37059)
  Upgrade `golang.org/x/image` to v0.38.0 (go-gitea#37054)
  Increase e2e test timeouts on CI to fix flaky tests (go-gitea#37053)
  Refactor "org teams" page and help new users to "add member" to an org (go-gitea#37051)
  Refactor issue sidebar and fix various problems (go-gitea#37045)
  Add tests for pull request's content_version in API (go-gitea#37044)
  Enable concurrent vitest execution (go-gitea#36998)
  Fix theme discovery and Vite dev server in dev mode (go-gitea#37033)
  ...

# Conflicts:
#	templates/user/dashboard/feeds.tmpl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

If there is just 1 login provider configured, redirect to it

5 participants