add unit tests for permission

models/fixtures/repo_unit.yml

@@ -108,4 +108,88 @@
  repo_id: 33
  type: 5
  config: "{}"
- created_unix: 1535593231
+ created_unix: 1535593231
+
+-
+ id: 17
+ repo_id: 4
+ type: 4
+ config: "{}"
+ created_unix: 946684810
+
+-
+ id: 18
+ repo_id: 4
+ type: 5
+ config: "{}"
+ created_unix: 946684810
+
+-
+ id: 19
+ repo_id: 4
+ type: 1
+ config: "{}"
+ created_unix: 946684810
+
+-
+ id: 20
+ repo_id: 4
+ type: 2
+ config: "{\"EnableTimetracker\":true,\"AllowOnlyContributorsToTrackTime\":true}"
+ created_unix: 946684810
+
+-
+ id: 21
+ repo_id: 4
+ type: 3
+ config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowSquash\":true}"
+ created_unix: 946684810
+
+-
+ id: 22
+ repo_id: 2
+ type: 4
+ config: "{}"
+ created_unix: 946684810
+
+-
+ id: 23
+ repo_id: 2
+ type: 5
+ config: "{}"
+ created_unix: 946684810
+
+-
+ id: 24
+ repo_id: 2
+ type: 1
+ config: "{}"
+ created_unix: 946684810
+
+-
+ id: 25
+ repo_id: 32
+ type: 1
+ config: "{}"
+ created_unix: 1524304355
+
+-
+ id: 26
+ repo_id: 32
+ type: 2
+ config: "{}"
+ created_unix: 1524304355
+
+-
+ id: 27
+ repo_id: 24
+ type: 1
+ config: "{}"
+ created_unix: 1524304355
+
+-
+ id: 28
+ repo_id: 24
+ type: 2
+ config: "{}"
+ created_unix: 1524304355

models/fixtures/repository.yml

@@ -391,7 +391,7 @@
  is_mirror: false
 
 -
- id: 32
+ id: 32 # org public repo
  owner_id: 3
  lower_name: repo21
  name: repo21

models/fixtures/team.yml

@@ -51,3 +51,30 @@
  authorize: 4 # owner
  num_repos: 2
  num_members: 1
+
+-
+ id: 7
+ org_id: 3
+ lower_name: test_team
+ name: test_team
+ authorize: 2 # write
+ num_repos: 1
+ num_members: 1
+
+-
+ id: 8
+ org_id: 17
+ lower_name: test_team
+ name: test_team
+ authorize: 2 # write
+ num_repos: 1
+ num_members: 1
+
+-
+ id: 9
+ org_id: 17
+ lower_name: review_team
+ name: review_team
+ authorize: 1 # read
+ num_repos: 1
+ num_members: 1

models/fixtures/team_repo.yml

@@ -45,3 +45,21 @@
  org_id: 3
  team_id: 1
  repo_id: 32
+
+-
+ id: 9
+ org_id: 3
+ team_id: 7
+ repo_id: 32
+
+-
+ id: 10
+ org_id: 17
+ team_id: 8
+ repo_id: 24
+
+-
+ id: 11
+ org_id: 17
+ team_id: 9
+ repo_id: 24

models/fixtures/team_unit.yml

@@ -207,3 +207,18 @@
  id: 42
  team_id: 6
  type: 7
+
+-
+ id: 43
+ team_id: 7
+ type: 2 # issues
+
+-
+ id: 44
+ team_id: 8
+ type: 2 # issues
+
+-
+ id: 45
+ team_id: 9
+ type: 1 # code

models/fixtures/team_user.yml

@@ -44,4 +44,22 @@
  id: 8
  org_id: 19
  team_id: 6
+ uid: 20
+
+-
+ id: 9
+ org_id: 3
+ team_id: 7
+ uid: 15
+
+-
+ id: 10
+ org_id: 17
+ team_id: 8
+ uid: 2
+
+-
+ id: 11
+ org_id: 17
+ team_id: 9
  uid: 20

models/repo.go

@@ -325,58 +325,16 @@ func (repo *Repository) CheckUnitUser(userID int64, isAdmin bool, unitType UnitT
 }
 
 func (repo *Repository) checkUnitUser(e Engine, userID int64, isAdmin bool, unitType UnitType) bool {
- if err := repo.getUnitsByUserID(e, userID, isAdmin); err != nil {
+ user, err := getUserByID(e, userID)
+ if err != nil {
  return false
  }
-
- for _, unit := range repo.Units {
- if unit.Type == unitType {
- return true
- }
- }
- return false
-}
-
-func (repo *Repository) getUnitsByUserID(e Engine, userID int64, isAdmin bool) (err error) {
- if repo.Units != nil {
- return nil
- }
-
- if err = repo.getUnits(e); err != nil {
- return err
- } else if err = repo.getOwner(e); err != nil {
- return err
- }
-
- if !repo.Owner.IsOrganization() || userID == 0 || isAdmin || !repo.IsPrivate {
- return nil
- }
-
- // Collaborators will not be limited
- if isCollaborator, err := repo.isCollaborator(e, userID); err != nil {
- return err
- } else if isCollaborator {
- return nil
- }
-
- teams, err := getUserRepoTeams(e, repo.OwnerID, userID, repo.ID)
+ perm, err := getUserRepoPermission(e, repo, user)
  if err != nil {
- return err
- }
-
- // unique
- var newRepoUnits = make([]*RepoUnit, 0, len(repo.Units))
- for _, u := range repo.Units {
- for _, team := range teams {
- if team.unitEnabled(e, u.Type) {
- newRepoUnits = append(newRepoUnits, u)
- break
- }
- }
+ return false
  }
 
- repo.Units = newRepoUnits
- return nil
+ return perm.CanAccess(unitType)
 }
 
 // UnitEnabled if this repository has the given unit enabled

models/repo_permission.go

@@ -47,6 +47,16 @@ func (p *Permission) CanAccess(unitType UnitType) bool {
  return p.UnitAccessMode(unitType) >= AccessModeRead
 }
 
+// CanAccessAny returns true if user has read access to any of the units of the repository
+func (p *Permission) CanAccessAny(unitTypes ...UnitType) bool {
+ for _, u := range unitTypes {
+ if p.CanAccess(u) {
+ return true
+ }
+ }
+ return false
+}
+
 // CanWrite returns true if user could write to this unit
 func (p *Permission) CanWrite(unitType UnitType) bool {
  return p.UnitAccessMode(unitType) >= AccessModeWrite
@@ -105,19 +115,23 @@ func getUserRepoPermission(e Engine, repo *Repository, user *User) (perm Permiss
  return
  }
 
- // Collaborators on organization repos will not be limited by teams units
+ perm.UnitsMode = make(map[UnitType]AccessMode)
+
+ // Collaborators on organization
  if isCollaborator, err := repo.isCollaborator(e, user.ID); err != nil {
  return perm, err
  } else if isCollaborator {
- return perm, nil
+ for _, u := range repo.Units {
+ perm.UnitsMode[u.Type] = perm.AccessMode
+ }
  }
 
+ // get units mode from teams
  teams, err := getUserRepoTeams(e, repo.OwnerID, user.ID, repo.ID)
  if err != nil {
  return
  }
 
- perm.UnitsMode = make(map[UnitType]AccessMode)
  for _, u := range repo.Units {
  var found bool
  for _, team := range teams {
@@ -130,13 +144,17 @@ func getUserRepoPermission(e Engine, repo *Repository, user *User) (perm Permiss
  }
  }
 
+ // for a public repo on an organization, user have read permission on non-team defined units.
  if !found && !repo.IsPrivate {
- perm.UnitsMode[u.Type] = AccessModeRead
+ if _, ok := perm.UnitsMode[u.Type]; !ok {
+ perm.UnitsMode[u.Type] = AccessModeRead
+ }
  }
  }
 
+ // remove no permission units
  perm.Units = make([]*RepoUnit, 0, len(repo.Units))
- for t, _ := range perm.UnitsMode {
+ for t := range perm.UnitsMode {
  for _, u := range repo.Units {
  if u.Type == t {
  perm.Units = append(perm.Units, u)