Browse Source

db: add tests for authentication through login sources (#7049)

Joe Chen 3 years ago
parent
commit
2e19f5a3c8

+ 1 - 1
.deepsource.toml

@@ -1,6 +1,6 @@
 version = 1
 
-exclude_patterns = ["**/mocks.go"]
+exclude_patterns = ["**/mocks_test.go"]
 
 [[analyzers]]
 name = "docker"

+ 0 - 4
.github/workflows/go.yml

@@ -38,10 +38,6 @@ jobs:
           args: --timeout=30m
       - name: Install Task
         uses: arduino/setup-task@v1
-      - name: Install goimports and go-mockgen
-        run: |
-          go install github.com/derision-test/go-mockgen/cmd/go-mockgen@v1.2.0
-          go install golang.org/x/tools/cmd/goimports@latest
       - name: Check Go module tidiness and generated files
         shell: bash
         run: |

+ 2 - 2
docs/dev/local_development.md

@@ -41,7 +41,7 @@ Gogs has the following dependencies:
     brew install go postgresql git npm go-task/tap/go-task
     npm install -g less
     npm install -g less-plugin-clean-css
-    go install github.com/derision-test/go-mockgen/cmd/go-mockgen@v1.2.0
+    go install github.com/derision-test/go-mockgen/cmd/go-mockgen@v1.3.3
     go install golang.org/x/tools/cmd/goimports@latest
     ```
 
@@ -79,7 +79,7 @@ Gogs has the following dependencies:
     sudo apt install -y make git-all postgresql postgresql-contrib golang-go nodejs
     npm install -g less
     go install github.com/go-task/task/v3/cmd/task@latest
-    go install github.com/derision-test/go-mockgen/cmd/go-mockgen@v1.2.0
+    go install github.com/derision-test/go-mockgen/cmd/go-mockgen@v1.3.3
     go install golang.org/x/tools/cmd/goimports@latest
     ```
 

+ 8 - 0
gen.go

@@ -0,0 +1,8 @@
+// Copyright 2022 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 main
+
+//go:generate go install golang.org/x/tools/cmd/goimports@v0.1.10
+//go:generate go run github.com/derision-test/go-mockgen/cmd/go-mockgen@v1.3.3

+ 0 - 67
internal/db/mock_gen.go

@@ -1,67 +0,0 @@
-// 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"
-)
-
-//go:generate go-mockgen -f gogs.io/gogs/internal/db -i AccessTokensStore -i LFSStore -i LoginSourcesStore -i LoginSourceFilesStore -i loginSourceFileStore -i PermsStore -i ReposStore -i TwoFactorsStore -i UsersStore -o mocks.go
-
-func SetMockAccessTokensStore(t *testing.T, mock AccessTokensStore) {
-	before := AccessTokens
-	AccessTokens = mock
-	t.Cleanup(func() {
-		AccessTokens = before
-	})
-}
-
-func SetMockLFSStore(t *testing.T, mock LFSStore) {
-	before := LFS
-	LFS = mock
-	t.Cleanup(func() {
-		LFS = before
-	})
-}
-
-func setMockLoginSourceFilesStore(t *testing.T, db *loginSources, mock loginSourceFilesStore) {
-	before := db.files
-	db.files = mock
-	t.Cleanup(func() {
-		db.files = before
-	})
-}
-
-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
-	t.Cleanup(func() {
-		Repos = before
-	})
-}
-
-func SetMockTwoFactorsStore(t *testing.T, mock TwoFactorsStore) {
-	before := TwoFactors
-	TwoFactors = mock
-	t.Cleanup(func() {
-		TwoFactors = before
-	})
-}
-
-func SetMockUsersStore(t *testing.T, mock UsersStore) {
-	before := Users
-	Users = mock
-	t.Cleanup(func() {
-		Users = before
-	})
-}

File diff suppressed because it is too large
+ 44 - 4458
internal/db/mocks.go


File diff suppressed because it is too large
+ 2411 - 0
internal/db/mocks_test.go


+ 62 - 2
internal/db/users_test.go

@@ -6,6 +6,7 @@ package db
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -51,8 +52,6 @@ func TestUsers(t *testing.T) {
 	}
 }
 
-// TODO: Only local account is tested, tests for external account will be added
-//  along with addressing https://github.com/gogs/gogs/issues/6115.
 func usersAuthenticate(t *testing.T, db *users) {
 	ctx := context.Background()
 
@@ -87,6 +86,67 @@ func usersAuthenticate(t *testing.T, db *users) {
 		require.NoError(t, err)
 		assert.Equal(t, alice.Name, user.Name)
 	})
+
+	t.Run("login source mismatch", func(t *testing.T) {
+		_, err := db.Authenticate(ctx, alice.Email, password, 1)
+		gotErr := fmt.Sprintf("%v", err)
+		wantErr := ErrLoginSourceMismatch{args: map[string]interface{}{"actual": 0, "expect": 1}}.Error()
+		assert.Equal(t, wantErr, gotErr)
+	})
+
+	t.Run("via login source", func(t *testing.T) {
+		mockLoginSources := NewMockLoginSourcesStore()
+		mockLoginSources.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int64) (*LoginSource, error) {
+			mockProvider := NewMockProvider()
+			mockProvider.AuthenticateFunc.SetDefaultReturn(&auth.ExternalAccount{}, nil)
+			s := &LoginSource{
+				IsActived: true,
+				Provider:  mockProvider,
+			}
+			return s, nil
+		})
+		setMockLoginSourcesStore(t, mockLoginSources)
+
+		bob, err := db.Create(ctx, "bob", "bob@example.com",
+			CreateUserOpts{
+				Password:    password,
+				LoginSource: 1,
+			},
+		)
+		require.NoError(t, err)
+
+		user, err := db.Authenticate(ctx, bob.Email, password, 1)
+		require.NoError(t, err)
+		assert.Equal(t, bob.Name, user.Name)
+	})
+
+	t.Run("new user via login source", func(t *testing.T) {
+		mockLoginSources := NewMockLoginSourcesStore()
+		mockLoginSources.GetByIDFunc.SetDefaultHook(func(ctx context.Context, id int64) (*LoginSource, error) {
+			mockProvider := NewMockProvider()
+			mockProvider.AuthenticateFunc.SetDefaultReturn(
+				&auth.ExternalAccount{
+					Name:  "cindy",
+					Email: "cindy@example.com",
+				},
+				nil,
+			)
+			s := &LoginSource{
+				IsActived: true,
+				Provider:  mockProvider,
+			}
+			return s, nil
+		})
+		setMockLoginSourcesStore(t, mockLoginSources)
+
+		user, err := db.Authenticate(ctx, "cindy", password, 1)
+		require.NoError(t, err)
+		assert.Equal(t, "cindy", user.Name)
+
+		user, err = db.GetByUsername(ctx, "cindy")
+		require.NoError(t, err)
+		assert.Equal(t, "cindy@example.com", user.Email)
+	})
 }
 
 func usersCreate(t *testing.T, db *users) {

+ 8 - 8
internal/route/lfs/basic_test.go

@@ -69,7 +69,7 @@ func Test_basicHandler_serveDownload(t *testing.T) {
 		{
 			name: "object does not exist",
 			mockLFSStore: func() db.LFSStore {
-				mock := db.NewMockLFSStore()
+				mock := NewMockLFSStore()
 				mock.GetObjectByOIDFunc.SetDefaultReturn(nil, db.ErrLFSObjectNotExist{})
 				return mock
 			},
@@ -82,7 +82,7 @@ func Test_basicHandler_serveDownload(t *testing.T) {
 		{
 			name: "storage not found",
 			mockLFSStore: func() db.LFSStore {
-				mock := db.NewMockLFSStore()
+				mock := NewMockLFSStore()
 				mock.GetObjectByOIDFunc.SetDefaultReturn(&db.LFSObject{Storage: "bad_storage"}, nil)
 				return mock
 			},
@@ -97,7 +97,7 @@ func Test_basicHandler_serveDownload(t *testing.T) {
 			name:    "object exists",
 			content: "Hello world!",
 			mockLFSStore: func() db.LFSStore {
-				mock := db.NewMockLFSStore()
+				mock := NewMockLFSStore()
 				mock.GetObjectByOIDFunc.SetDefaultReturn(
 					&db.LFSObject{
 						Size:    12,
@@ -168,7 +168,7 @@ func Test_basicHandler_serveUpload(t *testing.T) {
 		{
 			name: "object already exists",
 			mockLFSStore: func() db.LFSStore {
-				mock := db.NewMockLFSStore()
+				mock := NewMockLFSStore()
 				mock.GetObjectByOIDFunc.SetDefaultReturn(&db.LFSObject{}, nil)
 				return mock
 			},
@@ -177,7 +177,7 @@ func Test_basicHandler_serveUpload(t *testing.T) {
 		{
 			name: "new object",
 			mockLFSStore: func() db.LFSStore {
-				mock := db.NewMockLFSStore()
+				mock := NewMockLFSStore()
 				mock.GetObjectByOIDFunc.SetDefaultReturn(nil, db.ErrLFSObjectNotExist{})
 				return mock
 			},
@@ -233,7 +233,7 @@ func Test_basicHandler_serveVerify(t *testing.T) {
 			name: "object does not exist",
 			body: `{"oid":"ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f"}`,
 			mockLFSStore: func() db.LFSStore {
-				mock := db.NewMockLFSStore()
+				mock := NewMockLFSStore()
 				mock.GetObjectByOIDFunc.SetDefaultReturn(nil, db.ErrLFSObjectNotExist{})
 				return mock
 			},
@@ -244,7 +244,7 @@ func Test_basicHandler_serveVerify(t *testing.T) {
 			name: "object size mismatch",
 			body: `{"oid":"ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f"}`,
 			mockLFSStore: func() db.LFSStore {
-				mock := db.NewMockLFSStore()
+				mock := NewMockLFSStore()
 				mock.GetObjectByOIDFunc.SetDefaultReturn(&db.LFSObject{Size: 12}, nil)
 				return mock
 			},
@@ -256,7 +256,7 @@ func Test_basicHandler_serveVerify(t *testing.T) {
 			name: "object exists",
 			body: `{"oid":"ef797c8118f02dfb649607dd5d3f8c7623048c9c063d532cc95c5ed7a898a64f", "size":12}`,
 			mockLFSStore: func() db.LFSStore {
-				mock := db.NewMockLFSStore()
+				mock := NewMockLFSStore()
 				mock.GetObjectByOIDFunc.SetDefaultReturn(&db.LFSObject{Size: 12}, nil)
 				return mock
 			},

+ 1 - 1
internal/route/lfs/batch_test.go

@@ -83,7 +83,7 @@ func Test_serveBatch(t *testing.T) {
 	{"oid": "5cac0a318669fadfee734fb340a5f5b70b428ac57a9f4b109cb6e150b2ba7e57", "size": 456}
 ]}`,
 			mockLFSStore: func() db.LFSStore {
-				mock := db.NewMockLFSStore()
+				mock := NewMockLFSStore()
 				mock.GetObjectsByOIDsFunc.SetDefaultReturn(
 					[]*db.LFSObject{
 						{

File diff suppressed because it is too large
+ 2702 - 0
internal/route/lfs/mocks_test.go


+ 17 - 17
internal/route/lfs/route_test.go

@@ -52,12 +52,12 @@ func Test_authenticate(t *testing.T) {
 				"Authorization": []string{"Basic dXNlcm5hbWU6cGFzc3dvcmQ="},
 			},
 			mockUsersStore: func() db.UsersStore {
-				mock := db.NewMockUsersStore()
+				mock := NewMockUsersStore()
 				mock.AuthenticateFunc.SetDefaultReturn(&db.User{}, nil)
 				return mock
 			},
 			mockTwoFactorsStore: func() db.TwoFactorsStore {
-				mock := db.NewMockTwoFactorsStore()
+				mock := NewMockTwoFactorsStore()
 				mock.IsUserEnabledFunc.SetDefaultReturn(true)
 				return mock
 			},
@@ -71,12 +71,12 @@ func Test_authenticate(t *testing.T) {
 				"Authorization": []string{"Basic dXNlcm5hbWU="},
 			},
 			mockUsersStore: func() db.UsersStore {
-				mock := db.NewMockUsersStore()
+				mock := NewMockUsersStore()
 				mock.AuthenticateFunc.SetDefaultReturn(nil, auth.ErrBadCredentials{})
 				return mock
 			},
 			mockAccessTokensStore: func() db.AccessTokensStore {
-				mock := db.NewMockAccessTokensStore()
+				mock := NewMockAccessTokensStore()
 				mock.GetBySHA1Func.SetDefaultReturn(nil, db.ErrAccessTokenNotExist{})
 				return mock
 			},
@@ -94,12 +94,12 @@ func Test_authenticate(t *testing.T) {
 				"Authorization": []string{"Basic dXNlcm5hbWU6cGFzc3dvcmQ="},
 			},
 			mockUsersStore: func() db.UsersStore {
-				mock := db.NewMockUsersStore()
+				mock := NewMockUsersStore()
 				mock.AuthenticateFunc.SetDefaultReturn(&db.User{ID: 1, Name: "unknwon"}, nil)
 				return mock
 			},
 			mockTwoFactorsStore: func() db.TwoFactorsStore {
-				mock := db.NewMockTwoFactorsStore()
+				mock := NewMockTwoFactorsStore()
 				mock.IsUserEnabledFunc.SetDefaultReturn(false)
 				return mock
 			},
@@ -113,13 +113,13 @@ func Test_authenticate(t *testing.T) {
 				"Authorization": []string{"Basic dXNlcm5hbWU="},
 			},
 			mockUsersStore: func() db.UsersStore {
-				mock := db.NewMockUsersStore()
+				mock := NewMockUsersStore()
 				mock.AuthenticateFunc.SetDefaultReturn(nil, auth.ErrBadCredentials{})
 				mock.GetByIDFunc.SetDefaultReturn(&db.User{ID: 1, Name: "unknwon"}, nil)
 				return mock
 			},
 			mockAccessTokensStore: func() db.AccessTokensStore {
-				mock := db.NewMockAccessTokensStore()
+				mock := NewMockAccessTokensStore()
 				mock.GetBySHA1Func.SetDefaultReturn(&db.AccessToken{}, nil)
 				return mock
 			},
@@ -176,7 +176,7 @@ func Test_authorize(t *testing.T) {
 			name:      "user does not exist",
 			authroize: authorize(db.AccessModeNone),
 			mockUsersStore: func() db.UsersStore {
-				mock := db.NewMockUsersStore()
+				mock := NewMockUsersStore()
 				mock.GetByUsernameFunc.SetDefaultReturn(nil, db.ErrUserNotExist{})
 				return mock
 			},
@@ -186,14 +186,14 @@ func Test_authorize(t *testing.T) {
 			name:      "repository does not exist",
 			authroize: authorize(db.AccessModeNone),
 			mockUsersStore: func() db.UsersStore {
-				mock := db.NewMockUsersStore()
+				mock := NewMockUsersStore()
 				mock.GetByUsernameFunc.SetDefaultHook(func(ctx context.Context, username string) (*db.User, error) {
 					return &db.User{Name: username}, nil
 				})
 				return mock
 			},
 			mockReposStore: func() db.ReposStore {
-				mock := db.NewMockReposStore()
+				mock := NewMockReposStore()
 				mock.GetByNameFunc.SetDefaultReturn(nil, db.ErrRepoNotExist{})
 				return mock
 			},
@@ -203,21 +203,21 @@ func Test_authorize(t *testing.T) {
 			name:      "actor is not authorized",
 			authroize: authorize(db.AccessModeWrite),
 			mockUsersStore: func() db.UsersStore {
-				mock := db.NewMockUsersStore()
+				mock := NewMockUsersStore()
 				mock.GetByUsernameFunc.SetDefaultHook(func(ctx context.Context, username string) (*db.User, error) {
 					return &db.User{Name: username}, nil
 				})
 				return mock
 			},
 			mockReposStore: func() db.ReposStore {
-				mock := db.NewMockReposStore()
+				mock := NewMockReposStore()
 				mock.GetByNameFunc.SetDefaultHook(func(ctx context.Context, ownerID int64, name string) (*db.Repository, error) {
 					return &db.Repository{Name: name}, nil
 				})
 				return mock
 			},
 			mockPermsStore: func() db.PermsStore {
-				mock := db.NewMockPermsStore()
+				mock := NewMockPermsStore()
 				mock.AuthorizeFunc.SetDefaultHook(func(ctx context.Context, userID int64, repoID int64, desired db.AccessMode, opts db.AccessModeOptions) bool {
 					return desired <= db.AccessModeRead
 				})
@@ -230,21 +230,21 @@ func Test_authorize(t *testing.T) {
 			name:      "actor is authorized",
 			authroize: authorize(db.AccessModeRead),
 			mockUsersStore: func() db.UsersStore {
-				mock := db.NewMockUsersStore()
+				mock := NewMockUsersStore()
 				mock.GetByUsernameFunc.SetDefaultHook(func(ctx context.Context, username string) (*db.User, error) {
 					return &db.User{Name: username}, nil
 				})
 				return mock
 			},
 			mockReposStore: func() db.ReposStore {
-				mock := db.NewMockReposStore()
+				mock := NewMockReposStore()
 				mock.GetByNameFunc.SetDefaultHook(func(ctx context.Context, ownerID int64, name string) (*db.Repository, error) {
 					return &db.Repository{Name: name}, nil
 				})
 				return mock
 			},
 			mockPermsStore: func() db.PermsStore {
-				mock := db.NewMockPermsStore()
+				mock := NewMockPermsStore()
 				mock.AuthorizeFunc.SetDefaultHook(func(ctx context.Context, userID int64, repoID int64, desired db.AccessMode, opts db.AccessModeOptions) bool {
 					return desired <= db.AccessModeRead
 				})

+ 44 - 0
mockgen.yaml

@@ -0,0 +1,44 @@
+force: true
+goimports: goimports
+file-prefix: |
+  This file was generated by running `go-mockgen` at the root of this repository.
+  To add additional mocks to this or another package, add a new entry to the
+  mockgen.yaml file in the root of this repository.
+mocks:
+  # To generate a new mock struct from an interface definition, add a new value to this
+  # list. Each item will need to supply two pieces of information:
+  #
+  #  (1) First, you will need to give a target filename
+  #  (2) Second, you will need to supply a target import path and interface name. If the
+  #      set of interface definitions you are mocking are all from the same package, then
+  #      you can supply a `path` and `interfaces` key which take a string and string array,
+  #      respectively. If the set of interface definitions you are mocking come from multiple
+  #      import paths, you can supply a `sources` array, each item containing a `path` and
+  #      `interfaces` key.
+  #
+  # By convention, you should generate mocks next to the CONSUMER of an interface, not the
+  # definition. It is NOT considered an anti-pattern to generate multiple mocks for the same
+  # shared interface.
+  #
+  # By convention, the filename containing generated mocks should be `mocks_test.go`.
+  - filename: internal/db/mocks_test.go
+    sources:
+      - path: gogs.io/gogs/internal/db
+        interfaces:
+          - LoginSourcesStore
+          - LoginSourceFilesStore
+          - LoginSourceFileStore
+          - loginSourceFileStore
+      - path: gogs.io/gogs/internal/auth
+        interfaces:
+          - Provider
+  - filename: internal/route/lfs/mocks_test.go
+    sources:
+      - path: gogs.io/gogs/internal/db
+        interfaces:
+          - LFSStore
+          - UsersStore
+          - TwoFactorsStore
+          - AccessTokensStore
+          - ReposStore
+          - PermsStore