Skip to content

Commit ba0fcac

Browse files
ethantkoenigethantkoenig
committed
Bug fixes for repo permissions in API
Also move duplicated code into repo.APIFormat(..)
1 parent 83ed234 commit ba0fcac

File tree

7 files changed

+45
-26
lines changed

7 files changed

+45
-26
lines changed

‎models/action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ func CommitRepoAction(opts CommitRepoActionOptions) error {
539539
}()
540540

541541
apiPusher := pusher.APIFormat()
542-
apiRepo := repo.APIFormat(nil)
542+
apiRepo := repo.APIFormat(AccessModeNone)
543543

544544
var shaSum string
545545
switch opType {

‎models/issue.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ func (issue *Issue) sendLabelUpdatedWebhook(doer *User) {
265265
Action: api.HookIssueLabelUpdated,
266266
Index: issue.Index,
267267
PullRequest: issue.PullRequest.APIFormat(),
268-
Repository: issue.Repo.APIFormat(nil),
268+
Repository: issue.Repo.APIFormat(AccessModeNone),
269269
Sender: doer.APIFormat(),
270270
})
271271
}
@@ -371,7 +371,7 @@ func (issue *Issue) ClearLabels(doer *User) (err error) {
371371
Action: api.HookIssueLabelCleared,
372372
Index: issue.Index,
373373
PullRequest: issue.PullRequest.APIFormat(),
374-
Repository: issue.Repo.APIFormat(nil),
374+
Repository: issue.Repo.APIFormat(AccessModeNone),
375375
Sender: doer.APIFormat(),
376376
})
377377
}
@@ -493,7 +493,7 @@ func (issue *Issue) ChangeStatus(doer *User, repo *Repository, isClosed bool) (e
493493
apiPullRequest := &api.PullRequestPayload{
494494
Index: issue.Index,
495495
PullRequest: issue.PullRequest.APIFormat(),
496-
Repository: repo.APIFormat(nil),
496+
Repository: repo.APIFormat(AccessModeNone),
497497
Sender: doer.APIFormat(),
498498
}
499499
if isClosed {
@@ -531,7 +531,7 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
531531
},
532532
},
533533
PullRequest: issue.PullRequest.APIFormat(),
534-
Repository: issue.Repo.APIFormat(nil),
534+
Repository: issue.Repo.APIFormat(AccessModeNone),
535535
Sender: doer.APIFormat(),
536536
})
537537
}
@@ -563,7 +563,7 @@ func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
563563
},
564564
},
565565
PullRequest: issue.PullRequest.APIFormat(),
566-
Repository: issue.Repo.APIFormat(nil),
566+
Repository: issue.Repo.APIFormat(AccessModeNone),
567567
Sender: doer.APIFormat(),
568568
})
569569
}
@@ -596,7 +596,7 @@ func (issue *Issue) ChangeAssignee(doer *User, assigneeID int64) (err error) {
596596
apiPullRequest := &api.PullRequestPayload{
597597
Index: issue.Index,
598598
PullRequest: issue.PullRequest.APIFormat(),
599-
Repository: issue.Repo.APIFormat(nil),
599+
Repository: issue.Repo.APIFormat(AccessModeNone),
600600
Sender: doer.APIFormat(),
601601
}
602602
if isRemoveAssignee {

‎models/pull.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,14 @@ func (pr *PullRequest) APIFormat() *api.PullRequest {
160160
Ref: pr.BaseBranch,
161161
Sha: baseCommit.ID.String(),
162162
RepoID: pr.BaseRepoID,
163-
Repository: pr.BaseRepo.APIFormat(nil),
163+
Repository: pr.BaseRepo.APIFormat(AccessModeNone),
164164
}
165165
apiHeadBranchInfo := &api.PRBranchInfo{
166166
Name: pr.HeadBranch,
167167
Ref: pr.HeadBranch,
168168
Sha: headCommit.ID.String(),
169169
RepoID: pr.HeadRepoID,
170-
Repository: pr.HeadRepo.APIFormat(nil),
170+
Repository: pr.HeadRepo.APIFormat(AccessModeNone),
171171
}
172172
apiPullRequest := &api.PullRequest{
173173
ID: pr.ID,
@@ -355,7 +355,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
355355
Action: api.HookIssueClosed,
356356
Index: pr.Index,
357357
PullRequest: pr.APIFormat(),
358-
Repository: pr.Issue.Repo.APIFormat(nil),
358+
Repository: pr.Issue.Repo.APIFormat(AccessModeNone),
359359
Sender: doer.APIFormat(),
360360
}); err != nil {
361361
log.Error(4, "PrepareWebhooks: %v", err)
@@ -385,7 +385,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
385385
After: pr.MergedCommitID,
386386
CompareURL: setting.AppURL + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID),
387387
Commits: ListToPushCommits(l).ToAPIPayloadCommits(pr.BaseRepo.HTMLURL()),
388-
Repo: pr.BaseRepo.APIFormat(nil),
388+
Repo: pr.BaseRepo.APIFormat(AccessModeNone),
389389
Pusher: pr.HeadRepo.MustOwner().APIFormat(),
390390
Sender: doer.APIFormat(),
391391
}
@@ -514,7 +514,7 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str
514514
Action: api.HookIssueOpened,
515515
Index: pull.Index,
516516
PullRequest: pr.APIFormat(),
517-
Repository: repo.APIFormat(nil),
517+
Repository: repo.APIFormat(AccessModeNone),
518518
Sender: pull.Poster.APIFormat(),
519519
}); err != nil {
520520
log.Error(4, "PrepareWebhooks: %v", err)
@@ -840,7 +840,7 @@ func AddTestPullRequestTask(doer *User, repoID int64, branch string, isSync bool
840840
Action: api.HookIssueSynchronized,
841841
Index: pr.Issue.Index,
842842
PullRequest: pr.Issue.PullRequest.APIFormat(),
843-
Repository: pr.Issue.Repo.APIFormat(nil),
843+
Repository: pr.Issue.Repo.APIFormat(AccessModeNone),
844844
Sender: doer.APIFormat(),
845845
}); err != nil {
846846
log.Error(4, "PrepareWebhooks [pull_id: %v]: %v", pr.ID, err)

‎models/repo.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,8 +277,13 @@ func (repo *Repository) HTMLURL() string {
277277

278278
// APIFormat converts a Repository to api.Repository
279279
// Arguments that are allowed to be nil: permission
280-
func (repo *Repository) APIFormat(permission *api.Permission) *api.Repository {
280+
func (repo *Repository) APIFormat(mode AccessMode) *api.Repository {
281281
cloneLink := repo.CloneLink()
282+
permission := &api.Permission{
283+
Admin: mode >= AccessModeAdmin,
284+
Push: mode >= AccessModeWrite,
285+
Pull: mode >= AccessModeRead,
286+
}
282287
return &api.Repository{
283288
ID: repo.ID,
284289
Owner: repo.Owner.APIFormat(),

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

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,16 +95,12 @@ func ListMyRepos(ctx *context.APIContext) {
9595

9696
repos := make([]*api.Repository, numOwnRepos+len(accessibleRepos))
9797
for i := range ownRepos {
98-
repos[i] = ownRepos[i].APIFormat(&api.Permission{true, true, true})
98+
repos[i] = ownRepos[i].APIFormat(models.AccessModeOwner)
9999
}
100100
i := numOwnRepos
101101

102102
for repo, access := range accessibleRepos {
103-
repos[i] = repo.APIFormat(&api.Permission{
104-
Admin: access >= models.AccessModeAdmin,
105-
Push: access >= models.AccessModeWrite,
106-
Pull: true,
107-
})
103+
repos[i] = repo.APIFormat(access)
108104
i++
109105
}
110106

@@ -138,7 +134,7 @@ func CreateUserRepo(ctx *context.APIContext, owner *models.User, opt api.CreateR
138134
return
139135
}
140136

141-
ctx.JSON(201, repo.APIFormat(&api.Permission{true, true, true}))
137+
ctx.JSON(201, repo.APIFormat(models.AccessModeOwner))
142138
}
143139

144140
// Create one repository of mine
@@ -241,14 +237,19 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
241237
}
242238

243239
log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName)
244-
ctx.JSON(201, repo.APIFormat(&api.Permission{true, true, true}))
240+
ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin))
245241
}
246242

247243
// Get one repository
248244
// see https://github.com/gogits/go-gogs-client/wiki/Repositories#get
249245
func Get(ctx *context.APIContext) {
250246
repo := ctx.Repo.Repository
251-
ctx.JSON(200, repo.APIFormat(&api.Permission{true, true, true}))
247+
access, err := models.AccessLevel(ctx.User, repo)
248+
if err != nil {
249+
ctx.Error(500, "GetRepository", err)
250+
return
251+
}
252+
ctx.JSON(200, repo.APIFormat(access))
252253
}
253254

254255
// GetByID returns a single Repository
@@ -263,7 +264,12 @@ func GetByID(ctx *context.APIContext) {
263264
return
264265
}
265266

266-
ctx.JSON(200, repo.APIFormat(&api.Permission{true, true, true}))
267+
access, err := models.AccessLevel(ctx.User, repo)
268+
if err != nil {
269+
ctx.Error(500, "GetRepositoryByID", err)
270+
return
271+
}
272+
ctx.JSON(200, repo.APIFormat(access))
267273
}
268274

269275
// Delete one repository

‎routers/api/v1/user/star.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,17 @@ func getStarredRepos(userID int64, private bool) ([]*api.Repository, error) {
1818
if err != nil {
1919
return nil, err
2020
}
21+
user, err := models.GetUserByID(userID)
22+
if err != nil {
23+
return nil, err
24+
}
2125
repos := make([]*api.Repository, len(starredRepos))
2226
for i, starred := range starredRepos {
23-
repos[i] = starred.APIFormat(&api.Permission{true, true, true})
27+
access, err := models.AccessLevel(user, starred)
28+
if err != nil {
29+
return nil, err
30+
}
31+
repos[i] = starred.APIFormat(access)
2432
}
2533
return repos, nil
2634
}

‎routers/repo/webhook.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ func TestWebhook(ctx *context.Context) {
388388
},
389389
},
390390
},
391-
Repo: ctx.Repo.Repository.APIFormat(nil),
391+
Repo: ctx.Repo.Repository.APIFormat(models.AccessModeNone),
392392
Pusher: apiUser,
393393
Sender: apiUser,
394394
}

0 commit comments

Comments
 (0)