Skip to content

Commit 309a32e

Browse files
authored
Improve CORS settings (#20154)
* Remove allowCredentials * fix tests * Revert "Remove allowCredentials" This reverts commit 06624ee. * Use FeatureFlag `ws_proxy_cors_enabled` * Fix test failed * fixup * fix network * fixup
1 parent 5661c6e commit 309a32e

File tree

9 files changed

+101
-10
lines changed

9 files changed

+101
-10
lines changed

components/ws-proxy/go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ require (
3030

3131
require (
3232
github.com/beorn7/perks v1.0.1 // indirect
33+
github.com/blang/semver v3.5.1+incompatible // indirect
3334
github.com/cenkalti/backoff/v4 v4.2.1 // indirect
3435
github.com/cespare/xxhash/v2 v2.2.0 // indirect
36+
github.com/configcat/go-sdk/v7 v7.6.0 // indirect
3537
github.com/davecgh/go-spew v1.1.1 // indirect
3638
github.com/emicklei/go-restful/v3 v3.11.0 // indirect
3739
github.com/evanphx/json-patch/v5 v5.8.0 // indirect

components/ws-proxy/go.sum

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

components/ws-proxy/pkg/config/config.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,20 @@
55
package config
66

77
import (
8+
"context"
89
"encoding/json"
910
"os"
11+
"time"
1012

1113
"golang.org/x/xerrors"
1214

15+
"github.com/gitpod-io/gitpod/common-go/experiments"
16+
"github.com/gitpod-io/gitpod/common-go/log"
1317
"github.com/gitpod-io/gitpod/ws-proxy/pkg/proxy"
1418
)
1519

20+
const experimentsCorsEnabled = "ws_proxy_cors_enabled"
21+
1622
// Config configures this service.
1723
type Config struct {
1824
Ingress proxy.HostBasedIngressConfig `json:"ingress"`
@@ -64,5 +70,31 @@ func GetConfig(fn string) (*Config, error) {
6470
return nil, xerrors.Errorf("config validation error: %w", err)
6571
}
6672

73+
timeout := time.Minute * 5
74+
log.WithField("timeout", timeout).Info("waiting for Feature Flag")
75+
experimentsClient := experiments.NewClient(experiments.WithPollInterval(time.Second * 3))
76+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
77+
defer cancel()
78+
ffValue := waitExperimentsStringValue(ctx, experimentsClient, experimentsCorsEnabled, "nope", experiments.Attributes{})
79+
corsEnabled := ffValue == "true"
80+
cfg.Proxy.CorsEnabled = corsEnabled
81+
log.WithField("ffValue", ffValue).WithField("corsEnabled", cfg.Proxy.CorsEnabled).Info("feature flag final value")
82+
6783
return &cfg, nil
6884
}
85+
86+
func waitExperimentsStringValue(ctx context.Context, client experiments.Client, experimentName, nopeValue string, attributes experiments.Attributes) string {
87+
ticker := time.NewTicker(1 * time.Second)
88+
defer ticker.Stop()
89+
for {
90+
select {
91+
case <-ctx.Done():
92+
return nopeValue
93+
case <-ticker.C:
94+
value := client.GetStringValue(ctx, experimentName, nopeValue, attributes)
95+
if value != nopeValue {
96+
return value
97+
}
98+
}
99+
}
100+
}

components/ws-proxy/pkg/proxy/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type Config struct {
2929

3030
BuiltinPages BuiltinPagesConfig `json:"builtinPages"`
3131
SSHGatewayCAKeyFile string `json:"sshCAKeyFile"`
32+
CorsEnabled bool
3233
}
3334

3435
// Validate validates the configuration to catch issues during startup and not at runtime.

components/ws-proxy/pkg/proxy/routes.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,10 @@ func WithDefaultAuth(infoprov common.WorkspaceInfoProvider) RouteHandlerConfigOp
6161

6262
// NewRouteHandlerConfig creates a new instance.
6363
func NewRouteHandlerConfig(config *Config, opts ...RouteHandlerConfigOpt) (*RouteHandlerConfig, error) {
64-
corsHandler, err := corsHandler(config.GitpodInstallation.Scheme, config.GitpodInstallation.HostName)
64+
corsHandler, err := corsHandler(config.CorsEnabled, config.GitpodInstallation.Scheme, config.GitpodInstallation.HostName)
6565
if err != nil {
6666
return nil, err
6767
}
68-
6968
cfg := &RouteHandlerConfig{
7069
Config: config,
7170
DefaultTransport: createDefaultTransport(config.TransportConfig),
@@ -190,6 +189,7 @@ func (ir *ideRoutes) HandleCreateKeyRoute(route *mux.Route, hostKeyList []ssh.Si
190189
r := route.Subrouter()
191190
r.Use(logRouteHandlerHandler("HandleCreateKeyRoute"))
192191
r.Use(ir.Config.CorsHandler)
192+
193193
r.Use(ir.workspaceMustExistHandler)
194194
r.Use(ir.Config.WorkspaceAuthHandler)
195195

@@ -655,7 +655,13 @@ func buildWorkspacePodURL(protocol api.PortProtocol, ipAddress string, port stri
655655
}
656656

657657
// corsHandler produces the CORS handler for workspaces.
658-
func corsHandler(scheme, hostname string) (mux.MiddlewareFunc, error) {
658+
func corsHandler(enabled bool, scheme, hostname string) (mux.MiddlewareFunc, error) {
659+
if !enabled {
660+
// empty handler
661+
return func(h http.Handler) http.Handler {
662+
return h
663+
}, nil
664+
}
659665
origin := fmt.Sprintf("%s://%s", scheme, hostname)
660666

661667
domainRegex := strings.ReplaceAll(hostname, ".", "\\.")

components/ws-proxy/pkg/proxy/routes_test.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ var (
9595
BuiltinPages: BuiltinPagesConfig{
9696
Location: "../../public",
9797
},
98+
CorsEnabled: false,
9899
}
99100
)
100101

@@ -450,7 +451,7 @@ func TestRoutes(t *testing.T) {
450451
},
451452
},
452453
{
453-
Desc: "CORS preflight",
454+
Desc: "no CORS allow in workspace urls",
454455
Config: &config,
455456
Request: modifyRequest(httptest.NewRequest("GET", workspaces[0].URL+"somewhere/in/the/ide", nil),
456457
addHostHeader,
@@ -462,12 +463,9 @@ func TestRoutes(t *testing.T) {
462463
Expectation: Expectation{
463464
Status: http.StatusOK,
464465
Header: http.Header{
465-
"Access-Control-Allow-Credentials": {"true"},
466-
"Access-Control-Allow-Origin": {"test-domain.com"},
467-
"Access-Control-Expose-Headers": {"Authorization"},
468-
"Content-Length": {"37"},
469-
"Content-Type": {"text/plain; charset=utf-8"},
470-
"Vary": {"Accept-Encoding"},
466+
"Content-Length": {"37"},
467+
"Content-Type": {"text/plain; charset=utf-8"},
468+
"Vary": {"Accept-Encoding"},
471469
},
472470
Body: "workspace hit: /somewhere/in/the/ide\n",
473471
},

install/installer/pkg/common/common.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,31 @@ func ConfigcatEnv(ctx *RenderContext) []corev1.EnvVar {
409409
}
410410
}
411411

412+
func ConfigcatEnvOutOfCluster(ctx *RenderContext) []corev1.EnvVar {
413+
var sdkKey string
414+
_ = ctx.WithExperimental(func(cfg *experimental.Config) error {
415+
if cfg.WebApp != nil && cfg.WebApp.ConfigcatKey != "" {
416+
sdkKey = cfg.WebApp.ConfigcatKey
417+
}
418+
return nil
419+
})
420+
421+
if sdkKey == "" {
422+
return nil
423+
}
424+
425+
return []corev1.EnvVar{
426+
{
427+
Name: "CONFIGCAT_SDK_KEY",
428+
Value: "gitpod",
429+
},
430+
{
431+
Name: "CONFIGCAT_BASE_URL",
432+
Value: fmt.Sprintf("https://%s/configcat", ctx.Config.Domain),
433+
},
434+
}
435+
}
436+
412437
func ConfigcatProxyEnv(ctx *RenderContext) []corev1.EnvVar {
413438
var (
414439
sdkKey string

install/installer/pkg/common/common_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@ import (
1111
"github.com/gitpod-io/gitpod/common-go/baseserver"
1212
"github.com/gitpod-io/gitpod/installer/pkg/common"
1313
config "github.com/gitpod-io/gitpod/installer/pkg/config/v1"
14+
"github.com/gitpod-io/gitpod/installer/pkg/config/v1/experimental"
1415
"github.com/gitpod-io/gitpod/installer/pkg/config/versions"
1516
"github.com/stretchr/testify/require"
1617
corev1 "k8s.io/api/core/v1"
18+
v1 "k8s.io/api/core/v1"
1719
)
1820

1921
func TestKubeRBACProxyContainer_DefaultPorts(t *testing.T) {
@@ -73,3 +75,19 @@ func TestServerComponentWaiterContainer(t *testing.T) {
7375
require.Equal(t, labels, "app=gitpod,component=server")
7476
require.Equal(t, []string{"-v", "component", "--namespace", "test_namespace", "--component", common.ServerComponent, "--labels", labels, "--image", ctx.Config.Repository + "/server:" + "happy_path_server_image"}, container.Args)
7577
}
78+
79+
func TestConfigcatEnvOutOfCluster(t *testing.T) {
80+
ctx, err := common.NewRenderContext(config.Config{
81+
Domain: "gitpod.io",
82+
Experimental: &experimental.Config{
83+
WebApp: &experimental.WebAppConfig{
84+
ConfigcatKey: "foo",
85+
},
86+
},
87+
}, versions.Manifest{}, "test_namespace")
88+
require.NoError(t, err)
89+
90+
envVars := common.ConfigcatEnvOutOfCluster(ctx)
91+
require.Equal(t, len(envVars), 2)
92+
require.Equal(t, envVars, []v1.EnvVar([]v1.EnvVar{{Name: "CONFIGCAT_SDK_KEY", Value: "gitpod"}, {Name: "CONFIGCAT_BASE_URL", Value: "https://gitpod.io/configcat"}}))
93+
}

install/installer/pkg/components/ws-proxy/deployment.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,8 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
142142
common.DefaultEnv(&ctx.Config),
143143
common.WorkspaceTracingEnv(ctx, Component),
144144
common.AnalyticsEnv(&ctx.Config),
145+
// ws-proxy and proxy may not in the same cluster
146+
common.ConfigcatEnvOutOfCluster(ctx),
145147
)),
146148
ReadinessProbe: &corev1.Probe{
147149
InitialDelaySeconds: int32(2),

0 commit comments

Comments
 (0)