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

codecov.yml

@@ -6,4 +6,4 @@ coverage:
         threshold: 1%
 
 comment:
-  layout: 'diff, files'
+  layout: 'diff'

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 {

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,

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 {

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
+	MockAccessMode   func(userID int64, repo *Repository) AccessMode
-	MockAuthorize  func(userID int64, repo *Repository, desired AccessMode) bool
+	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

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
+	// Authorize returns true if the user has as good as desired access mode to the repository.
-	// 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 {
+func (db *perms) AccessMode(userID int64, repo *Repository) (mode AccessMode) {
-	var 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
+	})
+}

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)
+}