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

integrations/attachement_test.go → integrations/attachment_test.go

@@ -9,10 +9,14 @@ import (
 	"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"
 )
@@ -58,7 +62,7 @@ func TestCreateAnonymousAttachment(t *testing.T) {
 	createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusFound)
 }
 
-func TestCreateIssueAttachement(t *testing.T) {
+func TestCreateIssueAttachment(t *testing.T) {
 	prepareTestEnv(t)
 	const repoURL = "user2/repo1"
 	session := loginUser(t, "user2")
@@ -73,7 +77,7 @@ func TestCreateIssueAttachement(t *testing.T) {
 
 	postData := map[string]string{
 		"_csrf":    htmlDoc.GetCSRF(),
-		"title":    "New Issue With Attachement",
+		"title":    "New Issue With Attachment",
 		"content":  "some content",
 		"files[0]": uuid,
 	}
@@ -82,7 +86,49 @@ func TestCreateIssueAttachement(t *testing.T) {
 	resp = session.MakeRequest(t, req, http.StatusFound)
 	test.RedirectURL(resp) // check that redirect URL exists
 
-	//Validate that attachement is available
+	//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", "not-existing-uuid", false, user2Session, http.StatusNotFound},
+		{"FileMissing", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a18", false, user2Session, http.StatusInternalServerError},
+		{"NotLinked", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20", true, user2Session, http.StatusNotFound},
+		{"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)
+		})
+	}
+}

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

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

routers/routes/routes.go

@@ -495,6 +495,53 @@ func RegisterRoutes(m *macaron.Macaron) {
 				return
 			}
 
+			if attach.IssueID != 0 {
+				iss, err := models.GetIssueByID(attach.IssueID)
+				if err != nil {
+					ctx.ServerError("GetAttachmentByUUID.GetIssueByID", err)
+					return
+				}
+				repo, err := models.GetRepositoryByID(iss.RepoID)
+				if err != nil {
+					ctx.ServerError("GetAttachmentByUUID.GetRepositoryByID", err)
+					return
+				}
+				if repo.IsPrivate {
+					if !ctx.IsSigned {
+						ctx.Error(http.StatusNotFound)
+						return
+					}
+					if !repo.CheckUnitUser(ctx.User.ID, ctx.User.IsAdmin, models.UnitTypeIssues) {
+						ctx.Error(http.StatusForbidden)
+						return
+					}
+				}
+			} else if attach.ReleaseID != 0 {
+				rel, err := models.GetReleaseByID(attach.ReleaseID)
+				if err != nil {
+					ctx.ServerError("GetAttachmentByUUID.GetReleaseByID", err)
+					return
+				}
+				repo, err := models.GetRepositoryByID(rel.RepoID)
+				if err != nil {
+					ctx.ServerError("GetAttachmentByUUID.GetRepositoryByID", err)
+					return
+				}
+				if repo.IsPrivate {
+					if !ctx.IsSigned {
+						ctx.Error(http.StatusNotFound)
+						return
+					}
+					if !repo.CheckUnitUser(ctx.User.ID, ctx.User.IsAdmin, models.UnitTypeReleases) {
+						ctx.Error(http.StatusForbidden)
+						return
+					}
+				}
+			} else {
+				ctx.Error(http.StatusNotFound)
+			}
+
+			//If we have matched and access to release or issue
 			fr, err := os.Open(attach.LocalPath())
 			if err != nil {
 				ctx.ServerError("Open", err)