Restrict permission check on repositories and fix some problems (#5314)

* fix units permission problems

* fix some bugs and merge LoadUnits to repoAssignment

* refactor permission struct and add some copyright heads

* remove unused codes

* fix routes units check

* improve permission check

* add unit tests for permission

* fix typo

* fix tests

* fix some routes

* fix api permission check

* improve permission check

* fix some permission check

* fix tests

* fix tests

* improve some permission check

* fix some permission check

* refactor AccessLevel

* fix bug

* fix tests

* fix tests

* fix tests

* fix AccessLevel

* rename CanAccess

* fix tests

* fix comment

* fix bug

* add missing unit for test repos

* fix bug

* rename some functions

* fix routes check

27 files changed, 795 insertions, 205 deletions

diff --git a/models/access.go b/models/access.go
index 98ead19a0b..34d76953f5 100644
--- a/models/access.go
+++ b/models/access.go
@@ -80,22 +80,6 @@ func accessLevel(e Engine, userID int64, repo *Repository) (AccessMode, error) {
 	return a.Mode, nil
 }
 
-// AccessLevel returns the Access a user has to a repository. Will return NoneAccess if the
-// user does not have access.
-func AccessLevel(userID int64, repo *Repository) (AccessMode, error) {
-	return accessLevel(x, userID, repo)
-}
-
-func hasAccess(e Engine, userID int64, repo *Repository, testMode AccessMode) (bool, error) {
-	mode, err := accessLevel(e, userID, repo)
-	return testMode <= mode, err
-}
-
-// HasAccess returns true if user has access to repo
-func HasAccess(userID int64, repo *Repository, testMode AccessMode) (bool, error) {
-	return hasAccess(x, userID, repo, testMode)
-}
-
 type repoAccess struct {
 	Access     `xorm:"extends"`
 	Repository `xorm:"extends"`
diff --git a/models/access_test.go b/models/access_test.go
index 46d6f723ea..d6a1c92b90 100644
--- a/models/access_test.go
+++ b/models/access_test.go
@@ -20,28 +20,28 @@ var accessModes = []AccessMode{
 func TestAccessLevel(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 
-	user1 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	user2 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
+	user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	user5 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
 	// A public repository owned by User 2
 	repo1 := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
 	assert.False(t, repo1.IsPrivate)
 	// A private repository owned by Org 3
-	repo2 := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
-	assert.True(t, repo2.IsPrivate)
+	repo3 := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
+	assert.True(t, repo3.IsPrivate)
 
-	level, err := AccessLevel(user1.ID, repo1)
+	level, err := AccessLevel(user2, repo1)
 	assert.NoError(t, err)
 	assert.Equal(t, AccessModeOwner, level)
 
-	level, err = AccessLevel(user1.ID, repo2)
+	level, err = AccessLevel(user2, repo3)
 	assert.NoError(t, err)
-	assert.Equal(t, AccessModeWrite, level)
+	assert.Equal(t, AccessModeOwner, level)
 
-	level, err = AccessLevel(user2.ID, repo1)
+	level, err = AccessLevel(user5, repo1)
 	assert.NoError(t, err)
 	assert.Equal(t, AccessModeRead, level)
 
-	level, err = AccessLevel(user2.ID, repo2)
+	level, err = AccessLevel(user5, repo3)
 	assert.NoError(t, err)
 	assert.Equal(t, AccessModeNone, level)
 }
@@ -58,23 +58,18 @@ func TestHasAccess(t *testing.T) {
 	repo2 := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
 	assert.True(t, repo2.IsPrivate)
 
-	for _, accessMode := range accessModes {
-		has, err := HasAccess(user1.ID, repo1, accessMode)
-		assert.NoError(t, err)
-		assert.True(t, has)
+	has, err := HasAccess(user1.ID, repo1)
+	assert.NoError(t, err)
+	assert.True(t, has)
 
-		has, err = HasAccess(user1.ID, repo2, accessMode)
-		assert.NoError(t, err)
-		assert.Equal(t, accessMode <= AccessModeWrite, has)
+	has, err = HasAccess(user1.ID, repo2)
+	assert.NoError(t, err)
 
-		has, err = HasAccess(user2.ID, repo1, accessMode)
-		assert.NoError(t, err)
-		assert.Equal(t, accessMode <= AccessModeRead, has)
+	has, err = HasAccess(user2.ID, repo1)
+	assert.NoError(t, err)
 
-		has, err = HasAccess(user2.ID, repo2, accessMode)
-		assert.NoError(t, err)
-		assert.Equal(t, accessMode <= AccessModeNone, has)
-	}
+	has, err = HasAccess(user2.ID, repo2)
+	assert.NoError(t, err)
 }
 
 func TestUser_GetRepositoryAccesses(t *testing.T) {
diff --git a/models/branches.go b/models/branches.go
index 3de76a5cc1..bbcd342baa 100644
--- a/models/branches.go
+++ b/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
 		}
 
diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml
index 581f9d6ed5..24d77b9997 100644
--- a/models/fixtures/repo_unit.yml
+++ b/models/fixtures/repo_unit.yml
@@ -108,4 +108,116 @@
   repo_id: 33
   type: 5
   config: "{}"
-  created_unix: 1535593231
\ No newline at end of file
+  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
+
+-
+  id: 29
+  repo_id: 16
+  type: 1
+  config: "{}"
+  created_unix: 1524304355
+
+-
+  id: 30
+  repo_id: 23
+  type: 1
+  config: "{}"
+  created_unix: 1524304355
+
+-
+  id: 31
+  repo_id: 27
+  type: 1
+  config: "{}"
+  created_unix: 1524304355
+
+-
+  id: 32
+  repo_id: 28
+  type: 1
+  config: "{}"
+  created_unix: 1524304355
\ No newline at end of file
diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml
index c2987b9658..aa96656530 100644
--- a/models/fixtures/repository.yml
+++ b/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
diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml
index 4b4a1d798b..2d0dd9cd56 100644
--- a/models/fixtures/team.yml
+++ b/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
\ No newline at end of file
diff --git a/models/fixtures/team_repo.yml b/models/fixtures/team_repo.yml
index b324e09415..a523a90b20 100644
--- a/models/fixtures/team_repo.yml
+++ b/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
\ No newline at end of file
diff --git a/models/fixtures/team_unit.yml b/models/fixtures/team_unit.yml
index ad5466a5c1..943745c000 100644
--- a/models/fixtures/team_unit.yml
+++ b/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
\ No newline at end of file
diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml
index b1dfcdfdef..e20b5c9684 100644
--- a/models/fixtures/team_user.yml
+++ b/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
\ No newline at end of file
diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml
index b3850e3599..dc3de2a2e1 100644
--- a/models/fixtures/user.yml
+++ b/models/fixtures/user.yml
@@ -47,7 +47,7 @@
   avatar_email: user3@example.com
   num_repos: 3
   num_members: 2
-  num_teams: 2
+  num_teams: 3
 
 -
   id: 4
@@ -266,7 +266,7 @@
   num_repos: 2
   is_active: true
   num_members: 2
-  num_teams: 1
+  num_teams: 3
 
 -
   id: 18
diff --git a/models/issue.go b/models/issue.go
index ab4fe3107b..26196274fe 100644
--- a/models/issue.go
+++ b/models/issue.go
@@ -385,7 +385,7 @@ func (issue *Issue) sendLabelUpdatedWebhook(doer *User) {
 		return
 	}
 
-	mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
+	mode, _ := AccessLevel(issue.Poster, issue.Repo)
 	if issue.IsPull {
 		if err = issue.loadPullRequest(x); err != nil {
 			log.Error(4, "loadPullRequest: %v", err)
@@ -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{}
 	}
 
@@ -529,7 +533,7 @@ func (issue *Issue) ClearLabels(doer *User) (err error) {
 		return fmt.Errorf("loadPoster: %v", err)
 	}
 
-	mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
+	mode, _ := AccessLevel(issue.Poster, issue.Repo)
 	if issue.IsPull {
 		err = issue.PullRequest.LoadIssue()
 		if err != nil {
@@ -723,7 +727,7 @@ func (issue *Issue) ChangeStatus(doer *User, repo *Repository, isClosed bool) (e
 	}
 	sess.Close()
 
-	mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
+	mode, _ := AccessLevel(issue.Poster, issue.Repo)
 	if issue.IsPull {
 		// Merge pull request calls issue.changeStatus so we need to handle separately.
 		issue.PullRequest.Issue = issue
@@ -785,7 +789,7 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
 		return err
 	}
 
-	mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
+	mode, _ := AccessLevel(issue.Poster, issue.Repo)
 	if issue.IsPull {
 		issue.PullRequest.Issue = issue
 		err = PrepareWebhooks(issue.Repo, HookEventPullRequest, &api.PullRequestPayload{
@@ -851,7 +855,7 @@ func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
 		return fmt.Errorf("UpdateIssueCols: %v", err)
 	}
 
-	mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
+	mode, _ := AccessLevel(issue.Poster, issue.Repo)
 	if issue.IsPull {
 		issue.PullRequest.Issue = issue
 		err = PrepareWebhooks(issue.Repo, HookEventPullRequest, &api.PullRequestPayload{
@@ -946,9 +950,13 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
 	// Check for and validate assignees
 	if len(opts.AssigneeIDs) > 0 {
 		for _, assigneeID := range opts.AssigneeIDs {
-			valid, err := hasAccess(e, assigneeID, opts.Repo, AccessModeWrite)
+			user, err := getUserByID(e, assigneeID)
+			if err != nil {
+				return fmt.Errorf("getUserByID [user_id: %d, repo_id: %d]: %v", assigneeID, opts.Repo.ID, err)
+			}
+			valid, err := canBeAssigned(e, user, opts.Repo)
 			if err != nil {
-				return fmt.Errorf("hasAccess [user_id: %d, repo_id: %d]: %v", assigneeID, opts.Repo.ID, err)
+				return fmt.Errorf("canBeAssigned [user_id: %d, repo_id: %d]: %v", assigneeID, opts.Repo.ID, err)
 			}
 			if !valid {
 				return ErrUserDoesNotHaveAccessToRepo{UserID: assigneeID, RepoName: opts.Repo.Name}
@@ -1071,7 +1079,7 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []in
 		log.Error(4, "MailParticipants: %v", err)
 	}
 
-	mode, _ := AccessLevel(issue.Poster.ID, issue.Repo)
+	mode, _ := AccessLevel(issue.Poster, issue.Repo)
 	if err = PrepareWebhooks(repo, HookEventIssues, &api.IssuePayload{
 		Action:     api.HookIssueOpened,
 		Index:      issue.Index,
diff --git a/models/issue_assignees.go b/models/issue_assignees.go
index 09d4d310dc..f330ade1c8 100644
--- a/models/issue_assignees.go
+++ b/models/issue_assignees.go
@@ -159,13 +159,14 @@ func (issue *Issue) changeAssignee(sess *xorm.Session, doer *User, assigneeID in
 		return fmt.Errorf("createAssigneeComment: %v", err)
 	}
 
-	// if issue/pull is in the middle of creation - don't call webhook
+	// if pull request is in the middle of creation - don't call webhook
 	if isCreate {
 		return nil
 	}
 
-	mode, _ := accessLevel(sess, doer.ID, issue.Repo)
 	if issue.IsPull {
+		mode, _ := accessLevelUnit(sess, doer, issue.Repo, UnitTypePullRequests)
+
 		if err = issue.loadPullRequest(sess); err != nil {
 			return fmt.Errorf("loadPullRequest: %v", err)
 		}
@@ -186,6 +187,8 @@ func (issue *Issue) changeAssignee(sess *xorm.Session, doer *User, assigneeID in
 			return nil
 		}
 	} else {
+		mode, _ := accessLevelUnit(sess, doer, issue.Repo, UnitTypeIssues)
+
 		apiIssue := &api.IssuePayload{
 			Index:      issue.Index,
 			Issue:      issue.APIFormat(),
diff --git a/models/issue_comment.go b/models/issue_comment.go
index 0085c7a732..96b162ca19 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -795,7 +795,7 @@ func CreateIssueComment(doer *User, repo *Repository, issue *Issue, content stri
 		return nil, fmt.Errorf("CreateComment: %v", err)
 	}
 
-	mode, _ := AccessLevel(doer.ID, repo)
+	mode, _ := AccessLevel(doer, repo)
 	if err = PrepareWebhooks(repo, HookEventIssueComment, &api.IssueCommentPayload{
 		Action:     api.HookIssueCommentCreated,
 		Issue:      issue.APIFormat(),
@@ -990,7 +990,7 @@ func UpdateComment(doer *User, c *Comment, oldContent string) error {
 		return err
 	}
 
-	mode, _ := AccessLevel(doer.ID, c.Issue.Repo)
+	mode, _ := AccessLevel(doer, c.Issue.Repo)
 	if err := PrepareWebhooks(c.Issue.Repo, HookEventIssueComment, &api.IssueCommentPayload{
 		Action:  api.HookIssueCommentEdited,
 		Issue:   c.Issue.APIFormat(),
@@ -1047,7 +1047,7 @@ func DeleteComment(doer *User, comment *Comment) error {
 		return err
 	}
 
-	mode, _ := AccessLevel(doer.ID, comment.Issue.Repo)
+	mode, _ := AccessLevel(doer, comment.Issue.Repo)
 
 	if err := PrepareWebhooks(comment.Issue.Repo, HookEventIssueComment, &api.IssueCommentPayload{
 		Action:     api.HookIssueCommentDeleted,
diff --git a/models/issue_milestone.go b/models/issue_milestone.go
index ead3e5a4f6..2e512d7ba4 100644
--- a/models/issue_milestone.go
+++ b/models/issue_milestone.go
@@ -377,7 +377,7 @@ func ChangeMilestoneAssign(issue *Issue, doer *User, oldMilestoneID int64) (err
 		return err
 	}
 
-	mode, _ := AccessLevel(doer.ID, issue.Repo)
+	mode, _ := AccessLevel(doer, issue.Repo)
 	if issue.IsPull {
 		err = issue.PullRequest.LoadIssue()
 		if err != nil {
diff --git a/models/lfs_lock.go b/models/lfs_lock.go
index 52e877b156..66fc2b2ec4 100644
--- a/models/lfs_lock.go
+++ b/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.CanAccess(mode, UnitTypeCode) {
 		return ErrLFSUnauthorizedAction{repo.ID, u.DisplayName(), mode}
 	}
 	return nil
diff --git a/models/org_team.go b/models/org_team.go
index 53c1ec34d8..cad4af2506 100644
--- a/models/org_team.go
+++ b/models/org_team.go
@@ -177,7 +177,7 @@ func (t *Team) removeRepository(e Engine, repo *Repository, recalculate bool) (e
 		return fmt.Errorf("getTeamUsersByTeamID: %v", err)
 	}
 	for _, teamUser := range teamUsers {
-		has, err := hasAccess(e, teamUser.UID, repo, AccessModeRead)
+		has, err := hasAccess(e, teamUser.UID, repo)
 		if err != nil {
 			return err
 		} else if has {
@@ -434,7 +434,7 @@ func DeleteTeam(t *Team) error {
 
 		// Remove watches from all users and now unaccessible repos
 		for _, user := range t.Members {
-			has, err := hasAccess(sess, user.ID, repo, AccessModeRead)
+			has, err := hasAccess(sess, user.ID, repo)
 			if err != nil {
 				return err
 			} else if has {
@@ -652,7 +652,7 @@ func removeTeamMember(e *xorm.Session, team *Team, userID int64) error {
 		}
 
 		// Remove watches from now unaccessible
-		has, err := hasAccess(e, userID, repo, AccessModeRead)
+		has, err := hasAccess(e, userID, repo)
 		if err != nil {
 			return err
 		} else if has {
diff --git a/models/org_team_test.go b/models/org_team_test.go
index 05429c7cc2..f9f1f289ec 100644
--- a/models/org_team_test.go
+++ b/models/org_team_test.go
@@ -243,7 +243,7 @@ func TestDeleteTeam(t *testing.T) {
 	// check that team members don't have "leftover" access to repos
 	user := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User)
 	repo := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
-	accessMode, err := AccessLevel(user.ID, repo)
+	accessMode, err := AccessLevel(user, repo)
 	assert.NoError(t, err)
 	assert.True(t, accessMode < AccessModeWrite)
 }
diff --git a/models/org_test.go b/models/org_test.go
index c54e7a93bf..4790f69971 100644
--- a/models/org_test.go
+++ b/models/org_test.go
@@ -84,9 +84,10 @@ func TestUser_GetTeams(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 	org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User)
 	assert.NoError(t, org.GetTeams())
-	if assert.Len(t, org.Teams, 2) {
+	if assert.Len(t, org.Teams, 3) {
 		assert.Equal(t, int64(1), org.Teams[0].ID)
 		assert.Equal(t, int64(2), org.Teams[1].ID)
+		assert.Equal(t, int64(7), org.Teams[2].ID)
 	}
 }
 
diff --git a/models/pull.go b/models/pull.go
index 79f6d7005d..e97faa8f51 100644
--- a/models/pull.go
+++ b/models/pull.go
@@ -458,7 +458,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle
 		return nil
 	}
 
-	mode, _ := AccessLevel(doer.ID, pr.Issue.Repo)
+	mode, _ := AccessLevel(doer, pr.Issue.Repo)
 	if err = PrepareWebhooks(pr.Issue.Repo, HookEventPullRequest, &api.PullRequestPayload{
 		Action:      api.HookIssueClosed,
 		Index:       pr.Index,
@@ -787,7 +787,7 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str
 
 	pr.Issue = pull
 	pull.PullRequest = pr
-	mode, _ := AccessLevel(pull.Poster.ID, repo)
+	mode, _ := AccessLevel(pull.Poster, repo)
 	if err = PrepareWebhooks(repo, HookEventPullRequest, &api.PullRequestPayload{
 		Action:      api.HookIssueOpened,
 		Index:       pull.Index,
diff --git a/models/release.go b/models/release.go
index 44d028a26f..c18e152293 100644
--- a/models/release.go
+++ b/models/release.go
@@ -200,7 +200,7 @@ func CreateRelease(gitRepo *git.Repository, rel *Release, attachmentUUIDs []stri
 		if err := rel.LoadAttributes(); err != nil {
 			log.Error(2, "LoadAttributes: %v", err)
 		} else {
-			mode, _ := AccessLevel(rel.PublisherID, rel.Repo)
+			mode, _ := AccessLevel(rel.Publisher, rel.Repo)
 			if err := PrepareWebhooks(rel.Repo, HookEventRelease, &api.ReleasePayload{
 				Action:     api.HookReleasePublished,
 				Release:    rel.APIFormat(),
@@ -392,7 +392,7 @@ func UpdateRelease(doer *User, gitRepo *git.Repository, rel *Release, attachment
 
 	err = addReleaseAttachments(rel.ID, attachmentUUIDs)
 
-	mode, _ := accessLevel(x, doer.ID, rel.Repo)
+	mode, _ := AccessLevel(doer, rel.Repo)
 	if err1 := PrepareWebhooks(rel.Repo, HookEventRelease, &api.ReleasePayload{
 		Action:     api.HookReleaseUpdated,
 		Release:    rel.APIFormat(),
@@ -419,13 +419,6 @@ func DeleteReleaseByID(id int64, u *User, delTag bool) error {
 		return fmt.Errorf("GetRepositoryByID: %v", err)
 	}
 
-	has, err := HasAccess(u.ID, repo, AccessModeWrite)
-	if err != nil {
-		return fmt.Errorf("HasAccess: %v", err)
-	} else if !has {
-		return fmt.Errorf("DeleteReleaseByID: permission denied")
-	}
-
 	if delTag {
 		_, stderr, err := process.GetManager().ExecDir(-1, repo.RepoPath(),
 			fmt.Sprintf("DeleteReleaseByID (git tag -d): %d", rel.ID),
@@ -454,7 +447,7 @@ func DeleteReleaseByID(id int64, u *User, delTag bool) error {
 		return fmt.Errorf("LoadAttributes: %v", err)
 	}
 
-	mode, _ := accessLevel(x, u.ID, rel.Repo)
+	mode, _ := AccessLevel(u, rel.Repo)
 	if err := PrepareWebhooks(rel.Repo, HookEventRelease, &api.ReleasePayload{
 		Action:     api.HookReleaseDeleted,
 		Release:    rel.APIFormat(),
diff --git a/models/repo.go b/models/repo.go
index b7be50e9d1..b86226ec82 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -325,63 +325,19 @@ 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 {
-		return false
-	}
-
-	for _, unit := range repo.Units {
-		if unit.Type == unitType {
-			return true
-		}
-	}
-	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
-	}
-
-	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
+	if isAdmin {
+		return true
 	}
-
-	teams, err := getUserRepoTeams(e, repo.OwnerID, userID, repo.ID)
+	user, err := getUserByID(e, userID)
 	if err != nil {
-		return err
+		return false
 	}
-
-	// 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
-			}
-		}
+	perm, err := getUserRepoPermission(e, repo, user)
+	if err != nil {
+		return false
 	}
 
-	repo.Units = newRepoUnits
-	return nil
+	return perm.CanRead(unitType)
 }
 
 // UnitEnabled if this repository has the given unit enabled
@@ -397,21 +353,6 @@ func (repo *Repository) UnitEnabled(tp UnitType) bool {
 	return false
 }
 
-// AnyUnitEnabled if this repository has the any of the given units enabled
-func (repo *Repository) AnyUnitEnabled(tps ...UnitType) bool {
-	if err := repo.getUnits(x); err != nil {
-		log.Warn("Error loading repository (ID: %d) units: %s", repo.ID, err.Error())
-	}
-	for _, unit := range repo.Units {
-		for _, tp := range tps {
-			if unit.Type == tp {
-				return true
-			}
-		}
-	}
-	return false
-}
-
 var (
 	// ErrUnitNotExist organization does not exist
 	ErrUnitNotExist = errors.New("Unit does not exist")
@@ -600,11 +541,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)
@@ -671,12 +607,6 @@ func (repo *Repository) ComposeCompareURL(oldCommitID, newCommitID string) strin
 	return fmt.Sprintf("%s/%s/compare/%s...%s", repo.MustOwner().Name, repo.Name, oldCommitID, newCommitID)
 }
 
-// HasAccess returns true when user has access to this repository
-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 {
 	_, err := x.ID(repo.ID).Cols("default_branch").Update(repo)
@@ -704,11 +634,6 @@ func (repo *Repository) UpdateSize() error {
 	return repo.updateSize(x)
 }
 
-// CanBeForked returns true if repository meets the requirements of being forked.
-func (repo *Repository) CanBeForked() bool {
-	return !repo.IsBare && repo.UnitEnabled(UnitTypeCode)
-}
-
 // CanUserFork returns true if specified user can fork repository.
 func (repo *Repository) CanUserFork(user *User) (bool, error) {
 	if user == nil {
@@ -2486,8 +2411,8 @@ func ForkRepository(doer, u *User, oldRepo *Repository, name, desc string) (_ *R
 		return nil, err
 	}
 
-	oldMode, _ := AccessLevel(doer.ID, oldRepo)
-	mode, _ := AccessLevel(doer.ID, repo)
+	oldMode, _ := AccessLevel(doer, oldRepo)
+	mode, _ := AccessLevel(doer, repo)
 
 	if err = PrepareWebhooks(oldRepo, HookEventFork, &api.ForkPayload{
 		Forkee: oldRepo.APIFormat(oldMode),
diff --git a/models/repo_permission.go b/models/repo_permission.go
new file mode 100644
index 0000000000..9dd7cc559d
--- /dev/null
+++ b/models/repo_permission.go
@@ -0,0 +1,270 @@
+// Copyright 2018 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package models
+
+// Permission contains all the permissions related variables to a repository for a user
+type Permission struct {
+	AccessMode AccessMode
+	Units      []*RepoUnit
+	UnitsMode  map[UnitType]AccessMode
+}
+
+// IsOwner returns true if current user is the owner of repository.
+func (p *Permission) IsOwner() bool {
+	return p.AccessMode >= AccessModeOwner
+}
+
+// IsAdmin returns true if current user has admin or higher access of repository.
+func (p *Permission) IsAdmin() bool {
+	return p.AccessMode >= AccessModeAdmin
+}
+
+// HasAccess returns true if the current user has at least read access to any unit of this repository
+func (p *Permission) HasAccess() bool {
+	if p.UnitsMode == nil {
+		return p.AccessMode >= AccessModeRead
+	}
+	return len(p.UnitsMode) > 0
+}
+
+// UnitAccessMode returns current user accessmode to the specify unit of the repository
+func (p *Permission) UnitAccessMode(unitType UnitType) AccessMode {
+	if p.UnitsMode == nil {
+		for _, u := range p.Units {
+			if u.Type == unitType {
+				return p.AccessMode
+			}
+		}
+		return AccessModeNone
+	}
+	return p.UnitsMode[unitType]
+}
+
+// CanAccess returns true if user has mode access to the unit of the repository
+func (p *Permission) CanAccess(mode AccessMode, unitType UnitType) bool {
+	return p.UnitAccessMode(unitType) >= mode
+}
+
+// CanAccessAny returns true if user has mode access to any of the units of the repository
+func (p *Permission) CanAccessAny(mode AccessMode, unitTypes ...UnitType) bool {
+	for _, u := range unitTypes {
+		if p.CanAccess(mode, u) {
+			return true
+		}
+	}
+	return false
+}
+
+// CanRead returns true if user could read to this unit
+func (p *Permission) CanRead(unitType UnitType) bool {
+	return p.CanAccess(AccessModeRead, unitType)
+}
+
+// CanReadAny returns true if user has read access to any of the units of the repository
+func (p *Permission) CanReadAny(unitTypes ...UnitType) bool {
+	return p.CanAccessAny(AccessModeRead, unitTypes...)
+}
+
+// CanReadIssuesOrPulls returns true if isPull is true and user could read pull requests and
+// returns true if isPull is false and user could read to issues
+func (p *Permission) CanReadIssuesOrPulls(isPull bool) bool {
+	if isPull {
+		return p.CanRead(UnitTypePullRequests)
+	}
+	return p.CanRead(UnitTypeIssues)
+}
+
+// CanWrite returns true if user could write to this unit
+func (p *Permission) CanWrite(unitType UnitType) bool {
+	return p.CanAccess(AccessModeWrite, unitType)
+}
+
+// CanWriteIssuesOrPulls returns true if isPull is true and user could write to pull requests and
+// returns true if isPull is false and user could write to issues
+func (p *Permission) CanWriteIssuesOrPulls(isPull bool) bool {
+	if isPull {
+		return p.CanWrite(UnitTypePullRequests)
+	}
+	return p.CanWrite(UnitTypeIssues)
+}
+
+// GetUserRepoPermission returns the user permissions to the repository
+func GetUserRepoPermission(repo *Repository, user *User) (Permission, error) {
+	return getUserRepoPermission(x, repo, user)
+}
+
+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???
+	if user == nil && repo.IsPrivate {
+		perm.AccessMode = AccessModeNone
+		return
+	}
+
+	if err = repo.getUnits(e); err != nil {
+		return
+	}
+
+	perm.Units = repo.Units
+
+	// anonymous visit public repo
+	if user == nil {
+		perm.AccessMode = AccessModeRead
+		return
+	}
+
+	// Admin or the owner has super access to the repository
+	if user.IsAdmin || user.ID == repo.OwnerID {
+		perm.AccessMode = AccessModeOwner
+		return
+	}
+
+	// plain user
+	perm.AccessMode, err = accessLevel(e, user.ID, repo)
+	if err != nil {
+		return
+	}
+
+	if err = repo.getOwner(e); err != nil {
+		return
+	}
+	if !repo.Owner.IsOrganization() {
+		return
+	}
+
+	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 {
+		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
+	}
+
+	for _, u := range repo.Units {
+		var found bool
+		for _, team := range teams {
+			if team.unitEnabled(e, u.Type) {
+				m := perm.UnitsMode[u.Type]
+				if m < team.Authorize {
+					perm.UnitsMode[u.Type] = team.Authorize
+				}
+				found = true
+			}
+		}
+
+		// for a public repo on an organization, user have read permission on non-team defined units.
+		if !found && !repo.IsPrivate {
+			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 _, u := range repo.Units {
+			if u.Type == t {
+				perm.Units = append(perm.Units, u)
+			}
+		}
+	}
+
+	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
+	}
+	if mode >= AccessModeAdmin {
+		return true, nil
+	}
+
+	teams, err := getUserRepoTeams(e, repo.OwnerID, user.ID, repo.ID)
+	if err != nil {
+		return false, err
+	}
+
+	for _, team := range teams {
+		if team.Authorize >= AccessModeAdmin {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+// AccessLevel returns the Access a user has to a repository. Will return NoneAccess if the
+// user does not have access.
+func AccessLevel(user *User, repo *Repository) (AccessMode, error) {
+	return accessLevelUnit(x, user, repo, UnitTypeCode)
+}
+
+func accessLevelUnit(e Engine, user *User, repo *Repository, unitType UnitType) (AccessMode, error) {
+	perm, err := getUserRepoPermission(e, repo, user)
+	if err != nil {
+		return AccessModeNone, err
+	}
+	return perm.UnitAccessMode(UnitTypeCode), nil
+}
+
+func hasAccessUnit(e Engine, user *User, repo *Repository, unitType UnitType, testMode AccessMode) (bool, error) {
+	mode, err := accessLevelUnit(e, user, repo, unitType)
+	return testMode <= mode, err
+}
+
+// HasAccessUnit returns ture if user has testMode to the unit of the repository
+func HasAccessUnit(user *User, repo *Repository, unitType UnitType, testMode AccessMode) (bool, error) {
+	return hasAccessUnit(x, user, repo, unitType, testMode)
+}
+
+// canBeAssigned return true if user could be assigned to a repo
+// FIXME: user could send PullRequest also could be assigned???
+func canBeAssigned(e Engine, user *User, repo *Repository) (bool, error) {
+	return hasAccessUnit(e, user, repo, UnitTypeCode, AccessModeWrite)
+}
+
+func hasAccess(e Engine, userID int64, repo *Repository) (bool, error) {
+	var user *User
+	var err error
+	if userID > 0 {
+		user, err = getUserByID(e, userID)
+		if err != nil {
+			return false, err
+		}
+	}
+	perm, err := getUserRepoPermission(e, repo, user)
+	if err != nil {
+		return false, err
+	}
+	return perm.HasAccess(), nil
+}
+
+// HasAccess returns true if user has access to repo
+func HasAccess(userID int64, repo *Repository) (bool, error) {
+	return hasAccess(x, userID, repo)
+}
diff --git a/models/repo_permission_test.go b/models/repo_permission_test.go
new file mode 100644
index 0000000000..fd55ae5e52
--- /dev/null
+++ b/models/repo_permission_test.go
@@ -0,0 +1,246 @@
+// Copyright 2018 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package models
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRepoPermissionPublicNonOrgRepo(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	// public non-organization repo
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 4}).(*Repository)
+	assert.NoError(t, repo.getUnits(x))
+
+	// plain user
+	user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	perm, err := GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.False(t, perm.CanWrite(unit.Type))
+	}
+
+	// change to collaborator
+	assert.NoError(t, repo.AddCollaborator(user))
+	perm, err = GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+
+	// collaborator
+	collaborator := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User)
+	perm, err = GetUserRepoPermission(repo, collaborator)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+
+	// owner
+	owner := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
+	perm, err = GetUserRepoPermission(repo, owner)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+
+	// admin
+	admin := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
+	perm, err = GetUserRepoPermission(repo, admin)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+}
+
+func TestRepoPermissionPrivateNonOrgRepo(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	// private non-organization repo
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
+	assert.NoError(t, repo.getUnits(x))
+
+	// plain user
+	user := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User)
+	perm, err := GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.False(t, perm.CanRead(unit.Type))
+		assert.False(t, perm.CanWrite(unit.Type))
+	}
+
+	// change to collaborator to default write access
+	assert.NoError(t, repo.AddCollaborator(user))
+	perm, err = GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+
+	assert.NoError(t, repo.ChangeCollaborationAccessMode(user.ID, AccessModeRead))
+	perm, err = GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.False(t, perm.CanWrite(unit.Type))
+	}
+
+	// owner
+	owner := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	perm, err = GetUserRepoPermission(repo, owner)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+
+	// admin
+	admin := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
+	perm, err = GetUserRepoPermission(repo, admin)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+}
+
+func TestRepoPermissionPublicOrgRepo(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	// public organization repo
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 32}).(*Repository)
+	assert.NoError(t, repo.getUnits(x))
+
+	// plain user
+	user := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
+	perm, err := GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.False(t, perm.CanWrite(unit.Type))
+	}
+
+	// change to collaborator to default write access
+	assert.NoError(t, repo.AddCollaborator(user))
+	perm, err = GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+
+	assert.NoError(t, repo.ChangeCollaborationAccessMode(user.ID, AccessModeRead))
+	perm, err = GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.False(t, perm.CanWrite(unit.Type))
+	}
+
+	// org member team owner
+	owner := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	perm, err = GetUserRepoPermission(repo, owner)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+
+	// org member team tester
+	member := AssertExistsAndLoadBean(t, &User{ID: 15}).(*User)
+	perm, err = GetUserRepoPermission(repo, member)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+	}
+	assert.True(t, perm.CanWrite(UnitTypeIssues))
+	assert.False(t, perm.CanWrite(UnitTypeCode))
+
+	// admin
+	admin := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
+	perm, err = GetUserRepoPermission(repo, admin)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+}
+
+func TestRepoPermissionPrivateOrgRepo(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	// private organization repo
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 24}).(*Repository)
+	assert.NoError(t, repo.getUnits(x))
+
+	// plain user
+	user := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
+	perm, err := GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.False(t, perm.CanRead(unit.Type))
+		assert.False(t, perm.CanWrite(unit.Type))
+	}
+
+	// change to collaborator to default write access
+	assert.NoError(t, repo.AddCollaborator(user))
+	perm, err = GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+
+	assert.NoError(t, repo.ChangeCollaborationAccessMode(user.ID, AccessModeRead))
+	perm, err = GetUserRepoPermission(repo, user)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.False(t, perm.CanWrite(unit.Type))
+	}
+
+	// org member team owner
+	owner := AssertExistsAndLoadBean(t, &User{ID: 15}).(*User)
+	perm, err = GetUserRepoPermission(repo, owner)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+
+	// org member team tester
+	tester := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	perm, err = GetUserRepoPermission(repo, tester)
+	assert.NoError(t, err)
+	assert.True(t, perm.CanWrite(UnitTypeIssues))
+	assert.False(t, perm.CanWrite(UnitTypeCode))
+	assert.False(t, perm.CanRead(UnitTypeCode))
+
+	// org member team reviewer
+	reviewer := AssertExistsAndLoadBean(t, &User{ID: 20}).(*User)
+	perm, err = GetUserRepoPermission(repo, reviewer)
+	assert.NoError(t, err)
+	assert.False(t, perm.CanRead(UnitTypeIssues))
+	assert.False(t, perm.CanWrite(UnitTypeCode))
+	assert.True(t, perm.CanRead(UnitTypeCode))
+
+	// admin
+	admin := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
+	perm, err = GetUserRepoPermission(repo, admin)
+	assert.NoError(t, err)
+	for _, unit := range repo.Units {
+		assert.True(t, perm.CanRead(unit.Type))
+		assert.True(t, perm.CanWrite(unit.Type))
+	}
+}
diff --git a/models/repo_unit.go b/models/repo_unit.go
index 1e1778356a..9eaec884bb 100644
--- a/models/repo_unit.go
+++ b/models/repo_unit.go
@@ -166,10 +166,7 @@ func (r *RepoUnit) IssuesConfig() *IssuesConfig {
 func (r *RepoUnit) ExternalTrackerConfig() *ExternalTrackerConfig {
 	return r.Config.(*ExternalTrackerConfig)
 }
+
 func getUnitsByRepoID(e Engine, repoID int64) (units []*RepoUnit, err error) {
 	return units, e.Where("repo_id = ?", repoID).Find(&units)
 }
-
-func getUnitsByRepoIDAndIDs(e Engine, repoID int64, types []UnitType) (units []*RepoUnit, err error) {
-	return units, e.Where("repo_id = ?", repoID).In("`type`", types).Find(&units)
-}
diff --git a/models/ssh_key.go b/models/ssh_key.go
index 5d046c0201..b7dd81b49b 100644
--- a/models/ssh_key.go
+++ b/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"}
 		}
 	}
diff --git a/models/user.go b/models/user.go
index 9becee7760..8f1b170b0d 100644
--- a/models/user.go
+++ b/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)
diff --git a/models/user_test.go b/models/user_test.go
index 4713b6864c..ba700d3659 100644
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -169,7 +169,7 @@ func TestGetOrgRepositoryIDs(t *testing.T) {
 	accessibleRepos, err := user2.GetOrgRepositoryIDs()
 	assert.NoError(t, err)
 	// User 2's team has access to private repos 3, 5, repo 32 is a public repo of the organization
-	assert.Equal(t, []int64{3, 5, 32}, accessibleRepos)
+	assert.Equal(t, []int64{3, 5, 23, 24, 32}, accessibleRepos)
 
 	accessibleRepos, err = user4.GetOrgRepositoryIDs()
 	assert.NoError(t, err)