[server, ws-proxy] Test cookie filter against real name generator (#1…

…9770) * [server, ws-proxy] Extract CookieNameFromDomain into server/go, so installer (for config generation) and ws-proxy (for tests) can both depend on it * review comment
gitpod-io · May 27, 2024 · f079d8d · f079d8d
1 parent 504189b
commit f079d8d
Show file tree

Hide file tree

Showing 11 changed files with 67 additions and 29 deletions.
diff --git a/components/server/go/BUILD.yaml b/components/server/go/BUILD.yaml
@@ -0,0 +1,12 @@
+packages:
+  - name: lib
+    type: go
+    srcs:
+      - "**/*.go"
+      - "go.mod"
+      - "go.sum"
+    env:
+      - CGO_ENABLED=0
+      - GOOS=linux
+    config:
+      packaging: library
diff --git a/components/server/go/go.mod b/components/server/go/go.mod
@@ -0,0 +1,3 @@
+module github.com/gitpod-io/gitpod/server/go
+
+go 1.22.2
diff --git a/components/server/go/go.sum b/components/server/go/go.sum
diff --git a/components/server/go/pkg/lib/cookie.go b/components/server/go/pkg/lib/cookie.go
@@ -0,0 +1,13 @@
+// Copyright (c) 2024 Gitpod GmbH. All rights reserved.
+// Licensed under the GNU Affero General Public License (AGPL).
+// See License.AGPL.txt in the project root for license information.
+
+package lib
+
+import "regexp"
+
+func CookieNameFromDomain(domain string) string {
+	// replace all non-word characters with underscores
+	derived := regexp.MustCompile(`[\W_]+`).ReplaceAllString(domain, "_")
+	return "_" + derived + "_jwt2_"
+}
diff --git a/components/ws-proxy/BUILD.yaml b/components/ws-proxy/BUILD.yaml
@@ -14,6 +14,7 @@ packages:
       - components/registry-facade-api/go:lib
       - components/supervisor-api/go:lib
       - components/ws-manager-api/go:lib
+      - components/server/go:lib
     env:
       - CGO_ENABLED=0
       - GOOS=linux
@@ -52,6 +53,7 @@ packages:
       - components/registry-facade-api/go:lib
       - components/supervisor-api/go:lib
       - components/ws-manager-api/go:lib
+      - components/server/go:lib
     env:
       - CGO_ENABLED=0
       - GOOS=linux

diff --git a/components/ws-proxy/go.mod b/components/ws-proxy/go.mod
@@ -1,11 +1,12 @@
 module github.com/gitpod-io/gitpod/ws-proxy
 
-go 1.22
+go 1.22.2
 
 require (
 	github.com/bombsimon/logrusr/v2 v2.0.1
 	github.com/gitpod-io/gitpod/common-go v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/gitpod/gitpod-protocol v0.0.0-00010101000000-000000000000
+	github.com/gitpod-io/gitpod/server/go v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/gitpod/supervisor/api v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/gitpod/ws-manager/api v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/golang-crypto v0.0.0-20231122075959-de838e9cb174
@@ -118,6 +119,8 @@ replace github.com/gitpod-io/gitpod/supervisor/api => ../supervisor-api/go // le
 
 replace github.com/gitpod-io/gitpod/ws-manager/api => ../ws-manager-api/go // leeway
 
+replace github.com/gitpod-io/gitpod/server/go => ../server/go // leeway
+
 replace k8s.io/api => k8s.io/api v0.29.3 // leeway indirect from components/common-go:lib
 
 replace k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.29.3 // leeway indirect from components/common-go:lib

diff --git a/components/ws-proxy/pkg/proxy/routes_test.go b/components/ws-proxy/pkg/proxy/routes_test.go
@@ -27,6 +27,7 @@ import (
 
 	"github.com/gitpod-io/gitpod/common-go/log"
 	"github.com/gitpod-io/gitpod/common-go/util"
+	server_lib "github.com/gitpod-io/gitpod/server/go/pkg/lib"
 	"github.com/gitpod-io/gitpod/ws-manager/api"
 	"github.com/gitpod-io/gitpod/ws-proxy/pkg/common"
 	"github.com/gitpod-io/gitpod/ws-proxy/pkg/sshproxy"
@@ -978,27 +979,29 @@ func TestNoSSHGatewayRouter(t *testing.T) {
 
 func TestRemoveSensitiveCookies(t *testing.T) {
 	var (
-		domain            = "test-domain.com"
-		sessionCookie     = &http.Cookie{Domain: domain, Name: "_test_domain_com_", Value: "fobar"}
-		sessionCookieJwt2 = &http.Cookie{Domain: domain, Name: "_test_domain_com_jwt2_", Value: "fobar"}
-		portAuthCookie    = &http.Cookie{Domain: domain, Name: "_test_domain_com_ws_77f6b236_3456_4b88_8284_81ca543a9d65_port_auth_", Value: "some-token"}
-		ownerCookie       = &http.Cookie{Domain: domain, Name: "_test_domain_com_ws_77f6b236_3456_4b88_8284_81ca543a9d65_owner_", Value: "some-other-token"}
-		miscCookie        = &http.Cookie{Domain: domain, Name: "some-other-cookie", Value: "I like cookies"}
-		invalidCookieName = &http.Cookie{Domain: domain, Name: "foobar[0]", Value: "violates RFC6266"}
+		domain                  = "test-domain.com"
+		sessionCookie           = &http.Cookie{Domain: domain, Name: "_test_domain_com_", Value: "fobar"}
+		sessionCookieJwt2       = &http.Cookie{Domain: domain, Name: "_test_domain_com_jwt2_", Value: "fobar"}
+		realGitpodSessionCookie = &http.Cookie{Domain: domain, Name: server_lib.CookieNameFromDomain(domain), Value: "fobar"}
+		portAuthCookie          = &http.Cookie{Domain: domain, Name: "_test_domain_com_ws_77f6b236_3456_4b88_8284_81ca543a9d65_port_auth_", Value: "some-token"}
+		ownerCookie             = &http.Cookie{Domain: domain, Name: "_test_domain_com_ws_77f6b236_3456_4b88_8284_81ca543a9d65_owner_", Value: "some-other-token"}
+		miscCookie              = &http.Cookie{Domain: domain, Name: "some-other-cookie", Value: "I like cookies"}
+		invalidCookieName       = &http.Cookie{Domain: domain, Name: "foobar[0]", Value: "violates RFC6266"}
 	)
 
 	tests := []struct {
 		Name     string
 		Input    []*http.Cookie
 		Expected []*http.Cookie
 	}{
-		{"no cookies", []*http.Cookie{}, []*http.Cookie{}},
-		{"session cookie", []*http.Cookie{sessionCookie, miscCookie}, []*http.Cookie{miscCookie}},
-		{"session cookie ending on _jwt2_", []*http.Cookie{sessionCookieJwt2, miscCookie}, []*http.Cookie{miscCookie}},
-		{"portAuth cookie", []*http.Cookie{portAuthCookie, miscCookie}, []*http.Cookie{miscCookie}},
-		{"owner cookie", []*http.Cookie{ownerCookie, miscCookie}, []*http.Cookie{miscCookie}},
-		{"misc cookie", []*http.Cookie{miscCookie}, []*http.Cookie{miscCookie}},
-		{"invalid cookie name", []*http.Cookie{invalidCookieName}, []*http.Cookie{invalidCookieName}},
+		{Name: "no cookies", Input: []*http.Cookie{}, Expected: []*http.Cookie{}},
+		{Name: "session cookie", Input: []*http.Cookie{sessionCookie, miscCookie}, Expected: []*http.Cookie{miscCookie}},
+		{Name: "session cookie ending on _jwt2_", Input: []*http.Cookie{sessionCookieJwt2, miscCookie}, Expected: []*http.Cookie{miscCookie}},
+		{Name: "real Gitpod session cookie", Input: []*http.Cookie{realGitpodSessionCookie, miscCookie}, Expected: []*http.Cookie{miscCookie}},
+		{Name: "portAuth cookie", Input: []*http.Cookie{portAuthCookie, miscCookie}, Expected: []*http.Cookie{miscCookie}},
+		{Name: "owner cookie", Input: []*http.Cookie{ownerCookie, miscCookie}, Expected: []*http.Cookie{miscCookie}},
+		{Name: "misc cookie", Input: []*http.Cookie{miscCookie}, Expected: []*http.Cookie{miscCookie}},
+		{Name: "invalid cookie name", Input: []*http.Cookie{invalidCookieName}, Expected: []*http.Cookie{invalidCookieName}},
 	}
 	for _, test := range tests {
 		t.Run(test.Name, func(t *testing.T) {
@@ -1020,9 +1023,9 @@ func TestSensitiveCookieHandler(t *testing.T) {
 		Input    string
 		Expected string
 	}{
-		{"no cookies", "", ""},
-		{"valid cookie", miscCookie.String(), `some-other-cookie="I like cookies";Domain=test-domain.com`},
-		{"invalid cookie", `foobar[0]="violates RFC6266"`, `foobar[0]="violates RFC6266"`},
+		{Name: "no cookies", Input: "", Expected: ""},
+		{Name: "valid cookie", Input: miscCookie.String(), Expected: `some-other-cookie="I like cookies";Domain=test-domain.com`},
+		{Name: "invalid cookie", Input: `foobar[0]="violates RFC6266"`, Expected: `foobar[0]="violates RFC6266"`},
 	}
 	for _, test := range tests {
 		t.Run(test.Name, func(t *testing.T) {

diff --git a/install/installer/BUILD.yaml b/install/installer/BUILD.yaml
@@ -41,6 +41,7 @@ packages:
       - components/node-labeler:lib
       - dev/addlicense:app
       - components/spicedb:lib
+      - components/server/go:lib
     env:
       - CGO_ENABLED=0
     argdeps:

diff --git a/install/installer/go.mod b/install/installer/go.mod
@@ -1,6 +1,6 @@
 module github.com/gitpod-io/gitpod/installer
 
-go 1.22
+go 1.22.2
 
 require (
 	github.com/Masterminds/semver v1.5.0
@@ -19,6 +19,7 @@ require (
 	github.com/gitpod-io/gitpod/image-builder/api v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/gitpod/openvsx-proxy v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/gitpod/registry-facade/api v0.0.0-00010101000000-000000000000
+	github.com/gitpod-io/gitpod/server/go v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/gitpod/usage v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/gitpod/ws-daemon v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/gitpod/ws-daemon/api v0.0.0-00010101000000-000000000000
@@ -362,6 +363,8 @@ replace github.com/gitpod-io/gitpod/ws-proxy => ../../components/ws-proxy // lee
 
 replace github.com/gitpod-io/gitpod/node-labeler => ../../components/node-labeler // leeway
 
+replace github.com/gitpod-io/gitpod/server/go => ../../components/server/go // leeway
+
 replace k8s.io/api => k8s.io/api v0.29.3 // leeway indirect from components/common-go:lib
 
 replace k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.29.3 // leeway indirect from components/common-go:lib

diff --git a/install/installer/pkg/components/auth/config.go b/install/installer/pkg/components/auth/config.go
@@ -6,10 +6,10 @@ package auth
 
 import (
 	"fmt"
-	"regexp"
 	"time"
 
 	"github.com/gitpod-io/gitpod/installer/pkg/common"
+	server_lib "github.com/gitpod-io/gitpod/server/go/pkg/lib"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -45,7 +45,7 @@ func GetConfig(ctx *common.RenderContext) ([]corev1.Volume, []corev1.VolumeMount
 			Issuer:          fmt.Sprintf("https://%s", ctx.Config.Domain),
 			Cookie: CookieConfig{
 				// Caution: changing these have security implications for the application. Make sure you understand what you're doing.
-				Name:     cookieNameFromDomain(ctx.Config.Domain),
+				Name:     server_lib.CookieNameFromDomain(ctx.Config.Domain),
 				MaxAge:   lifetime,
 				SameSite: "lax",
 				Secure:   true,
@@ -54,9 +54,3 @@ func GetConfig(ctx *common.RenderContext) ([]corev1.Volume, []corev1.VolumeMount
 		},
 	}
 }
-
-func cookieNameFromDomain(domain string) string {
-	// replace all non-word characters with underscores
-	derived := regexp.MustCompile(`[\W_]+`).ReplaceAllString(domain, "_")
-	return "_" + derived + "_jwt2_"
-}
diff --git a/install/installer/pkg/components/auth/config_test.go b/install/installer/pkg/components/auth/config_test.go
@@ -4,7 +4,11 @@
 
 package auth
 
-import "testing"
+import (
+	"testing"
+
+	server_lib "github.com/gitpod-io/gitpod/server/go/pkg/lib"
+)
 
 func TestCookieNameFromDomain(t *testing.T) {
 	tests := []struct {
@@ -56,7 +60,7 @@ func TestCookieNameFromDomain(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			actual := cookieNameFromDomain(tt.domain)
+			actual := server_lib.CookieNameFromDomain(tt.domain)
 			if actual != tt.expectedOutcome {
 				t.Errorf("expected %q, got %q", tt.expectedOutcome, actual)
 			}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		module github.com/gitpod-io/gitpod/server/go

		go 1.22.2