improve permission check

cmd/serv.go

@@ -236,7 +236,7 @@ func runServ(c *cli.Context) error {
  user.Name, repoPath)
  }
 
- mode, err := private.AccessLevel(user.ID, repo.ID)
+ mode, err := private.CheckUnitUser(user.ID, repo.ID, user.IsAdmin, unitType)
  if err != nil {
  fail("Internal error", "Failed to check access: %v", err)
  } else if *mode < requestedMode {
@@ -249,16 +249,6 @@ func runServ(c *cli.Context) error {
  user.Name, requestedMode, repoPath)
  }
 
- check, err := private.CheckUnitUser(user.ID, repo.ID, user.IsAdmin, unitType)
- if err != nil {
- fail("You do not have allowed for this action", "Failed to access internal api: [user.Name: %s, repoPath: %s]", user.Name, repoPath)
- }
- if !check {
- fail("You do not have allowed for this action",
- "User %s does not have allowed access to repository %s 's code",
- user.Name, repoPath)
- }
-
  os.Setenv(models.EnvPusherName, user.Name)
  os.Setenv(models.EnvPusherID, fmt.Sprintf("%d", user.ID))
  }

models/repo.go

@@ -337,11 +337,6 @@ func (repo *Repository) checkUnitUser(e Engine, userID int64, isAdmin bool, unit
  return false
 }
 
-// LoadUnitsByUserID loads units according userID's permissions
-func (repo *Repository) LoadUnitsByUserID(userID int64, isAdmin bool) error {
- return repo.getUnitsByUserID(x, userID, isAdmin)
-}
-
 func (repo *Repository) getUnitsByUserID(e Engine, userID int64, isAdmin bool) (err error) {
  if repo.Units != nil {
  return nil

models/repo_permission.go

@@ -32,7 +32,12 @@ func (p *Permission) HasAccess() bool {
 // UnitAccessMode returns current user accessmode to the specify unit of the repository
 func (p *Permission) UnitAccessMode(unitType UnitType) AccessMode {
  if p.UnitsMode == nil {
- return p.AccessMode
+ for _, u := range p.Units {
+ if u.Type == unitType {
+ return p.AccessMode
+ }
+ }
+ return AccessModeNone
  }
  return p.UnitsMode[unitType]
 }
@@ -62,7 +67,8 @@ func GetUserRepoPermission(repo *Repository, user *User) (Permission, error) {
 }
 
 func getUserRepoPermission(e Engine, repo *Repository, user *User) (perm Permission, err error) {
- // anonymous user visit private repo. TODO: anonymous user visit public unit of private repo???
+ // anonymous user visit private repo.
+ // TODO: anonymous user visit public unit of private repo???
  if user == nil && repo.IsPrivate {
  perm.AccessMode = AccessModeNone
  return
@@ -99,6 +105,13 @@ func getUserRepoPermission(e Engine, repo *Repository, user *User) (perm Permiss
  return
  }
 
+ // Collaborators on organization repos will not be limited by teams units
+ if isCollaborator, err := repo.isCollaborator(e, user.ID); err != nil {
+ return perm, err
+ } else if isCollaborator {
+ return perm, nil
+ }
+
  teams, err := getUserRepoTeams(e, repo.OwnerID, user.ID, repo.ID)
  if err != nil {
  return

modules/context/repo.go

@@ -422,9 +422,6 @@ func RepoAssignment() macaron.Handler {
  ctx.Repo.PullRequest.HeadInfo = ctx.Repo.BranchName
  }
  }
-
- // Reset repo units as otherwise user specific units wont be loaded later
- ctx.Repo.Repository.Units = nil
  }
  ctx.Data["PullRequestCtx"] = ctx.Repo.PullRequest
 

modules/private/internal.go

@@ -51,20 +51,26 @@ func newInternalRequest(url, method string) *httplib.Request {
 }
 
 // CheckUnitUser check whether user could visit the unit of this repository
-func CheckUnitUser(userID, repoID int64, isAdmin bool, unitType models.UnitType) (bool, error) {
+func CheckUnitUser(userID, repoID int64, isAdmin bool, unitType models.UnitType) (*models.AccessMode, error) {
  reqURL := setting.LocalURL + fmt.Sprintf("api/internal/repositories/%d/user/%d/checkunituser?isAdmin=%t&unitType=%d", repoID, userID, isAdmin, unitType)
  log.GitLogger.Trace("AccessLevel: %s", reqURL)
 
  resp, err := newInternalRequest(reqURL, "GET").Response()
  if err != nil {
- return false, err
+ return nil, err
  }
  defer resp.Body.Close()
 
- if resp.StatusCode == 200 {
- return true, nil
+ if resp.StatusCode != 200 {
+ return nil, fmt.Errorf("Failed to CheckUnitUser: %s", decodeJSONError(resp).Err)
  }
- return false, nil
+
+ var a models.AccessMode
+ if err := json.NewDecoder(resp.Body).Decode(&a); err != nil {
+ return nil, err
+ }
+
+ return &a, nil
 }
 
 // AccessLevel returns the Access a user has to a repository. Will return NoneAccess if the

routers/private/internal.go

@@ -70,11 +70,24 @@ func CheckUnitUser(ctx *macaron.Context) {
  })
  return
  }
- if repo.CheckUnitUser(userID, ctx.QueryBool("isAdmin"), models.UnitType(ctx.QueryInt("unitType"))) {
- ctx.PlainText(200, []byte("success"))
+
+ user, err := models.GetUserByID(userID)
+ if err != nil {
+ ctx.JSON(500, map[string]interface{}{
+ "err": err.Error(),
+ })
  return
  }
- ctx.PlainText(404, []byte("no access"))
+
+ perm, err := models.GetUserRepoPermission(repo, user)
+ if err != nil {
+ ctx.JSON(500, map[string]interface{}{
+ "err": err.Error(),
+ })
+ return
+ }
+
+ ctx.JSON(200, perm.UnitAccessMode(models.UnitType(ctx.QueryInt("unitType"))))
 }
 
 // RegisterRoutes registers all internal APIs routes to web application.

routers/repo/http.go

@@ -182,36 +182,19 @@ func HTTP(ctx *context.Context) {
  }
  }
 
- if !isPublicPull {
- has, err := models.HasAccess(authUser.ID, repo, accessMode)
- if err != nil {
- ctx.ServerError("HasAccess", err)
- return
- } else if !has {
- if accessMode == models.AccessModeRead {
- has, err = models.HasAccess(authUser.ID, repo, models.AccessModeWrite)
- if err != nil {
- ctx.ServerError("HasAccess2", err)
- return
- } else if !has {
- ctx.HandleText(http.StatusForbidden, "User permission denied")
- return
- }
- } else {
- ctx.HandleText(http.StatusForbidden, "User permission denied")
- return
- }
- }
+ perm, err := models.GetUserRepoPermission(repo, authUser)
+ if err != nil {
+ ctx.ServerError("GetUserRepoPermission", err)
+ return
+ }
 
- if !isPull && repo.IsMirror {
- ctx.HandleText(http.StatusForbidden, "mirror repository is read-only")
- return
- }
+ if perm.UnitAccessMode(unitType) < accessMode {
+ ctx.HandleText(http.StatusForbidden, "User permission denied")
+ return
  }
 
- if !repo.CheckUnitUser(authUser.ID, authUser.IsAdmin, unitType) {
- ctx.HandleText(http.StatusForbidden, fmt.Sprintf("User %s does not have allowed access to repository %s 's code",
- authUser.Name, repo.RepoPath()))
+ if !isPull && repo.IsMirror {
+ ctx.HandleText(http.StatusForbidden, "mirror repository is read-only")
  return
  }
 

routers/repo/view.go

@@ -282,9 +282,9 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
 
 // Home render repository home page
 func Home(ctx *context.Context) {
- if len(ctx.Repo.Repository.Units) > 0 {
+ if len(ctx.Repo.Units) > 0 {
  var firstUnit *models.Unit
- for _, repoUnit := range ctx.Repo.Repository.Units {
+ for _, repoUnit := range ctx.Repo.Units {
  if repoUnit.Type == models.UnitTypeCode {
  renderCode(ctx)
  return