diff --git a/src/server/middleware/csrf/csrf.go b/src/server/middleware/csrf/csrf.go index 459b996031c4..c715df07eca9 100644 --- a/src/server/middleware/csrf/csrf.go +++ b/src/server/middleware/csrf/csrf.go @@ -67,9 +67,16 @@ func attach(handler http.Handler) http.Handler { func Middleware() func(handler http.Handler) http.Handler { once.Do(func() { key := os.Getenv(csrfKeyEnv) - if len(key) != 32 { - log.Warningf("Invalid CSRF key from environment: %s, generating random key...", key) + if len(key) == 0 { key = utils.GenerateRandomString() + } else if len(key) != 32 { + log.Errorf("Invalid CSRF key length from the environment: %s. Please ensure the key length is 32 characters.", key) + protect = func(handler http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lib_http.SendError(w, errors.New("Invalid CSRF key length from the environment. Please ensure the key length is 32 characters.")) + }) + } + return } secureFlag = secureCookie() protect = csrf.Protect([]byte(key), csrf.RequestHeader(tokenHeader), diff --git a/src/server/middleware/csrf/csrf_test.go b/src/server/middleware/csrf/csrf_test.go index a4975ae58be0..054baf91925a 100644 --- a/src/server/middleware/csrf/csrf_test.go +++ b/src/server/middleware/csrf/csrf_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/utils" "github.com/goharbor/harbor/src/common/utils/test" "github.com/goharbor/harbor/src/lib/config" _ "github.com/goharbor/harbor/src/pkg/config/inmemory" @@ -32,34 +33,50 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { } func TestMiddleware(t *testing.T) { - srv := Middleware()(&handler{}) cases := []struct { req *http.Request statusCode int returnToken bool + validKey bool }{ { req: httptest.NewRequest(http.MethodGet, "/", nil), statusCode: http.StatusOK, returnToken: true, + validKey: true, }, { req: httptest.NewRequest(http.MethodDelete, "/", nil), statusCode: http.StatusForbidden, returnToken: true, + validKey: true, }, { req: httptest.NewRequest(http.MethodGet, "/api/2.0/projects", nil), // should be skipped statusCode: http.StatusOK, returnToken: false, + validKey: true, }, { req: httptest.NewRequest(http.MethodDelete, "/v2/library/hello-world/manifests/latest", nil), // should be skipped statusCode: http.StatusOK, returnToken: false, + validKey: true, + }, + { + req: httptest.NewRequest(http.MethodGet, "/", nil), + statusCode: http.StatusInternalServerError, + returnToken: false, + validKey: false, }, } for _, c := range cases { + if c.validKey { + os.Setenv(csrfKeyEnv, utils.GenerateRandomStringWithLen(32)) + } else { + os.Setenv(csrfKeyEnv, utils.GenerateRandomStringWithLen(10)) + } + srv := Middleware()(&handler{}) rec := httptest.NewRecorder() srv.ServeHTTP(rec, c.req) assert.Equal(t, c.statusCode, rec.Result().StatusCode) @@ -67,15 +84,6 @@ func TestMiddleware(t *testing.T) { } } -func hasCookie(resp *http.Response, name string) bool { - for _, c := range resp.Cookies() { - if c != nil && c.Name == name { - return true - } - } - return false -} - func TestSecureCookie(t *testing.T) { assert.True(t, secureCookie()) conf := map[string]interface{}{