refactor(db): migrate perms methods off `user.go` (#7207)

internal/db/repo.go

@@ -391,8 +391,17 @@ func (repo *Repository) APIFormatLegacy(permission *api.Permission, user ...*Use
 	if repo.IsFork {
 		p := &api.Permission{Pull: true}
 		if len(user) != 0 {
-			p.Admin = user[0].IsAdminOfRepo(repo)
+			accessMode := Perms.AccessMode(
-			p.Push = user[0].IsWriterOfRepo(repo)
+				context.TODO(),
+				user[0].ID,
+				repo.ID,
+				AccessModeOptions{
+					OwnerID: repo.OwnerID,
+					Private: repo.IsPrivate,
+				},
+			)
+			p.Admin = accessMode >= AccessModeAdmin
+			p.Push = accessMode >= AccessModeWrite
 		}
 		apiRepo.Parent = repo.BaseRepo.APIFormatLegacy(p)
 	}

internal/db/two_factors.go

@@ -32,8 +32,8 @@ type TwoFactorsStore interface {
 	// GetByUserID returns the 2FA token of given user. It returns
 	// ErrTwoFactorNotFound when not found.
 	GetByUserID(ctx context.Context, userID int64) (*TwoFactor, error)
-	// IsUserEnabled returns true if the user has enabled 2FA.
+	// IsEnabled returns true if the user has enabled 2FA.
-	IsUserEnabled(ctx context.Context, userID int64) bool
+	IsEnabled(ctx context.Context, userID int64) bool
 }
 
 var TwoFactors TwoFactorsStore
@@ -114,7 +114,7 @@ func (db *twoFactors) GetByUserID(ctx context.Context, userID int64) (*TwoFactor
 	return tf, nil
 }
 
-func (db *twoFactors) IsUserEnabled(ctx context.Context, userID int64) bool {
+func (db *twoFactors) IsEnabled(ctx context.Context, userID int64) bool {
 	var count int64
 	err := db.WithContext(ctx).Model(new(TwoFactor)).Where("user_id = ?", userID).Count(&count).Error
 	if err != nil {

internal/db/two_factors_test.go

@@ -58,7 +58,7 @@ func TestTwoFactors(t *testing.T) {
 	}{
 		{"Create", twoFactorsCreate},
 		{"GetByUserID", twoFactorsGetByUserID},
-		{"IsUserEnabled", twoFactorsIsUserEnabled},
+		{"IsEnabled", twoFactorsIsEnabled},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Cleanup(func() {
@@ -109,13 +109,13 @@ func twoFactorsGetByUserID(t *testing.T, db *twoFactors) {
 	assert.Equal(t, wantErr, err)
 }
 
-func twoFactorsIsUserEnabled(t *testing.T, db *twoFactors) {
+func twoFactorsIsEnabled(t *testing.T, db *twoFactors) {
 	ctx := context.Background()
 
 	// Create a 2FA token for user 1
 	err := db.Create(ctx, 1, "secure-key", "secure-secret")
 	require.NoError(t, err)
 
-	assert.True(t, db.IsUserEnabled(ctx, 1))
+	assert.True(t, db.IsEnabled(ctx, 1))
-	assert.False(t, db.IsUserEnabled(ctx, 2))
+	assert.False(t, db.IsEnabled(ctx, 2))
 }

internal/db/user.go

@@ -53,46 +53,6 @@ func (u *User) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
-// IsAdminOfRepo returns true if user has admin or higher access of repository.
-func (u *User) IsAdminOfRepo(repo *Repository) bool {
-	return Perms.Authorize(context.TODO(), u.ID, repo.ID, AccessModeAdmin,
-		AccessModeOptions{
-			OwnerID: repo.OwnerID,
-			Private: repo.IsPrivate,
-		},
-	)
-}
-
-// IsWriterOfRepo returns true if user has write access to given repository.
-func (u *User) IsWriterOfRepo(repo *Repository) bool {
-	return Perms.Authorize(context.TODO(), u.ID, repo.ID, AccessModeWrite,
-		AccessModeOptions{
-			OwnerID: repo.OwnerID,
-			Private: repo.IsPrivate,
-		},
-	)
-}
-
-// IsOrganization returns true if user is actually a organization.
-func (u *User) IsOrganization() bool {
-	return u.Type == UserTypeOrganization
-}
-
-// IsUserOrgOwner returns true if user is in the owner team of given organization.
-func (u *User) IsUserOrgOwner(orgId int64) bool {
-	return IsOrganizationOwner(orgId, u.ID)
-}
-
-// IsPublicMember returns true if user public his/her membership in give organization.
-func (u *User) IsPublicMember(orgId int64) bool {
-	return IsPublicMembership(orgId, u.ID)
-}
-
-// IsEnabledTwoFactor returns true if user has enabled two-factor authentication.
-func (u *User) IsEnabledTwoFactor() bool {
-	return TwoFactors.IsUserEnabled(context.TODO(), u.ID)
-}
-
 func (u *User) getOrganizationCount(e Engine) (int64, error) {
 	return e.Where("uid=?", u.ID).Count(new(OrgUser))
 }

internal/db/users.go

@@ -511,11 +511,16 @@ func (u *User) AfterFind(_ *gorm.DB) error {
 	return nil
 }
 
-// IsLocal returns true if user is created as local account.
+// IsLocal returns true if the user is created as local account.
 func (u *User) IsLocal() bool {
 	return u.LoginSource <= 0
 }
 
+// IsOrganization returns true if the user is an organization.
+func (u *User) IsOrganization() bool {
+	return u.Type == UserTypeOrganization
+}
+
 // APIFormat returns the API format of a user.
 func (u *User) APIFormat() *api.User {
 	return &api.User{
@@ -626,3 +631,21 @@ func (u *User) AvatarURL() string {
 func (u *User) IsFollowing(followID int64) bool {
 	return Follows.IsFollowing(context.TODO(), u.ID, followID)
 }
+
+// IsUserOrgOwner returns true if the user is in the owner team of the given
+// organization.
+//
+// TODO(unknwon): This is also used in templates, which should be fixed by
+// having a dedicated type `template.User`.
+func (u *User) IsUserOrgOwner(orgId int64) bool {
+	return IsOrganizationOwner(orgId, u.ID)
+}
+
+// IsPublicMember returns true if the user has public membership of the given
+// organization.
+//
+// TODO(unknwon): This is also used in templates, which should be fixed by
+// having a dedicated type `template.User`.
+func (u *User) IsPublicMember(orgId int64) bool {
+	return IsPublicMembership(orgId, u.ID)
+}

internal/route/api/v1/repo/repo.go

@@ -352,11 +352,24 @@ func ListForks(c *context.APIContext) {
 			c.Error(err, "get owner")
 			return
 		}
-		apiForks[i] = forks[i].APIFormatLegacy(&api.Permission{
+
-			Admin: c.User.IsAdminOfRepo(forks[i]),
+		accessMode := db.Perms.AccessMode(
-			Push:  c.User.IsWriterOfRepo(forks[i]),
+			c.Req.Context(),
-			Pull:  true,
+			c.User.ID,
-		})
+			forks[i].ID,
+			db.AccessModeOptions{
+				OwnerID: forks[i].OwnerID,
+				Private: forks[i].IsPrivate,
+			},
+		)
+
+		apiForks[i] = forks[i].APIFormatLegacy(
+			&api.Permission{
+				Admin: accessMode >= db.AccessModeAdmin,
+				Push:  accessMode >= db.AccessModeWrite,
+				Pull:  true,
+			},
+		)
 	}
 
 	c.JSONSuccess(&apiForks)

internal/route/lfs/mocks_test.go

@@ -1898,9 +1898,9 @@ type MockTwoFactorsStore struct {
 	// GetByUserIDFunc is an instance of a mock function object controlling
 	// the behavior of the method GetByUserID.
 	GetByUserIDFunc *TwoFactorsStoreGetByUserIDFunc
-	// IsUserEnabledFunc is an instance of a mock function object
+	// IsEnabledFunc is an instance of a mock function object controlling
-	// controlling the behavior of the method IsUserEnabled.
+	// the behavior of the method IsEnabled.
-	IsUserEnabledFunc *TwoFactorsStoreIsUserEnabledFunc
+	IsEnabledFunc *TwoFactorsStoreIsEnabledFunc
 }
 
 // NewMockTwoFactorsStore creates a new mock of the TwoFactorsStore
@@ -1918,7 +1918,7 @@ func NewMockTwoFactorsStore() *MockTwoFactorsStore {
 				return
 			},
 		},
-		IsUserEnabledFunc: &TwoFactorsStoreIsUserEnabledFunc{
+		IsEnabledFunc: &TwoFactorsStoreIsEnabledFunc{
 			defaultHook: func(context.Context, int64) (r0 bool) {
 				return
 			},
@@ -1940,9 +1940,9 @@ func NewStrictMockTwoFactorsStore() *MockTwoFactorsStore {
 				panic("unexpected invocation of MockTwoFactorsStore.GetByUserID")
 			},
 		},
-		IsUserEnabledFunc: &TwoFactorsStoreIsUserEnabledFunc{
+		IsEnabledFunc: &TwoFactorsStoreIsEnabledFunc{
 			defaultHook: func(context.Context, int64) bool {
-				panic("unexpected invocation of MockTwoFactorsStore.IsUserEnabled")
+				panic("unexpected invocation of MockTwoFactorsStore.IsEnabled")
 			},
 		},
 	}
@@ -1959,8 +1959,8 @@ func NewMockTwoFactorsStoreFrom(i db.TwoFactorsStore) *MockTwoFactorsStore {
 		GetByUserIDFunc: &TwoFactorsStoreGetByUserIDFunc{
 			defaultHook: i.GetByUserID,
 		},
-		IsUserEnabledFunc: &TwoFactorsStoreIsUserEnabledFunc{
+		IsEnabledFunc: &TwoFactorsStoreIsEnabledFunc{
-			defaultHook: i.IsUserEnabled,
+			defaultHook: i.IsEnabled,
 		},
 	}
 }
@@ -2184,36 +2184,35 @@ func (c TwoFactorsStoreGetByUserIDFuncCall) Results() []interface{} {
 	return []interface{}{c.Result0, c.Result1}
 }
 
-// TwoFactorsStoreIsUserEnabledFunc describes the behavior when the
+// TwoFactorsStoreIsEnabledFunc describes the behavior when the IsEnabled
-// IsUserEnabled method of the parent MockTwoFactorsStore instance is
+// method of the parent MockTwoFactorsStore instance is invoked.
-// invoked.
+type TwoFactorsStoreIsEnabledFunc struct {
-type TwoFactorsStoreIsUserEnabledFunc struct {
 	defaultHook func(context.Context, int64) bool
 	hooks       []func(context.Context, int64) bool
-	history     []TwoFactorsStoreIsUserEnabledFuncCall
+	history     []TwoFactorsStoreIsEnabledFuncCall
 	mutex       sync.Mutex
 }
 
-// IsUserEnabled delegates to the next hook function in the queue and stores
+// IsEnabled delegates to the next hook function in the queue and stores the
-// the parameter and result values of this invocation.
+// parameter and result values of this invocation.
-func (m *MockTwoFactorsStore) IsUserEnabled(v0 context.Context, v1 int64) bool {
+func (m *MockTwoFactorsStore) IsEnabled(v0 context.Context, v1 int64) bool {
-	r0 := m.IsUserEnabledFunc.nextHook()(v0, v1)
+	r0 := m.IsEnabledFunc.nextHook()(v0, v1)
-	m.IsUserEnabledFunc.appendCall(TwoFactorsStoreIsUserEnabledFuncCall{v0, v1, r0})
+	m.IsEnabledFunc.appendCall(TwoFactorsStoreIsEnabledFuncCall{v0, v1, r0})
 	return r0
 }
 
-// SetDefaultHook sets function that is called when the IsUserEnabled method
+// SetDefaultHook sets function that is called when the IsEnabled method of
-// of the parent MockTwoFactorsStore instance is invoked and the hook queue
+// the parent MockTwoFactorsStore instance is invoked and the hook queue is
-// is empty.
+// empty.
-func (f *TwoFactorsStoreIsUserEnabledFunc) SetDefaultHook(hook func(context.Context, int64) bool) {
+func (f *TwoFactorsStoreIsEnabledFunc) SetDefaultHook(hook func(context.Context, int64) bool) {
 	f.defaultHook = hook
 }
 
 // PushHook adds a function to the end of hook queue. Each invocation of the
-// IsUserEnabled method of the parent MockTwoFactorsStore instance invokes
+// IsEnabled method of the parent MockTwoFactorsStore instance invokes the
-// the hook at the front of the queue and discards it. After the queue is
+// hook at the front of the queue and discards it. After the queue is empty,
-// empty, the default hook function is invoked for any future action.
+// the default hook function is invoked for any future action.
-func (f *TwoFactorsStoreIsUserEnabledFunc) PushHook(hook func(context.Context, int64) bool) {
+func (f *TwoFactorsStoreIsEnabledFunc) PushHook(hook func(context.Context, int64) bool) {
 	f.mutex.Lock()
 	f.hooks = append(f.hooks, hook)
 	f.mutex.Unlock()
@@ -2221,20 +2220,20 @@ func (f *TwoFactorsStoreIsUserEnabledFunc) PushHook(hook func(context.Context, i
 
 // SetDefaultReturn calls SetDefaultHook with a function that returns the
 // given values.
-func (f *TwoFactorsStoreIsUserEnabledFunc) SetDefaultReturn(r0 bool) {
+func (f *TwoFactorsStoreIsEnabledFunc) SetDefaultReturn(r0 bool) {
 	f.SetDefaultHook(func(context.Context, int64) bool {
 		return r0
 	})
 }
 
 // PushReturn calls PushHook with a function that returns the given values.
-func (f *TwoFactorsStoreIsUserEnabledFunc) PushReturn(r0 bool) {
+func (f *TwoFactorsStoreIsEnabledFunc) PushReturn(r0 bool) {
 	f.PushHook(func(context.Context, int64) bool {
 		return r0
 	})
 }
 
-func (f *TwoFactorsStoreIsUserEnabledFunc) nextHook() func(context.Context, int64) bool {
+func (f *TwoFactorsStoreIsEnabledFunc) nextHook() func(context.Context, int64) bool {
 	f.mutex.Lock()
 	defer f.mutex.Unlock()
 
@@ -2247,26 +2246,26 @@ func (f *TwoFactorsStoreIsUserEnabledFunc) nextHook() func(context.Context, int6
 	return hook
 }
 
-func (f *TwoFactorsStoreIsUserEnabledFunc) appendCall(r0 TwoFactorsStoreIsUserEnabledFuncCall) {
+func (f *TwoFactorsStoreIsEnabledFunc) appendCall(r0 TwoFactorsStoreIsEnabledFuncCall) {
 	f.mutex.Lock()
 	f.history = append(f.history, r0)
 	f.mutex.Unlock()
 }
 
-// History returns a sequence of TwoFactorsStoreIsUserEnabledFuncCall
+// History returns a sequence of TwoFactorsStoreIsEnabledFuncCall objects
-// objects describing the invocations of this function.
+// describing the invocations of this function.
-func (f *TwoFactorsStoreIsUserEnabledFunc) History() []TwoFactorsStoreIsUserEnabledFuncCall {
+func (f *TwoFactorsStoreIsEnabledFunc) History() []TwoFactorsStoreIsEnabledFuncCall {
 	f.mutex.Lock()
-	history := make([]TwoFactorsStoreIsUserEnabledFuncCall, len(f.history))
+	history := make([]TwoFactorsStoreIsEnabledFuncCall, len(f.history))
 	copy(history, f.history)
 	f.mutex.Unlock()
 
 	return history
 }
 
-// TwoFactorsStoreIsUserEnabledFuncCall is an object that describes an
+// TwoFactorsStoreIsEnabledFuncCall is an object that describes an
-// invocation of method IsUserEnabled on an instance of MockTwoFactorsStore.
+// invocation of method IsEnabled on an instance of MockTwoFactorsStore.
-type TwoFactorsStoreIsUserEnabledFuncCall struct {
+type TwoFactorsStoreIsEnabledFuncCall struct {
 	// Arg0 is the value of the 1st argument passed to this method
 	// invocation.
 	Arg0 context.Context
@@ -2280,13 +2279,13 @@ type TwoFactorsStoreIsUserEnabledFuncCall struct {
 
 // Args returns an interface slice containing the arguments of this
 // invocation.
-func (c TwoFactorsStoreIsUserEnabledFuncCall) Args() []interface{} {
+func (c TwoFactorsStoreIsEnabledFuncCall) Args() []interface{} {
 	return []interface{}{c.Arg0, c.Arg1}
 }
 
 // Results returns an interface slice containing the results of this
 // invocation.
-func (c TwoFactorsStoreIsUserEnabledFuncCall) Results() []interface{} {
+func (c TwoFactorsStoreIsEnabledFuncCall) Results() []interface{} {
 	return []interface{}{c.Result0}
 }
 

internal/route/lfs/route.go

@@ -67,7 +67,7 @@ func authenticate() macaron.Handler {
 			return
 		}
 
-		if err == nil && user.IsEnabledTwoFactor() {
+		if err == nil && db.TwoFactors.IsEnabled(c.Req.Context(), user.ID) {
 			c.Error(http.StatusBadRequest, "Users with 2FA enabled are not allowed to authenticate via username and password.")
 			return
 		}

internal/route/lfs/route_test.go

@@ -58,7 +58,7 @@ func Test_authenticate(t *testing.T) {
 			},
 			mockTwoFactorsStore: func() db.TwoFactorsStore {
 				mock := NewMockTwoFactorsStore()
-				mock.IsUserEnabledFunc.SetDefaultReturn(true)
+				mock.IsEnabledFunc.SetDefaultReturn(true)
 				return mock
 			},
 			expStatusCode: http.StatusBadRequest,
@@ -100,7 +100,7 @@ func Test_authenticate(t *testing.T) {
 			},
 			mockTwoFactorsStore: func() db.TwoFactorsStore {
 				mock := NewMockTwoFactorsStore()
-				mock.IsUserEnabledFunc.SetDefaultReturn(false)
+				mock.IsEnabledFunc.SetDefaultReturn(false)
 				return mock
 			},
 			expStatusCode: http.StatusOK,

internal/route/repo/http.go

@@ -153,7 +153,7 @@ func HTTPContexter() macaron.Handler {
 					return
 				}
 			}
-		} else if authUser.IsEnabledTwoFactor() {
+		} else if db.TwoFactors.IsEnabled(c.Req.Context(), authUser.ID) {
 			askCredentials(c, http.StatusUnauthorized, `User with two-factor authentication enabled cannot perform HTTP/HTTPS operations via plain username and password
 Please create and use personal access token on user settings page`)
 			return

internal/route/repo/issue.go

@@ -612,7 +612,16 @@ func viewIssue(c *context.Context, isPullList bool) {
 			if repo.IsOwnedBy(comment.PosterID) ||
 				(repo.Owner.IsOrganization() && repo.Owner.IsOwnedBy(comment.PosterID)) {
 				comment.ShowTag = db.COMMENT_TAG_OWNER
-			} else if comment.Poster.IsWriterOfRepo(repo) {
+			} else if db.Perms.Authorize(
+				c.Req.Context(),
+				comment.PosterID,
+				repo.ID,
+				db.AccessModeWrite,
+				db.AccessModeOptions{
+					OwnerID: repo.OwnerID,
+					Private: repo.IsPrivate,
+				},
+			) {
 				comment.ShowTag = db.COMMENT_TAG_WRITER
 			} else if comment.PosterID == issue.PosterID {
 				comment.ShowTag = db.COMMENT_TAG_POSTER

internal/route/repo/pull.go

@@ -510,7 +510,16 @@ func ParseCompareInfo(c *context.Context) (*db.User, *db.Repository, *git.Reposi
 		headGitRepo = c.Repo.GitRepo
 	}
 
-	if !c.User.IsWriterOfRepo(headRepo) && !c.User.IsAdmin {
+	if !db.Perms.Authorize(
+		c.Req.Context(),
+		c.User.ID,
+		headRepo.ID,
+		db.AccessModeWrite,
+		db.AccessModeOptions{
+			OwnerID: headRepo.OwnerID,
+			Private: headRepo.IsPrivate,
+		},
+	) && !c.User.IsAdmin {
 		log.Trace("ParseCompareInfo [base_repo_id: %d]: does not have write access or site admin", baseRepo.ID)
 		c.NotFound()
 		return nil, nil, nil, nil, "", ""

internal/route/user/auth.go

@@ -184,7 +184,7 @@ func LoginPost(c *context.Context, f form.SignIn) {
 		return
 	}
 
-	if !u.IsEnabledTwoFactor() {
+	if !db.TwoFactors.IsEnabled(c.Req.Context(), u.ID) {
 		afterLogin(c, u, f.Remember)
 		return
 	}

internal/route/user/setting.go

@@ -386,7 +386,7 @@ func SettingsSecurity(c *context.Context) {
 }
 
 func SettingsTwoFactorEnable(c *context.Context) {
-	if c.User.IsEnabledTwoFactor() {
+	if db.TwoFactors.IsEnabled(c.Req.Context(), c.User.ID) {
 		c.NotFound()
 		return
 	}
@@ -456,7 +456,7 @@ func SettingsTwoFactorEnablePost(c *context.Context) {
 }
 
 func SettingsTwoFactorRecoveryCodes(c *context.Context) {
-	if !c.User.IsEnabledTwoFactor() {
+	if !db.TwoFactors.IsEnabled(c.Req.Context(), c.User.ID) {
 		c.NotFound()
 		return
 	}
@@ -475,7 +475,7 @@ func SettingsTwoFactorRecoveryCodes(c *context.Context) {
 }
 
 func SettingsTwoFactorRecoveryCodesPost(c *context.Context) {
-	if !c.User.IsEnabledTwoFactor() {
+	if !db.TwoFactors.IsEnabled(c.Req.Context(), c.User.ID) {
 		c.NotFound()
 		return
 	}
@@ -490,7 +490,7 @@ func SettingsTwoFactorRecoveryCodesPost(c *context.Context) {
 }
 
 func SettingsTwoFactorDisable(c *context.Context) {
-	if !c.User.IsEnabledTwoFactor() {
+	if !db.TwoFactors.IsEnabled(c.Req.Context(), c.User.ID) {
 		c.NotFound()
 		return
 	}