fix some permission check

models/branches.go

@@ -243,10 +243,16 @@ func updateUserWhitelist(repo *Repository, currentWhitelist, newWhitelist []int6
 
  whitelist = make([]int64, 0, len(newWhitelist))
  for _, userID := range newWhitelist {
- has, err := hasAccess(x, userID, repo, AccessModeWrite)
+ user, err := GetUserByID(userID)
  if err != nil {
- return nil, fmt.Errorf("HasAccess [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
- } else if !has {
+ return nil, fmt.Errorf("GetUserByID [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
+ }
+ perm, err := GetUserRepoPermission(repo, user)
+ if err != nil {
+ return nil, fmt.Errorf("GetUserRepoPermission [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
+ }
+
+ if !perm.CanWrite(UnitTypeCode) {
  continue // Drop invalid user ID
  }
 

models/issue.go

@@ -468,9 +468,11 @@ func (issue *Issue) RemoveLabel(doer *User, label *Label) error {
  return err
  }
 
- if has, err := HasAccess(doer.ID, issue.Repo, AccessModeWrite); err != nil {
+ perm, err := GetUserRepoPermission(issue.Repo, doer)
+ if err != nil {
  return err
- } else if !has {
+ }
+ if !perm.CanWriteIssuesOrPulls(issue.IsPull) {
  return ErrLabelNotExist{}
  }
 
@@ -511,9 +513,11 @@ func (issue *Issue) ClearLabels(doer *User) (err error) {
  return err
  }
 
- if has, err := hasAccess(sess, doer.ID, issue.Repo, AccessModeWrite); err != nil {
+ perm, err := getUserRepoPermission(sess, issue.Repo, doer)
+ if err != nil {
  return err
- } else if !has {
+ }
+ if !perm.CanWriteIssuesOrPulls(issue.IsPull) {
  return ErrLabelNotExist{}
  }
 

models/lfs_lock.go

@@ -139,10 +139,11 @@ func CheckLFSAccessForRepo(u *User, repo *Repository, mode AccessMode) error {
  if u == nil {
  return ErrLFSUnauthorizedAction{repo.ID, "undefined", mode}
  }
- has, err := HasAccess(u.ID, repo, mode)
+ perm, err := GetUserRepoPermission(repo, u)
  if err != nil {
  return err
- } else if !has {
+ }
+ if !perm.CanWrite(UnitTypeCode) {
  return ErrLFSUnauthorizedAction{repo.ID, u.DisplayName(), mode}
  }
  return nil

models/repo.go

@@ -553,11 +553,6 @@ func (repo *Repository) GetAssignees() (_ []*User, err error) {
  return repo.getAssignees(x)
 }
 
-// GetUserIfHasWriteAccess returns the user that has write access of repository by given ID.
-func (repo *Repository) GetUserIfHasWriteAccess(userID int64) (*User, error) {
- return GetUserIfHasWriteAccess(repo, userID)
-}
-
 // GetMilestoneByID returns the milestone belongs to repository by given ID.
 func (repo *Repository) GetMilestoneByID(milestoneID int64) (*Milestone, error) {
  return GetMilestoneByRepoID(repo.ID, milestoneID)
@@ -625,10 +620,10 @@ func (repo *Repository) ComposeCompareURL(oldCommitID, newCommitID string) strin
 }
 
 // HasAccess returns true when user has access to this repository
-func (repo *Repository) HasAccess(u *User) bool {
+/*func (repo *Repository) HasAccess(u *User) bool {
  has, _ := HasAccess(u.ID, repo, AccessModeRead)
  return has
-}
+}*/
 
 // UpdateDefaultBranch updates the default branch
 func (repo *Repository) UpdateDefaultBranch() error {

models/repo_permission.go

@@ -173,3 +173,23 @@ func getUserRepoPermission(e Engine, repo *Repository, user *User) (perm Permiss
 
  return
 }
+
+// IsUserRepoAdmin return ture if user has admin right of a repo
+func IsUserRepoAdmin(repo *Repository, user *User) (bool, error) {
+ return isUserRepoAdmin(x, repo, user)
+}
+
+func isUserRepoAdmin(e Engine, repo *Repository, user *User) (bool, error) {
+ if user == nil || repo == nil {
+ return false, nil
+ }
+ if user.IsAdmin {
+ return true, nil
+ }
+
+ mode, err := accessLevel(e, user.ID, repo)
+ if err != nil {
+ return false, err
+ }
+ return mode >= AccessModeAdmin, nil
+}

models/ssh_key.go

@@ -807,10 +807,10 @@ func DeleteDeployKey(doer *User, id int64) error {
  if err != nil {
  return fmt.Errorf("GetRepositoryByID: %v", err)
  }
- yes, err := HasAccess(doer.ID, repo, AccessModeAdmin)
+ has, err := IsUserRepoAdmin(repo, doer)
  if err != nil {
- return fmt.Errorf("HasAccess: %v", err)
- } else if !yes {
+ return fmt.Errorf("GetUserRepoPermission: %v", err)
+ } else if !has {
  return ErrKeyAccessDenied{doer.ID, key.ID, "deploy"}
  }
  }

models/user.go

@@ -496,24 +496,6 @@ func (u *User) DeleteAvatar() error {
  return nil
 }
 
-// IsAdminOfRepo returns true if user has admin or higher access of repository.
-func (u *User) IsAdminOfRepo(repo *Repository) bool {
- has, err := HasAccess(u.ID, repo, AccessModeAdmin)
- if err != nil {
- log.Error(3, "HasAccess: %v", err)
- }
- return has
-}
-
-// IsWriterOfRepo returns true if user has write access to given repository.
-func (u *User) IsWriterOfRepo(repo *Repository) bool {
- has, err := HasAccess(u.ID, repo, AccessModeWrite)
- if err != nil {
- log.Error(3, "HasAccess: %v", err)
- }
- return has
-}
-
 // IsOrganization returns true if user is actually a organization.
 func (u *User) IsOrganization() bool {
  return u.Type == UserTypeOrganization
@@ -1170,17 +1152,6 @@ func GetUserByID(id int64) (*User, error) {
  return getUserByID(x, id)
 }
 
-// GetUserIfHasWriteAccess returns the user with write access of repository by given ID.
-func GetUserIfHasWriteAccess(repo *Repository, userID int64) (*User, error) {
- has, err := HasAccess(userID, repo, AccessModeWrite)
- if err != nil {
- return nil, err
- } else if !has {
- return nil, ErrUserNotExist{userID, "", 0}
- }
- return GetUserByID(userID)
-}
-
 // GetUserByName returns user by given name.
 func GetUserByName(name string) (*User, error) {
  return getUserByName(x, name)

modules/lfs/server.go

@@ -497,12 +497,12 @@ func authenticate(ctx *context.Context, repository *models.Repository, authoriza
  accessMode = models.AccessModeWrite
  }
 
- if !repository.IsPrivate && !requireWrite {
- return true
+ perm, err := models.GetUserRepoPermission(repository, ctx.User)
+ if err != nil {
+ return false
  }
  if ctx.IsSigned {
- accessCheck, _ := models.HasAccess(ctx.User.ID, repository, accessMode)
- return accessCheck
+ return perm.UnitAccessMode(models.UnitTypeCode) >= accessMode
  }
 
  user, repo, opStr, err := parseToken(authorization)
@@ -511,8 +511,11 @@ func authenticate(ctx *context.Context, repository *models.Repository, authoriza
  }
  ctx.User = user
  if opStr == "basic" {
- accessCheck, _ := models.HasAccess(ctx.User.ID, repository, accessMode)
- return accessCheck
+ perm, err = models.GetUserRepoPermission(repository, ctx.User)
+ if err != nil {
+ return false
+ }
+ return perm.UnitAccessMode(models.UnitTypeCode) >= accessMode
  }
  if repository.ID == repo.ID {
  if requireWrite && opStr != "upload" {

routers/api/v1/repo/pull.go

@@ -663,7 +663,12 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
  }
  }
 
- if !ctx.User.IsWriterOfRepo(headRepo) && !ctx.User.IsAdmin {
+ perm, err := models.GetUserRepoPermission(headRepo, ctx.User)
+ if err != nil {
+ ctx.ServerError("GetUserRepoPermission", err)
+ return nil, nil, nil, nil, "", ""
+ }
+ if !perm.CanWrite(models.UnitTypeCode) {
  log.Trace("ParseCompareInfo[%d]: does not have write access or site admin", baseRepo.ID)
  ctx.Status(404)
  return nil, nil, nil, nil, "", ""

routers/repo/issue.go

@@ -370,7 +370,7 @@ func NewIssue(ctx *context.Context) {
 }
 
 // ValidateRepoMetas check and returns repository's meta informations
-func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm) ([]int64, []int64, int64) {
+func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm, isPull bool) ([]int64, []int64, int64) {
  var (
  repo = ctx.Repo.Repository
  err error
@@ -428,9 +428,19 @@ func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm) ([]int64
 
  // Check if the passed assignees actually exists and has write access to the repo
  for _, aID := range assigneeIDs {
- _, err = repo.GetUserIfHasWriteAccess(aID)
+ user, err := models.GetUserByID(aID)
  if err != nil {
- ctx.ServerError("GetUserIfHasWriteAccess", err)
+ ctx.ServerError("GetUserByID", err)
+ return nil, nil, 0
+ }
+
+ perm, err := models.GetUserRepoPermission(repo, user)
+ if err != nil {
+ ctx.ServerError("GetUserRepoPermission", err)
+ return nil, nil, 0
+ }
+ if !perm.CanWriteIssuesOrPulls(isPull) {
+ ctx.ServerError("CanWriteIssuesOrPulls", fmt.Errorf("No permission for %s", user.Name))
  return nil, nil, 0
  }
  }
@@ -459,7 +469,7 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) {
  attachments []string
  )
 
- labelIDs, assigneeIDs, milestoneID := ValidateRepoMetas(ctx, form)
+ labelIDs, assigneeIDs, milestoneID := ValidateRepoMetas(ctx, form, false)
  if ctx.Written() {
  return
  }
@@ -499,31 +509,23 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) {
 
 // commentTag returns the CommentTag for a comment in/with the given repo, poster and issue
 func commentTag(repo *models.Repository, poster *models.User, issue *models.Issue) (models.CommentTag, error) {
- if repo.IsOwnedBy(poster.ID) {
- return models.CommentTagOwner, nil
- } else if repo.Owner.IsOrganization() {
- isOwner, err := repo.Owner.IsOwnedBy(poster.ID)
- if err != nil {
- return models.CommentTagNone, err
- } else if isOwner {
- return models.CommentTagOwner, nil
- }
+ perm, err := models.GetUserRepoPermission(repo, poster)
+ if err != nil {
+ return models.CommentTagNone, err
  }
- if poster.IsWriterOfRepo(repo) {
- return models.CommentTagWriter, nil
+ if perm.IsOwner() {
+ return models.CommentTagOwner, nil
  } else if poster.ID == issue.PosterID {
  return models.CommentTagPoster, nil
+ } else if perm.CanWrite(models.UnitTypeCode) {
+ return models.CommentTagWriter, nil
  }
+
  return models.CommentTagNone, nil
 }
 
 // ViewIssue render issue view page
 func ViewIssue(ctx *context.Context) {
- ctx.Data["RequireHighlightJS"] = true
- ctx.Data["RequireDropzone"] = true
- ctx.Data["RequireTribute"] = true
- renderAttachmentSettings(ctx)
-
  issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
  if err != nil {
  if models.IsErrIssueNotExist(err) {
@@ -533,25 +535,6 @@ func ViewIssue(ctx *context.Context) {
  }
  return
  }
- ctx.Data["Title"] = fmt.Sprintf("#%d - %s", issue.Index, issue.Title)
-
- var iw *models.IssueWatch
- var exists bool
- if ctx.User != nil {
- iw, exists, err = models.GetIssueWatch(ctx.User.ID, issue.ID)
- if err != nil {
- ctx.ServerError("GetIssueWatch", err)
- return
- }
- if !exists {
- iw = &models.IssueWatch{
- UserID: ctx.User.ID,
- IssueID: issue.ID,
- IsWatching: models.IsWatching(ctx.User.ID, ctx.Repo.Repository.ID),
- }
- }
- }
- ctx.Data["IssueWatch"] = iw
 
  // Make sure type and URL matches.
  if ctx.Params(":type") == "issues" && issue.IsPull {
@@ -577,6 +560,31 @@ func ViewIssue(ctx *context.Context) {
  ctx.Data["PageIsIssueList"] = true
  }
 
+ ctx.Data["RequireHighlightJS"] = true
+ ctx.Data["RequireDropzone"] = true
+ ctx.Data["RequireTribute"] = true
+ renderAttachmentSettings(ctx)
+
+ ctx.Data["Title"] = fmt.Sprintf("#%d - %s", issue.Index, issue.Title)
+
+ var iw *models.IssueWatch
+ var exists bool
+ if ctx.User != nil {
+ iw, exists, err = models.GetIssueWatch(ctx.User.ID, issue.ID)
+ if err != nil {
+ ctx.ServerError("GetIssueWatch", err)
+ return
+ }
+ if !exists {
+ iw = &models.IssueWatch{
+ UserID: ctx.User.ID,
+ IssueID: issue.ID,
+ IsWatching: models.IsWatching(ctx.User.ID, ctx.Repo.Repository.ID),
+ }
+ }
+ }
+ ctx.Data["IssueWatch"] = iw
+
  issue.RenderedContent = string(markdown.Render([]byte(issue.Content), ctx.Repo.RepoLink,
  ctx.Repo.Repository.ComposeMetas()))
 
@@ -762,13 +770,20 @@ func ViewIssue(ctx *context.Context) {
  if ctx.IsSigned {
  if err := pull.GetHeadRepo(); err != nil {
  log.Error(4, "GetHeadRepo: %v", err)
- } else if pull.HeadRepo != nil && pull.HeadBranch != pull.HeadRepo.DefaultBranch && ctx.User.IsWriterOfRepo(pull.HeadRepo) {
- // Check if branch is not protected
- if protected, err := pull.HeadRepo.IsProtectedBranch(pull.HeadBranch, ctx.User); err != nil {
- log.Error(4, "IsProtectedBranch: %v", err)
- } else if !protected {
- canDelete = true
- ctx.Data["DeleteBranchLink"] = ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index) + "/cleanup"
+ } else if pull.HeadRepo != nil && pull.HeadBranch != pull.HeadRepo.DefaultBranch {
+ perm, err := models.GetUserRepoPermission(pull.HeadRepo, ctx.User)
+ if err != nil {
+ ctx.ServerError("GetUserRepoPermission", err)
+ return
+ }
+ if perm.CanWrite(models.UnitTypeCode) {
+ // Check if branch is not protected
+ if protected, err := pull.HeadRepo.IsProtectedBranch(pull.HeadBranch, ctx.User); err != nil {
+ log.Error(4, "IsProtectedBranch: %v", err)
+ } else if !protected {
+ canDelete = true
+ ctx.Data["DeleteBranchLink"] = ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index) + "/cleanup"
+ }
  }
  }
  }