diff options
author | ᴜɴᴋɴᴡᴏɴ <u@gogs.io> | 2020-04-11 05:39:45 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-04-11 05:39:45 +0800 |
commit | 76bb647d2437dbdea86ac1a0caf5d768ab924e18 (patch) | |
tree | 94a59406f014521d17e662ba4dcf2545b6d3a5ff /internal/db | |
parent | e077ecdd9d95ecb76d91105b3858ee48d52c0cc2 (diff) |
db: add tests for permissions (#6088)
* Add flag to print SQLs
* Add tests for perms
* Make results stable
* codecov: only show diff
* Once again, stable find results
Diffstat (limited to 'internal/db')
-rw-r--r-- | internal/db/access.go | 2 | ||||
-rw-r--r-- | internal/db/lfs.go | 10 | ||||
-rw-r--r-- | internal/db/main_test.go | 5 | ||||
-rw-r--r-- | internal/db/mocks.go | 9 | ||||
-rw-r--r-- | internal/db/perms.go | 42 | ||||
-rw-r--r-- | internal/db/perms_test.go | 288 |
6 files changed, 343 insertions, 13 deletions
diff --git a/internal/db/access.go b/internal/db/access.go index 5f9960af..987abe80 100644 --- a/internal/db/access.go +++ b/internal/db/access.go @@ -146,7 +146,7 @@ func maxAccessMode(modes ...AccessMode) AccessMode { return max } -// FIXME: do corss-comparison so reduce deletions and additions to the minimum? +// Deprecated: Use Perms.SetRepoPerms instead. func (repo *Repository) refreshAccesses(e Engine, accessMap map[int64]AccessMode) (err error) { newAccesses := make([]Access, 0, len(accessMap)) for userID, mode := range accessMap { diff --git a/internal/db/lfs.go b/internal/db/lfs.go index 128069ed..4db2fa2d 100644 --- a/internal/db/lfs.go +++ b/internal/db/lfs.go @@ -30,10 +30,6 @@ type LFSStore interface { var LFS LFSStore -type lfs struct { - *gorm.DB -} - // LFSObject is the relation between an LFS object and a repository. type LFSObject struct { RepoID int64 `gorm:"PRIMARY_KEY;AUTO_INCREMENT:false"` @@ -43,6 +39,12 @@ type LFSObject struct { CreatedAt time.Time `gorm:"NOT NULL"` } +var _ LFSStore = (*lfs)(nil) + +type lfs struct { + *gorm.DB +} + func (db *lfs) CreateObject(repoID int64, oid lfsutil.OID, size int64, storage lfsutil.Storage) error { object := &LFSObject{ RepoID: repoID, diff --git a/internal/db/main_test.go b/internal/db/main_test.go index 8b4190da..fdd7447f 100644 --- a/internal/db/main_test.go +++ b/internal/db/main_test.go @@ -21,6 +21,8 @@ import ( "gogs.io/gogs/internal/testutil" ) +var printSQL = flag.Bool("print-sql", false, "Print SQL executed") + func TestMain(m *testing.M) { flag.Parse() if !testing.Verbose() { @@ -75,6 +77,9 @@ func initTestDB(t *testing.T, suite string, tables ...interface{}) *gorm.DB { if !testing.Verbose() { db.SetLogger(&dbutil.Writer{Writer: ioutil.Discard}) } + if *printSQL { + db.LogMode(true) + } err = db.AutoMigrate(tables...).Error if err != nil { diff --git a/internal/db/mocks.go b/internal/db/mocks.go index 12622a7c..7e6ddced 100644 --- a/internal/db/mocks.go +++ b/internal/db/mocks.go @@ -81,8 +81,9 @@ func SetMockLFSStore(t *testing.T, mock LFSStore) { var _ PermsStore = (*MockPermsStore)(nil) type MockPermsStore struct { - MockAccessMode func(userID int64, repo *Repository) AccessMode - MockAuthorize func(userID int64, repo *Repository, desired AccessMode) bool + MockAccessMode func(userID int64, repo *Repository) AccessMode + MockAuthorize func(userID int64, repo *Repository, desired AccessMode) bool + MockSetRepoPerms func(repoID int64, accessMap map[int64]AccessMode) error } func (m *MockPermsStore) AccessMode(userID int64, repo *Repository) AccessMode { @@ -93,6 +94,10 @@ func (m *MockPermsStore) Authorize(userID int64, repo *Repository, desired Acces return m.MockAuthorize(userID, repo, desired) } +func (m *MockPermsStore) SetRepoPerms(repoID int64, accessMap map[int64]AccessMode) error { + return m.MockSetRepoPerms(repoID, accessMap) +} + func SetMockPermsStore(t *testing.T, mock PermsStore) { before := Perms Perms = mock diff --git a/internal/db/perms.go b/internal/db/perms.go index 6dc5d423..b0db295e 100644 --- a/internal/db/perms.go +++ b/internal/db/perms.go @@ -5,6 +5,8 @@ package db import ( + "strings" + "github.com/jinzhu/gorm" log "unknwon.dev/clog/v2" ) @@ -15,25 +17,32 @@ import ( type PermsStore interface { // AccessMode returns the access mode of given user has to the repository. AccessMode(userID int64, repo *Repository) AccessMode - // Authorize returns true if the user has as good as desired access mode to - // the repository. + // Authorize returns true if the user has as good as desired access mode to the repository. Authorize(userID int64, repo *Repository, desired AccessMode) 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(repoID int64, accessMap map[int64]AccessMode) error } var Perms PermsStore +var _ PermsStore = (*perms)(nil) + type perms struct { *gorm.DB } -func (db *perms) AccessMode(userID int64, repo *Repository) AccessMode { - var mode AccessMode +func (db *perms) AccessMode(userID int64, repo *Repository) (mode AccessMode) { + if repo == nil { + return AccessModeNone + } + // Everyone has read access to public repository. if !repo.IsPrivate { mode = AccessModeRead } - // Quick check to avoid a DB query. + // Anonymous user gets the default access. if userID <= 0 { return mode } @@ -45,7 +54,9 @@ func (db *perms) AccessMode(userID int64, repo *Repository) AccessMode { access := new(Access) err := db.Where("user_id = ? AND repo_id = ?", userID, repo.ID).First(access).Error if err != nil { - log.Error("Failed to get access [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err) + if !gorm.IsRecordNotFoundError(err){ + log.Error("Failed to get access [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err) + } return mode } return access.Mode @@ -54,3 +65,22 @@ func (db *perms) AccessMode(userID int64, repo *Repository) AccessMode { func (db *perms) Authorize(userID int64, repo *Repository, desired AccessMode) bool { return desired <= db.AccessMode(userID, repo) } + +func (db *perms) SetRepoPerms(repoID int64, accessMap map[int64]AccessMode) error { + vals := make([]string, 0, len(accessMap)) + items := make([]interface{}, 0, len(accessMap)*3) + for userID, mode := range accessMap { + vals = append(vals, "(?, ?, ?)") + items = append(items, userID, repoID, mode) + } + + return db.Transaction(func(tx *gorm.DB) error { + err := tx.Where("repo_id = ?", repoID).Delete(new(Access)).Error + if err != nil { + return err + } + + sql := "INSERT INTO access (user_id, repo_id, mode) VALUES " + strings.Join(vals, ", ") + return tx.Exec(sql, items...).Error + }) +} diff --git a/internal/db/perms_test.go b/internal/db/perms_test.go new file mode 100644 index 00000000..9f3f4b1b --- /dev/null +++ b/internal/db/perms_test.go @@ -0,0 +1,288 @@ +// Copyright 2020 The Gogs 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 db + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_perms(t *testing.T) { + if testing.Short() { + t.Skip() + } + + t.Parallel() + + db := &perms{ + DB: initTestDB(t, "perms", new(Access)), + } + + for _, tc := range []struct { + name string + test func(*testing.T, *perms) + }{ + {"AccessMode", test_perms_AccessMode}, + {"Authorize", test_perms_Authorize}, + {"SetRepoPerms", test_perms_SetRepoPerms}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Cleanup(func() { + err := deleteTables(db.DB, new(Access)) + if err != nil { + t.Fatal(err) + } + }) + tc.test(t, db) + }) + } +} +func test_perms_AccessMode(t *testing.T, db *perms) { + // Set up permissions + err := db.SetRepoPerms(1, map[int64]AccessMode{ + 2: AccessModeWrite, + 3: AccessModeAdmin, + }) + if err != nil { + t.Fatal(err) + } + err = db.SetRepoPerms(2, map[int64]AccessMode{ + 1: AccessModeRead, + }) + if err != nil { + t.Fatal(err) + } + + publicRepo := &Repository{ + ID: 1, + OwnerID: 98, + } + privateRepo := &Repository{ + ID: 2, + OwnerID: 99, + IsPrivate: true, + } + + tests := []struct { + name string + userID int64 + repo *Repository + expAccessMode AccessMode + }{ + { + name: "nil repository", + expAccessMode: AccessModeNone, + }, + + { + name: "anonymous user has read access to public repository", + repo: publicRepo, + expAccessMode: AccessModeRead, + }, + { + name: "anonymous user has no access to private repository", + repo: privateRepo, + expAccessMode: AccessModeNone, + }, + + { + name: "user is the owner", + userID: 98, + repo: publicRepo, + expAccessMode: AccessModeOwner, + }, + { + name: "user 1 has read access to public repo", + userID: 1, + repo: publicRepo, + expAccessMode: AccessModeRead, + }, + { + name: "user 2 has write access to public repo", + userID: 2, + repo: publicRepo, + expAccessMode: AccessModeWrite, + }, + { + name: "user 3 has admin access to public repo", + userID: 3, + repo: publicRepo, + expAccessMode: AccessModeAdmin, + }, + + { + name: "user 1 has read access to private repo", + userID: 1, + repo: privateRepo, + expAccessMode: AccessModeRead, + }, + { + name: "user 2 has no access to private repo", + userID: 2, + repo: privateRepo, + expAccessMode: AccessModeNone, + }, + { + name: "user 3 has no access to private repo", + userID: 3, + repo: privateRepo, + expAccessMode: AccessModeNone, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mode := db.AccessMode(test.userID, test.repo) + assert.Equal(t, test.expAccessMode, mode) + }) + } +} + +func test_perms_Authorize(t *testing.T, db *perms) { + // Set up permissions + err := db.SetRepoPerms(1, map[int64]AccessMode{ + 1: AccessModeRead, + 2: AccessModeWrite, + 3: AccessModeAdmin, + }) + if err != nil { + t.Fatal(err) + } + + repo := &Repository{ + ID: 1, + OwnerID: 98, + } + + tests := []struct { + name string + userID int64 + desired AccessMode + expAuthorized bool + }{ + { + name: "user 1 has read and wants read", + userID: 1, + desired: AccessModeRead, + expAuthorized: true, + }, + { + name: "user 1 has read and wants write", + userID: 1, + desired: AccessModeWrite, + expAuthorized: false, + }, + + { + name: "user 2 has write and wants read", + userID: 2, + desired: AccessModeRead, + expAuthorized: true, + }, + { + name: "user 2 has write and wants write", + userID: 2, + desired: AccessModeWrite, + expAuthorized: true, + }, + { + name: "user 2 has write and wants admin", + userID: 2, + desired: AccessModeAdmin, + expAuthorized: false, + }, + + { + name: "user 3 has admin and wants read", + userID: 3, + desired: AccessModeRead, + expAuthorized: true, + }, + { + name: "user 3 has admin and wants write", + userID: 3, + desired: AccessModeWrite, + expAuthorized: true, + }, + { + name: "user 3 has admin and wants admin", + userID: 3, + desired: AccessModeAdmin, + expAuthorized: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + authorized := db.Authorize(test.userID, repo, test.desired) + assert.Equal(t, test.expAuthorized, authorized) + }) + } +} + +func test_perms_SetRepoPerms(t *testing.T, db *perms) { + for _, update := range []struct { + repoID int64 + accessMap map[int64]AccessMode + }{ + { + repoID: 1, + accessMap: map[int64]AccessMode{ + 1: AccessModeWrite, + 2: AccessModeWrite, + 3: AccessModeAdmin, + 4: AccessModeWrite, + }, + }, + { + repoID: 2, + accessMap: map[int64]AccessMode{ + 1: AccessModeWrite, + 2: AccessModeRead, + 4: AccessModeWrite, + 5: AccessModeWrite, + }, + }, + { + repoID: 1, + accessMap: map[int64]AccessMode{ + 2: AccessModeWrite, + 3: AccessModeAdmin, + }, + }, + { + repoID: 2, + accessMap: map[int64]AccessMode{ + 1: AccessModeWrite, + 2: AccessModeRead, + 5: AccessModeWrite, + }, + }, + } { + err := db.SetRepoPerms(update.repoID, update.accessMap) + if err != nil { + t.Fatal(err) + } + } + + var accesses []*Access + err := db.Order("user_id, repo_id").Find(&accesses).Error + if err != nil { + t.Fatal(err) + } + + // Ignore ID fields + for _, a := range accesses { + a.ID = 0 + } + + expAccesses := []*Access{ + {UserID: 1, RepoID: 2, Mode: AccessModeWrite}, + {UserID: 2, RepoID: 1, Mode: AccessModeWrite}, + {UserID: 2, RepoID: 2, Mode: AccessModeRead}, + {UserID: 3, RepoID: 1, Mode: AccessModeAdmin}, + {UserID: 5, RepoID: 2, Mode: AccessModeWrite}, + } + assert.Equal(t, expAccesses, accesses) +} |