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 <wxiaoguang@gmail.com>

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 {

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) {

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(`<img class="tw-object-contain tw-mr-2" width="%d" height="%d" src="%s" alt="%s">`,
+		img := fmt.Sprintf(`<img class="tw-object-contain" width="%d" height="%d" src="%s" alt="%s">`,
 			size,
 			size,
 			html.EscapeString(p.iconURL), html.EscapeString(p.DisplayName()),

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
 }

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

templates/user/auth/external_auth_methods.tmpl

@@ -0,0 +1,18 @@
+<div id="external-login-navigator" class="tw-py-1 tw-flex tw-flex-col tw-gap-3">
+	{{range $provider := .OAuth2Providers}}
+		{{/* use QueryEscape for consistent with frontend urlQueryEscape, it is right for a path component */}}
+		<a class="ui button external-login-link tw-gap-3" data-require-appurl-check="true" rel="nofollow" href="{{AppSubUrl}}/user/oauth2/{{$provider.DisplayName | QueryEscape}}">
+			{{$provider.IconHTML 24}} {{ctx.Locale.Tr "sign_in_with_provider" $provider.DisplayName}}
+		</a>
+	{{end}}
+	{{if .EnableOpenIDSignIn}}
+		<a class="ui button external-login-link tw-gap-3" data-require-appurl-check="true" rel="nofollow" href="{{AppSubUrl}}/user/login/openid">
+			{{svg "fontawesome-openid" 24}} {{ctx.Locale.Tr "sign_in_with_provider" "OpenID"}}
+		</a>
+	{{end}}
+	{{if .EnableSSPI}}
+		<a class="ui button external-login-link tw-gap-3" rel="nofollow" href="{{AppSubUrl}}/user/login?auth_with_sspi=1">
+			{{svg "fontawesome-windows" 24}} Windows SSPI
+		</a>
+	{{end}}
+</div>

templates/user/auth/oauth_container.tmpl

@@ -1,24 +0,0 @@
-<div id="oauth2-login-navigator" class="tw-py-1">
-	<div class="tw-flex tw-flex-col tw-justify-center">
-		<div id="oauth2-login-navigator-inner" class="tw-flex tw-flex-col tw-flex-wrap tw-items-center tw-gap-2">
-			{{range $provider := .OAuth2Providers}}
-				<a class="{{$provider.Name}} ui button tw-flex tw-items-center tw-justify-center tw-py-2 tw-w-full oauth-login-link" href="{{AppSubUrl}}/user/oauth2/{{$provider.DisplayName}}">
-					{{$provider.IconHTML 28}}
-					{{ctx.Locale.Tr "sign_in_with_provider" $provider.DisplayName}}
-				</a>
-			{{end}}
-			{{if .EnableOpenIDSignIn}}
-				<a class="openid ui button tw-flex tw-items-center tw-justify-center tw-py-2 tw-w-full" href="{{AppSubUrl}}/user/login/openid">
-				{{svg "fontawesome-openid" 28 "tw-mr-2"}}
-				{{ctx.Locale.Tr "sign_in_with_provider" "OpenID"}}
-				</a>
-			{{end}}
-			{{if .EnableSSPI}}
-				<a class="ui button tw-flex tw-items-center tw-justify-center tw-py-2 tw-w-full" rel="nofollow" href="{{AppSubUrl}}/user/login?auth_with_sspi=1">
-					{{svg "fontawesome-windows"}}
-					&nbsp;SSPI
-				</a>
-			{{end}}
-		</div>
-	</div>
-</div>

templates/user/auth/signin_inner.tmpl

@@ -46,14 +46,13 @@
 				</button>
 			</div>
 		</form>
-		{{end}}{{/*if .EnablePasswordSignInForm*/}}
+		{{end}}{{/*end if .EnablePasswordSignInForm*/}}
-		{{/* "oauth_container" contains not only "oauth2" methods, but also "OIDC" and "SSPI" methods */}}
+		{{$showExternalAuthMethods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}}
-		{{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}}
+		{{if and $showExternalAuthMethods .EnablePasswordSignInForm}}
-		{{if and $showOAuth2Methods .EnablePasswordSignInForm}}
 			<div class="divider divider-text">{{ctx.Locale.Tr "sign_in_or"}}</div>
 		{{end}}
-		{{if $showOAuth2Methods}}
+		{{if $showExternalAuthMethods}}
-			{{template "user/auth/oauth_container" .}}
+			{{template "user/auth/external_auth_methods" .}}
 		{{end}}
 	</div>
 </div>

templates/user/auth/signup_inner.tmpl

@@ -49,12 +49,10 @@
 					</button>
 				</div>
 			{{end}}
-			{{/* "oauth_container" contains not only "oauth2" methods, but also "OIDC" and "SSPI" methods */}}
+			{{$showExternalAuthMethods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}}
-			{{/* TODO: it seems that "EnableSSPI" is only set in "sign-in" handlers, but it should use the same logic to control its display */}}
+			{{if $showExternalAuthMethods}}
-			{{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}}
-			{{if $showOAuth2Methods}}
 				<div class="divider divider-text">{{ctx.Locale.Tr "sign_in_or"}}</div>
-				{{template "user/auth/oauth_container" .}}
+				{{template "user/auth/external_auth_methods" .}}
 			{{end}}
 		</form>
 	</div>

web_src/js/features/user-auth.ts

@@ -5,23 +5,23 @@ export function initUserCheckAppUrl() {
   checkAppUrlScheme();
 }
 
-export function initUserAuthOauth2() {
+export function initUserExternalLogins() {
-  const outer = document.querySelector('#oauth2-login-navigator');
+  const container = document.querySelector('#external-login-navigator');
-  if (!outer) return;
+  if (!container) return;
-  const inner = document.querySelector('#oauth2-login-navigator-inner')!;
 
-  checkAppUrl();
+  // whether the auth method requires app url check (need consistent ROOT_URL with visited URL)
-
+  let needCheckAppUrl = false;
-  for (const link of outer.querySelectorAll('.oauth-login-link')) {
+  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');
+      container.classList.add('is-loading');
-      outer.classList.add('is-loading');
       setTimeout(() => {
-        // recover previous content to let user try again
+        // recover previous content to let user try again, usually redirection will be performed before this action
-        // usually redirection will be performed before this action
+        container.classList.remove('is-loading');
-        outer.classList.remove('is-loading');
-        inner.classList.remove('tw-invisible');
       }, 5000);
     });
   }
+  if (needCheckAppUrl) {
+    checkAppUrl();
+  }
 }

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,