Skip to content

Commit

Permalink
all: unwrap database.PermissionsStore interface (#7701)
Browse files Browse the repository at this point in the history
  • Loading branch information
unknwon committed Mar 24, 2024
1 parent 8d2386b commit 5cf0189
Show file tree
Hide file tree
Showing 20 changed files with 233 additions and 511 deletions.
2 changes: 1 addition & 1 deletion internal/cmd/serv.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func runServ(c *cli.Context) error {
fail("Internal error", "Failed to get user by key ID '%d': %v", key.ID, err)
}

mode := database.Perms.AccessMode(ctx, user.ID, repo.ID,
mode := database.Handle.Permissions().AccessMode(ctx, user.ID, repo.ID,
database.AccessModeOptions{
OwnerID: repo.OwnerID,
Private: repo.IsPrivate,
Expand Down
4 changes: 2 additions & 2 deletions internal/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func RepoAssignment(pages ...bool) macaron.Handler {
if c.IsLogged && c.User.IsAdmin {
c.Repo.AccessMode = database.AccessModeOwner
} else {
c.Repo.AccessMode = database.Perms.AccessMode(c.Req.Context(), c.UserID(), repo.ID,
c.Repo.AccessMode = database.Handle.Permissions().AccessMode(c.Req.Context(), c.UserID(), repo.ID,
database.AccessModeOptions{
OwnerID: repo.OwnerID,
Private: repo.IsPrivate,
Expand All @@ -182,7 +182,7 @@ func RepoAssignment(pages ...bool) macaron.Handler {
// If the authenticated user has no direct access, see if the repository is a fork
// and whether the user has access to the base repository.
if c.Repo.AccessMode == database.AccessModeNone && repo.BaseRepo != nil {
mode := database.Perms.AccessMode(c.Req.Context(), c.UserID(), repo.BaseRepo.ID,
mode := database.Handle.Permissions().AccessMode(c.Req.Context(), c.UserID(), repo.BaseRepo.ID,
database.AccessModeOptions{
OwnerID: repo.BaseRepo.OwnerID,
Private: repo.BaseRepo.IsPrivate,
Expand Down
5 changes: 4 additions & 1 deletion internal/database/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ func NewConnection(w logger.Writer) (*gorm.DB, error) {
}

// Initialize stores, sorted in alphabetical order.
Perms = NewPermsStore(db)
Repos = NewReposStore(db)
TwoFactors = &twoFactorsStore{DB: db}
Users = NewUsersStore(db)
Expand Down Expand Up @@ -176,3 +175,7 @@ func (db *DB) Notices() *NoticesStore {
func (db *DB) Organizations() *OrganizationsStore {
return newOrganizationsStoreStore(db.db)
}

func (db *DB) Permissions() *PermissionsStore {
return newPermissionsStore(db.db)
}
8 changes: 0 additions & 8 deletions internal/database/mocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@ import (
"testing"
)

func SetMockPermsStore(t *testing.T, mock PermsStore) {
before := Perms
Perms = mock
t.Cleanup(func() {
Perms = before
})
}

func SetMockReposStore(t *testing.T, mock ReposStore) {
before := Repos
Repos = mock
Expand Down
45 changes: 17 additions & 28 deletions internal/database/perms.go → internal/database/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,11 @@ package database
import (
"context"

"github.com/pkg/errors"
"gorm.io/gorm"
log "unknwon.dev/clog/v2"
)

// PermsStore is the persistent interface for permissions.
type PermsStore interface {
// AccessMode returns the access mode of given user has to the repository.
AccessMode(ctx context.Context, userID, repoID int64, opts AccessModeOptions) AccessMode
// Authorize returns true if the user has as good as desired access mode to the
// repository.
Authorize(ctx context.Context, userID, repoID int64, desired AccessMode, opts AccessModeOptions) bool
// SetRepoPerms does a full update to which users have which level of access to
// given repository. Keys of the "accessMap" are user IDs.
SetRepoPerms(ctx context.Context, repoID int64, accessMap map[int64]AccessMode) error
}

var Perms PermsStore

// Access represents the highest access level of a user has to a repository. The
// only access type that is not in this table is the real owner of a repository.
// In case of an organization repository, the members of the owners team are in
Expand Down Expand Up @@ -74,24 +61,22 @@ func ParseAccessMode(permission string) AccessMode {
}
}

var _ PermsStore = (*permsStore)(nil)

type permsStore struct {
*gorm.DB
// PermissionsStore is the storage layer for repository permissions.
type PermissionsStore struct {
db *gorm.DB
}

// NewPermsStore returns a persistent interface for permissions with given
// database connection.
func NewPermsStore(db *gorm.DB) PermsStore {
return &permsStore{DB: db}
func newPermissionsStore(db *gorm.DB) *PermissionsStore {
return &PermissionsStore{db: db}
}

type AccessModeOptions struct {
OwnerID int64 // The ID of the repository owner.
Private bool // Whether the repository is private.
}

func (s *permsStore) AccessMode(ctx context.Context, userID, repoID int64, opts AccessModeOptions) (mode AccessMode) {
// AccessMode returns the access mode of given user has to the repository.
func (s *PermissionsStore) AccessMode(ctx context.Context, userID, repoID int64, opts AccessModeOptions) (mode AccessMode) {
if repoID <= 0 {
return AccessModeNone
}
Expand All @@ -111,21 +96,25 @@ func (s *permsStore) AccessMode(ctx context.Context, userID, repoID int64, opts
}

access := new(Access)
err := s.WithContext(ctx).Where("user_id = ? AND repo_id = ?", userID, repoID).First(access).Error
err := s.db.WithContext(ctx).Where("user_id = ? AND repo_id = ?", userID, repoID).First(access).Error
if err != nil {
if err != gorm.ErrRecordNotFound {
if !errors.Is(err, gorm.ErrRecordNotFound) {
log.Error("Failed to get access [user_id: %d, repo_id: %d]: %v", userID, repoID, err)
}
return mode
}
return access.Mode
}

func (s *permsStore) Authorize(ctx context.Context, userID, repoID int64, desired AccessMode, opts AccessModeOptions) bool {
// Authorize returns true if the user has as good as desired access mode to the
// repository.
func (s *PermissionsStore) Authorize(ctx context.Context, userID, repoID int64, desired AccessMode, opts AccessModeOptions) bool {
return desired <= s.AccessMode(ctx, userID, repoID, opts)
}

func (s *permsStore) SetRepoPerms(ctx context.Context, repoID int64, accessMap map[int64]AccessMode) error {
// SetRepoPerms does a full update to which users have which level of access to
// given repository. Keys of the "accessMap" are user IDs.
func (s *PermissionsStore) SetRepoPerms(ctx context.Context, repoID int64, accessMap map[int64]AccessMode) error {
records := make([]*Access, 0, len(accessMap))
for userID, mode := range accessMap {
records = append(records, &Access{
Expand All @@ -135,7 +124,7 @@ func (s *permsStore) SetRepoPerms(ctx context.Context, repoID int64, accessMap m
})
}

return s.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
err := tx.Where("repo_id = ?", repoID).Delete(new(Access)).Error
if err != nil {
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,41 +19,41 @@ func TestPerms(t *testing.T) {
t.Parallel()

ctx := context.Background()
db := &permsStore{
DB: newTestDB(t, "permsStore"),
s := &PermissionsStore{
db: newTestDB(t, "PermissionsStore"),
}

for _, tc := range []struct {
name string
test func(t *testing.T, ctx context.Context, db *permsStore)
test func(t *testing.T, ctx context.Context, s *PermissionsStore)
}{
{"AccessMode", permsAccessMode},
{"Authorize", permsAuthorize},
{"SetRepoPerms", permsSetRepoPerms},
} {
t.Run(tc.name, func(t *testing.T) {
t.Cleanup(func() {
err := clearTables(t, db.DB)
err := clearTables(t, s.db)
require.NoError(t, err)
})
tc.test(t, ctx, db)
tc.test(t, ctx, s)
})
if t.Failed() {
break
}
}
}

func permsAccessMode(t *testing.T, ctx context.Context, db *permsStore) {
func permsAccessMode(t *testing.T, ctx context.Context, s *PermissionsStore) {
// Set up permissions
err := db.SetRepoPerms(ctx, 1,
err := s.SetRepoPerms(ctx, 1,
map[int64]AccessMode{
2: AccessModeWrite,
3: AccessModeAdmin,
},
)
require.NoError(t, err)
err = db.SetRepoPerms(ctx, 2,
err = s.SetRepoPerms(ctx, 2,
map[int64]AccessMode{
1: AccessModeRead,
},
Expand Down Expand Up @@ -149,15 +149,15 @@ func permsAccessMode(t *testing.T, ctx context.Context, db *permsStore) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
mode := db.AccessMode(ctx, test.userID, test.repoID, test.opts)
mode := s.AccessMode(ctx, test.userID, test.repoID, test.opts)
assert.Equal(t, test.wantAccessMode, mode)
})
}
}

func permsAuthorize(t *testing.T, ctx context.Context, db *permsStore) {
func permsAuthorize(t *testing.T, ctx context.Context, s *PermissionsStore) {
// Set up permissions
err := db.SetRepoPerms(ctx, 1,
err := s.SetRepoPerms(ctx, 1,
map[int64]AccessMode{
1: AccessModeRead,
2: AccessModeWrite,
Expand Down Expand Up @@ -230,7 +230,7 @@ func permsAuthorize(t *testing.T, ctx context.Context, db *permsStore) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
authorized := db.Authorize(ctx, test.userID, repo.ID, test.desired,
authorized := s.Authorize(ctx, test.userID, repo.ID, test.desired,
AccessModeOptions{
OwnerID: repo.OwnerID,
Private: repo.IsPrivate,
Expand All @@ -241,7 +241,7 @@ func permsAuthorize(t *testing.T, ctx context.Context, db *permsStore) {
}
}

func permsSetRepoPerms(t *testing.T, ctx context.Context, db *permsStore) {
func permsSetRepoPerms(t *testing.T, ctx context.Context, s *PermissionsStore) {
for _, update := range []struct {
repoID int64
accessMap map[int64]AccessMode
Expand Down Expand Up @@ -280,14 +280,14 @@ func permsSetRepoPerms(t *testing.T, ctx context.Context, db *permsStore) {
},
},
} {
err := db.SetRepoPerms(ctx, update.repoID, update.accessMap)
err := s.SetRepoPerms(ctx, update.repoID, update.accessMap)
if err != nil {
t.Fatal(err)
}
}

var accesses []*Access
err := db.Order("user_id, repo_id").Find(&accesses).Error
err := s.db.Order("user_id, repo_id").Find(&accesses).Error
require.NoError(t, err)

// Ignore ID fields
Expand Down
6 changes: 3 additions & 3 deletions internal/database/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ func (repo *Repository) APIFormatLegacy(permission *api.Permission, user ...*Use
if repo.IsFork {
p := &api.Permission{Pull: true}
if len(user) != 0 {
accessMode := Perms.AccessMode(
accessMode := Handle.Permissions().AccessMode(
context.TODO(),
user[0].ID,
repo.ID,
Expand Down Expand Up @@ -530,7 +530,7 @@ func (repo *Repository) GetAssignees() (_ []*User, err error) {
// GetAssigneeByID returns the user that has write access of repository by given ID.
func (repo *Repository) GetAssigneeByID(userID int64) (*User, error) {
ctx := context.TODO()
if !Perms.Authorize(
if !Handle.Permissions().Authorize(
ctx,
userID,
repo.ID,
Expand Down Expand Up @@ -592,7 +592,7 @@ func (repo *Repository) ComposeCompareURL(oldCommitID, newCommitID string) strin
}

func (repo *Repository) HasAccess(userID int64) bool {
return Perms.Authorize(context.TODO(), userID, repo.ID, AccessModeRead,
return Handle.Permissions().Authorize(context.TODO(), userID, repo.ID, AccessModeRead,
AccessModeOptions{
OwnerID: repo.OwnerID,
Private: repo.IsPrivate,
Expand Down
2 changes: 1 addition & 1 deletion internal/database/repo_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func UpdateOrgProtectBranch(repo *Repository, protectBranch *ProtectBranch, whit
userIDs := tool.StringsToInt64s(strings.Split(whitelistUserIDs, ","))
validUserIDs = make([]int64, 0, len(userIDs))
for _, userID := range userIDs {
if !Perms.Authorize(context.TODO(), userID, repo.ID, AccessModeWrite,
if !Handle.Permissions().Authorize(context.TODO(), userID, repo.ID, AccessModeWrite,
AccessModeOptions{
OwnerID: repo.OwnerID,
Private: repo.IsPrivate,
Expand Down
14 changes: 7 additions & 7 deletions internal/database/repos_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,10 @@ func reposGetByCollaboratorID(t *testing.T, ctx context.Context, db *reposStore)
repo2, err := db.Create(ctx, 2, CreateRepoOptions{Name: "repo2"})
require.NoError(t, err)

permsStore := NewPermsStore(db.DB)
err = permsStore.SetRepoPerms(ctx, repo1.ID, map[int64]AccessMode{3: AccessModeRead})
permissionsStore := newPermissionsStore(db.DB)
err = permissionsStore.SetRepoPerms(ctx, repo1.ID, map[int64]AccessMode{3: AccessModeRead})
require.NoError(t, err)
err = permsStore.SetRepoPerms(ctx, repo2.ID, map[int64]AccessMode{4: AccessModeAdmin})
err = permissionsStore.SetRepoPerms(ctx, repo2.ID, map[int64]AccessMode{4: AccessModeAdmin})
require.NoError(t, err)

t.Run("user 3 is a collaborator of repo1", func(t *testing.T) {
Expand All @@ -193,12 +193,12 @@ func reposGetByCollaboratorIDWithAccessMode(t *testing.T, ctx context.Context, d
repo3, err := db.Create(ctx, 2, CreateRepoOptions{Name: "repo3"})
require.NoError(t, err)

permsStore := NewPermsStore(db.DB)
err = permsStore.SetRepoPerms(ctx, repo1.ID, map[int64]AccessMode{3: AccessModeRead})
permissionsStore := newPermissionsStore(db.DB)
err = permissionsStore.SetRepoPerms(ctx, repo1.ID, map[int64]AccessMode{3: AccessModeRead})
require.NoError(t, err)
err = permsStore.SetRepoPerms(ctx, repo2.ID, map[int64]AccessMode{3: AccessModeAdmin, 4: AccessModeWrite})
err = permissionsStore.SetRepoPerms(ctx, repo2.ID, map[int64]AccessMode{3: AccessModeAdmin, 4: AccessModeWrite})
require.NoError(t, err)
err = permsStore.SetRepoPerms(ctx, repo3.ID, map[int64]AccessMode{4: AccessModeWrite})
err = permissionsStore.SetRepoPerms(ctx, repo3.ID, map[int64]AccessMode{4: AccessModeWrite})
require.NoError(t, err)

got, err := db.GetByCollaboratorIDWithAccessMode(ctx, 3)
Expand Down
2 changes: 1 addition & 1 deletion internal/database/ssh_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ func DeleteDeployKey(doer *User, id int64) error {
if err != nil {
return fmt.Errorf("GetRepositoryByID: %v", err)
}
if !Perms.Authorize(context.TODO(), doer.ID, repo.ID, AccessModeAdmin,
if !Handle.Permissions().Authorize(context.TODO(), doer.ID, repo.ID, AccessModeAdmin,
AccessModeOptions{
OwnerID: repo.OwnerID,
Private: repo.IsPrivate,
Expand Down
2 changes: 1 addition & 1 deletion internal/route/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func repoAssignment() macaron.Handler {
if c.IsTokenAuth && c.User.IsAdmin {
c.Repo.AccessMode = database.AccessModeOwner
} else {
c.Repo.AccessMode = database.Perms.AccessMode(c.Req.Context(), c.UserID(), repo.ID,
c.Repo.AccessMode = database.Handle.Permissions().AccessMode(c.Req.Context(), c.UserID(), repo.ID,
database.AccessModeOptions{
OwnerID: repo.OwnerID,
Private: repo.IsPrivate,
Expand Down
2 changes: 1 addition & 1 deletion internal/route/api/v1/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ func ListForks(c *context.APIContext) {
return
}

accessMode := database.Perms.AccessMode(
accessMode := database.Handle.Permissions().AccessMode(
c.Req.Context(),
c.User.ID,
forks[i].ID,
Expand Down

0 comments on commit 5cf0189

Please sign in to comment.