improve

Author: wxiaoguang

services/auth/auth.go

@@ -26,13 +26,17 @@ type globalVarsStruct struct {
 	gitRawOrAttachPathRe *regexp.Regexp
 	lfsPathRe            *regexp.Regexp
 	archivePathRe        *regexp.Regexp
+	feedPathRe           *regexp.Regexp
+	feedRefPathRe        *regexp.Regexp
 }
 
 var globalVars = sync.OnceValue(func() *globalVarsStruct {
 	return &globalVarsStruct{
-		gitRawOrAttachPathRe: regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/(?:(?:git-(?:(?:upload)|(?:receive))-pack$)|(?:info/refs$)|(?:HEAD$)|(?:objects/)|(?:raw/)|(?:releases/download/)|(?:attachments/))`),
-		lfsPathRe:            regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/info/lfs/`),
-		archivePathRe:        regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/archive/`),
+		gitRawOrAttachPathRe: regexp.MustCompile(`^/[-.\w]+/[-.\w]+/(?:(?:git-(?:(?:upload)|(?:receive))-pack$)|(?:info/refs$)|(?:HEAD$)|(?:objects/)|(?:raw/)|(?:releases/download/)|(?:attachments/))`),
+		lfsPathRe:            regexp.MustCompile(`^/[-.\w]+/[-.\w]+/info/lfs/`),
+		archivePathRe:        regexp.MustCompile(`^/[-.\w]+/[-.\w]+/archive/`),
+		feedPathRe:           regexp.MustCompile(`^/[-.\w]+(/[-.\w]+)?\.(rss|atom)$`), // "/owner.rss" or "/owner/repo.atom"
+		feedRefPathRe:        regexp.MustCompile(`^/[-.\w]+/[-.\w]+/(rss|atom)/`),     // "/owner/repo/rss/branch/..."
 	}
 })
 
@@ -61,9 +65,14 @@ func (a *authPathDetector) isAttachmentDownload() bool {
 	return strings.HasPrefix(a.req.URL.Path, "/attachments/") && a.req.Method == "GET"
 }
 
-// isFeed checks if the request targets a rss/atom feed
-func isFeed(req *http.Request) bool {
-	return setting.Other.EnableFeed && req.Method == "GET" && (strings.HasSuffix(req.URL.Path, ".rss") || strings.HasSuffix(req.URL.Path, ".atom"))
+func (a *authPathDetector) isFeedRequest(req *http.Request) bool {
+	if !setting.Other.EnableFeed {
+		return false
+	}
+	if req.Method != "GET" {
+		return false
+	}
+	return a.vars.feedPathRe.MatchString(req.URL.Path) || a.vars.feedRefPathRe.MatchString(req.URL.Path)
 }
 
 // isContainerPath checks if the request targets the container endpoint

services/auth/auth_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -92,20 +93,8 @@ func Test_isGitRawOrLFSPath(t *testing.T) {
 			true,
 		},
 	}
-	lfsTests := []string{
-		"/owner/repo/info/lfs/",
-		"/owner/repo/info/lfs/objects/batch",
-		"/owner/repo/info/lfs/objects/oid/filename",
-		"/owner/repo/info/lfs/objects/oid",
-		"/owner/repo/info/lfs/objects",
-		"/owner/repo/info/lfs/verify",
-		"/owner/repo/info/lfs/locks",
-		"/owner/repo/info/lfs/locks/verify",
-		"/owner/repo/info/lfs/locks/123/unlock",
-	}
-
-	origLFSStartServer := setting.LFS.StartServer
 
+	defer test.MockVariableValue(&setting.LFS.StartServer)()
 	for _, tt := range tests {
 		t.Run(tt.path, func(t *testing.T) {
 			req, _ := http.NewRequest("POST", "http://localhost"+tt.path, nil)
@@ -116,6 +105,18 @@ func Test_isGitRawOrLFSPath(t *testing.T) {
 			assert.Equal(t, tt.want, newAuthPathDetector(req).isGitRawOrAttachOrLFSPath())
 		})
 	}
+
+	lfsTests := []string{
+		"/owner/repo/info/lfs/",
+		"/owner/repo/info/lfs/objects/batch",
+		"/owner/repo/info/lfs/objects/oid/filename",
+		"/owner/repo/info/lfs/objects/oid",
+		"/owner/repo/info/lfs/objects",
+		"/owner/repo/info/lfs/verify",
+		"/owner/repo/info/lfs/locks",
+		"/owner/repo/info/lfs/locks/verify",
+		"/owner/repo/info/lfs/locks/123/unlock",
+	}
 	for _, tt := range lfsTests {
 		t.Run(tt, func(t *testing.T) {
 			req, _ := http.NewRequest("POST", tt, nil)
@@ -128,5 +129,27 @@ func Test_isGitRawOrLFSPath(t *testing.T) {
 			assert.Equalf(t, setting.LFS.StartServer, got, "isGitOrLFSPath(%q) = %v, want %v", tt, got, setting.LFS.StartServer)
 		})
 	}
-	setting.LFS.StartServer = origLFSStartServer
+}
+
+func Test_isFeedRequest(t *testing.T) {
+	tests := []struct {
+		want bool
+		path string
+	}{
+		{true, "/user.rss"},
+		{true, "/user/repo.atom"},
+		{false, "/user/repo"},
+		{false, "/use/repo/file.rss"},
+
+		{true, "/org/repo/rss/branch/xxx"},
+		{true, "/org/repo/atom/tag/xxx"},
+		{false, "/org/repo/branch/main/rss/any"},
+		{false, "/org/atom/any"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.path, func(t *testing.T) {
+			req, _ := http.NewRequest("GET", "http://localhost"+tt.path, nil)
+			assert.Equal(t, tt.want, newAuthPathDetector(req).isFeedRequest(req))
+		})
+	}
 }

services/auth/basic.go

@@ -47,9 +47,10 @@ func (b *Basic) Name() string {
 // name/token on successful validation.
 // Returns nil if header is empty or validation fails.
 func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
-	// Basic authentication should only fire on API, Download or on Git or LFSPaths
+	// Basic authentication should only fire on API, Feed, Download or on Git or LFSPaths
+	// Not all feed (rss/atom) clients feature the ability to add cookies or headers, so we need to allow basic auth for feeds
 	detector := newAuthPathDetector(req)
-	if !detector.isAPIPath() && !isFeed(req) && !detector.isContainerPath() && !detector.isAttachmentDownload() && !detector.isGitRawOrAttachOrLFSPath() {
+	if !detector.isAPIPath() && !detector.isFeedRequest(req) && !detector.isContainerPath() && !detector.isAttachmentDownload() && !detector.isGitRawOrAttachOrLFSPath() {
 		return nil, nil
 	}