From f1501a7d809f7e8683af4272ea9860bf739c9c92 Mon Sep 17 00:00:00 2001 From: Wesley van Tilburg Date: Thu, 23 Jan 2025 21:37:02 +0100 Subject: [PATCH 1/3] Add basic auth support to rss/atom feeds --- services/auth/auth.go | 5 +++++ services/auth/basic.go | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/services/auth/auth.go b/services/auth/auth.go index 43ff95f05302e..7237aa5aa6868 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -32,6 +32,11 @@ func isAttachmentDownload(req *http.Request) bool { return strings.HasPrefix(req.URL.Path, "/attachments/") && req.Method == "GET" } +// isFeed checks if the request targets a rss/atom feed +func isFeed(req *http.Request) bool { + return strings.HasSuffix(req.URL.Path, ".rss") || strings.HasSuffix(req.URL.Path, ".atom") +} + // isContainerPath checks if the request targets the container endpoint func isContainerPath(req *http.Request) bool { return strings.HasPrefix(req.URL.Path, "/v2/") diff --git a/services/auth/basic.go b/services/auth/basic.go index 6a05b2fe53043..a25f8e1beee62 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -48,8 +48,8 @@ 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 - if !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) { + // Basic authentication should only fire on API, rss/atom feeds, Download or on Git or LFSPaths + if !middleware.IsAPIPath(req) && !isFeed(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) { return nil, nil } From 7c6dab723fc7b8d39672a5d28b39b355ab61bd34 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 24 Jan 2025 09:43:42 +0800 Subject: [PATCH 2/3] Update auth.go --- services/auth/auth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/auth/auth.go b/services/auth/auth.go index 7237aa5aa6868..7576248dd2209 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -34,7 +34,7 @@ func isAttachmentDownload(req *http.Request) bool { // isFeed checks if the request targets a rss/atom feed func isFeed(req *http.Request) bool { - return strings.HasSuffix(req.URL.Path, ".rss") || strings.HasSuffix(req.URL.Path, ".atom") + return setting.Other.EnableFeed && req.Method == "GET" && (strings.HasSuffix(req.URL.Path, ".rss") || strings.HasSuffix(req.URL.Path, ".atom")) } // isContainerPath checks if the request targets the container endpoint From a37aea6df49ce15f3077d173d4724b7ad8aa721d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 26 Jan 2025 18:09:38 +0800 Subject: [PATCH 3/3] improve --- services/auth/auth.go | 21 +++++++++++----- services/auth/auth_test.go | 51 +++++++++++++++++++++++++++----------- services/auth/basic.go | 5 ++-- 3 files changed, 55 insertions(+), 22 deletions(-) diff --git a/services/auth/auth.go b/services/auth/auth.go index 0c0590ffc078b..7deca9bc3d759 100644 --- a/services/auth/auth.go +++ b/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 diff --git a/services/auth/auth_test.go b/services/auth/auth_test.go index 55ffdebe2d372..b8d33961636ec 100644 --- a/services/auth/auth_test.go +++ b/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)) + }) + } } diff --git a/services/auth/basic.go b/services/auth/basic.go index 884b31c14e264..e22b9e1eb777a 100644 --- a/services/auth/basic.go +++ b/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 }