Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only serve attachments when linked to issue/release and if accessible by user #9340

Merged
merged 30 commits into from
Jan 4, 2020
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
6b15314
test: add current attachement responses
sapk Dec 12, 2019
595f5bb
refactor: check if attachement is linked and accessible by user
sapk Dec 13, 2019
cd494ee
chore: clean TODO
sapk Dec 13, 2019
a87f112
fix: typo attachement -> attachment
sapk Dec 13, 2019
3a90576
revert un-needed go.sum change
sapk Dec 13, 2019
67551c4
refactor: move models logic to models
sapk Dec 13, 2019
4740e0c
fix TestCreateIssueAttachment which was wrongly successful
sapk Dec 13, 2019
fbddeac
fix unit tests with unittype added
sapk Dec 13, 2019
7ebe2c3
fix unit tests with changes
sapk Dec 13, 2019
b488f1d
use a valid uuid format for pgsql int. test
sapk Dec 13, 2019
0e3f0f1
Merge branch 'master' into not-found-not-linked
sapk Dec 13, 2019
ee21be2
Merge branch 'master' into not-found-not-linked
sapk Dec 13, 2019
5db84c2
Merge branch 'master' into not-found-not-linked
sapk Dec 14, 2019
3790b46
Merge branch 'master' into not-found-not-linked
sapk Dec 17, 2019
93c305b
Merge branch 'master' into not-found-not-linked
sapk Dec 18, 2019
ce4993a
test: add unit test TestLinkedRepository
sapk Dec 18, 2019
49a18a2
refactor: allow uploader to access unlinked attachement
sapk Dec 18, 2019
5292d3f
Merge branch 'master' into not-found-not-linked
sapk Dec 18, 2019
26df92c
Merge branch 'master' into not-found-not-linked
sapk Dec 19, 2019
48006bb
Merge branch 'master' into not-found-not-linked
sapk Dec 20, 2019
24476b2
Merge branch 'master' into not-found-not-linked
sapk Dec 27, 2019
71c5cfd
Merge branch 'master' into not-found-not-linked
sapk Jan 2, 2020
06162e3
add missing blank line
sapk Jan 2, 2020
19b9eb9
refactor: move to a separate function repo.GetAttachment
sapk Jan 2, 2020
219a4d0
typo
sapk Jan 2, 2020
9664c9f
Merge branch 'master' into not-found-not-linked
sapk Jan 2, 2020
f6f30d1
Merge branch 'master' into not-found-not-linked
sapk Jan 4, 2020
dc68569
test: remove err test return
sapk Jan 4, 2020
149d2f6
refactor: use repo perm for access checking generally + 404 for all r…
sapk Jan 4, 2020
63b8479
Merge branch 'master' into not-found-not-linked
sapk Jan 4, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
88 changes: 0 additions & 88 deletions integrations/attachement_test.go

This file was deleted.

136 changes: 136 additions & 0 deletions integrations/attachment_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// Copyright 2019 The Gitea Authors. All rights reserved.
sapk marked this conversation as resolved.
Show resolved Hide resolved
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package integrations

import (
"bytes"
"image"
"image/png"
"io"
"io/ioutil"
"mime/multipart"
"net/http"
"os"
"path"
"testing"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/test"

"github.com/stretchr/testify/assert"
sapk marked this conversation as resolved.
Show resolved Hide resolved
)

func generateImg() bytes.Buffer {
// Generate image
myImage := image.NewRGBA(image.Rect(0, 0, 32, 32))
var buff bytes.Buffer
png.Encode(&buff, myImage)
return buff
}

func createAttachment(t *testing.T, session *TestSession, repoURL, filename string, buff bytes.Buffer, expectedStatus int) string {
body := &bytes.Buffer{}

//Setup multi-part
writer := multipart.NewWriter(body)
part, err := writer.CreateFormFile("file", filename)
assert.NoError(t, err)
_, err = io.Copy(part, &buff)
assert.NoError(t, err)
err = writer.Close()
assert.NoError(t, err)

csrf := GetCSRF(t, session, repoURL)

req := NewRequestWithBody(t, "POST", "/attachments", body)
req.Header.Add("X-Csrf-Token", csrf)
req.Header.Add("Content-Type", writer.FormDataContentType())
resp := session.MakeRequest(t, req, expectedStatus)

if expectedStatus != http.StatusOK {
return ""
}
var obj map[string]string
DecodeJSON(t, resp, &obj)
return obj["uuid"]
}

func TestCreateAnonymousAttachment(t *testing.T) {
prepareTestEnv(t)
session := emptyTestSession(t)
createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusFound)
}

func TestCreateIssueAttachment(t *testing.T) {
prepareTestEnv(t)
const repoURL = "user2/repo1"
session := loginUser(t, "user2")
uuid := createAttachment(t, session, repoURL, "image.png", generateImg(), http.StatusOK)

req := NewRequest(t, "GET", repoURL+"/issues/new")
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)

link, exists := htmlDoc.doc.Find("form").Attr("action")
assert.True(t, exists, "The template has changed")

postData := map[string]string{
"_csrf": htmlDoc.GetCSRF(),
"title": "New Issue With Attachment",
"content": "some content",
"files": uuid,
}

req = NewRequestWithValues(t, "POST", link, postData)
resp = session.MakeRequest(t, req, http.StatusFound)
test.RedirectURL(resp) // check that redirect URL exists

//Validate that attachment is available
req = NewRequest(t, "GET", "/attachments/"+uuid)
session.MakeRequest(t, req, http.StatusOK)
}

func TestGetAttachment(t *testing.T) {
prepareTestEnv(t)
adminSession := loginUser(t, "user1")
user2Session := loginUser(t, "user2")
user8Session := loginUser(t, "user8")
emptySession := emptyTestSession(t)
testCases := []struct {
name string
uuid string
createFile bool
session *TestSession
want int
}{
{"LinkedIssueUUID", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", true, user2Session, http.StatusOK},
{"LinkedCommentUUID", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a17", true, user2Session, http.StatusOK},
{"linked_release_uuid", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a19", true, user2Session, http.StatusOK},
{"NotExistingUUID", "b0eebc99-9c0b-4ef8-bb6d-6bb9bd380a18", false, user2Session, http.StatusNotFound},
{"FileMissing", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a18", false, user2Session, http.StatusInternalServerError},
{"NotLinked", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20", true, user2Session, http.StatusNotFound},
{"NotLinkedAccessibleByUploader", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20", true, user8Session, http.StatusOK},
{"PublicByNonLogged", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", true, emptySession, http.StatusOK},
{"PrivateByNonLogged", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12", true, emptySession, http.StatusNotFound},
{"PrivateAccessibleByAdmin", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12", true, adminSession, http.StatusOK},
{"PrivateAccessibleByUser", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12", true, user2Session, http.StatusOK},
{"NotAccessibleByUser", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12", true, user8Session, http.StatusForbidden},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
//Write empty file to be available for response
if tc.createFile {
localPath := models.AttachmentLocalPath(tc.uuid)
err := os.MkdirAll(path.Dir(localPath), os.ModePerm)
assert.NoError(t, err)
err = ioutil.WriteFile(localPath, []byte("hello world"), 0644)
assert.NoError(t, err)
}
//Actual test
req := NewRequest(t, "GET", "/attachments/"+tc.uuid)
tc.session.MakeRequest(t, req, tc.want)
})
}
}
20 changes: 20 additions & 0 deletions models/attachment.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,26 @@ func (a *Attachment) DownloadURL() string {
return fmt.Sprintf("%sattachments/%s", setting.AppURL, a.UUID)
}

// LinkedRepository returns the linked repo if any
func (a *Attachment) LinkedRepository() (*Repository, UnitType, error) {
if a.IssueID != 0 {
iss, err := GetIssueByID(a.IssueID)
if err != nil {
return nil, UnitTypeIssues, err
}
repo, err := GetRepositoryByID(iss.RepoID)
return repo, UnitTypeIssues, err
} else if a.ReleaseID != 0 {
rel, err := GetReleaseByID(a.ReleaseID)
if err != nil {
return nil, UnitTypeReleases, err
}
repo, err := GetRepositoryByID(rel.RepoID)
return repo, UnitTypeReleases, err
}
return nil, -1, nil
}

// NewAttachment creates a new attachment object.
func NewAttachment(attach *Attachment, buf []byte, file io.Reader) (_ *Attachment, err error) {
attach.UUID = gouuid.NewV4().String()
Expand Down
32 changes: 30 additions & 2 deletions models/attachment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestGetByCommentOrIssueID(t *testing.T) {
// count of attachments from issue ID
attachments, err := GetAttachmentsByIssueID(1)
assert.NoError(t, err)
assert.Equal(t, 2, len(attachments))
assert.Equal(t, 1, len(attachments))

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

count, err := DeleteAttachmentsByIssue(4, false)
assert.NoError(t, err)
assert.Equal(t, 1, count)
assert.Equal(t, 2, count)

count, err = DeleteAttachmentsByComment(2, false)
assert.NoError(t, err)
Expand Down Expand Up @@ -128,3 +128,31 @@ func TestGetAttachmentsByUUIDs(t *testing.T) {
assert.Equal(t, int64(1), attachList[0].IssueID)
assert.Equal(t, int64(5), attachList[1].IssueID)
}

func TestLinkedRepository(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
testCases := []struct {
name string
attachID int64
expectedRepo *Repository
expectedUnitType UnitType
}{
{"LinkedIssue", 1, &Repository{ID: 1}, UnitTypeIssues},
{"LinkedComment", 3, &Repository{ID: 1}, UnitTypeIssues},
{"LinkedRelease", 9, &Repository{ID: 1}, UnitTypeReleases},
{"Notlinked", 10, nil, -1},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
attach, err := GetAttachmentByID(tc.attachID)
assert.NoError(t, err)
repo, unitType, err := attach.LinkedRepository()
assert.NoError(t, err)
if tc.expectedRepo != nil {
assert.Equal(t, tc.expectedRepo.ID, repo.ID)
}
assert.Equal(t, tc.expectedUnitType, unitType)

})
}
}
3 changes: 2 additions & 1 deletion models/fixtures/attachment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
-
id: 2
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12
issue_id: 1
issue_id: 4
comment_id: 0
name: attach2
download_count: 1
Expand Down Expand Up @@ -81,6 +81,7 @@
-
id: 10
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20
uploader_id: 8
name: attach1
download_count: 0
created_unix: 946684800
6 changes: 6 additions & 0 deletions models/fixtures/repo_unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -472,4 +472,10 @@
repo_id: 48
type: 7
config: "{\"ExternalTrackerURL\":\"https://tracker.com\",\"ExternalTrackerFormat\":\"https://tracker.com/{user}/{repo}/issues/{index}\",\"ExternalTrackerStyle\":\"alphanumeric\"}"
created_unix: 946684810
-
id: 69
repo_id: 2
type: 2
config: "{}"
created_unix: 946684810