Merge branch 'main' into enforce-2fa-2

Author: wxiaoguang

cmd/admin_user_create.go

@@ -81,6 +81,10 @@ var microcmdUserCreate = &cli.Command{
 			Name:  "restricted",
 			Usage: "Make a restricted user account",
 		},
+		&cli.StringFlag{
+			Name:  "fullname",
+			Usage: `The full, human-readable name of the user`,
+		},
 	},
 }
 
@@ -191,6 +195,7 @@ func runCreateUser(c *cli.Context) error {
 		Passwd:             password,
 		MustChangePassword: mustChangePassword,
 		Visibility:         visibility,
+		FullName:           c.String("fullname"),
 	}
 
 	overwriteDefault := &user_model.CreateUserOverwriteOptions{

cmd/admin_user_create_test.go

@@ -50,17 +50,17 @@ func TestAdminUserCreate(t *testing.T) {
 		assert.Equal(t, check{IsAdmin: false, MustChangePassword: false}, createCheck("u5", "--must-change-password=false"))
 	})
 
-	createUser := func(name, args string) error {
-		return app.Run(strings.Fields(fmt.Sprintf("./gitea admin user create --username %s --email %s@gitea.local %s", name, name, args)))
+	createUser := func(name string, args ...string) error {
+		return app.Run(append([]string{"./gitea", "admin", "user", "create", "--username", name, "--email", name + "@gitea.local"}, args...))
 	}
 
 	t.Run("UserType", func(t *testing.T) {
 		reset()
-		assert.ErrorContains(t, createUser("u", "--user-type invalid"), "invalid user type")
-		assert.ErrorContains(t, createUser("u", "--user-type bot --password 123"), "can only be set for individual users")
-		assert.ErrorContains(t, createUser("u", "--user-type bot --must-change-password"), "can only be set for individual users")
+		assert.ErrorContains(t, createUser("u", "--user-type", "invalid"), "invalid user type")
+		assert.ErrorContains(t, createUser("u", "--user-type", "bot", "--password", "123"), "can only be set for individual users")
+		assert.ErrorContains(t, createUser("u", "--user-type", "bot", "--must-change-password"), "can only be set for individual users")
 
-		assert.NoError(t, createUser("u", "--user-type bot"))
+		assert.NoError(t, createUser("u", "--user-type", "bot"))
 		u := unittest.AssertExistsAndLoadBean(t, &user_model.User{LowerName: "u"})
 		assert.Equal(t, user_model.UserTypeBot, u.Type)
 		assert.Empty(t, u.Passwd)
@@ -75,7 +75,7 @@ func TestAdminUserCreate(t *testing.T) {
 
 		// using "--access-token" only means "all" access
 		reset()
-		assert.NoError(t, createUser("u", "--random-password --access-token"))
+		assert.NoError(t, createUser("u", "--random-password", "--access-token"))
 		assert.Equal(t, 1, unittest.GetCount(t, &user_model.User{}))
 		assert.Equal(t, 1, unittest.GetCount(t, &auth_model.AccessToken{}))
 		accessToken := unittest.AssertExistsAndLoadBean(t, &auth_model.AccessToken{Name: "gitea-admin"})
@@ -85,7 +85,7 @@ func TestAdminUserCreate(t *testing.T) {
 
 		// using "--access-token" with name & scopes
 		reset()
-		assert.NoError(t, createUser("u", "--random-password --access-token --access-token-name new-token-name --access-token-scopes read:issue,read:user"))
+		assert.NoError(t, createUser("u", "--random-password", "--access-token", "--access-token-name", "new-token-name", "--access-token-scopes", "read:issue,read:user"))
 		assert.Equal(t, 1, unittest.GetCount(t, &user_model.User{}))
 		assert.Equal(t, 1, unittest.GetCount(t, &auth_model.AccessToken{}))
 		accessToken = unittest.AssertExistsAndLoadBean(t, &auth_model.AccessToken{Name: "new-token-name"})
@@ -98,23 +98,38 @@ func TestAdminUserCreate(t *testing.T) {
 
 		// using "--access-token-name" without "--access-token"
 		reset()
-		err = createUser("u", "--random-password --access-token-name new-token-name")
+		err = createUser("u", "--random-password", "--access-token-name", "new-token-name")
 		assert.Equal(t, 0, unittest.GetCount(t, &user_model.User{}))
 		assert.Equal(t, 0, unittest.GetCount(t, &auth_model.AccessToken{}))
 		assert.ErrorContains(t, err, "access-token-name and access-token-scopes flags are only valid when access-token flag is set")
 
 		// using "--access-token-scopes" without "--access-token"
 		reset()
-		err = createUser("u", "--random-password --access-token-scopes read:issue")
+		err = createUser("u", "--random-password", "--access-token-scopes", "read:issue")
 		assert.Equal(t, 0, unittest.GetCount(t, &user_model.User{}))
 		assert.Equal(t, 0, unittest.GetCount(t, &auth_model.AccessToken{}))
 		assert.ErrorContains(t, err, "access-token-name and access-token-scopes flags are only valid when access-token flag is set")
 
 		// empty permission
 		reset()
-		err = createUser("u", "--random-password --access-token --access-token-scopes public-only")
+		err = createUser("u", "--random-password", "--access-token", "--access-token-scopes", "public-only")
 		assert.Equal(t, 0, unittest.GetCount(t, &user_model.User{}))
 		assert.Equal(t, 0, unittest.GetCount(t, &auth_model.AccessToken{}))
 		assert.ErrorContains(t, err, "access token does not have any permission")
 	})
+
+	t.Run("UserFields", func(t *testing.T) {
+		reset()
+		assert.NoError(t, createUser("u-FullNameWithSpace", "--random-password", "--fullname", "First O'Middle Last"))
+		unittest.AssertExistsAndLoadBean(t, &user_model.User{
+			Name:      "u-FullNameWithSpace",
+			LowerName: "u-fullnamewithspace",
+			FullName:  "First O'Middle Last",
+			Email:     "[email protected]",
+		})
+
+		assert.NoError(t, createUser("u-FullNameEmpty", "--random-password", "--fullname", ""))
+		u := unittest.AssertExistsAndLoadBean(t, &user_model.User{LowerName: "u-fullnameempty"})
+		assert.Empty(t, u.FullName)
+	})
 }

custom/conf/app.example.ini

@@ -59,27 +59,16 @@ RUN_USER = ; git
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;
-;; The protocol the server listens on. One of 'http', 'https', 'http+unix', 'fcgi' or 'fcgi+unix'. Defaults to 'http'
-;; Note: Value must be lowercase.
+;; The protocol the server listens on. One of "http", "https", "http+unix", "fcgi" or "fcgi+unix".
 ;PROTOCOL = http
 ;;
-;; Expect PROXY protocol headers on connections
-;USE_PROXY_PROTOCOL = false
-;;
-;; Use PROXY protocol in TLS Bridging mode
-;PROXY_PROTOCOL_TLS_BRIDGING = false
-;;
-; Timeout to wait for PROXY protocol header (set to 0 to have no timeout)
-;PROXY_PROTOCOL_HEADER_TIMEOUT=5s
-;;
-; Accept PROXY protocol headers with UNKNOWN type
-;PROXY_PROTOCOL_ACCEPT_UNKNOWN=false
-;;
-;; Set the domain for the server
+;; Set the domain for the server.
+;; Most users should set it to the real website domain of their Gitea instance.
 ;DOMAIN = localhost
 ;;
 ;; The AppURL used by Gitea to generate absolute links, defaults to "{PROTOCOL}://{DOMAIN}:{HTTP_PORT}/".
-;; Most users should set it to the real website URL of their Gitea instance.
+;; Most users should set it to the real website URL of their Gitea instance when there is a reverse proxy.
+;; When it is empty, Gitea will use HTTP "Host" header to generate ROOT_URL, and fall back to the default one if no "Host" header.
 ;ROOT_URL =
 ;;
 ;; For development purpose only. It makes Gitea handle sub-path ("/sub-path/owner/repo/...") directly when debugging without a reverse proxy.
@@ -90,13 +79,25 @@ RUN_USER = ; git
 ;STATIC_URL_PREFIX =
 ;;
 ;; The address to listen on. Either a IPv4/IPv6 address or the path to a unix socket.
-;; If PROTOCOL is set to `http+unix` or `fcgi+unix`, this should be the name of the Unix socket file to use.
+;; If PROTOCOL is set to "http+unix" or "fcgi+unix", this should be the name of the Unix socket file to use.
 ;; Relative paths will be made absolute against the _`AppWorkPath`_.
 ;HTTP_ADDR = 0.0.0.0
 ;;
-;; The port to listen on. Leave empty when using a unix socket.
+;; The port to listen on for "http" or "https" protocol. Leave empty when using a unix socket.
 ;HTTP_PORT = 3000
 ;;
+;; Expect PROXY protocol headers on connections
+;USE_PROXY_PROTOCOL = false
+;;
+;; Use PROXY protocol in TLS Bridging mode
+;PROXY_PROTOCOL_TLS_BRIDGING = false
+;;
+;; Timeout to wait for PROXY protocol header (set to 0 to have no timeout)
+;PROXY_PROTOCOL_HEADER_TIMEOUT = 5s
+;;
+;; Accept PROXY protocol headers with UNKNOWN type
+;PROXY_PROTOCOL_ACCEPT_UNKNOWN = false
+;;
 ;; If REDIRECT_OTHER_PORT is true, and PROTOCOL is set to https an http server
 ;; will be started on PORT_TO_REDIRECT and it will redirect plain, non-secure http requests to the main
 ;; ROOT_URL.  Defaults are false for REDIRECT_OTHER_PORT and 80 for
@@ -520,7 +521,7 @@ INTERNAL_TOKEN =
 ;; RECORD_USER_SIGNUP_METADATA = false
 ;;
 ;; Set the two-factor auth behavior.
-;; Set to "enforced", to force users to enroll into Two-Factor Authentication, users without 2FA have no access to any repositories.
+;; Set to "enforced", to force users to enroll into Two-Factor Authentication, users without 2FA have no access to repositories via API or web.
 ;TWO_FACTOR_AUTH =
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

modules/httplib/url.go

@@ -70,11 +70,16 @@ func GuessCurrentHostURL(ctx context.Context) string {
 	// 1. The reverse proxy is configured correctly, it passes "X-Forwarded-Proto/Host" headers. Perfect, Gitea can handle it correctly.
 	// 2. The reverse proxy is not configured correctly, doesn't pass "X-Forwarded-Proto/Host" headers, eg: only one "proxy_pass http://gitea:3000" in Nginx.
 	// 3. There is no reverse proxy.
-	// Without an extra config option, Gitea is impossible to distinguish between case 2 and case 3,
-	// then case 2 would result in wrong guess like guessed AppURL becomes "http://gitea:3000/", which is not accessible by end users.
-	// So in the future maybe it should introduce a new config option, to let site admin decide how to guess the AppURL.
+	// Without more information, Gitea is impossible to distinguish between case 2 and case 3, then case 2 would result in
+	// wrong guess like guessed AppURL becomes "http://gitea:3000/" behind a "https" reverse proxy, which is not accessible by end users.
+	// So we introduced "UseHostHeader" option, it could be enabled by setting "ROOT_URL" to empty
 	reqScheme := getRequestScheme(req)
 	if reqScheme == "" {
+		// if no reverse proxy header, try to use "Host" header for absolute URL
+		if setting.UseHostHeader && req.Host != "" {
+			return util.Iif(req.TLS == nil, "http://", "https://") + req.Host
+		}
+		// fall back to default AppURL
 		return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/")
 	}
 	// X-Forwarded-Host has many problems: non-standard, not well-defined (X-Forwarded-Port or not), conflicts with Host header.

modules/httplib/url_test.go

@@ -5,6 +5,7 @@ package httplib
 
 import (
 	"context"
+	"crypto/tls"
 	"net/http"
 	"testing"
 
@@ -39,6 +40,25 @@ func TestIsRelativeURL(t *testing.T) {
 	}
 }
 
+func TestGuessCurrentHostURL(t *testing.T) {
+	defer test.MockVariableValue(&setting.AppURL, "http://cfg-host/sub/")()
+	defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
+	defer test.MockVariableValue(&setting.UseHostHeader, false)()
+
+	ctx := t.Context()
+	assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx))
+
+	ctx = context.WithValue(ctx, RequestContextKey, &http.Request{Host: "localhost:3000"})
+	assert.Equal(t, "http://cfg-host", GuessCurrentHostURL(ctx))
+
+	defer test.MockVariableValue(&setting.UseHostHeader, true)()
+	ctx = context.WithValue(ctx, RequestContextKey, &http.Request{Host: "http-host:3000"})
+	assert.Equal(t, "http://http-host:3000", GuessCurrentHostURL(ctx))
+
+	ctx = context.WithValue(ctx, RequestContextKey, &http.Request{Host: "http-host", TLS: &tls.ConnectionState{}})
+	assert.Equal(t, "https://http-host", GuessCurrentHostURL(ctx))
+}
+
 func TestMakeAbsoluteURL(t *testing.T) {
 	defer test.MockVariableValue(&setting.Protocol, "http")()
 	defer test.MockVariableValue(&setting.AppURL, "http://cfg-host/sub/")()

modules/markup/html.go

@@ -71,7 +71,8 @@ var globalVars = sync.OnceValue(func() *globalVarsType {
 	// it is still accepted by the CommonMark specification, as well as the HTML5 spec:
 	//   http://spec.commonmark.org/0.28/#email-address
 	//   https://html.spec.whatwg.org/multipage/input.html#e-mail-state-(type%3Demail)
-	v.emailRegex = regexp.MustCompile("(?:\\s|^|\\(|\\[)([a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9]{2,}(?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)+)(?:\\s|$|\\)|\\]|;|,|\\?|!|\\.(\\s|$))")
+	// At the moment, we use stricter rule for rendering purpose: only allow the "name" part starting after the word boundary
+	v.emailRegex = regexp.MustCompile(`\b([-\w.!#$%&'*+/=?^{|}~]*@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9]{2,}(?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)+)\b`)
 
 	// emojiShortCodeRegex find emoji by alias like :smile:
 	v.emojiShortCodeRegex = regexp.MustCompile(`:[-+\w]+:`)

modules/markup/html_email.go

@@ -3,7 +3,11 @@
 
 package markup
 
-import "golang.org/x/net/html"
+import (
+	"strings"
+
+	"golang.org/x/net/html"
+)
 
 // emailAddressProcessor replaces raw email addresses with a mailto: link.
 func emailAddressProcessor(ctx *RenderContext, node *html.Node) {
@@ -14,6 +18,14 @@ func emailAddressProcessor(ctx *RenderContext, node *html.Node) {
 			return
 		}
 
+		var nextByte byte
+		if len(node.Data) > m[3] {
+			nextByte = node.Data[m[3]]
+		}
+		if strings.IndexByte(":/", nextByte) != -1 {
+			// for cases: "[email protected]:owner/repo.git", "https://[email protected]/owner/repo.git"
+			return
+		}
 		mail := node.Data[m[2]:m[3]]
 		replaceContent(node, m[2], m[3], createLink(ctx, "mailto:"+mail, mail, "" /*mailto*/))
 		node = node.NextSibling.NextSibling

modules/markup/html_test.go

@@ -225,10 +225,10 @@ func TestRender_email(t *testing.T) {
 	test := func(input, expected string) {
 		res, err := markup.RenderString(markup.NewTestRenderContext().WithRelativePath("a.md"), input)
 		assert.NoError(t, err)
-		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res))
+		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res), "input: %s", input)
 	}
-	// Text that should be turned into email link
 
+	// Text that should be turned into email link
 	test(
 		"[email protected]",
 		`<p><a href="mailto:[email protected]" rel="nofollow">[email protected]</a></p>`)
@@ -260,28 +260,48 @@ func TestRender_email(t *testing.T) {
 <a href="mailto:[email protected]" rel="nofollow">[email protected]</a>?
 <a href="mailto:[email protected]" rel="nofollow">[email protected]</a>!</p>`)
 
+	// match GitHub behavior
+	test("email@[email protected]", `<p>email@<a href="mailto:[email protected]" rel="nofollow">[email protected]</a></p>`)
+
+	// match GitHub behavior
+	test(`"[email protected]"`, `<p>&#34;<a href="mailto:[email protected]" rel="nofollow">[email protected]</a>&#34;</p>`)
+
 	// Test that should *not* be turned into email links
-	test(
-		"\"[email protected]\"",
-		`<p>&#34;[email protected]&#34;</p>`)
 	test(
 		"/home/gitea/mailstore/info@gitea/com",
 		`<p>/home/gitea/mailstore/info@gitea/com</p>`)
 	test(
 		"[email protected]:go-gitea/gitea.git",
 		`<p>[email protected]:go-gitea/gitea.git</p>`)
+	test(
+		"https://foo:[email protected]",
+		`<p><a href="https://foo:[email protected]" rel="nofollow">https://foo:[email protected]</a></p>`)
 	test(
 		"gitea@3",
 		`<p>gitea@3</p>`)
 	test(
 		"[email protected]",
 		`<p>[email protected]</p>`)
-	test(
-		"email@[email protected]",
-		`<p>email@[email protected]</p>`)
 	test(
 		"[email protected]",
 		`<p>[email protected]</p>`)
+
+	cases := []struct {
+		input, expected string
+	}{
+		// match GitHub behavior
+		{"[email protected]", `<p>?<a href="mailto:[email protected]" rel="nofollow">[email protected]</a></p>`},
+		{"*[email protected]", `<p>*<a href="mailto:[email protected]" rel="nofollow">[email protected]</a></p>`},
+		{"[email protected]", `<p>~<a href="mailto:[email protected]" rel="nofollow">[email protected]</a></p>`},
+
+		// the following cases don't match GitHub behavior, but they are valid email addresses ...
+		// maybe we should reduce the candidate characters for the "name" part in the future
+		{"a*[email protected]", `<p><a href="mailto:a*[email protected]" rel="nofollow">a*[email protected]</a></p>`},
+		{"[email protected]", `<p><a href="mailto:[email protected]" rel="nofollow">[email protected]</a></p>`},
+	}
+	for _, c := range cases {
+		test(c.input, c.expected)
+	}
 }
 
 func TestRender_emoji(t *testing.T) {