Skip to content

Commit 93a1de4

Browse files
ethantkoenigbkcsoft
ethantkoenig
authored and
bkcsoft
committed
Fix repo API bug (#2133)
Don't require token when not necessary
1 parent da89afd commit 93a1de4

File tree

9 files changed

+132
-54
lines changed

9 files changed

+132
-54
lines changed

‎integrations/api_fork_test.go

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright 2017 The Gogs 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+
"net/http"
9+
"testing"
10+
11+
api "code.gitea.io/sdk/gitea"
12+
)
13+
14+
func TestCreateForkNoLogin(t *testing.T) {
15+
prepareTestEnv(t)
16+
req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/forks", &api.CreateForkOption{})
17+
MakeRequest(t, req, http.StatusUnauthorized)
18+
}

‎integrations/api_keys_test.go

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright 2017 The Gogs 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+
"net/http"
9+
"testing"
10+
11+
api "code.gitea.io/sdk/gitea"
12+
)
13+
14+
func TestViewDeployKeysNoLogin(t *testing.T) {
15+
prepareTestEnv(t)
16+
req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/keys")
17+
MakeRequest(t, req, http.StatusUnauthorized)
18+
}
19+
20+
func TestCreateDeployKeyNoLogin(t *testing.T) {
21+
prepareTestEnv(t)
22+
req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/keys", api.CreateKeyOption{
23+
Title: "title",
24+
Key: "key",
25+
})
26+
MakeRequest(t, req, http.StatusUnauthorized)
27+
}
28+
29+
func TestGetDeployKeyNoLogin(t *testing.T) {
30+
prepareTestEnv(t)
31+
req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/keys/1")
32+
MakeRequest(t, req, http.StatusUnauthorized)
33+
}
34+
35+
func TestDeleteDeployKeyNoLogin(t *testing.T) {
36+
prepareTestEnv(t)
37+
req := NewRequest(t, "DELETE", "/api/v1/repos/user2/repo1/keys/1")
38+
MakeRequest(t, req, http.StatusUnauthorized)
39+
}

‎integrations/api_repo_test.go

+12
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,15 @@ func TestAPISearchRepoNotLogin(t *testing.T) {
5151
assert.True(t, strings.Contains(repo.Name, keyword))
5252
}
5353
}
54+
55+
func TestAPIViewRepo(t *testing.T) {
56+
prepareTestEnv(t)
57+
58+
req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1")
59+
resp := MakeRequest(t, req, http.StatusOK)
60+
61+
var repo api.Repository
62+
DecodeJSON(t, resp, &repo)
63+
assert.EqualValues(t, 1, repo.ID)
64+
assert.EqualValues(t, "repo1", repo.Name)
65+
}

‎routers/api/v1/api.go

+43-33
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"code.gitea.io/gitea/routers/api/v1/org"
4343
"code.gitea.io/gitea/routers/api/v1/repo"
4444
"code.gitea.io/gitea/routers/api/v1/user"
45+
"code.gitea.io/gitea/routers/api/v1/utils"
4546
)
4647

4748
func repoAssignment() macaron.Handler {
@@ -92,7 +93,7 @@ func repoAssignment() macaron.Handler {
9293
if ctx.IsSigned && ctx.User.IsAdmin {
9394
ctx.Repo.AccessMode = models.AccessModeOwner
9495
} else {
95-
mode, err := models.AccessLevel(ctx.User.ID, repo)
96+
mode, err := models.AccessLevel(utils.UserID(ctx), repo)
9697
if err != nil {
9798
ctx.Error(500, "AccessLevel", err)
9899
return
@@ -341,27 +342,27 @@ func RegisterRoutes(m *macaron.Macaron) {
341342
m.Combo("/repositories/:id", reqToken()).Get(repo.GetByID)
342343

343344
m.Group("/repos", func() {
344-
m.Post("/migrate", bind(auth.MigrateRepoForm{}), repo.Migrate)
345+
m.Post("/migrate", reqToken(), bind(auth.MigrateRepoForm{}), repo.Migrate)
345346

346347
m.Group("/:username/:reponame", func() {
347-
m.Combo("").Get(repo.Get).Delete(repo.Delete)
348+
m.Combo("").Get(repo.Get).Delete(reqToken(), repo.Delete)
348349
m.Group("/hooks", func() {
349350
m.Combo("").Get(repo.ListHooks).
350351
Post(bind(api.CreateHookOption{}), repo.CreateHook)
351352
m.Combo("/:id").Get(repo.GetHook).
352353
Patch(bind(api.EditHookOption{}), repo.EditHook).
353354
Delete(repo.DeleteHook)
354-
}, reqRepoWriter())
355+
}, reqToken(), reqRepoWriter())
355356
m.Group("/collaborators", func() {
356357
m.Get("", repo.ListCollaborators)
357358
m.Combo("/:collaborator").Get(repo.IsCollaborator).
358359
Put(bind(api.AddCollaboratorOption{}), repo.AddCollaborator).
359360
Delete(repo.DeleteCollaborator)
360-
})
361+
}, reqToken())
361362
m.Get("/raw/*", context.RepoRef(), repo.GetRawFile)
362363
m.Get("/archive/*", repo.GetArchive)
363364
m.Combo("/forks").Get(repo.ListForks).
364-
Post(bind(api.CreateForkOption{}), repo.CreateFork)
365+
Post(reqToken(), bind(api.CreateForkOption{}), repo.CreateFork)
365366
m.Group("/branches", func() {
366367
m.Get("", repo.ListBranches)
367368
m.Get("/*", context.RepoRef(), repo.GetBranch)
@@ -371,78 +372,87 @@ func RegisterRoutes(m *macaron.Macaron) {
371372
Post(bind(api.CreateKeyOption{}), repo.CreateDeployKey)
372373
m.Combo("/:id").Get(repo.GetDeployKey).
373374
Delete(repo.DeleteDeploykey)
374-
})
375+
}, reqToken())
375376
m.Group("/issues", func() {
376-
m.Combo("").Get(repo.ListIssues).Post(bind(api.CreateIssueOption{}), repo.CreateIssue)
377+
m.Combo("").Get(repo.ListIssues).
378+
Post(reqToken(), bind(api.CreateIssueOption{}), repo.CreateIssue)
377379
m.Group("/comments", func() {
378380
m.Get("", repo.ListRepoIssueComments)
379-
m.Combo("/:id").Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueComment)
381+
m.Combo("/:id", reqToken()).
382+
Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueComment)
380383
})
381384
m.Group("/:index", func() {
382-
m.Combo("").Get(repo.GetIssue).Patch(bind(api.EditIssueOption{}), repo.EditIssue)
385+
m.Combo("").Get(repo.GetIssue).
386+
Patch(reqToken(), bind(api.EditIssueOption{}), repo.EditIssue)
383387

384388
m.Group("/comments", func() {
385-
m.Combo("").Get(repo.ListIssueComments).Post(bind(api.CreateIssueCommentOption{}), repo.CreateIssueComment)
386-
m.Combo("/:id").Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueComment).
389+
m.Combo("").Get(repo.ListIssueComments).
390+
Post(reqToken(), bind(api.CreateIssueCommentOption{}), repo.CreateIssueComment)
391+
m.Combo("/:id", reqToken()).Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueComment).
387392
Delete(repo.DeleteIssueComment)
388393
})
389394

390395
m.Group("/labels", func() {
391396
m.Combo("").Get(repo.ListIssueLabels).
392-
Post(bind(api.IssueLabelsOption{}), repo.AddIssueLabels).
393-
Put(bind(api.IssueLabelsOption{}), repo.ReplaceIssueLabels).
394-
Delete(repo.ClearIssueLabels)
395-
m.Delete("/:id", repo.DeleteIssueLabel)
397+
Post(reqToken(), bind(api.IssueLabelsOption{}), repo.AddIssueLabels).
398+
Put(reqToken(), bind(api.IssueLabelsOption{}), repo.ReplaceIssueLabels).
399+
Delete(reqToken(), repo.ClearIssueLabels)
400+
m.Delete("/:id", reqToken(), repo.DeleteIssueLabel)
396401
})
397402

398403
})
399404
}, mustEnableIssues)
400405
m.Group("/labels", func() {
401406
m.Combo("").Get(repo.ListLabels).
402-
Post(bind(api.CreateLabelOption{}), repo.CreateLabel)
403-
m.Combo("/:id").Get(repo.GetLabel).Patch(bind(api.EditLabelOption{}), repo.EditLabel).
404-
Delete(repo.DeleteLabel)
407+
Post(reqToken(), bind(api.CreateLabelOption{}), repo.CreateLabel)
408+
m.Combo("/:id").Get(repo.GetLabel).
409+
Patch(reqToken(), bind(api.EditLabelOption{}), repo.EditLabel).
410+
Delete(reqToken(), repo.DeleteLabel)
405411
})
406412
m.Group("/milestones", func() {
407413
m.Combo("").Get(repo.ListMilestones).
408-
Post(reqRepoWriter(), bind(api.CreateMilestoneOption{}), repo.CreateMilestone)
414+
Post(reqToken(), reqRepoWriter(), bind(api.CreateMilestoneOption{}), repo.CreateMilestone)
409415
m.Combo("/:id").Get(repo.GetMilestone).
410-
Patch(reqRepoWriter(), bind(api.EditMilestoneOption{}), repo.EditMilestone).
411-
Delete(reqRepoWriter(), repo.DeleteMilestone)
416+
Patch(reqToken(), reqRepoWriter(), bind(api.EditMilestoneOption{}), repo.EditMilestone).
417+
Delete(reqToken(), reqRepoWriter(), repo.DeleteMilestone)
412418
})
413419
m.Get("/stargazers", repo.ListStargazers)
414420
m.Get("/subscribers", repo.ListSubscribers)
415421
m.Group("/subscription", func() {
416422
m.Get("", user.IsWatching)
417-
m.Put("", user.Watch)
418-
m.Delete("", user.Unwatch)
423+
m.Put("", reqToken(), user.Watch)
424+
m.Delete("", reqToken(), user.Unwatch)
419425
})
420426
m.Group("/releases", func() {
421427
m.Combo("").Get(repo.ListReleases).
422-
Post(bind(api.CreateReleaseOption{}), repo.CreateRelease)
428+
Post(reqToken(), bind(api.CreateReleaseOption{}), repo.CreateRelease)
423429
m.Combo("/:id").Get(repo.GetRelease).
424-
Patch(bind(api.EditReleaseOption{}), repo.EditRelease).
425-
Delete(repo.DeleteRelease)
430+
Patch(reqToken(), bind(api.EditReleaseOption{}), repo.EditRelease).
431+
Delete(reqToken(), repo.DeleteRelease)
426432
})
427-
m.Post("/mirror-sync", repo.MirrorSync)
433+
m.Post("/mirror-sync", reqToken(), repo.MirrorSync)
428434
m.Get("/editorconfig/:filename", context.RepoRef(), repo.GetEditorconfig)
429435
m.Group("/pulls", func() {
430-
m.Combo("").Get(bind(api.ListPullRequestsOptions{}), repo.ListPullRequests).Post(reqRepoWriter(), bind(api.CreatePullRequestOption{}), repo.CreatePullRequest)
436+
m.Combo("").Get(bind(api.ListPullRequestsOptions{}), repo.ListPullRequests).
437+
Post(reqToken(), reqRepoWriter(), bind(api.CreatePullRequestOption{}), repo.CreatePullRequest)
431438
m.Group("/:index", func() {
432-
m.Combo("").Get(repo.GetPullRequest).Patch(reqRepoWriter(), bind(api.EditPullRequestOption{}), repo.EditPullRequest)
433-
m.Combo("/merge").Get(repo.IsPullRequestMerged).Post(reqRepoWriter(), repo.MergePullRequest)
439+
m.Combo("").Get(repo.GetPullRequest).
440+
Patch(reqToken(), reqRepoWriter(), bind(api.EditPullRequestOption{}), repo.EditPullRequest)
441+
m.Combo("/merge").Get(repo.IsPullRequestMerged).
442+
Post(reqToken(), reqRepoWriter(), repo.MergePullRequest)
434443
})
435444

436445
}, mustAllowPulls, context.ReferencesGitRepo())
437446
m.Group("/statuses", func() {
438-
m.Combo("/:sha").Get(repo.GetCommitStatuses).Post(reqRepoWriter(), bind(api.CreateStatusOption{}), repo.NewCommitStatus)
447+
m.Combo("/:sha").Get(repo.GetCommitStatuses).
448+
Post(reqToken(), reqRepoWriter(), bind(api.CreateStatusOption{}), repo.NewCommitStatus)
439449
})
440450
m.Group("/commits/:ref", func() {
441451
m.Get("/status", repo.GetCombinedCommitStatus)
442452
m.Get("/statuses", repo.GetCommitStatuses)
443453
})
444454
}, repoAssignment())
445-
}, reqToken())
455+
})
446456

447457
// Organizations
448458
m.Get("/user/orgs", reqToken(), org.ListMyOrgs)

‎routers/api/v1/repo/fork.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"code.gitea.io/gitea/models"
1111
"code.gitea.io/gitea/modules/context"
12+
"code.gitea.io/gitea/routers/api/v1/utils"
1213
)
1314

1415
// ListForks list a repository's forks
@@ -29,7 +30,7 @@ func ListForks(ctx *context.APIContext) {
2930
}
3031
apiForks := make([]*api.Repository, len(forks))
3132
for i, fork := range forks {
32-
access, err := models.AccessLevel(ctx.User.ID, fork)
33+
access, err := models.AccessLevel(utils.UserID(ctx), fork)
3334
if err != nil {
3435
ctx.Error(500, "AccessLevel", err)
3536
return

‎routers/api/v1/repo/release.go

+1-7
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,8 @@ func GetRelease(ctx *context.APIContext) {
3434

3535
// ListReleases list a repository's releases
3636
func ListReleases(ctx *context.APIContext) {
37-
access, err := models.AccessLevel(ctx.User.ID, ctx.Repo.Repository)
38-
if err != nil {
39-
ctx.Error(500, "AccessLevel", err)
40-
return
41-
}
42-
4337
releases, err := models.GetReleasesByRepoID(ctx.Repo.Repository.ID, models.FindReleasesOptions{
44-
IncludeDrafts: access >= models.AccessModeWrite,
38+
IncludeDrafts: ctx.Repo.AccessMode >= models.AccessModeWrite,
4539
}, 1, 2147483647)
4640
if err != nil {
4741
ctx.Error(500, "GetReleasesByRepoID", err)

‎routers/api/v1/repo/repo.go

+1-7
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,7 @@ func Get(ctx *context.APIContext) {
267267
// 200: Repository
268268
// 500: error
269269

270-
repo := ctx.Repo.Repository
271-
access, err := models.AccessLevel(ctx.User.ID, repo)
272-
if err != nil {
273-
ctx.Error(500, "GetRepository", err)
274-
return
275-
}
276-
ctx.JSON(200, repo.APIFormat(access))
270+
ctx.JSON(200, ctx.Repo.Repository.APIFormat(ctx.Repo.AccessMode))
277271
}
278272

279273
// GetByID returns a single Repository

‎routers/api/v1/repo/status.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,10 @@ func GetCombinedCommitStatus(ctx *context.APIContext) {
103103
return
104104
}
105105

106-
acl, err := models.AccessLevel(ctx.User.ID, repo)
107-
if err != nil {
108-
ctx.Error(500, "AccessLevel", fmt.Errorf("AccessLevel[%d, %s]: %v", ctx.User.ID, repo.FullName(), err))
109-
return
110-
}
111106
retStatus := &combinedCommitStatus{
112107
SHA: sha,
113108
TotalCount: len(statuses),
114-
Repo: repo.APIFormat(acl),
109+
Repo: repo.APIFormat(ctx.Repo.AccessMode),
115110
URL: "",
116111
}
117112

‎routers/api/v1/utils/utils.go

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright 2017 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 utils
6+
7+
import "code.gitea.io/gitea/modules/context"
8+
9+
// UserID user ID of authenticated user, or 0 if not authenticated
10+
func UserID(ctx *context.APIContext) int64 {
11+
if ctx.User == nil {
12+
return 0
13+
}
14+
return ctx.User.ID
15+
}

0 commit comments

Comments
 (0)