Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move access and repo permission to models/perm/access #19350

Merged
merged 9 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
lunny marked this conversation as resolved.
Show resolved Hide resolved
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