From 3ffccb8fe586c8e8ec680ec26a37809d48b2ee6f Mon Sep 17 00:00:00 2001 From: Navneet <144308688+navneet102@users.noreply.github.com> Date: Wed, 1 Apr 2026 18:20:57 +0530 Subject: [PATCH] Redirect to the only OAuth2 provider when no other login methods and fix various problems (#36901) 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) --------- Co-authored-by: wxiaoguang --- routers/web/auth/auth.go | 44 +++++++++++++++++-- routers/web/auth/auth_test.go | 31 +++++++++++++ services/auth/source/oauth2/providers.go | 2 +- services/auth/source/oauth2/providers_base.go | 4 +- .../auth/source/oauth2/providers_openid.go | 2 +- .../user/auth/external_auth_methods.tmpl | 18 ++++++++ templates/user/auth/oauth_container.tmpl | 24 ---------- templates/user/auth/signin_inner.tmpl | 11 +++-- templates/user/auth/signup_inner.tmpl | 8 ++-- web_src/js/features/user-auth.ts | 26 +++++------ web_src/js/index.ts | 4 +- 11 files changed, 116 insertions(+), 58 deletions(-) create mode 100644 templates/user/auth/external_auth_methods.tmpl delete mode 100644 templates/user/auth/oauth_container.tmpl diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 1219690200..b0f5a9ed79 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -218,18 +218,50 @@ func performAutoLogin(ctx *context.Context) bool { return false } -func prepareSignInPageData(ctx *context.Context) { +func performAutoLoginOAuth2(ctx *context.Context, data *preparedSignInData) bool { + // If only 1 OAuth provider is present and other login methods are disabled, redirect to the OAuth provider. + onlySingleOAuth2 := len(data.oauth2Providers) == 1 && + !setting.Service.EnablePasswordSignInForm && + !setting.Service.EnableOpenIDSignIn && + !setting.Service.EnablePasskeyAuth && + !data.enableSSPI + + if !onlySingleOAuth2 { + return false + } + + skipToOAuthURL := setting.AppSubURL + "/user/oauth2/" + url.QueryEscape(data.oauth2Providers[0].DisplayName()) + if redirectTo := ctx.FormString("redirect_to"); redirectTo != "" { + skipToOAuthURL += "?redirect_to=" + url.QueryEscape(redirectTo) + } + ctx.Redirect(skipToOAuthURL) + return true +} + +type preparedSignInData struct { + oauth2Providers []oauth2.Provider + enableSSPI bool +} + +func prepareSignInPageData(ctx *context.Context) (ret preparedSignInData) { + var err error + ret.enableSSPI = auth.IsSSPIEnabled(ctx) + ret.oauth2Providers, err = oauth2.GetOAuth2Providers(ctx, optional.Some(true)) + if err != nil { + log.Error("Failed to get OAuth2 providers: %v", err) + } ctx.Data["Title"] = ctx.Tr("sign_in") - ctx.Data["OAuth2Providers"], _ = oauth2.GetOAuth2Providers(ctx, optional.Some(true)) + ctx.Data["OAuth2Providers"] = ret.oauth2Providers ctx.Data["Title"] = ctx.Tr("sign_in") ctx.Data["SignInLink"] = setting.AppSubURL + "/user/login" ctx.Data["PageIsSignIn"] = true ctx.Data["PageIsLogin"] = true - ctx.Data["EnableSSPI"] = auth.IsSSPIEnabled(ctx) + ctx.Data["EnableSSPI"] = ret.enableSSPI prepareCommonAuthPageData(ctx, CommonAuthOptions{ EnableCaptcha: setting.Service.EnableCaptcha && setting.Service.RequireCaptchaForLogin, }) + return ret } // SignIn render sign in page @@ -241,7 +273,10 @@ func SignIn(ctx *context.Context) { redirectAfterAuth(ctx) return } - prepareSignInPageData(ctx) + data := prepareSignInPageData(ctx) + if performAutoLoginOAuth2(ctx, &data) { + return + } ctx.HTML(http.StatusOK, tplSignIn) } @@ -471,6 +506,7 @@ func prepareSignUpPageData(ctx *context.Context) bool { ctx.Data["Title"] = ctx.Tr("sign_up") ctx.Data["SignUpLink"] = setting.AppSubURL + "/user/sign_up" ctx.Data["PageIsSignUp"] = true + ctx.Data["EnableSSPI"] = auth.IsSSPIEnabled(ctx) hasUsers, err := user_model.HasUsers(ctx) if err != nil { diff --git a/routers/web/auth/auth_test.go b/routers/web/auth/auth_test.go index d1f808181a..943085a963 100644 --- a/routers/web/auth/auth_test.go +++ b/routers/web/auth/auth_test.go @@ -96,6 +96,37 @@ func TestWebAuthOAuth2(t *testing.T) { assert.Contains(t, ctx.Flash.ErrorMsg, "auth.oauth.signin.error.general") }) + t.Run("RedirectSingleProvider", func(t *testing.T) { + enablePassword := &setting.Service.EnablePasswordSignInForm + enableOpenID := &setting.Service.EnableOpenIDSignIn + enablePasskey := &setting.Service.EnablePasskeyAuth + defer test.MockVariableValue(enablePassword, false)() + defer test.MockVariableValue(enableOpenID, false)() + defer test.MockVariableValue(enablePasskey, false)() + + testSignIn := func(t *testing.T, link string, expectedCode int, expectedRedirect string) { + ctx, resp := contexttest.MockContext(t, link) + SignIn(ctx) + assert.Equal(t, expectedCode, resp.Code) + if expectedCode == http.StatusSeeOther { + assert.Equal(t, expectedRedirect, test.RedirectURL(resp)) + } + } + testSignIn(t, "/user/login", http.StatusSeeOther, "/user/oauth2/dummy-auth-source") + testSignIn(t, "/user/login?redirect_to=/", http.StatusSeeOther, "/user/oauth2/dummy-auth-source?redirect_to=%2F") + + *enablePassword, *enableOpenID, *enablePasskey = true, false, false + testSignIn(t, "/user/login", http.StatusOK, "") + *enablePassword, *enableOpenID, *enablePasskey = false, true, false + testSignIn(t, "/user/login", http.StatusOK, "") + *enablePassword, *enableOpenID, *enablePasskey = false, false, true + testSignIn(t, "/user/login", http.StatusOK, "") + + *enablePassword, *enableOpenID, *enablePasskey = false, false, false + addOAuth2Source(t, "dummy-auth-source-2", oauth2.Source{}) + testSignIn(t, "/user/login", http.StatusOK, "") + }) + t.Run("OIDCLogout", func(t *testing.T) { var mockServer *httptest.Server mockServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/services/auth/source/oauth2/providers.go b/services/auth/source/oauth2/providers.go index 68bd4a1d4c..f719d23fc3 100644 --- a/services/auth/source/oauth2/providers.go +++ b/services/auth/source/oauth2/providers.go @@ -62,7 +62,7 @@ func (p *AuthSourceProvider) DisplayName() string { func (p *AuthSourceProvider) IconHTML(size int) template.HTML { if p.iconURL != "" { - img := fmt.Sprintf(`%s`, + img := fmt.Sprintf(`%s`, size, size, html.EscapeString(p.iconURL), html.EscapeString(p.DisplayName()), diff --git a/services/auth/source/oauth2/providers_base.go b/services/auth/source/oauth2/providers_base.go index d34597d6d9..bc761e9742 100644 --- a/services/auth/source/oauth2/providers_base.go +++ b/services/auth/source/oauth2/providers_base.go @@ -42,10 +42,10 @@ func (b *BaseProvider) IconHTML(size int) template.HTML { case "github": svgName = "octicon-mark-github" } - svgHTML := svg.RenderHTML(svgName, size, "tw-mr-2") + svgHTML := svg.RenderHTML(svgName, size) if svgHTML == "" { log.Error("No SVG icon for oauth2 provider %q", b.name) - svgHTML = svg.RenderHTML("gitea-openid", size, "tw-mr-2") + svgHTML = svg.RenderHTML("gitea-openid", size) } return svgHTML } diff --git a/services/auth/source/oauth2/providers_openid.go b/services/auth/source/oauth2/providers_openid.go index e86dc48232..fc0d77a7e6 100644 --- a/services/auth/source/oauth2/providers_openid.go +++ b/services/auth/source/oauth2/providers_openid.go @@ -33,7 +33,7 @@ func (o *OpenIDProvider) DisplayName() string { // IconHTML returns icon HTML for this provider func (o *OpenIDProvider) IconHTML(size int) template.HTML { - return svg.RenderHTML("gitea-openid", size, "tw-mr-2") + return svg.RenderHTML("gitea-openid", size) } // CreateGothProvider creates a GothProvider from this Provider diff --git a/templates/user/auth/external_auth_methods.tmpl b/templates/user/auth/external_auth_methods.tmpl new file mode 100644 index 0000000000..c23cab6565 --- /dev/null +++ b/templates/user/auth/external_auth_methods.tmpl @@ -0,0 +1,18 @@ +
+ {{range $provider := .OAuth2Providers}} + {{/* use QueryEscape for consistent with frontend urlQueryEscape, it is right for a path component */}} + + {{end}} + {{if .EnableOpenIDSignIn}} + + {{end}} + {{if .EnableSSPI}} + + {{end}} +
diff --git a/templates/user/auth/oauth_container.tmpl b/templates/user/auth/oauth_container.tmpl deleted file mode 100644 index d01aaefe1a..0000000000 --- a/templates/user/auth/oauth_container.tmpl +++ /dev/null @@ -1,24 +0,0 @@ -
-
- -
-
diff --git a/templates/user/auth/signin_inner.tmpl b/templates/user/auth/signin_inner.tmpl index f86195b0ef..864e0993d6 100644 --- a/templates/user/auth/signin_inner.tmpl +++ b/templates/user/auth/signin_inner.tmpl @@ -46,14 +46,13 @@ - {{end}}{{/*if .EnablePasswordSignInForm*/}} - {{/* "oauth_container" contains not only "oauth2" methods, but also "OIDC" and "SSPI" methods */}} - {{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}} - {{if and $showOAuth2Methods .EnablePasswordSignInForm}} + {{end}}{{/*end if .EnablePasswordSignInForm*/}} + {{$showExternalAuthMethods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}} + {{if and $showExternalAuthMethods .EnablePasswordSignInForm}}
{{ctx.Locale.Tr "sign_in_or"}}
{{end}} - {{if $showOAuth2Methods}} - {{template "user/auth/oauth_container" .}} + {{if $showExternalAuthMethods}} + {{template "user/auth/external_auth_methods" .}} {{end}} diff --git a/templates/user/auth/signup_inner.tmpl b/templates/user/auth/signup_inner.tmpl index 339118ac78..0c1f1a3906 100644 --- a/templates/user/auth/signup_inner.tmpl +++ b/templates/user/auth/signup_inner.tmpl @@ -49,12 +49,10 @@ {{end}} - {{/* "oauth_container" contains not only "oauth2" methods, but also "OIDC" and "SSPI" methods */}} - {{/* TODO: it seems that "EnableSSPI" is only set in "sign-in" handlers, but it should use the same logic to control its display */}} - {{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}} - {{if $showOAuth2Methods}} + {{$showExternalAuthMethods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}} + {{if $showExternalAuthMethods}}
{{ctx.Locale.Tr "sign_in_or"}}
- {{template "user/auth/oauth_container" .}} + {{template "user/auth/external_auth_methods" .}} {{end}} diff --git a/web_src/js/features/user-auth.ts b/web_src/js/features/user-auth.ts index 24c34b4aa5..83e6fb71c5 100644 --- a/web_src/js/features/user-auth.ts +++ b/web_src/js/features/user-auth.ts @@ -5,23 +5,23 @@ export function initUserCheckAppUrl() { checkAppUrlScheme(); } -export function initUserAuthOauth2() { - const outer = document.querySelector('#oauth2-login-navigator'); - if (!outer) return; - const inner = document.querySelector('#oauth2-login-navigator-inner')!; +export function initUserExternalLogins() { + const container = document.querySelector('#external-login-navigator'); + if (!container) return; - checkAppUrl(); - - for (const link of outer.querySelectorAll('.oauth-login-link')) { + // whether the auth method requires app url check (need consistent ROOT_URL with visited URL) + let needCheckAppUrl = false; + for (const link of container.querySelectorAll('.external-login-link')) { + needCheckAppUrl = needCheckAppUrl || link.getAttribute('data-require-appurl-check') === 'true'; link.addEventListener('click', () => { - inner.classList.add('tw-invisible'); - outer.classList.add('is-loading'); + container.classList.add('is-loading'); setTimeout(() => { - // recover previous content to let user try again - // usually redirection will be performed before this action - outer.classList.remove('is-loading'); - inner.classList.remove('tw-invisible'); + // recover previous content to let user try again, usually redirection will be performed before this action + container.classList.remove('is-loading'); }, 5000); }); } + if (needCheckAppUrl) { + checkAppUrl(); + } } diff --git a/web_src/js/index.ts b/web_src/js/index.ts index 5e2cde28ee..d4d5ea6cff 100644 --- a/web_src/js/index.ts +++ b/web_src/js/index.ts @@ -20,7 +20,7 @@ import {initStopwatch} from './features/stopwatch.ts'; import {initRepoFileSearch} from './features/repo-findfile.ts'; import {initMarkupContent} from './markup/content.ts'; import {initRepoFileView} from './features/file-view.ts'; -import {initUserAuthOauth2, initUserCheckAppUrl} from './features/user-auth.ts'; +import {initUserExternalLogins, initUserCheckAppUrl} from './features/user-auth.ts'; import {initRepoPullRequestReview, initRepoIssueFilterItemLabel} from './features/repo-issue.ts'; import {initRepoEllipsisButton, initCommitStatuses} from './features/repo-commit.ts'; import {initRepoTopicBar} from './features/repo-home.ts'; @@ -149,7 +149,7 @@ const initPerformanceTracer = callInitFunctions([ initCaptcha, initUserCheckAppUrl, - initUserAuthOauth2, + initUserExternalLogins, initUserAuthWebAuthn, initUserAuthWebAuthnRegister, initUserSettings,