Skip to content

Commit

Permalink
Move access and repo permission to models/perm/access (go-gitea#19350)
Browse files Browse the repository at this point in the history
* Move access and repo permission to models/perm/access

* Remove unnecessary code
  • Loading branch information
lunny authored May 11, 2022
1 parent 8e8e936 commit cbd4547
Show file tree
Hide file tree
Showing 72 changed files with 608 additions and 511 deletions.
4 changes: 3 additions & 1 deletion integrations/api_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"testing"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
Expand Down Expand Up @@ -205,7 +207,7 @@ func TestAPISearchRepo(t *testing.T) {
assert.Len(t, repoNames, expected.count)
for _, repo := range body.Data {
r := getRepo(t, repo.ID)
hasAccess, err := models.HasAccess(userID, r)
hasAccess, err := access_model.HasAccess(db.DefaultContext, userID, r)
assert.NoError(t, err, "Error when checking if User: %d has access to %s: %v", userID, repo.FullName, err)
assert.True(t, hasAccess, "User: %d does not have access to %s", userID, repo.FullName)

Expand Down
3 changes: 2 additions & 1 deletion integrations/delete_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/organization"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
Expand All @@ -21,7 +22,7 @@ func assertUserDeleted(t *testing.T, userID int64) {
unittest.AssertNotExistsBean(t, &user_model.Follow{UserID: userID})
unittest.AssertNotExistsBean(t, &user_model.Follow{FollowID: userID})
unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerID: userID})
unittest.AssertNotExistsBean(t, &models.Access{UserID: userID})
unittest.AssertNotExistsBean(t, &access_model.Access{UserID: userID})
unittest.AssertNotExistsBean(t, &organization.OrgUser{UID: userID})
unittest.AssertNotExistsBean(t, &models.IssueUser{UID: userID})
unittest.AssertNotExistsBean(t, &organization.TeamUser{UID: userID})
Expand Down
3 changes: 2 additions & 1 deletion models/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
Expand Down Expand Up @@ -510,7 +511,7 @@ func notifyWatchers(ctx context.Context, actions ...*Action) error {
permPR[i] = false
continue
}
perm, err := GetUserRepoPermission(ctx, repo, user)
perm, err := access_model.GetUserRepoPermission(ctx, repo, user)
if err != nil {
permCode[i] = false
permIssue[i] = false
Expand Down
11 changes: 6 additions & 5 deletions models/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
Expand Down Expand Up @@ -79,7 +80,7 @@ func (protectBranch *ProtectedBranch) CanUserPush(userID int64) bool {
} else if repo, err := repo_model.GetRepositoryByID(protectBranch.RepoID); err != nil {
log.Error("repo_model.GetRepositoryByID: %v", err)
return false
} else if writeAccess, err := HasAccessUnit(user, repo, unit.TypeCode, perm.AccessModeWrite); err != nil {
} else if writeAccess, err := access_model.HasAccessUnit(db.DefaultContext, user, repo, unit.TypeCode, perm.AccessModeWrite); err != nil {
log.Error("HasAccessUnit: %v", err)
return false
} else {
Expand All @@ -104,7 +105,7 @@ func (protectBranch *ProtectedBranch) CanUserPush(userID int64) bool {
}

// IsUserMergeWhitelisted checks if some user is whitelisted to merge to this branch
func IsUserMergeWhitelisted(ctx context.Context, protectBranch *ProtectedBranch, userID int64, permissionInRepo Permission) bool {
func IsUserMergeWhitelisted(ctx context.Context, protectBranch *ProtectedBranch, userID int64, permissionInRepo access_model.Permission) bool {
if !protectBranch.EnableMergeWhitelist {
// Then we need to fall back on whether the user has write permission
return permissionInRepo.CanWrite(unit.TypeCode)
Expand Down Expand Up @@ -139,7 +140,7 @@ func isUserOfficialReviewer(ctx context.Context, protectBranch *ProtectedBranch,

if !protectBranch.EnableApprovalsWhitelist {
// Anyone with write access is considered official reviewer
writeAccess, err := hasAccessUnit(ctx, user, repo, unit.TypeCode, perm.AccessModeWrite)
writeAccess, err := access_model.HasAccessUnit(ctx, user, repo, unit.TypeCode, perm.AccessModeWrite)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -424,7 +425,7 @@ func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, c

whitelist = make([]int64, 0, len(newWhitelist))
for _, userID := range newWhitelist {
if reader, err := IsRepoReader(ctx, repo, userID); err != nil {
if reader, err := access_model.IsRepoReader(ctx, repo, userID); err != nil {
return nil, err
} else if !reader {
continue
Expand All @@ -449,7 +450,7 @@ func updateUserWhitelist(ctx context.Context, repo *repo_model.Repository, curre
if err != nil {
return nil, fmt.Errorf("GetUserByID [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
}
perm, err := GetUserRepoPermission(ctx, repo, user)
perm, err := access_model.GetUserRepoPermission(ctx, repo, user)
if err != nil {
return nil, fmt.Errorf("GetUserRepoPermission [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
}
Expand Down
5 changes: 3 additions & 2 deletions models/fixture_generation.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"

"code.gitea.io/gitea/models/db"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
)

Expand All @@ -22,14 +23,14 @@ func GetYamlFixturesAccess() (string, error) {

for _, repo := range repos {
repo.MustOwner()
if err := RecalculateAccesses(repo); err != nil {
if err := access_model.RecalculateAccesses(db.DefaultContext, repo); err != nil {
return "", err
}
}

var b strings.Builder

accesses := make([]*Access, 0, 200)
accesses := make([]*access_model.Access, 0, 200)
if err := db.GetEngine(db.DefaultContext).OrderBy("user_id, repo_id").Find(&accesses); err != nil {
return "", err
}
Expand Down
5 changes: 3 additions & 2 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
project_model "code.gitea.io/gitea/models/project"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
Expand Down Expand Up @@ -489,7 +490,7 @@ func ClearIssueLabels(issue *Issue, doer *user_model.User) (err error) {
return err
}

perm, err := GetUserRepoPermission(ctx, issue.Repo, doer)
perm, err := access_model.GetUserRepoPermission(ctx, issue.Repo, doer)
if err != nil {
return err
}
Expand Down Expand Up @@ -2314,7 +2315,7 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u
continue
}
// Normal users must have read access to the referencing issue
perm, err := GetUserRepoPermission(ctx, issue.Repo, user)
perm, err := access_model.GetUserRepoPermission(ctx, issue.Repo, user)
if err != nil {
return nil, fmt.Errorf("GetUserRepoPermission [%d]: %v", user.ID, err)
}
Expand Down
3 changes: 2 additions & 1 deletion models/issue_xref.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"

"code.gitea.io/gitea/models/db"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log"
Expand Down Expand Up @@ -215,7 +216,7 @@ func (issue *Issue) verifyReferencedIssue(stdCtx context.Context, ctx *crossRefe

// Check doer permissions; set action to None if the doer can't change the destination
if refIssue.RepoID != ctx.OrigIssue.RepoID || ref.Action != references.XRefActionNone {
perm, err := GetUserRepoPermission(stdCtx, refIssue.Repo, ctx.Doer)
perm, err := access_model.GetUserRepoPermission(stdCtx, refIssue.Repo, ctx.Doer)
if err != nil {
return nil, references.XRefActionNone, err
}
Expand Down
3 changes: 2 additions & 1 deletion models/lfs_lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
Expand Down Expand Up @@ -171,7 +172,7 @@ func CheckLFSAccessForRepo(ctx context.Context, ownerID int64, repo *repo_model.
if err != nil {
return err
}
perm, err := GetUserRepoPermission(ctx, repo, u)
perm, err := access_model.GetUserRepoPermission(ctx, repo, u)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion models/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"

Expand Down Expand Up @@ -142,7 +143,7 @@ func removeOrgUser(ctx context.Context, orgID, userID int64) error {
if _, err = sess.
Where("user_id = ?", userID).
In("repo_id", repoIDs).
Delete(new(Access)); err != nil {
Delete(new(access_model.Access)); err != nil {
return err
}
}
Expand Down
33 changes: 15 additions & 18 deletions models/org_team.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log"
Expand All @@ -33,7 +34,7 @@ func addRepository(ctx context.Context, t *organization.Team, repo *repo_model.R

t.NumRepos++

if err = recalculateTeamAccesses(ctx, repo, 0); err != nil {
if err = access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil {
return fmt.Errorf("recalculateAccesses: %v", err)
}

Expand Down Expand Up @@ -62,7 +63,7 @@ func addAllRepositories(ctx context.Context, t *organization.Team) error {
}

for _, repo := range orgRepos {
if !hasRepository(ctx, t, repo.ID) {
if !organization.HasTeamRepo(ctx, t.OrgID, t.ID, repo.ID) {
if err := addRepository(ctx, t, &repo); err != nil {
return fmt.Errorf("addRepository: %v", err)
}
Expand Down Expand Up @@ -108,11 +109,6 @@ func AddRepository(t *organization.Team, repo *repo_model.Repository) (err error
return committer.Commit()
}

// HasRepository returns true if given repository belong to team.
func HasRepository(t *organization.Team, repoID int64) bool {
return hasRepository(db.DefaultContext, t, repoID)
}

// RemoveAllRepositories removes all repositories from team and recalculates access
func RemoveAllRepositories(t *organization.Team) (err error) {
if t.IncludesAllRepositories {
Expand All @@ -138,13 +134,13 @@ func removeAllRepositories(ctx context.Context, t *organization.Team) (err error
e := db.GetEngine(ctx)
// Delete all accesses.
for _, repo := range t.Repos {
if err := recalculateTeamAccesses(ctx, repo, t.ID); err != nil {
if err := access_model.RecalculateTeamAccesses(ctx, repo, t.ID); err != nil {
return err
}

// Remove watches from all users and now unaccessible repos
for _, user := range t.Members {
has, err := hasAccess(ctx, user.ID, repo)
has, err := access_model.HasAccess(ctx, user.ID, repo)
if err != nil {
return err
} else if has {
Expand Down Expand Up @@ -177,8 +173,9 @@ func removeAllRepositories(ctx context.Context, t *organization.Team) (err error
return nil
}

func hasRepository(ctx context.Context, t *organization.Team, repoID int64) bool {
return organization.HasTeamRepo(ctx, t.OrgID, t.ID, repoID)
// HasRepository returns true if given repository belong to team.
func HasRepository(t *organization.Team, repoID int64) bool {
return organization.HasTeamRepo(db.DefaultContext, t.OrgID, t.ID, repoID)
}

// removeRepository removes a repository from a team and recalculates access
Expand All @@ -196,7 +193,7 @@ func removeRepository(ctx context.Context, t *organization.Team, repo *repo_mode

// Don't need to recalculate when delete a repository from organization.
if recalculate {
if err = recalculateTeamAccesses(ctx, repo, t.ID); err != nil {
if err = access_model.RecalculateTeamAccesses(ctx, repo, t.ID); err != nil {
return err
}
}
Expand All @@ -206,7 +203,7 @@ func removeRepository(ctx context.Context, t *organization.Team, repo *repo_mode
return fmt.Errorf("getTeamUsersByTeamID: %v", err)
}
for _, teamUser := range teamUsers {
has, err := hasAccess(ctx, teamUser.UID, repo)
has, err := access_model.HasAccess(ctx, teamUser.UID, repo)
if err != nil {
return err
} else if has {
Expand Down Expand Up @@ -378,7 +375,7 @@ func UpdateTeam(t *organization.Team, authChanged, includeAllChanged bool) (err
}

for _, repo := range t.Repos {
if err = recalculateTeamAccesses(ctx, repo, 0); err != nil {
if err = access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil {
return fmt.Errorf("recalculateTeamAccesses: %v", err)
}
}
Expand Down Expand Up @@ -522,7 +519,7 @@ func AddTeamMember(team *organization.Team, userID int64) error {
In("repo_id", subQuery).
And("mode < ?", team.AccessMode).
SetExpr("mode", team.AccessMode).
Update(new(Access)); err != nil {
Update(new(access_model.Access)); err != nil {
return fmt.Errorf("update user accesses: %v", err)
}

Expand All @@ -533,9 +530,9 @@ func AddTeamMember(team *organization.Team, userID int64) error {
return fmt.Errorf("select id accesses: %v", err)
}

accesses := make([]*Access, 0, 100)
accesses := make([]*access_model.Access, 0, 100)
for i, repoID := range repoIDs {
accesses = append(accesses, &Access{RepoID: repoID, UserID: userID, Mode: team.AccessMode})
accesses = append(accesses, &access_model.Access{RepoID: repoID, UserID: userID, Mode: team.AccessMode})
if (i%100 == 0 || i == len(repoIDs)-1) && len(accesses) > 0 {
if err = db.Insert(ctx, accesses); err != nil {
return fmt.Errorf("insert new user accesses: %v", err)
Expand Down Expand Up @@ -595,7 +592,7 @@ func removeTeamMember(ctx context.Context, team *organization.Team, userID int64

// Delete access to team repositories.
for _, repo := range team.Repos {
if err := recalculateUserAccess(ctx, repo, userID); err != nil {
if err := access_model.RecalculateUserAccess(ctx, repo, userID); err != nil {
return err
}

Expand Down
23 changes: 21 additions & 2 deletions models/org_team_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
Expand Down Expand Up @@ -129,7 +130,7 @@ func TestUpdateTeam(t *testing.T) {
team = unittest.AssertExistsAndLoadBean(t, &organization.Team{Name: "newName"}).(*organization.Team)
assert.True(t, strings.HasPrefix(team.Description, "A long description!"))

access := unittest.AssertExistsAndLoadBean(t, &Access{UserID: 4, RepoID: 3}).(*Access)
access := unittest.AssertExistsAndLoadBean(t, &access_model.Access{UserID: 4, RepoID: 3}).(*access_model.Access)
assert.EqualValues(t, perm.AccessModeAdmin, access.Mode)

unittest.CheckConsistencyFor(t, &organization.Team{ID: team.ID})
Expand Down Expand Up @@ -161,7 +162,7 @@ func TestDeleteTeam(t *testing.T) {
// check that team members don't have "leftover" access to repos
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}).(*user_model.User)
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}).(*repo_model.Repository)
accessMode, err := AccessLevel(user, repo)
accessMode, err := access_model.AccessLevel(user, repo)
assert.NoError(t, err)
assert.True(t, accessMode < perm.AccessModeWrite)
}
Expand Down Expand Up @@ -198,3 +199,21 @@ func TestRemoveTeamMember(t *testing.T) {
err := RemoveTeamMember(team, 2)
assert.True(t, organization.IsErrLastOrgOwner(err))
}

func TestRepository_RecalculateAccesses3(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
team5 := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 5}).(*organization.Team)
user29 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 29}).(*user_model.User)

has, err := db.GetEngine(db.DefaultContext).Get(&access_model.Access{UserID: 29, RepoID: 23})
assert.NoError(t, err)
assert.False(t, has)

// adding user29 to team5 should add an explicit access row for repo 23
// even though repo 23 is public
assert.NoError(t, AddTeamMember(team5, user29.ID))

has, err = db.GetEngine(db.DefaultContext).Get(&access_model.Access{UserID: 29, RepoID: 23})
assert.NoError(t, err)
assert.True(t, has)
}
Loading

0 comments on commit cbd4547

Please sign in to comment.