update csrf key generation

Fixes #21060

Do not generate a random key if the provided key has an invalid length.

Signed-off-by: wang yan <[email protected]>

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),

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,50 +33,57 @@ 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)
 		assert.Equal(t, c.returnToken, rec.Result().Header.Get(tokenHeader) != "")
 	}
 }
 
-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{}{