Fix Account linking page (#33325) (#33327)

Backport #33325 by CrimsonEdgeHope

Fix password form missing whilst linking account even with
`ENABLE_PASSWORD_SIGNIN_FORM = true`.

Remove redundant empty box in account linking sign up page when
`LinkAccountMode` is true.

Co-authored-by: CrimsonEdgeHope <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>

Author: 3 people

routers/web/auth/linkaccount.go

@@ -29,6 +29,7 @@ var tplLinkAccount base.TplName = "user/auth/link_account"
 
 // LinkAccount shows the page where the user can decide to login or create a new account
 func LinkAccount(ctx *context.Context) {
+	// FIXME: these common template variables should be prepared in one common function, but not just copy-paste again and again.
 	ctx.Data["DisablePassword"] = !setting.Service.RequireExternalRegistrationPassword || setting.Service.AllowOnlyExternalRegistration
 	ctx.Data["Title"] = ctx.Tr("link_account")
 	ctx.Data["LinkAccountMode"] = true
@@ -43,13 +44,19 @@ func LinkAccount(ctx *context.Context) {
 	ctx.Data["CfTurnstileSitekey"] = setting.Service.CfTurnstileSitekey
 	ctx.Data["DisableRegistration"] = setting.Service.DisableRegistration
 	ctx.Data["AllowOnlyInternalRegistration"] = setting.Service.AllowOnlyInternalRegistration
+	ctx.Data["EnablePasswordSignInForm"] = setting.Service.EnablePasswordSignInForm
 	ctx.Data["ShowRegistrationButton"] = false
 
 	// use this to set the right link into the signIn and signUp templates in the link_account template
 	ctx.Data["SignInLink"] = setting.AppSubURL + "/user/link_account_signin"
 	ctx.Data["SignUpLink"] = setting.AppSubURL + "/user/link_account_signup"
 
 	gothUser, ok := ctx.Session.Get("linkAccountGothUser").(goth.User)
+
+	// If you'd like to quickly debug the "link account" page layout, just uncomment the blow line
+	// Don't worry, when the below line exists, the lint won't pass: ineffectual assignment to gothUser (ineffassign)
+	// gothUser, ok = goth.User{Email: "invalid-email", Name: "."}, true // intentionally use invalid data to avoid pass the registration check
+
 	if !ok {
 		// no account in session, so just redirect to the login page, then the user could restart the process
 		ctx.Redirect(setting.AppSubURL + "/user/login")
@@ -135,6 +142,8 @@ func LinkAccountPostSignIn(ctx *context.Context) {
 	ctx.Data["McaptchaURL"] = setting.Service.McaptchaURL
 	ctx.Data["CfTurnstileSitekey"] = setting.Service.CfTurnstileSitekey
 	ctx.Data["DisableRegistration"] = setting.Service.DisableRegistration
+	ctx.Data["AllowOnlyInternalRegistration"] = setting.Service.AllowOnlyInternalRegistration
+	ctx.Data["EnablePasswordSignInForm"] = setting.Service.EnablePasswordSignInForm
 	ctx.Data["ShowRegistrationButton"] = false
 
 	// use this to set the right link into the signIn and signUp templates in the link_account template
@@ -223,6 +232,8 @@ func LinkAccountPostRegister(ctx *context.Context) {
 	ctx.Data["McaptchaURL"] = setting.Service.McaptchaURL
 	ctx.Data["CfTurnstileSitekey"] = setting.Service.CfTurnstileSitekey
 	ctx.Data["DisableRegistration"] = setting.Service.DisableRegistration
+	ctx.Data["AllowOnlyInternalRegistration"] = setting.Service.AllowOnlyInternalRegistration
+	ctx.Data["EnablePasswordSignInForm"] = setting.Service.EnablePasswordSignInForm
 	ctx.Data["ShowRegistrationButton"] = false
 
 	// use this to set the right link into the signIn and signUp templates in the link_account template

templates/user/auth/link_account.tmpl

@@ -16,13 +16,18 @@
 		</div>
 	</overflow-menu>
 	<div class="ui middle very relaxed page grid">
-		<div class="column">
+		<div class="column tw-my-5">
+			{{/* these styles are quite tricky but it needs to be the same as the signin page */}}
 			<div class="ui tab {{if not .user_exists}}active{{end}}" data-tab="auth-link-signup-tab">
+				<div class="tw-flex tw-flex-col tw-gap-4 tw-max-w-2xl tw-m-auto">
 				{{if .AutoRegistrationFailedPrompt}}<div class="ui message">{{.AutoRegistrationFailedPrompt}}</div>{{end}}
 				{{template "user/auth/signup_inner" .}}
+				</div>
 			</div>
 			<div class="ui tab {{if .user_exists}}active{{end}}" data-tab="auth-link-signin-tab">
+				<div class="tw-flex tw-flex-col tw-gap-4 tw-max-w-2xl tw-m-auto">
 				{{template "user/auth/signin_inner" .}}
+				</div>
 			</div>
 		</div>
 	</div>

templates/user/auth/signin.tmpl

@@ -1,6 +1,7 @@
 {{template "base/head" .}}
 <div role="main" aria-label="{{.Title}}" class="page-content user signin{{if .LinkAccountMode}} icon{{end}}">
 	<div class="ui middle very relaxed page grid">
+		{{/* these styles are quite tricky and should also apply to the signup and link_account pages */}}
 		<div class="column tw-flex tw-flex-col tw-gap-4 tw-max-w-2xl tw-m-auto">
 			{{template "user/auth/signin_inner" .}}
 		</div>

templates/user/auth/signup_inner.tmpl

@@ -59,12 +59,12 @@
 </div>
 
 <div class="ui container fluid">
+	{{if not .LinkAccountMode}}
 	<div class="ui attached segment header top tw-flex tw-flex-col tw-items-center">
-		{{if not .LinkAccountMode}}
 		<div class="field">
 			<span>{{ctx.Locale.Tr "auth.already_have_account"}}</span>
 			<a href="{{AppSubUrl}}/user/login">{{ctx.Locale.Tr "auth.sign_in_now"}}</a>
 		</div>
-		{{end}}
 	</div>
+	{{end}}
 </div>

tests/integration/signin_test.go

@@ -14,8 +14,11 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/modules/translation"
+	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/tests"
 
+	"github.com/markbates/goth"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -96,6 +99,11 @@ func TestSigninWithRememberMe(t *testing.T) {
 func TestEnablePasswordSignInForm(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
+	mockLinkAccount := func(ctx *context.Context) {
+		gothUser := goth.User{Email: "invalid-email", Name: "."}
+		_ = ctx.Session.Set("linkAccountGothUser", gothUser)
+	}
+
 	t.Run("EnablePasswordSignInForm=false", func(t *testing.T) {
 		defer tests.PrintCurrentTest(t)()
 		defer test.MockVariableValue(&setting.Service.EnablePasswordSignInForm, false)()
@@ -106,6 +114,12 @@ func TestEnablePasswordSignInForm(t *testing.T) {
 
 		req = NewRequest(t, "POST", "/user/login")
 		MakeRequest(t, req, http.StatusForbidden)
+
+		req = NewRequest(t, "GET", "/user/link_account")
+		defer web.RouteMockReset()
+		web.RouteMock(web.MockAfterMiddlewares, mockLinkAccount)
+		resp = MakeRequest(t, req, http.StatusOK)
+		NewHTMLParser(t, resp.Body).AssertElement(t, "form[action='/user/link_account_signin']", false)
 	})
 
 	t.Run("EnablePasswordSignInForm=true", func(t *testing.T) {
@@ -118,5 +132,11 @@ func TestEnablePasswordSignInForm(t *testing.T) {
 
 		req = NewRequest(t, "POST", "/user/login")
 		MakeRequest(t, req, http.StatusOK)
+
+		req = NewRequest(t, "GET", "/user/link_account")
+		defer web.RouteMockReset()
+		web.RouteMock(web.MockAfterMiddlewares, mockLinkAccount)
+		resp = MakeRequest(t, req, http.StatusOK)
+		NewHTMLParser(t, resp.Body).AssertElement(t, "form[action='/user/link_account_signin']", true)
 	})
 }