From 6029d78ab5006e8fb4f42adb5a8c491f19fa7b0a Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Thu, 10 Oct 2024 17:04:42 +0800
Subject: [PATCH] Improve the maintainblity of the reserved username list
 (#32229)

---
 models/user/user.go            | 56 ++++++++++++++++++----------------
 services/user/user_test.go     |  8 ++---
 tests/integration/user_test.go | 45 ++++++---------------------
 3 files changed, 41 insertions(+), 68 deletions(-)

diff --git a/models/user/user.go b/models/user/user.go
index c1e3d5d1c76..c1cb988e43d 100644
--- a/models/user/user.go
+++ b/models/user/user.go
@@ -565,41 +565,43 @@ var (
 		".",
 		"..",
 		".well-known",
-		"api",
-		"assets",
-		"attachments",
-		"avatar",
-		"avatars",
+
+		"api",     // gitea api
+		"metrics", // prometheus metrics api
+		"v2",      // container registry api
+
+		"assets",      // static asset files
+		"attachments", // issue attachments
+
+		"avatar",  // avatar by email hash
+		"avatars", // user avatars by file name
+		"repo-avatars",
+
 		"captcha",
-		"commits",
-		"debug",
-		"error",
+		"login", // oauth2 login
+		"org",   // org create/manage, or "/org/{org}", BUT if an org is named as "invite" then it goes wrong
+		"repo",  // repo create/migrate, etc
+		"user",  // user login/activate/settings, etc
+
 		"explore",
-		"favicon.ico",
-		"ghost",
 		"issues",
-		"login",
-		"manifest.json",
-		"metrics",
+		"pulls",
 		"milestones",
-		"new",
 		"notifications",
-		"org",
-		"pulls",
-		"raw",
-		"repo",
-		"repo-avatars",
-		"robots.txt",
-		"search",
-		"serviceworker.js",
-		"ssh_info",
+
+		"favicon.ico",
+		"manifest.json", // web app manifests
+		"robots.txt",    // search engine robots
+		"sitemap.xml",   // search engine sitemap
+		"ssh_info",      // agit info
 		"swagger.v1.json",
-		"user",
-		"v2",
-		"gitea-actions",
+
+		"ghost",         // reserved name for deleted users (id: -1)
+		"gitea-actions", // gitea builtin user (id: -2)
 	}
 
-	// DON'T ADD ANY NEW STUFF, WE SOLVE THIS WITH `/user/{obj}` PATHS!
+	// These names are reserved for user accounts: user's keys, user's rss feed, user's avatar, etc.
+	// DO NOT add any new stuff! The paths with these names are processed by `/{username}` handler (UsernameSubRoute) manually.
 	reservedUserPatterns = []string{"*.keys", "*.gpg", "*.rss", "*.atom", "*.png"}
 )
 
diff --git a/services/user/user_test.go b/services/user/user_test.go
index cd0f5975015..efcbc669c8a 100644
--- a/services/user/user_test.go
+++ b/services/user/user_test.go
@@ -114,12 +114,10 @@ func TestRenameUser(t *testing.T) {
 	})
 
 	t.Run("Non usable username", func(t *testing.T) {
-		usernames := []string{"--diff", "aa.png", ".well-known", "search", "aaa.atom"}
+		usernames := []string{"--diff", ".well-known", "gitea-actions", "aaa.atom", "aa.png"}
 		for _, username := range usernames {
-			t.Run(username, func(t *testing.T) {
-				assert.Error(t, user_model.IsUsableUsername(username))
-				assert.Error(t, RenameUser(db.DefaultContext, user, username))
-			})
+			assert.Error(t, user_model.IsUsableUsername(username), "non-usable username: %s", username)
+			assert.Error(t, RenameUser(db.DefaultContext, user, username), "non-usable username: %s", username)
 		}
 	})
 
diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go
index 2ba16b3d362..99e413c6d95 100644
--- a/tests/integration/user_test.go
+++ b/tests/integration/user_test.go
@@ -5,6 +5,7 @@ package integration
 
 import (
 	"net/http"
+	"strings"
 	"testing"
 
 	auth_model "code.gitea.io/gitea/models/auth"
@@ -98,41 +99,12 @@ func TestRenameReservedUsername(t *testing.T) {
 	reservedUsernames := []string{
 		// ".", "..", ".well-known", // The names are not only reserved but also invalid
 		"api",
-		"assets",
-		"attachments",
-		"avatar",
-		"avatars",
-		"captcha",
-		"commits",
-		"debug",
-		"error",
-		"explore",
-		"favicon.ico",
-		"ghost",
-		"issues",
-		"login",
-		"manifest.json",
-		"metrics",
-		"milestones",
-		"new",
-		"notifications",
-		"org",
-		"pulls",
-		"raw",
-		"repo",
-		"repo-avatars",
-		"robots.txt",
-		"search",
-		"serviceworker.js",
-		"ssh_info",
-		"swagger.v1.json",
-		"user",
-		"v2",
+		"name.keys",
 	}
 
 	session := loginUser(t, "user2")
+	locale := translation.NewLocale("en-US")
 	for _, reservedUsername := range reservedUsernames {
-		t.Logf("Testing username %s", reservedUsername)
 		req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
 			"_csrf":    GetUserCSRFToken(t, session),
 			"name":     reservedUsername,
@@ -144,11 +116,12 @@ func TestRenameReservedUsername(t *testing.T) {
 		req = NewRequest(t, "GET", test.RedirectURL(resp))
 		resp = session.MakeRequest(t, req, http.StatusOK)
 		htmlDoc := NewHTMLParser(t, resp.Body)
-		assert.Contains(t,
-			htmlDoc.doc.Find(".ui.negative.message").Text(),
-			translation.NewLocale("en-US").TrString("user.form.name_reserved", reservedUsername),
-		)
-
+		actualMsg := strings.TrimSpace(htmlDoc.doc.Find(".ui.negative.message").Text())
+		expectedMsg := locale.TrString("user.form.name_reserved", reservedUsername)
+		if strings.Contains(reservedUsername, ".") {
+			expectedMsg = locale.TrString("user.form.name_pattern_not_allowed", reservedUsername)
+		}
+		assert.Equal(t, expectedMsg, actualMsg)
 		unittest.AssertNotExistsBean(t, &user_model.User{Name: reservedUsername})
 	}
 }