Bug fixes for repo permissions in API

Also move duplicated code into repo.APIFormat(..)

diff --git a/models/action.go b/models/action.go
index e2ac1756ab3d72fc44aa9f444488d86edefe33b9..9c79dc5df09479a9067039ec92cb9c5ac4a25772 100644 (file)
--- a/models/action.go
+++ b/models/action.go
@@ -539,7 +539,7 @@ func CommitRepoAction(opts CommitRepoActionOptions) error {
        }()
 
        apiPusher := pusher.APIFormat()
-       apiRepo := repo.APIFormat(nil)
+       apiRepo := repo.APIFormat(AccessModeNone)
 
        var shaSum string
        switch opType {

diff --git a/models/issue.go b/models/issue.go
index 7ef3d00e41f118e78d2b1f24d7ad81881482b982..4937bf1b56d47b8626c42af31754a8f58cb8f2fe 100644 (file)
--- a/models/issue.go
+++ b/models/issue.go
@@ -265,7 +265,7 @@ func (issue *Issue) sendLabelUpdatedWebhook(doer *User) {
                        Action:      api.HookIssueLabelUpdated,
                        Index:       issue.Index,
                        PullRequest: issue.PullRequest.APIFormat(),
-                       Repository:  issue.Repo.APIFormat(nil),
+                       Repository:  issue.Repo.APIFormat(AccessModeNone),
                        Sender:      doer.APIFormat(),
                })
        }
@@ -371,7 +371,7 @@ func (issue *Issue) ClearLabels(doer *User) (err error) {
                        Action:      api.HookIssueLabelCleared,
                        Index:       issue.Index,
                        PullRequest: issue.PullRequest.APIFormat(),
-                       Repository:  issue.Repo.APIFormat(nil),
+                       Repository:  issue.Repo.APIFormat(AccessModeNone),
                        Sender:      doer.APIFormat(),
                })
        }
@@ -493,7 +493,7 @@ func (issue *Issue) ChangeStatus(doer *User, repo *Repository, isClosed bool) (e
                apiPullRequest := &api.PullRequestPayload{
                        Index:       issue.Index,
                        PullRequest: issue.PullRequest.APIFormat(),
-                       Repository:  repo.APIFormat(nil),
+                       Repository:  repo.APIFormat(AccessModeNone),
                        Sender:      doer.APIFormat(),
                }
                if isClosed {
@@ -531,7 +531,7 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
                                },
                        },
                        PullRequest: issue.PullRequest.APIFormat(),
-                       Repository:  issue.Repo.APIFormat(nil),
+                       Repository:  issue.Repo.APIFormat(AccessModeNone),
                        Sender:      doer.APIFormat(),
                })
        }
@@ -563,7 +563,7 @@ func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
                                },
                        },
                        PullRequest: issue.PullRequest.APIFormat(),
-                       Repository:  issue.Repo.APIFormat(nil),
+                       Repository:  issue.Repo.APIFormat(AccessModeNone),
                        Sender:      doer.APIFormat(),
                })
        }
@@ -596,7 +596,7 @@ func (issue *Issue) ChangeAssignee(doer *User, assigneeID int64) (err error) {
                apiPullRequest := &api.PullRequestPayload{
                        Index:       issue.Index,
                        PullRequest: issue.PullRequest.APIFormat(),
-                       Repository:  issue.Repo.APIFormat(nil),
+                       Repository:  issue.Repo.APIFormat(AccessModeNone),
                        Sender:      doer.APIFormat(),
                }
                if isRemoveAssignee {

diff --git a/models/pull.go b/models/pull.go
index d149a142d0cc64946921366ad4c47ae176fd01c9..b7a1ce5d2e1482650311bd10ccea48e34630a2b9 100644 (file)
--- a/models/pull.go
+++ b/models/pull.go
@@ -160,14 +160,14 @@ func (pr *PullRequest) APIFormat() *api.PullRequest {
                Ref:        pr.BaseBranch,
                Sha:        baseCommit.ID.String(),
                RepoID:     pr.BaseRepoID,
-               Repository: pr.BaseRepo.APIFormat(nil),
+               Repository: pr.BaseRepo.APIFormat(AccessModeNone),
        }
        apiHeadBranchInfo := &api.PRBranchInfo{
                Name:       pr.HeadBranch,
                Ref:        pr.HeadBranch,
                Sha:        headCommit.ID.String(),
                RepoID:     pr.HeadRepoID,
-               Repository: pr.HeadRepo.APIFormat(nil),
+               Repository: pr.HeadRepo.APIFormat(AccessModeNone),
        }
        apiPullRequest := &api.PullRequest{
                ID:        pr.ID,
@@ -355,7 +355,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
                Action:      api.HookIssueClosed,
                Index:       pr.Index,
                PullRequest: pr.APIFormat(),
-               Repository:  pr.Issue.Repo.APIFormat(nil),
+               Repository:  pr.Issue.Repo.APIFormat(AccessModeNone),
                Sender:      doer.APIFormat(),
        }); err != nil {
                log.Error(4, "PrepareWebhooks: %v", err)
@@ -385,7 +385,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
                After:      pr.MergedCommitID,
                CompareURL: setting.AppURL + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID),
                Commits:    ListToPushCommits(l).ToAPIPayloadCommits(pr.BaseRepo.HTMLURL()),
-               Repo:       pr.BaseRepo.APIFormat(nil),
+               Repo:       pr.BaseRepo.APIFormat(AccessModeNone),
                Pusher:     pr.HeadRepo.MustOwner().APIFormat(),
                Sender:     doer.APIFormat(),
        }
@@ -514,7 +514,7 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str
                Action:      api.HookIssueOpened,
                Index:       pull.Index,
                PullRequest: pr.APIFormat(),
-               Repository:  repo.APIFormat(nil),
+               Repository:  repo.APIFormat(AccessModeNone),
                Sender:      pull.Poster.APIFormat(),
        }); err != nil {
                log.Error(4, "PrepareWebhooks: %v", err)
@@ -840,7 +840,7 @@ func AddTestPullRequestTask(doer *User, repoID int64, branch string, isSync bool
                                        Action:      api.HookIssueSynchronized,
                                        Index:       pr.Issue.Index,
                                        PullRequest: pr.Issue.PullRequest.APIFormat(),
-                                       Repository:  pr.Issue.Repo.APIFormat(nil),
+                                       Repository:  pr.Issue.Repo.APIFormat(AccessModeNone),
                                        Sender:      doer.APIFormat(),
                                }); err != nil {
                                        log.Error(4, "PrepareWebhooks [pull_id: %v]: %v", pr.ID, err)

diff --git a/models/repo.go b/models/repo.go
index af74e6c8fc815bb82321ac4c93dcb13c2723ca2e..f339234fb46c9485293e8a0161391ccde5c93b85 100644 (file)
--- a/models/repo.go
+++ b/models/repo.go
@@ -277,8 +277,13 @@ func (repo *Repository) HTMLURL() string {
 
 // APIFormat converts a Repository to api.Repository
 // Arguments that are allowed to be nil: permission
-func (repo *Repository) APIFormat(permission *api.Permission) *api.Repository {
+func (repo *Repository) APIFormat(mode AccessMode) *api.Repository {
        cloneLink := repo.CloneLink()
+       permission := &api.Permission{
+               Admin: mode >= AccessModeAdmin,
+               Push:  mode >= AccessModeWrite,
+               Pull:  mode >= AccessModeRead,
+       }
        return &api.Repository{
                ID:            repo.ID,
                Owner:         repo.Owner.APIFormat(),

diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go
index 0c5c1ef5fda4d138a0e22093538cc8c14fd9938f..35e6554273e820e3fd6c9f0e57381afb2baf9d15 100644 (file)
--- a/routers/api/v1/repo/repo.go
+++ b/routers/api/v1/repo/repo.go
@@ -95,16 +95,12 @@ func ListMyRepos(ctx *context.APIContext) {
 
        repos := make([]*api.Repository, numOwnRepos+len(accessibleRepos))
        for i := range ownRepos {
-               repos[i] = ownRepos[i].APIFormat(&api.Permission{true, true, true})
+               repos[i] = ownRepos[i].APIFormat(models.AccessModeOwner)
        }
        i := numOwnRepos
 
        for repo, access := range accessibleRepos {
-               repos[i] = repo.APIFormat(&api.Permission{
-                       Admin: access >= models.AccessModeAdmin,
-                       Push:  access >= models.AccessModeWrite,
-                       Pull:  true,
-               })
+               repos[i] = repo.APIFormat(access)
                i++
        }
 
@@ -138,7 +134,7 @@ func CreateUserRepo(ctx *context.APIContext, owner *models.User, opt api.CreateR
                return
        }
 
-       ctx.JSON(201, repo.APIFormat(&api.Permission{true, true, true}))
+       ctx.JSON(201, repo.APIFormat(models.AccessModeOwner))
 }
 
 // Create one repository of mine
@@ -241,14 +237,19 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
        }
 
        log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName)
-       ctx.JSON(201, repo.APIFormat(&api.Permission{true, true, true}))
+       ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin))
 }
 
 // Get one repository
 // see https://github.com/gogits/go-gogs-client/wiki/Repositories#get
 func Get(ctx *context.APIContext) {
        repo := ctx.Repo.Repository
-       ctx.JSON(200, repo.APIFormat(&api.Permission{true, true, true}))
+       access, err := models.AccessLevel(ctx.User, repo)
+       if err != nil {
+               ctx.Error(500, "GetRepository", err)
+               return
+       }
+       ctx.JSON(200, repo.APIFormat(access))
 }
 
 // GetByID returns a single Repository
@@ -263,7 +264,12 @@ func GetByID(ctx *context.APIContext) {
                return
        }
 
-       ctx.JSON(200, repo.APIFormat(&api.Permission{true, true, true}))
+       access, err := models.AccessLevel(ctx.User, repo)
+       if err != nil {
+               ctx.Error(500, "GetRepositoryByID", err)
+               return
+       }
+       ctx.JSON(200, repo.APIFormat(access))
 }
 
 // Delete one repository

diff --git a/routers/api/v1/user/star.go b/routers/api/v1/user/star.go
index c856ecefdd9eb2b0b5e66a982a269f29de5ffca7..0937fd1903ebe279ff182a7bc5139f2d88a324db 100644 (file)
--- a/routers/api/v1/user/star.go
+++ b/routers/api/v1/user/star.go
@@ -18,9 +18,17 @@ func getStarredRepos(userID int64, private bool) ([]*api.Repository, error) {
        if err != nil {
                return nil, err
        }
+       user, err := models.GetUserByID(userID)
+       if err != nil {
+               return nil, err
+       }
        repos := make([]*api.Repository, len(starredRepos))
        for i, starred := range starredRepos {
-               repos[i] = starred.APIFormat(&api.Permission{true, true, true})
+               access, err := models.AccessLevel(user, starred)
+               if err != nil {
+                       return nil, err
+               }
+               repos[i] = starred.APIFormat(access)
        }
        return repos, nil
 }

diff --git a/routers/repo/webhook.go b/routers/repo/webhook.go
index 5a4770cdf31cc4cbc9ef0b4e4b00763d2ad7189c..c3150a02f6992fe5622c07cbe32f18019a403d99 100644 (file)
--- a/routers/repo/webhook.go
+++ b/routers/repo/webhook.go
@@ -388,7 +388,7 @@ func TestWebhook(ctx *context.Context) {
                                },
                        },
                },
-               Repo:   ctx.Repo.Repository.APIFormat(nil),
+               Repo:   ctx.Repo.Repository.APIFormat(models.AccessModeNone),
                Pusher: apiUser,
                Sender: apiUser,
        }