Skip to content

Commit 8b24073

Browse files
sapklafriks
sapk
authored and
lafriks
committed
Only serve attachments when linked to issue/release and if accessible by user (#9340)
* test: add current attachement responses * refactor: check if attachement is linked and accessible by user * chore: clean TODO * fix: typo attachement -> attachment * revert un-needed go.sum change * refactor: move models logic to models * fix TestCreateIssueAttachment which was wrongly successful * fix unit tests with unittype added * fix unit tests with changes * use a valid uuid format for pgsql int. test * test: add unit test TestLinkedRepository * refactor: allow uploader to access unlinked attachement * add missing blank line * refactor: move to a separate function repo.GetAttachment * typo * test: remove err test return * refactor: use repo perm for access checking generally + 404 for all reject
1 parent 6a5a2f4 commit 8b24073

File tree

10 files changed

+279
-124
lines changed

10 files changed

+279
-124
lines changed

‎integrations/attachement_test.go

Lines changed: 0 additions & 88 deletions
This file was deleted.

‎integrations/attachment_test.go

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Copyright 2019 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package integrations
6+
7+
import (
8+
"bytes"
9+
"image"
10+
"image/png"
11+
"io"
12+
"io/ioutil"
13+
"mime/multipart"
14+
"net/http"
15+
"os"
16+
"path"
17+
"testing"
18+
19+
"code.gitea.io/gitea/models"
20+
"code.gitea.io/gitea/modules/test"
21+
22+
"github.com/stretchr/testify/assert"
23+
)
24+
25+
func generateImg() bytes.Buffer {
26+
// Generate image
27+
myImage := image.NewRGBA(image.Rect(0, 0, 32, 32))
28+
var buff bytes.Buffer
29+
png.Encode(&buff, myImage)
30+
return buff
31+
}
32+
33+
func createAttachment(t *testing.T, session *TestSession, repoURL, filename string, buff bytes.Buffer, expectedStatus int) string {
34+
body := &bytes.Buffer{}
35+
36+
//Setup multi-part
37+
writer := multipart.NewWriter(body)
38+
part, err := writer.CreateFormFile("file", filename)
39+
assert.NoError(t, err)
40+
_, err = io.Copy(part, &buff)
41+
assert.NoError(t, err)
42+
err = writer.Close()
43+
assert.NoError(t, err)
44+
45+
csrf := GetCSRF(t, session, repoURL)
46+
47+
req := NewRequestWithBody(t, "POST", "/attachments", body)
48+
req.Header.Add("X-Csrf-Token", csrf)
49+
req.Header.Add("Content-Type", writer.FormDataContentType())
50+
resp := session.MakeRequest(t, req, expectedStatus)
51+
52+
if expectedStatus != http.StatusOK {
53+
return ""
54+
}
55+
var obj map[string]string
56+
DecodeJSON(t, resp, &obj)
57+
return obj["uuid"]
58+
}
59+
60+
func TestCreateAnonymousAttachment(t *testing.T) {
61+
prepareTestEnv(t)
62+
session := emptyTestSession(t)
63+
createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusFound)
64+
}
65+
66+
func TestCreateIssueAttachment(t *testing.T) {
67+
prepareTestEnv(t)
68+
const repoURL = "user2/repo1"
69+
session := loginUser(t, "user2")
70+
uuid := createAttachment(t, session, repoURL, "image.png", generateImg(), http.StatusOK)
71+
72+
req := NewRequest(t, "GET", repoURL+"/issues/new")
73+
resp := session.MakeRequest(t, req, http.StatusOK)
74+
htmlDoc := NewHTMLParser(t, resp.Body)
75+
76+
link, exists := htmlDoc.doc.Find("form").Attr("action")
77+
assert.True(t, exists, "The template has changed")
78+
79+
postData := map[string]string{
80+
"_csrf": htmlDoc.GetCSRF(),
81+
"title": "New Issue With Attachment",
82+
"content": "some content",
83+
"files": uuid,
84+
}
85+
86+
req = NewRequestWithValues(t, "POST", link, postData)
87+
resp = session.MakeRequest(t, req, http.StatusFound)
88+
test.RedirectURL(resp) // check that redirect URL exists
89+
90+
//Validate that attachment is available
91+
req = NewRequest(t, "GET", "/attachments/"+uuid)
92+
session.MakeRequest(t, req, http.StatusOK)
93+
}
94+
95+
func TestGetAttachment(t *testing.T) {
96+
prepareTestEnv(t)
97+
adminSession := loginUser(t, "user1")
98+
user2Session := loginUser(t, "user2")
99+
user8Session := loginUser(t, "user8")
100+
emptySession := emptyTestSession(t)
101+
testCases := []struct {
102+
name string
103+
uuid string
104+
createFile bool
105+
session *TestSession
106+
want int
107+
}{
108+
{"LinkedIssueUUID", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", true, user2Session, http.StatusOK},
109+
{"LinkedCommentUUID", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a17", true, user2Session, http.StatusOK},
110+
{"linked_release_uuid", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a19", true, user2Session, http.StatusOK},
111+
{"NotExistingUUID", "b0eebc99-9c0b-4ef8-bb6d-6bb9bd380a18", false, user2Session, http.StatusNotFound},
112+
{"FileMissing", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a18", false, user2Session, http.StatusInternalServerError},
113+
{"NotLinked", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20", true, user2Session, http.StatusNotFound},
114+
{"NotLinkedAccessibleByUploader", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20", true, user8Session, http.StatusOK},
115+
{"PublicByNonLogged", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", true, emptySession, http.StatusOK},
116+
{"PrivateByNonLogged", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12", true, emptySession, http.StatusNotFound},
117+
{"PrivateAccessibleByAdmin", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12", true, adminSession, http.StatusOK},
118+
{"PrivateAccessibleByUser", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12", true, user2Session, http.StatusOK},
119+
{"RepoNotAccessibleByUser", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12", true, user8Session, http.StatusNotFound},
120+
{"OrgNotAccessibleByUser", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a21", true, user8Session, http.StatusNotFound},
121+
}
122+
for _, tc := range testCases {
123+
t.Run(tc.name, func(t *testing.T) {
124+
//Write empty file to be available for response
125+
if tc.createFile {
126+
localPath := models.AttachmentLocalPath(tc.uuid)
127+
err := os.MkdirAll(path.Dir(localPath), os.ModePerm)
128+
assert.NoError(t, err)
129+
err = ioutil.WriteFile(localPath, []byte("hello world"), 0644)
130+
assert.NoError(t, err)
131+
}
132+
//Actual test
133+
req := NewRequest(t, "GET", "/attachments/"+tc.uuid)
134+
tc.session.MakeRequest(t, req, tc.want)
135+
})
136+
}
137+
}

‎models/attachment.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,26 @@ func (a *Attachment) DownloadURL() string {
7171
return fmt.Sprintf("%sattachments/%s", setting.AppURL, a.UUID)
7272
}
7373

74+
// LinkedRepository returns the linked repo if any
75+
func (a *Attachment) LinkedRepository() (*Repository, UnitType, error) {
76+
if a.IssueID != 0 {
77+
iss, err := GetIssueByID(a.IssueID)
78+
if err != nil {
79+
return nil, UnitTypeIssues, err
80+
}
81+
repo, err := GetRepositoryByID(iss.RepoID)
82+
return repo, UnitTypeIssues, err
83+
} else if a.ReleaseID != 0 {
84+
rel, err := GetReleaseByID(a.ReleaseID)
85+
if err != nil {
86+
return nil, UnitTypeReleases, err
87+
}
88+
repo, err := GetRepositoryByID(rel.RepoID)
89+
return repo, UnitTypeReleases, err
90+
}
91+
return nil, -1, nil
92+
}
93+
7494
// NewAttachment creates a new attachment object.
7595
func NewAttachment(attach *Attachment, buf []byte, file io.Reader) (_ *Attachment, err error) {
7696
attach.UUID = gouuid.NewV4().String()

‎models/attachment_test.go

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func TestGetByCommentOrIssueID(t *testing.T) {
6161
// count of attachments from issue ID
6262
attachments, err := GetAttachmentsByIssueID(1)
6363
assert.NoError(t, err)
64-
assert.Equal(t, 2, len(attachments))
64+
assert.Equal(t, 1, len(attachments))
6565

6666
attachments, err = GetAttachmentsByCommentID(1)
6767
assert.NoError(t, err)
@@ -73,7 +73,7 @@ func TestDeleteAttachments(t *testing.T) {
7373

7474
count, err := DeleteAttachmentsByIssue(4, false)
7575
assert.NoError(t, err)
76-
assert.Equal(t, 1, count)
76+
assert.Equal(t, 2, count)
7777

7878
count, err = DeleteAttachmentsByComment(2, false)
7979
assert.NoError(t, err)
@@ -128,3 +128,31 @@ func TestGetAttachmentsByUUIDs(t *testing.T) {
128128
assert.Equal(t, int64(1), attachList[0].IssueID)
129129
assert.Equal(t, int64(5), attachList[1].IssueID)
130130
}
131+
132+
func TestLinkedRepository(t *testing.T) {
133+
assert.NoError(t, PrepareTestDatabase())
134+
testCases := []struct {
135+
name string
136+
attachID int64
137+
expectedRepo *Repository
138+
expectedUnitType UnitType
139+
}{
140+
{"LinkedIssue", 1, &Repository{ID: 1}, UnitTypeIssues},
141+
{"LinkedComment", 3, &Repository{ID: 1}, UnitTypeIssues},
142+
{"LinkedRelease", 9, &Repository{ID: 1}, UnitTypeReleases},
143+
{"Notlinked", 10, nil, -1},
144+
}
145+
for _, tc := range testCases {
146+
t.Run(tc.name, func(t *testing.T) {
147+
attach, err := GetAttachmentByID(tc.attachID)
148+
assert.NoError(t, err)
149+
repo, unitType, err := attach.LinkedRepository()
150+
assert.NoError(t, err)
151+
if tc.expectedRepo != nil {
152+
assert.Equal(t, tc.expectedRepo.ID, repo.ID)
153+
}
154+
assert.Equal(t, tc.expectedUnitType, unitType)
155+
156+
})
157+
}
158+
}

‎models/fixtures/attachment.yml

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
-
1111
id: 2
1212
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12
13-
issue_id: 1
13+
issue_id: 4
1414
comment_id: 0
1515
name: attach2
1616
download_count: 1
@@ -81,6 +81,15 @@
8181
-
8282
id: 10
8383
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20
84+
uploader_id: 8
85+
name: attach1
86+
download_count: 0
87+
created_unix: 946684800
88+
89+
-
90+
id: 11
91+
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a21
92+
release_id: 2
8493
name: attach1
8594
download_count: 0
86-
created_unix: 946684800
95+
created_unix: 946684800

‎models/fixtures/release.yml

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,19 @@
1111
is_draft: false
1212
is_prerelease: false
1313
is_tag: false
14-
created_unix: 946684800
14+
created_unix: 946684800
15+
16+
-
17+
id: 2
18+
repo_id: 40
19+
publisher_id: 2
20+
tag_name: "v1.1"
21+
lower_tag_name: "v1.1"
22+
target: "master"
23+
title: "testing-release"
24+
sha1: "65f1bf27bc3bf70f64657658635e66094edbcb4d"
25+
num_commits: 10
26+
is_draft: false
27+
is_prerelease: false
28+
is_tag: false
29+
created_unix: 946684800

‎models/fixtures/repo_unit.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,4 +472,10 @@
472472
repo_id: 48
473473
type: 7
474474
config: "{\"ExternalTrackerURL\":\"https://tracker.com\",\"ExternalTrackerFormat\":\"https://tracker.com/{user}/{repo}/issues/{index}\",\"ExternalTrackerStyle\":\"alphanumeric\"}"
475+
created_unix: 946684810
476+
-
477+
id: 69
478+
repo_id: 2
479+
type: 2
480+
config: "{}"
475481
created_unix: 946684810

0 commit comments

Comments
 (0)