diff options
author | wxiaoguang <wxiaoguang@gmail.com> | 2023-06-18 15:59:09 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-06-18 09:59:09 +0200 |
commit | 4e2f1ee58d1aa49f85c3a28a4f96e915f12bdb21 (patch) | |
tree | e2751e1c4673f6a051ef04ca156f428a819fdb66 /services | |
parent | fc2115b494e9ba7e4cf7a1440404dce53738b514 (diff) | |
download | gitea-4e2f1ee58d1aa49f85c3a28a4f96e915f12bdb21.tar.gz gitea-4e2f1ee58d1aa49f85c3a28a4f96e915f12bdb21.zip |
Refactor web package and context package (#25298)
1. The "web" package shouldn't depends on "modules/context" package,
instead, let each "web context" register themselves to the "web"
package.
2. The old Init/Free doesn't make sense, so simplify it
* The ctx in "Init(ctx)" is never used, and shouldn't be used that way
* The "Free" is never called and shouldn't be called because the SSPI
instance is shared
---------
Co-authored-by: Giteabot <teabot@gitea.io>
Diffstat (limited to 'services')
-rw-r--r-- | services/auth/group.go | 34 | ||||
-rw-r--r-- | services/auth/interface.go | 15 | ||||
-rw-r--r-- | services/auth/sspi_windows.go | 36 | ||||
-rw-r--r-- | services/repository/archiver/archiver_test.go | 2 | ||||
-rw-r--r-- | services/repository/files/content_test.go | 14 | ||||
-rw-r--r-- | services/repository/files/diff_test.go | 4 | ||||
-rw-r--r-- | services/repository/files/file_test.go | 2 | ||||
-rw-r--r-- | services/repository/files/tree_test.go | 2 |
8 files changed, 27 insertions, 82 deletions
diff --git a/services/auth/group.go b/services/auth/group.go index 0a0330b3aa..a1ff65f203 100644 --- a/services/auth/group.go +++ b/services/auth/group.go @@ -4,7 +4,6 @@ package auth import ( - "context" "net/http" "reflect" "strings" @@ -14,9 +13,7 @@ import ( // Ensure the struct implements the interface. var ( - _ Method = &Group{} - _ Initializable = &Group{} - _ Freeable = &Group{} + _ Method = &Group{} ) // Group implements the Auth interface with serval Auth. @@ -49,35 +46,6 @@ func (b *Group) Name() string { return strings.Join(names, ",") } -// Init does nothing as the Basic implementation does not need to allocate any resources -func (b *Group) Init(ctx context.Context) error { - for _, method := range b.methods { - initializable, ok := method.(Initializable) - if !ok { - continue - } - - if err := initializable.Init(ctx); err != nil { - return err - } - } - return nil -} - -// Free does nothing as the Basic implementation does not have to release any resources -func (b *Group) Free() error { - for _, method := range b.methods { - freeable, ok := method.(Freeable) - if !ok { - continue - } - if err := freeable.Free(); err != nil { - return err - } - } - return nil -} - // Verify extracts and validates func (b *Group) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) { // Try to sign in with each of the enabled plugins diff --git a/services/auth/interface.go b/services/auth/interface.go index c4a8a20d01..508291fa43 100644 --- a/services/auth/interface.go +++ b/services/auth/interface.go @@ -29,26 +29,11 @@ type Method interface { Verify(http *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) } -// Initializable represents a structure that requires initialization -// It usually should only be called once before anything else is called -type Initializable interface { - // Init should be called exactly once before using any of the other methods, - // in order to allow the plugin to allocate necessary resources - Init(ctx context.Context) error -} - // Named represents a named thing type Named interface { Name() string } -// Freeable represents a structure that is required to be freed -type Freeable interface { - // Free should be called exactly once before application closes, in order to - // give chance to the plugin to free any allocated resources - Free() error -} - // PasswordAuthenticator represents a source of authentication type PasswordAuthenticator interface { Authenticate(user *user_model.User, login, password string) (*user_model.User, error) diff --git a/services/auth/sspi_windows.go b/services/auth/sspi_windows.go index d49497e19c..c162810797 100644 --- a/services/auth/sspi_windows.go +++ b/services/auth/sspi_windows.go @@ -4,10 +4,10 @@ package auth import ( - "context" "errors" "net/http" "strings" + "sync" "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/avatars" @@ -32,13 +32,12 @@ var ( // sspiAuth is a global instance of the websspi authentication package, // which is used to avoid acquiring the server credential handle on // every request - sspiAuth *websspi.Authenticator + sspiAuth *websspi.Authenticator + sspiAuthOnce sync.Once // Ensure the struct implements the interface. - _ Method = &SSPI{} - _ Named = &SSPI{} - _ Initializable = &SSPI{} - _ Freeable = &SSPI{} + _ Method = &SSPI{} + _ Named = &SSPI{} ) // SSPI implements the SingleSignOn interface and authenticates requests @@ -47,32 +46,25 @@ var ( // Returns nil if authentication fails. type SSPI struct{} -// Init creates a new global websspi.Authenticator object -func (s *SSPI) Init(ctx context.Context) error { - config := websspi.NewConfig() - var err error - sspiAuth, err = websspi.New(config) - if err != nil { - return err - } - return nil -} - // Name represents the name of auth method func (s *SSPI) Name() string { return "sspi" } -// Free releases resources used by the global websspi.Authenticator object -func (s *SSPI) Free() error { - return sspiAuth.Free() -} - // Verify uses SSPI (Windows implementation of SPNEGO) to authenticate the request. // If authentication is successful, returns the corresponding user object. // If negotiation should continue or authentication fails, immediately returns a 401 HTTP // response code, as required by the SPNEGO protocol. func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) { + var errInit error + sspiAuthOnce.Do(func() { + config := websspi.NewConfig() + sspiAuth, errInit = websspi.New(config) + }) + if errInit != nil { + return nil, errInit + } + if !s.shouldAuthenticate(req) { return nil, nil } diff --git a/services/repository/archiver/archiver_test.go b/services/repository/archiver/archiver_test.go index 3cd6e81351..4b6fb7446d 100644 --- a/services/repository/archiver/archiver_test.go +++ b/services/repository/archiver/archiver_test.go @@ -24,7 +24,7 @@ func TestMain(m *testing.M) { func TestArchive_Basic(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - ctx := test.MockContext(t, "user27/repo49") + ctx, _ := test.MockContext(t, "user27/repo49") firstCommit, secondCommit := "51f84af23134", "aacbdfe9e1c4" test.LoadRepo(t, ctx, 49) diff --git a/services/repository/files/content_test.go b/services/repository/files/content_test.go index a43b71cf31..8ff96822c9 100644 --- a/services/repository/files/content_test.go +++ b/services/repository/files/content_test.go @@ -54,7 +54,7 @@ func getExpectedReadmeContentsResponse() *api.ContentsResponse { func TestGetContents(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) @@ -82,7 +82,7 @@ func TestGetContents(t *testing.T) { func TestGetContentsOrListForDir(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) @@ -117,7 +117,7 @@ func TestGetContentsOrListForDir(t *testing.T) { func TestGetContentsOrListForFile(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) @@ -145,7 +145,7 @@ func TestGetContentsOrListForFile(t *testing.T) { func TestGetContentsErrors(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) @@ -176,7 +176,7 @@ func TestGetContentsErrors(t *testing.T) { func TestGetContentsOrListErrors(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) @@ -207,7 +207,7 @@ func TestGetContentsOrListErrors(t *testing.T) { func TestGetContentsOrListOfEmptyRepos(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user30/empty") + ctx, _ := test.MockContext(t, "user30/empty") ctx.SetParams(":id", "52") test.LoadRepo(t, ctx, 52) test.LoadUser(t, ctx, 30) @@ -225,7 +225,7 @@ func TestGetContentsOrListOfEmptyRepos(t *testing.T) { func TestGetBlobBySHA(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) diff --git a/services/repository/files/diff_test.go b/services/repository/files/diff_test.go index 621816e97d..0346e0e9e9 100644 --- a/services/repository/files/diff_test.go +++ b/services/repository/files/diff_test.go @@ -17,7 +17,7 @@ import ( func TestGetDiffPreview(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) @@ -139,7 +139,7 @@ func TestGetDiffPreview(t *testing.T) { func TestGetDiffPreviewErrors(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) diff --git a/services/repository/files/file_test.go b/services/repository/files/file_test.go index e1c7d5d7fb..d14a049438 100644 --- a/services/repository/files/file_test.go +++ b/services/repository/files/file_test.go @@ -98,7 +98,7 @@ func getExpectedFileResponse() *api.FileResponse { func TestGetFileResponseFromCommit(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) diff --git a/services/repository/files/tree_test.go b/services/repository/files/tree_test.go index a500dbdb22..51a2190e8f 100644 --- a/services/repository/files/tree_test.go +++ b/services/repository/files/tree_test.go @@ -15,7 +15,7 @@ import ( func TestGetTreeBySHA(t *testing.T) { unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user2/repo1") + ctx, _ := test.MockContext(t, "user2/repo1") test.LoadRepo(t, ctx, 1) test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) |