diff options
| -rw-r--r-- | .golangci.yml | 1 | ||||
| -rw-r--r-- | app.go | 26 | ||||
| -rw-r--r-- | internal/auth/auth.go | 4 | ||||
| -rw-r--r-- | internal/vfs/zip/vfs.go | 4 | ||||
| -rw-r--r-- | test/acceptance/auth_test.go | 26 | ||||
| -rw-r--r-- | test/acceptance/helpers_test.go | 12 | ||||
| -rw-r--r-- | test/acceptance/metrics_test.go | 3 | ||||
| -rw-r--r-- | test/acceptance/zip_test.go | 12 |
8 files changed, 40 insertions, 48 deletions
diff --git a/.golangci.yml b/.golangci.yml index abb21b68..6c239d92 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -45,6 +45,7 @@ linters: - structcheck - typecheck - unconvert + - unparam - unused - varcheck - whitespace @@ -79,14 +79,6 @@ func (a *theApp) ServeTLS(ch *cryptotls.ClientHelloInfo) (*cryptotls.Certificate return nil, nil } -func (a *theApp) healthCheck(w http.ResponseWriter, r *http.Request, https bool) { - if a.isReady() { - w.Write([]byte("success\n")) - } else { - http.Error(w, "not yet ready", http.StatusServiceUnavailable) - } -} - func (a *theApp) redirectToHTTPS(w http.ResponseWriter, r *http.Request, statusCode int) { u := *r.URL u.Scheme = request.SchemeHTTPS @@ -103,15 +95,14 @@ func (a *theApp) domain(ctx context.Context, host string) (*domain.Domain, error // checkAuthAndServeNotFound performs the auth process if domain can't be found // the main purpose of this process is to avoid leaking the project existence/not-existence // by behaving the same if user has no access to the project or if project simply does not exists -func (a *theApp) checkAuthAndServeNotFound(domain *domain.Domain, w http.ResponseWriter, r *http.Request) bool { +func (a *theApp) checkAuthAndServeNotFound(domain *domain.Domain, w http.ResponseWriter, r *http.Request) { // To avoid user knowing if pages exist, we will force user to login and authorize pages if a.Auth.CheckAuthenticationWithoutProject(w, r, domain) { - return true + return } // auth succeeded try to serve the correct 404 page domain.ServeNotFoundAuthFailed(w, r) - return true } func (a *theApp) tryAuxiliaryHandlers(w http.ResponseWriter, r *http.Request, https bool, host string, domain *domain.Domain) bool { @@ -138,9 +129,8 @@ func (a *theApp) tryAuxiliaryHandlers(w http.ResponseWriter, r *http.Request, ht } // redirect to auth and serve not found - if a.checkAuthAndServeNotFound(domain, w, r) { - return true - } + a.checkAuthAndServeNotFound(domain, w, r) + return true } if !https && domain.IsHTTPSOnly(r) { @@ -153,8 +143,12 @@ func (a *theApp) tryAuxiliaryHandlers(w http.ResponseWriter, r *http.Request, ht // healthCheckMiddleware is serving the application status check func (a *theApp) healthCheckMiddleware(handler http.Handler) (http.Handler, error) { - healthCheck := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - a.healthCheck(w, r, request.IsHTTPS(r)) + healthCheck := http.HandlerFunc(func(w http.ResponseWriter, _r *http.Request) { + if a.isReady() { + w.Write([]byte("success\n")) + } else { + http.Error(w, "not yet ready", http.StatusServiceUnavailable) + } }) loggedHealthCheck, err := logging.BasicAccessLogger(healthCheck, a.config.Log.Format, nil) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 07af99fe..05f58187 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -283,7 +283,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit // If auth request callback should be proxied to custom domain // redirect to originating domain set in the cookie as proxy_auth_domain - if shouldProxyCallbackToCustomDomain(r, session) { + if shouldProxyCallbackToCustomDomain(session) { // Get domain started auth process proxyDomain := session.Values["proxy_auth_domain"].(string) @@ -348,7 +348,7 @@ func shouldProxyAuthToGitlab(r *http.Request) bool { return r.URL.Query().Get("domain") != "" && r.URL.Query().Get("state") != "" } -func shouldProxyCallbackToCustomDomain(r *http.Request, session *sessions.Session) bool { +func shouldProxyCallbackToCustomDomain(session *sessions.Session) bool { return session.Values["proxy_auth_domain"] != nil } diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go index a9825190..18078915 100644 --- a/internal/vfs/zip/vfs.go +++ b/internal/vfs/zip/vfs.go @@ -200,7 +200,7 @@ func (zfs *zipVFS) Name() string { // otherwise creates the archive entry in a cache and try to save it, // if saving fails it's because the archive has already been cached // (e.g. by another concurrent request) -func (zfs *zipVFS) findOrCreateArchive(ctx context.Context, key string) (*zipArchive, error) { +func (zfs *zipVFS) findOrCreateArchive(key string) (*zipArchive, error) { // This needs to happen in lock to ensure that // concurrent access will not remove it // it is needed due to the bug https://github.com/patrickmn/go-cache/issues/48 @@ -259,7 +259,7 @@ func (zfs *zipVFS) findOrCreateArchive(ctx context.Context, key string) (*zipArc // findOrOpenArchive gets archive from cache and tries to open it func (zfs *zipVFS) findOrOpenArchive(ctx context.Context, key, path string) (*zipArchive, error) { - zipArchive, err := zfs.findOrCreateArchive(ctx, key) + zipArchive, err := zfs.findOrCreateArchive(key) if err != nil { return nil, err } diff --git a/test/acceptance/auth_test.go b/test/acceptance/auth_test.go index 49d2806d..ad5c343a 100644 --- a/test/acceptance/auth_test.go +++ b/test/acceptance/auth_test.go @@ -27,7 +27,7 @@ func TestWhenAuthIsEnabledPrivateWillRedirectToAuthorize(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{httpsListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -63,7 +63,7 @@ func TestWhenAuthDeniedWillCauseUnauthorized(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{httpsListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -78,7 +78,7 @@ func TestWhenLoginCallbackWithWrongStateShouldFail(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{httpsListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -100,7 +100,7 @@ func TestWhenLoginCallbackWithUnencryptedCode(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{httpsListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -133,7 +133,7 @@ func TestAccessControlUnderCustomDomain(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{httpListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -214,7 +214,7 @@ func TestCustomErrorPageWithAuth(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{httpListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -323,7 +323,7 @@ func TestAccessControlUnderCustomDomainWithHTTPSProxy(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{proxyListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -390,7 +390,7 @@ func TestAccessControlGroupDomain404RedirectsAuth(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{httpListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -409,7 +409,7 @@ func TestAccessControlProject404DoesNotRedirect(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{httpListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -568,7 +568,7 @@ func TestHijackedCode(t *testing.T) { RunPagesProcess(t, withListeners([]ListenSpec{proxyListener}), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), ) @@ -643,13 +643,13 @@ func getValidCookieAndState(t *testing.T, domain string) (string, string) { return cookie, state } -func defaultAuthConfigWith(t *testing.T, configs ...string) string { +func defaultAuthConfig(t *testing.T) string { t.Helper() - configs = append(configs, + configs := []string{ "gitlab-server=https://public-gitlab-auth.com", "auth-redirect-uri=https://projects.gitlab-example.com/auth", - ) + } configFile := defaultConfigFileWith(t, configs...) diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go index 8433646a..ce2d47d3 100644 --- a/test/acceptance/helpers_test.go +++ b/test/acceptance/helpers_test.go @@ -245,7 +245,7 @@ func RunPagesProcessWithSSLCertFile(t *testing.T, listeners []ListenSpec, sslCer RunPagesProcess(t, withListeners(listeners), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), withEnv([]string{"SSL_CERT_FILE=" + sslCertFile}), ) @@ -263,7 +263,7 @@ func RunPagesProcessWithSSLCertDir(t *testing.T, listeners []ListenSpec, sslCert RunPagesProcess(t, withListeners(listeners), withArguments([]string{ - "-config=" + defaultAuthConfigWith(t), + "-config=" + defaultAuthConfig(t), }), withEnv([]string{"SSL_CERT_DIR=" + sslCertDir}), ) @@ -464,14 +464,14 @@ func NewGitlabDomainsSourceStub(t *testing.T, opts *stubOpts) *httptest.Server { router.HandleFunc("/api/v4/internal/pages", pagesHandler) - authHandler := defaultAuthHandler(t, opts) + authHandler := defaultAuthHandler(t) if opts.authHandler != nil { authHandler = opts.authHandler } router.HandleFunc("/oauth/token", authHandler) - userHandler := defaultUserHandler(t, opts) + userHandler := defaultUserHandler(t) if opts.userHandler != nil { userHandler = opts.userHandler } @@ -549,7 +549,7 @@ func defaultAPIHandler(t *testing.T, opts *stubOpts) http.HandlerFunc { } } -func defaultAuthHandler(t *testing.T, opts *stubOpts) http.HandlerFunc { +func defaultAuthHandler(t *testing.T) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { require.Equal(t, "POST", r.Method) err := json.NewEncoder(w).Encode(struct { @@ -561,7 +561,7 @@ func defaultAuthHandler(t *testing.T, opts *stubOpts) http.HandlerFunc { } } -func defaultUserHandler(t *testing.T, opts *stubOpts) http.HandlerFunc { +func defaultUserHandler(t *testing.T) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { require.Equal(t, "Bearer abc", r.Header.Get("Authorization")) w.WriteHeader(http.StatusOK) diff --git a/test/acceptance/metrics_test.go b/test/acceptance/metrics_test.go index cb4262fb..32a36638 100644 --- a/test/acceptance/metrics_test.go +++ b/test/acceptance/metrics_test.go @@ -9,8 +9,7 @@ import ( ) func TestPrometheusMetricsCanBeScraped(t *testing.T) { - _, cleanup := newZipFileServerURL(t, "../../shared/pages/group/zip.gitlab.io/public.zip") - defer cleanup() + runObjectStorage(t, "../../shared/pages/group/zip.gitlab.io/public.zip") RunPagesProcess(t, withExtraArgument("max-conns", "10"), diff --git a/test/acceptance/zip_test.go b/test/acceptance/zip_test.go index f7623340..fcaecf25 100644 --- a/test/acceptance/zip_test.go +++ b/test/acceptance/zip_test.go @@ -12,8 +12,7 @@ import ( ) func TestZipServing(t *testing.T) { - _, cleanup := newZipFileServerURL(t, "../../shared/pages/group/zip.gitlab.io/public.zip") - defer cleanup() + runObjectStorage(t, "../../shared/pages/group/zip.gitlab.io/public.zip") RunPagesProcess(t, withListeners([]ListenSpec{httpListener}), @@ -98,8 +97,7 @@ func TestZipServing(t *testing.T) { } func TestZipServingCache(t *testing.T) { - _, cleanup := newZipFileServerURL(t, "../../shared/pages/group/zip.gitlab.io/public.zip") - t.Cleanup(cleanup) + runObjectStorage(t, "../../shared/pages/group/zip.gitlab.io/public.zip") RunPagesProcess(t, withListeners([]ListenSpec{httpListener}), @@ -246,7 +244,7 @@ func TestZipServingConfigShortTimeout(t *testing.T) { require.Equal(t, http.StatusInternalServerError, response.StatusCode, "should fail to serve") } -func newZipFileServerURL(t *testing.T, zipFilePath string) (string, func()) { +func runObjectStorage(t *testing.T, zipFilePath string) { t.Helper() m := http.NewServeMux() @@ -271,8 +269,8 @@ func newZipFileServerURL(t *testing.T, zipFilePath string) (string, func()) { // Start the server. testServer.Start() - return testServer.URL, func() { + t.Cleanup(func() { // Cleanup. testServer.Close() - } + }) } |