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

5 files changed, 83 insertions, 5 deletions

diff --git a/models/attachment.go b/models/attachment.go
index 487ddd4ad5..6cfa6cb64e 100644
--- a/models/attachment.go
+++ b/models/attachment.go
@@ -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()
diff --git a/models/attachment_test.go b/models/attachment_test.go
index f38a5beeee..ddb6abad32 100644
--- a/models/attachment_test.go
+++ b/models/attachment_test.go
@@ -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)
@@ -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)
@@ -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)
+
+		})
+	}
+}
diff --git a/models/fixtures/attachment.yml b/models/fixtures/attachment.yml
index 289d4d0efd..2606d52b47 100644
--- a/models/fixtures/attachment.yml
+++ b/models/fixtures/attachment.yml
@@ -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
@@ -81,6 +81,15 @@
 -
   id: 10
   uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20
+  uploader_id: 8
+  name: attach1
+  download_count: 0
+  created_unix: 946684800
+
+-
+  id: 11
+  uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a21
+  release_id: 2
   name: attach1
   download_count: 0
-  created_unix: 946684800 
\ No newline at end of file
+  created_unix: 946684800
diff --git a/models/fixtures/release.yml b/models/fixtures/release.yml
index db9a6b503d..f95eb048be 100644
--- a/models/fixtures/release.yml
+++ b/models/fixtures/release.yml
@@ -11,4 +11,19 @@
   is_draft: false
   is_prerelease: false
   is_tag: false
-  created_unix: 946684800
\ No newline at end of file
+  created_unix: 946684800
+
+-
+  id: 2
+  repo_id: 40
+  publisher_id: 2
+  tag_name: "v1.1"
+  lower_tag_name: "v1.1"
+  target: "master"
+  title: "testing-release"
+  sha1: "65f1bf27bc3bf70f64657658635e66094edbcb4d"
+  num_commits: 10
+  is_draft: false
+  is_prerelease: false
+  is_tag: false
+  created_unix: 946684800
diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml
index f7522d3031..5ced38b003 100644
--- a/models/fixtures/repo_unit.yml
+++ b/models/fixtures/repo_unit.yml
@@ -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
\ No newline at end of file