Fix repo API bug (#2133)

Don't require token when not necessary

integrations/api_fork_test.go

@@ -0,0 +1,18 @@
+// Copyright 2017 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package integrations
+
+import (
+	"net/http"
+	"testing"
+
+	api "code.gitea.io/sdk/gitea"
+)
+
+func TestCreateForkNoLogin(t *testing.T) {
+	prepareTestEnv(t)
+	req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/forks", &api.CreateForkOption{})
+	MakeRequest(t, req, http.StatusUnauthorized)
+}

integrations/api_keys_test.go

@@ -0,0 +1,39 @@
+// Copyright 2017 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package integrations
+
+import (
+	"net/http"
+	"testing"
+
+	api "code.gitea.io/sdk/gitea"
+)
+
+func TestViewDeployKeysNoLogin(t *testing.T) {
+	prepareTestEnv(t)
+	req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/keys")
+	MakeRequest(t, req, http.StatusUnauthorized)
+}
+
+func TestCreateDeployKeyNoLogin(t *testing.T) {
+	prepareTestEnv(t)
+	req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/keys", api.CreateKeyOption{
+		Title: "title",
+		Key:   "key",
+	})
+	MakeRequest(t, req, http.StatusUnauthorized)
+}
+
+func TestGetDeployKeyNoLogin(t *testing.T) {
+	prepareTestEnv(t)
+	req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/keys/1")
+	MakeRequest(t, req, http.StatusUnauthorized)
+}
+
+func TestDeleteDeployKeyNoLogin(t *testing.T) {
+	prepareTestEnv(t)
+	req := NewRequest(t, "DELETE", "/api/v1/repos/user2/repo1/keys/1")
+	MakeRequest(t, req, http.StatusUnauthorized)
+}

integrations/api_repo_test.go

@@ -51,3 +51,15 @@ func TestAPISearchRepoNotLogin(t *testing.T) {
 		assert.True(t, strings.Contains(repo.Name, keyword))
 	}
 }
+
+func TestAPIViewRepo(t *testing.T) {
+	prepareTestEnv(t)
+
+	req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1")
+	resp := MakeRequest(t, req, http.StatusOK)
+
+	var repo api.Repository
+	DecodeJSON(t, resp, &repo)
+	assert.EqualValues(t, 1, repo.ID)
+	assert.EqualValues(t, "repo1", repo.Name)
+}

routers/api/v1/api.go

@@ -42,6 +42,7 @@ import (
 	"code.gitea.io/gitea/routers/api/v1/org"
 	"code.gitea.io/gitea/routers/api/v1/repo"
 	"code.gitea.io/gitea/routers/api/v1/user"
+	"code.gitea.io/gitea/routers/api/v1/utils"
 )
 
 func repoAssignment() macaron.Handler {
@@ -92,7 +93,7 @@ func repoAssignment() macaron.Handler {
 		if ctx.IsSigned && ctx.User.IsAdmin {
 			ctx.Repo.AccessMode = models.AccessModeOwner
 		} else {
-			mode, err := models.AccessLevel(ctx.User.ID, repo)
+			mode, err := models.AccessLevel(utils.UserID(ctx), repo)
 			if err != nil {
 				ctx.Error(500, "AccessLevel", err)
 				return
@@ -341,27 +342,27 @@ func RegisterRoutes(m *macaron.Macaron) {
 		m.Combo("/repositories/:id", reqToken()).Get(repo.GetByID)
 
 		m.Group("/repos", func() {
-			m.Post("/migrate", bind(auth.MigrateRepoForm{}), repo.Migrate)
+			m.Post("/migrate", reqToken(), bind(auth.MigrateRepoForm{}), repo.Migrate)
 
 			m.Group("/:username/:reponame", func() {
-				m.Combo("").Get(repo.Get).Delete(repo.Delete)
+				m.Combo("").Get(repo.Get).Delete(reqToken(), repo.Delete)
 				m.Group("/hooks", func() {
 					m.Combo("").Get(repo.ListHooks).
 						Post(bind(api.CreateHookOption{}), repo.CreateHook)
 					m.Combo("/:id").Get(repo.GetHook).
 						Patch(bind(api.EditHookOption{}), repo.EditHook).
 						Delete(repo.DeleteHook)
-				}, reqRepoWriter())
+				}, reqToken(), reqRepoWriter())
 				m.Group("/collaborators", func() {
 					m.Get("", repo.ListCollaborators)
 					m.Combo("/:collaborator").Get(repo.IsCollaborator).
 						Put(bind(api.AddCollaboratorOption{}), repo.AddCollaborator).
 						Delete(repo.DeleteCollaborator)
-				})
+				}, reqToken())
 				m.Get("/raw/*", context.RepoRef(), repo.GetRawFile)
 				m.Get("/archive/*", repo.GetArchive)
 				m.Combo("/forks").Get(repo.ListForks).
-					Post(bind(api.CreateForkOption{}), repo.CreateFork)
+					Post(reqToken(), bind(api.CreateForkOption{}), repo.CreateFork)
 				m.Group("/branches", func() {
 					m.Get("", repo.ListBranches)
 					m.Get("/*", context.RepoRef(), repo.GetBranch)
@@ -371,78 +372,87 @@ func RegisterRoutes(m *macaron.Macaron) {
 						Post(bind(api.CreateKeyOption{}), repo.CreateDeployKey)
 					m.Combo("/:id").Get(repo.GetDeployKey).
 						Delete(repo.DeleteDeploykey)
-				})
+				}, reqToken())
 				m.Group("/issues", func() {
-					m.Combo("").Get(repo.ListIssues).Post(bind(api.CreateIssueOption{}), repo.CreateIssue)
+					m.Combo("").Get(repo.ListIssues).
+						Post(reqToken(), bind(api.CreateIssueOption{}), repo.CreateIssue)
 					m.Group("/comments", func() {
 						m.Get("", repo.ListRepoIssueComments)
-						m.Combo("/:id").Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueComment)
+						m.Combo("/:id", reqToken()).
+							Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueComment)
 					})
 					m.Group("/:index", func() {
-						m.Combo("").Get(repo.GetIssue).Patch(bind(api.EditIssueOption{}), repo.EditIssue)
+						m.Combo("").Get(repo.GetIssue).
+							Patch(reqToken(), bind(api.EditIssueOption{}), repo.EditIssue)
 
 						m.Group("/comments", func() {
-							m.Combo("").Get(repo.ListIssueComments).Post(bind(api.CreateIssueCommentOption{}), repo.CreateIssueComment)
+							m.Combo("").Get(repo.ListIssueComments).
-							m.Combo("/:id").Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueComment).
+								Post(reqToken(), bind(api.CreateIssueCommentOption{}), repo.CreateIssueComment)
+							m.Combo("/:id", reqToken()).Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueComment).
 								Delete(repo.DeleteIssueComment)
 						})
 
 						m.Group("/labels", func() {
 							m.Combo("").Get(repo.ListIssueLabels).
-								Post(bind(api.IssueLabelsOption{}), repo.AddIssueLabels).
+								Post(reqToken(), bind(api.IssueLabelsOption{}), repo.AddIssueLabels).
-								Put(bind(api.IssueLabelsOption{}), repo.ReplaceIssueLabels).
+								Put(reqToken(), bind(api.IssueLabelsOption{}), repo.ReplaceIssueLabels).
-								Delete(repo.ClearIssueLabels)
+								Delete(reqToken(), repo.ClearIssueLabels)
-							m.Delete("/:id", repo.DeleteIssueLabel)
+							m.Delete("/:id", reqToken(), repo.DeleteIssueLabel)
 						})
 
 					})
 				}, mustEnableIssues)
 				m.Group("/labels", func() {
 					m.Combo("").Get(repo.ListLabels).
-						Post(bind(api.CreateLabelOption{}), repo.CreateLabel)
+						Post(reqToken(), bind(api.CreateLabelOption{}), repo.CreateLabel)
-					m.Combo("/:id").Get(repo.GetLabel).Patch(bind(api.EditLabelOption{}), repo.EditLabel).
+					m.Combo("/:id").Get(repo.GetLabel).
-						Delete(repo.DeleteLabel)
+						Patch(reqToken(), bind(api.EditLabelOption{}), repo.EditLabel).
+						Delete(reqToken(), repo.DeleteLabel)
 				})
 				m.Group("/milestones", func() {
 					m.Combo("").Get(repo.ListMilestones).
-						Post(reqRepoWriter(), bind(api.CreateMilestoneOption{}), repo.CreateMilestone)
+						Post(reqToken(), reqRepoWriter(), bind(api.CreateMilestoneOption{}), repo.CreateMilestone)
 					m.Combo("/:id").Get(repo.GetMilestone).
-						Patch(reqRepoWriter(), bind(api.EditMilestoneOption{}), repo.EditMilestone).
+						Patch(reqToken(), reqRepoWriter(), bind(api.EditMilestoneOption{}), repo.EditMilestone).
-						Delete(reqRepoWriter(), repo.DeleteMilestone)
+						Delete(reqToken(), reqRepoWriter(), repo.DeleteMilestone)
 				})
 				m.Get("/stargazers", repo.ListStargazers)
 				m.Get("/subscribers", repo.ListSubscribers)
 				m.Group("/subscription", func() {
 					m.Get("", user.IsWatching)
-					m.Put("", user.Watch)
+					m.Put("", reqToken(), user.Watch)
-					m.Delete("", user.Unwatch)
+					m.Delete("", reqToken(), user.Unwatch)
 				})
 				m.Group("/releases", func() {
 					m.Combo("").Get(repo.ListReleases).
-						Post(bind(api.CreateReleaseOption{}), repo.CreateRelease)
+						Post(reqToken(), bind(api.CreateReleaseOption{}), repo.CreateRelease)
 					m.Combo("/:id").Get(repo.GetRelease).
-						Patch(bind(api.EditReleaseOption{}), repo.EditRelease).
+						Patch(reqToken(), bind(api.EditReleaseOption{}), repo.EditRelease).
-						Delete(repo.DeleteRelease)
+						Delete(reqToken(), repo.DeleteRelease)
 				})
-				m.Post("/mirror-sync", repo.MirrorSync)
+				m.Post("/mirror-sync", reqToken(), repo.MirrorSync)
 				m.Get("/editorconfig/:filename", context.RepoRef(), repo.GetEditorconfig)
 				m.Group("/pulls", func() {
-					m.Combo("").Get(bind(api.ListPullRequestsOptions{}), repo.ListPullRequests).Post(reqRepoWriter(), bind(api.CreatePullRequestOption{}), repo.CreatePullRequest)
+					m.Combo("").Get(bind(api.ListPullRequestsOptions{}), repo.ListPullRequests).
+						Post(reqToken(), reqRepoWriter(), bind(api.CreatePullRequestOption{}), repo.CreatePullRequest)
 					m.Group("/:index", func() {
-						m.Combo("").Get(repo.GetPullRequest).Patch(reqRepoWriter(), bind(api.EditPullRequestOption{}), repo.EditPullRequest)
+						m.Combo("").Get(repo.GetPullRequest).
-						m.Combo("/merge").Get(repo.IsPullRequestMerged).Post(reqRepoWriter(), repo.MergePullRequest)
+							Patch(reqToken(), reqRepoWriter(), bind(api.EditPullRequestOption{}), repo.EditPullRequest)
+						m.Combo("/merge").Get(repo.IsPullRequestMerged).
+							Post(reqToken(), reqRepoWriter(), repo.MergePullRequest)
 					})
 
 				}, mustAllowPulls, context.ReferencesGitRepo())
 				m.Group("/statuses", func() {
-					m.Combo("/:sha").Get(repo.GetCommitStatuses).Post(reqRepoWriter(), bind(api.CreateStatusOption{}), repo.NewCommitStatus)
+					m.Combo("/:sha").Get(repo.GetCommitStatuses).
+						Post(reqToken(), reqRepoWriter(), bind(api.CreateStatusOption{}), repo.NewCommitStatus)
 				})
 				m.Group("/commits/:ref", func() {
 					m.Get("/status", repo.GetCombinedCommitStatus)
 					m.Get("/statuses", repo.GetCommitStatuses)
 				})
 			}, repoAssignment())
-		}, reqToken())
+		})
 
 		// Organizations
 		m.Get("/user/orgs", reqToken(), org.ListMyOrgs)

routers/api/v1/repo/fork.go

@@ -9,6 +9,7 @@ import (
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/routers/api/v1/utils"
 )
 
 // ListForks list a repository's forks
@@ -29,7 +30,7 @@ func ListForks(ctx *context.APIContext) {
 	}
 	apiForks := make([]*api.Repository, len(forks))
 	for i, fork := range forks {
-		access, err := models.AccessLevel(ctx.User.ID, fork)
+		access, err := models.AccessLevel(utils.UserID(ctx), fork)
 		if err != nil {
 			ctx.Error(500, "AccessLevel", err)
 			return

routers/api/v1/repo/release.go

@@ -34,14 +34,8 @@ func GetRelease(ctx *context.APIContext) {
 
 // ListReleases list a repository's releases
 func ListReleases(ctx *context.APIContext) {
-	access, err := models.AccessLevel(ctx.User.ID, ctx.Repo.Repository)
-	if err != nil {
-		ctx.Error(500, "AccessLevel", err)
-		return
-	}
-
 	releases, err := models.GetReleasesByRepoID(ctx.Repo.Repository.ID, models.FindReleasesOptions{
-		IncludeDrafts: access >= models.AccessModeWrite,
+		IncludeDrafts: ctx.Repo.AccessMode >= models.AccessModeWrite,
 	}, 1, 2147483647)
 	if err != nil {
 		ctx.Error(500, "GetReleasesByRepoID", err)

routers/api/v1/repo/repo.go

@@ -267,13 +267,7 @@ func Get(ctx *context.APIContext) {
 	//       200: Repository
 	//       500: error
 
-	repo := ctx.Repo.Repository
+	ctx.JSON(200, ctx.Repo.Repository.APIFormat(ctx.Repo.AccessMode))
-	access, err := models.AccessLevel(ctx.User.ID, repo)
-	if err != nil {
-		ctx.Error(500, "GetRepository", err)
-		return
-	}
-	ctx.JSON(200, repo.APIFormat(access))
 }
 
 // GetByID returns a single Repository

routers/api/v1/repo/status.go

@@ -103,15 +103,10 @@ func GetCombinedCommitStatus(ctx *context.APIContext) {
 		return
 	}
 
-	acl, err := models.AccessLevel(ctx.User.ID, repo)
-	if err != nil {
-		ctx.Error(500, "AccessLevel", fmt.Errorf("AccessLevel[%d, %s]: %v", ctx.User.ID, repo.FullName(), err))
-		return
-	}
 	retStatus := &combinedCommitStatus{
 		SHA:        sha,
 		TotalCount: len(statuses),
-		Repo:       repo.APIFormat(acl),
+		Repo:       repo.APIFormat(ctx.Repo.AccessMode),
 		URL:        "",
 	}
 

routers/api/v1/utils/utils.go

@@ -0,0 +1,15 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package utils
+
+import "code.gitea.io/gitea/modules/context"
+
+// UserID user ID of authenticated user, or 0 if not authenticated
+func UserID(ctx *context.APIContext) int64 {
+	if ctx.User == nil {
+		return 0
+	}
+	return ctx.User.ID
+}