enhance(admin): show impersonation banner and keep password change with the user (#38924)

Follow-up to https://github.com/go-gitea/gitea/pull/38614

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

options/locale/locale_en-US.json

@@ -3052,6 +3052,8 @@
   "admin.users.new_success": "The user account \"%s\" has been created.",
   "admin.users.edit": "Edit",
   "admin.users.impersonate": "Impersonate",
+  "admin.users.impersonate_stop": "Stop impersonating",
+  "admin.users.impersonating_notice": "You are impersonating <strong>%s</strong>. Actions you take are performed as this user.",
   "admin.users.auth_source": "Authentication Source",
   "admin.users.local": "Local",
   "admin.users.auth_login_name": "Authentication Sign-In Name",

routers/web/home.go

@@ -38,7 +38,7 @@ func Home(ctx *context.Context) {
 			log.Info("Failed authentication attempt for %s from %s", ctx.Doer.Name, ctx.RemoteAddr())
 			ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
 			ctx.HTML(http.StatusOK, "user/auth/prohibit_login")
-		} else if ctx.Doer.MustChangePassword {
+		} else if doerMustChangePassword(ctx) {
 			ctx.Data["Title"] = ctx.Tr("auth.must_change_password")
 			ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/change_password"
 			middleware.SetRedirectToCookie(ctx.Resp, setting.AppSubURL+ctx.Req.URL.RequestURI())

routers/web/web.go

@@ -166,6 +166,11 @@ func newWebAuthMiddleware() *AuthMiddleware {
 	return webAuth
 }
 
+func doerMustChangePassword(ctx *context.Context) bool {
+	// an impersonating admin must not be forced to set the impersonated user's password
+	return ctx.Doer != nil && ctx.Doer.MustChangePassword && !ctx.DoerIsImpersonated()
+}
+
 // verifyAuthWithOptions checks authentication according to options
 func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.Context) {
 	crossOriginProtection := http.NewCrossOriginProtection()
@@ -185,7 +190,7 @@ func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.Cont
 				return
 			}
 
-			if ctx.Doer.MustChangePassword {
+			if doerMustChangePassword(ctx) {
 				if ctx.Req.URL.Path != "/user/settings/change_password" {
 					if strings.HasPrefix(ctx.Req.UserAgent(), "git") {
 						ctx.HTTPError(http.StatusUnauthorized, ctx.Locale.TrString("auth.must_change_password"))

services/context/context.go

@@ -228,6 +228,11 @@ func (ctx *Context) DoerNeedTwoFactorAuth() bool {
 	return ctx.Session.Get(session.KeyUserHasTwoFactorAuth) == false
 }
 
+// DoerIsImpersonated returns true if the current session is an admin impersonating the doer
+func (ctx *Context) DoerIsImpersonated() bool {
+	return ctx.Session.Get(session.KeyImpersonatorData) != nil
+}
+
 // HasError returns true if error occurs in form validation.
 // Attention: this function changes ctx.Data and ctx.Flash
 // If HasError is called, then before Redirect, the error message should be stored by ctx.Flash.Error(ctx.GetErrMsg()) again.

services/context/context_template.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/htmlutil"
 	"gitea.dev/modules/httplib"
 	"gitea.dev/modules/public"
@@ -66,6 +67,14 @@ func (c TemplateContext) CurrentWebTheme() *webtheme.ThemeMetaInfo {
 	return webtheme.GuaranteeGetThemeMetaInfo(themeName)
 }
 
+func (c TemplateContext) ImpersonatedUser() *user_model.User {
+	webCtx := GetWebContext(c)
+	if webCtx == nil || webCtx.Doer == nil || !webCtx.DoerIsImpersonated() {
+		return nil
+	}
+	return webCtx.Doer
+}
+
 func (c TemplateContext) CurrentWebBanner() *setting.WebBannerType {
 	// Using revision as a simple approach to determine if the banner has been changed after the user dismissed it.
 	// There could be some false-positives because revision can be changed even if the banner isn't.

templates/admin/config_settings/instance.tmpl

@@ -37,7 +37,7 @@
 				<label>{{ctx.Locale.Tr "admin.config.instance_web_banner.enabled"}}</label>
 			</div>
 			{{template "shared/combomarkdowneditor" (dict
-				"ContainerClasses" "web-banner-content-editor"
+				"ContainerClasses" "site-banner-content-editor"
 				"TextareaName" (print $cfgKey ".ContentMessage")
 				"TextareaContent" $banner.ContentMessage
 				"TextareaPlaceholder" (ctx.Locale.Tr "admin.config.instance_web_banner.message_placeholder")

templates/base/head_banner.tmpl

@@ -1,10 +1,10 @@
 {{$banner := ctx.CurrentWebBanner}}
 {{if $banner}}
-<div class="ui info message web-banner-container">
-	<div class="render-content markup web-banner-content">
+<div class="ui info message site-banner-container">
+	<div class="render-content markup site-banner-content">
 		{{ctx.RenderUtils.MarkdownToHtml $banner.ContentMessage}}
 	</div>
-	<button type="button" class="btn dismiss-banner link-action" aria-label="{{ctx.Locale.Tr "dismiss"}}" data-url="{{AppSubUrl}}/-/web-banner/dismiss">
+	<button type="button" class="btn site-banner-close link-action" aria-label="{{ctx.Locale.Tr "dismiss"}}" data-url="{{AppSubUrl}}/-/web-banner/dismiss">
 		{{svg "octicon-x"}}
 	</button>
 </div>

templates/base/head_impersonate_banner.tmpl

@@ -0,0 +1,10 @@
+{{$impersonated := ctx.ImpersonatedUser}}
+{{if $impersonated}}
+<div class="ui warning message site-banner-container">
+	<div class="flex-text-block site-banner-content">
+		{{svg "octicon-alert"}}
+		<span class="tw-flex-1">{{ctx.Locale.Tr "admin.users.impersonating_notice" $impersonated.Name}}</span>
+		<a class="ui compact tiny button hover-opaque" href="{{AppSubUrl}}/user/logout">{{ctx.Locale.Tr "admin.users.impersonate_stop"}}</a>
+	</div>
+</div>
+{{end}}

templates/base/head_navbar.tmpl

@@ -178,4 +178,5 @@
 		</div>
 	</div>
 </nav>
+{{template "base/head_impersonate_banner"}}
 {{template "base/head_banner"}}

templates/devtest/severity-colors.tmpl

@@ -59,7 +59,7 @@
 	<div class="ui attached warning segment">Warning section body content.</div>
 
 <h2>Banner Preview (info-tinted)</h2>
-	<div class="web-banner-content-editor">
+	<div class="site-banner-content-editor">
 		<div class="render-content render-preview">Banner preview content</div>
 	</div>
 

tests/integration/admin_user_test.go

@@ -14,6 +14,7 @@ import (
 	"gitea.dev/tests"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestAdminViewUsers(t *testing.T) {
@@ -106,29 +107,41 @@ func TestAdminDeleteUser(t *testing.T) {
 func TestAdminImpersonatedUser(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
+	// user2 never signed in yet, only the user themselves should be asked to set a password
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	user2.MustChangePassword = true
+	require.NoError(t, user_model.UpdateUserCols(t.Context(), user2, "must_change_password"))
+
 	session := loginUser(t, "user1")
-	currentUsername := func(t *testing.T) string {
+	homeDoc := func(t *testing.T) *HTMLDoc {
 		t.Helper()
 		resp := session.MakeRequest(t, NewRequest(t, "GET", "/"), http.StatusOK)
-		doc := NewHTMLParser(t, resp.Body)
+		return NewHTMLParser(t, resp.Body)
+	}
+	currentUsername := func(doc *HTMLDoc) string {
 		return doc.Find("[data-signed-in-username]").AttrOr("data-signed-in-username", "")
 	}
 
 	// user1 is admin, can visit admin pages
-	assert.Equal(t, "user1", currentUsername(t))
+	assert.Equal(t, "user1", currentUsername(homeDoc(t)))
+	assert.Equal(t, 0, homeDoc(t).Find(".site-banner-container").Length())
 	session.MakeRequest(t, NewRequest(t, "GET", "/-/admin/users/2"), http.StatusOK)
 
 	// impersonate to user2, user2 can't visit admin pages
 	session.MakeRequest(t, NewRequest(t, "POST", "/-/admin/users/2/impersonate"), http.StatusOK)
-	assert.Equal(t, "user2", currentUsername(t))
+	doc := homeDoc(t)
+	assert.Equal(t, "user2", currentUsername(doc))
+	assert.Contains(t, doc.Find(".site-banner-container").Text(), "user2")
 	session.MakeRequest(t, NewRequest(t, "GET", "/-/admin/users/2"), http.StatusForbidden)
+	// the impersonating admin must not set the password of the impersonated user
+	session.MakeRequest(t, NewRequest(t, "GET", "/user/settings/change_password"), http.StatusSeeOther)
 
 	// exit impersonation, current user is user1(admin) again
 	session.MakeRequest(t, NewRequest(t, "GET", "/user/logout"), http.StatusSeeOther)
-	assert.Equal(t, "user1", currentUsername(t))
+	assert.Equal(t, "user1", currentUsername(homeDoc(t)))
 	session.MakeRequest(t, NewRequest(t, "GET", "/-/admin/users/2"), http.StatusOK)
 
 	// completely logout
 	session.MakeRequest(t, NewRequest(t, "GET", "/user/logout"), http.StatusSeeOther)
-	assert.Equal(t, "", currentUsername(t))
+	assert.Equal(t, "", currentUsername(homeDoc(t)))
 }

web_src/css/admin.css

@@ -44,7 +44,7 @@
   margin-bottom: 1rem;
 }
 
-.web-banner-content-editor .render-content.render-preview {
+.site-banner-content-editor .render-content.render-preview {
   /* use the styles from ".ui.message" */
   padding: 1em 1.5em;
   border: 1px solid var(--color-info-border);

web_src/css/modules/button.css

@@ -32,6 +32,12 @@
   color: var(--color-text);
 }
 
+.ui.button.hover-opaque:hover {
+  /* ".ui.button:hover" uses alpha channel to "highlight", it doesn't work when the background is not body: the inherited color is not right
+   for such case, use this opaque color */
+  background: var(--color-hover-opaque);
+}
+
 .ui.active.button,
 .ui.button:active,
 .ui.active.button:active,

web_src/css/modules/container.css

@@ -15,19 +15,24 @@
   width: 800px;
 }
 
-.ui.message.web-banner-container {
+.ui.message.site-banner-container {
   position: relative;
   margin: 0;
   border-radius: 0;
 }
 
-.ui.message.web-banner-container > .web-banner-content {
+#navbar + .ui.message.site-banner-container,
+.ui.message.site-banner-container + .ui.message.site-banner-container {
+  border-top: none;
+}
+
+.ui.message.site-banner-container > .site-banner-content {
   width: 1280px;
   max-width: calc(100% - calc(2 * var(--page-margin-x)));
   margin: auto;
 }
 
-.ui.message.web-banner-container > button.dismiss-banner {
+.ui.message.site-banner-container > button.site-banner-close {
   position: absolute;
   right: 20px;
   top: 15px;

web_src/css/modules/navbar.css

@@ -7,11 +7,6 @@
   padding: 0 10px;
 }
 
-/* When notification message is present after navbar, hide border to avoid double border */
-#navbar:has(+ .ui.message) {
-  border-bottom: none;
-}
-
 #navbar .navbar-left,
 #navbar .navbar-right {
   display: flex;