From 07cf6d3140babc4561390b9d34054f6095ffc462 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 7 Oct 2023 01:11:38 +0800 Subject: [PATCH 1/3] Fix attachment download bug --- services/auth/auth.go | 8 ++++---- services/auth/basic.go | 2 +- services/auth/oauth2.go | 2 +- services/auth/reverseproxy.go | 2 +- services/convert/attachment.go | 10 +--------- 5 files changed, 8 insertions(+), 16 deletions(-) diff --git a/services/auth/auth.go b/services/auth/auth.go index 0c8acac61f053..ea6c834942019 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -36,12 +36,12 @@ func isContainerPath(req *http.Request) bool { } var ( - gitRawReleasePathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/(?:(?:git-(?:(?:upload)|(?:receive))-pack$)|(?:info/refs$)|(?:HEAD$)|(?:objects/)|(?:raw/)|(?:releases/download/))`) - lfsPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/info/lfs/`) + 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/`) ) -func isGitRawReleaseOrLFSPath(req *http.Request) bool { - if gitRawReleasePathRe.MatchString(req.URL.Path) { +func isGitRawOrAttachOrLFSPath(req *http.Request) bool { + if gitRawOrAttachPathRe.MatchString(req.URL.Path) { return true } if setting.LFS.StartServer { diff --git a/services/auth/basic.go b/services/auth/basic.go index bb9eb7a321636..6c3fbf595e44c 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -42,7 +42,7 @@ func (b *Basic) Name() string { // 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) && !isGitRawReleaseOrLFSPath(req) { + if !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) { return nil, nil } diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 38b705cc5b8f7..0b49f15281dbb 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -127,7 +127,7 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store Dat func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) { // These paths are not API paths, but we still want to check for tokens because they maybe in the API returned URLs if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) && - !gitRawReleasePathRe.MatchString(req.URL.Path) { + !gitRawOrAttachPathRe.MatchString(req.URL.Path) { return nil, nil } diff --git a/services/auth/reverseproxy.go b/services/auth/reverseproxy.go index ad525f5c95fef..359c1f2473784 100644 --- a/services/auth/reverseproxy.go +++ b/services/auth/reverseproxy.go @@ -117,7 +117,7 @@ func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, store Da } // Make sure requests to API paths, attachment downloads, git and LFS do not create a new session - if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isGitRawReleaseOrLFSPath(req) { + if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) { if sess != nil && (sess.Get("uid") == nil || sess.Get("uid").(int64) != user.ID) { handleSignIn(w, req, sess, user) } diff --git a/services/convert/attachment.go b/services/convert/attachment.go index ab36a1c577856..4a8f10f7b02cd 100644 --- a/services/convert/attachment.go +++ b/services/convert/attachment.go @@ -4,10 +4,7 @@ package convert import ( - "strconv" - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" ) @@ -16,12 +13,7 @@ func WebAssetDownloadURL(repo *repo_model.Repository, attach *repo_model.Attachm } func APIAssetDownloadURL(repo *repo_model.Repository, attach *repo_model.Attachment) string { - if attach.CustomDownloadURL != "" { - return attach.CustomDownloadURL - } - - // /repos/{owner}/{repo}/releases/{id}/assets/{attachment_id} - return setting.AppURL + "api/repos/" + repo.FullName() + "/releases/" + strconv.FormatInt(attach.ReleaseID, 10) + "/assets/" + strconv.FormatInt(attach.ID, 10) + return attach.DownloadURL() } // ToAttachment converts models.Attachment to api.Attachment for API usage From 18dddd3487025c7d0e2e2672ef468d06206bc445 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 7 Oct 2023 09:10:29 +0800 Subject: [PATCH 2/3] Fix lint --- services/auth/auth_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/services/auth/auth_test.go b/services/auth/auth_test.go index f4e3cdf0d32dc..3adaa2866444e 100644 --- a/services/auth/auth_test.go +++ b/services/auth/auth_test.go @@ -85,6 +85,10 @@ func Test_isGitRawOrLFSPath(t *testing.T) { "/owner/repo/releases/download/tag/repo.tar.gz", true, }, + { + "/owner/repo/attachments/6d92a9ee-5d8b-4993-97c9-6181bdaa8955", + true, + }, } lfsTests := []string{ "/owner/repo/info/lfs/", @@ -104,11 +108,11 @@ func Test_isGitRawOrLFSPath(t *testing.T) { t.Run(tt.path, func(t *testing.T) { req, _ := http.NewRequest("POST", "http://localhost"+tt.path, nil) setting.LFS.StartServer = false - if got := isGitRawReleaseOrLFSPath(req); got != tt.want { + if got := isGitRawOrAttachOrLFSPath(req); got != tt.want { t.Errorf("isGitOrLFSPath() = %v, want %v", got, tt.want) } setting.LFS.StartServer = true - if got := isGitRawReleaseOrLFSPath(req); got != tt.want { + if got := isGitRawOrAttachOrLFSPath(req); got != tt.want { t.Errorf("isGitOrLFSPath() = %v, want %v", got, tt.want) } }) @@ -117,11 +121,11 @@ func Test_isGitRawOrLFSPath(t *testing.T) { t.Run(tt, func(t *testing.T) { req, _ := http.NewRequest("POST", tt, nil) setting.LFS.StartServer = false - if got := isGitRawReleaseOrLFSPath(req); got != setting.LFS.StartServer { - t.Errorf("isGitOrLFSPath(%q) = %v, want %v, %v", tt, got, setting.LFS.StartServer, gitRawReleasePathRe.MatchString(tt)) + if got := isGitRawOrAttachOrLFSPath(req); got != setting.LFS.StartServer { + t.Errorf("isGitOrLFSPath(%q) = %v, want %v, %v", tt, got, setting.LFS.StartServer, gitRawOrAttachPathRe.MatchString(tt)) } setting.LFS.StartServer = true - if got := isGitRawReleaseOrLFSPath(req); got != setting.LFS.StartServer { + if got := isGitRawOrAttachOrLFSPath(req); got != setting.LFS.StartServer { t.Errorf("isGitOrLFSPath(%q) = %v, want %v", tt, got, setting.LFS.StartServer) } }) From b6ea4edd5741607968f3c4af3eb2ec86347cd9b9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 9 Oct 2023 13:57:25 +0800 Subject: [PATCH 3/3] follow @delvh suggestion --- services/auth/auth.go | 6 +++++- services/auth/oauth2.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/services/auth/auth.go b/services/auth/auth.go index ea6c834942019..713463a3d47ed 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -40,8 +40,12 @@ var ( lfsPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/info/lfs/`) ) +func isGitRawOrAttachPath(req *http.Request) bool { + return gitRawOrAttachPathRe.MatchString(req.URL.Path) +} + func isGitRawOrAttachOrLFSPath(req *http.Request) bool { - if gitRawOrAttachPathRe.MatchString(req.URL.Path) { + if isGitRawOrAttachPath(req) { return true } if setting.LFS.StartServer { diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 0b49f15281dbb..08a2a05539b5a 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -127,7 +127,7 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store Dat func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) { // These paths are not API paths, but we still want to check for tokens because they maybe in the API returned URLs if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) && - !gitRawOrAttachPathRe.MatchString(req.URL.Path) { + !isGitRawOrAttachPath(req) { return nil, nil }