From 0cfe1cdedf2b05f0c3a3ed13ff73a66d427f5749 Mon Sep 17 00:00:00 2001 From: jannfis Date: Tue, 19 Nov 2019 00:12:35 +0100 Subject: [PATCH] Set X-Frame-Options on serving static assets (#2706) (#2711) * Add some test data for testing static assets * Optional send X-Frame-Options header for static assets * Allow fake server some time to settle in tests * Retrigger CI --- cmd/argocd-server/commands/root.go | 3 + server/server.go | 10 ++- server/server_test.go | 104 +++++++++++++++++++++++++++-- test/testdata/static/test.html | 8 +++ 4 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 test/testdata/static/test.html diff --git a/cmd/argocd-server/commands/root.go b/cmd/argocd-server/commands/root.go index bda3ef8833..9bc70ad0aa 100644 --- a/cmd/argocd-server/commands/root.go +++ b/cmd/argocd-server/commands/root.go @@ -36,6 +36,7 @@ func NewCommand() *cobra.Command { disableAuth bool tlsConfigCustomizerSrc func() (tls.ConfigCustomizer, error) cacheSrc func() (*servercache.Cache, error) + frameOptions string ) var command = &cobra.Command{ Use: cliName, @@ -76,6 +77,7 @@ func NewCommand() *cobra.Command { DisableAuth: disableAuth, TLSConfigCustomizer: tlsConfigCustomizer, Cache: cache, + XFrameOptions: frameOptions, } stats.RegisterStackDumper() @@ -105,6 +107,7 @@ func NewCommand() *cobra.Command { command.Flags().IntVar(&listenPort, "port", common.DefaultPortAPIServer, "Listen on given port") command.Flags().IntVar(&metricsPort, "metrics-port", common.DefaultPortArgoCDAPIServerMetrics, "Start metrics on given port") command.Flags().IntVar(&repoServerTimeoutSeconds, "repo-server-timeout-seconds", 60, "Repo server RPC call timeout seconds.") + command.Flags().StringVar(&frameOptions, "x-frame-options", "sameorigin", "Set X-Frame-Options header in HTTP responses to `value`. To disable, set to \"\".") tlsConfigCustomizerSrc = tls.AddTLSFlagsToCmd(command) cacheSrc = servercache.AddCacheFlagsToCmd(command) return command diff --git a/server/server.go b/server/server.go index 065c5f36c9..79cbed5c38 100644 --- a/server/server.go +++ b/server/server.go @@ -147,6 +147,7 @@ type ArgoCDServerOpts struct { RepoClientset repoapiclient.Clientset Cache *servercache.Cache TLSConfigCustomizer tlsutil.ConfigCustomizer + XFrameOptions string } // initializeDefaultProject creates the default project if it does not already exist @@ -582,7 +583,7 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl // Serve UI static assets if a.StaticAssetsDir != "" { - mux.HandleFunc("/", newStaticAssetsHandler(a.StaticAssetsDir, a.BaseHRef)) + mux.HandleFunc("/", a.newStaticAssetsHandler(a.StaticAssetsDir, a.BaseHRef)) } return &httpS } @@ -688,7 +689,7 @@ func fileExists(filename string) bool { } // newStaticAssetsHandler returns an HTTP handler to serve UI static assets -func newStaticAssetsHandler(dir string, baseHRef string) func(http.ResponseWriter, *http.Request) { +func (server *ArgoCDServer) newStaticAssetsHandler(dir string, baseHRef string) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { acceptHTML := false for _, acceptType := range strings.Split(r.Header.Get("Accept"), ",") { @@ -699,6 +700,11 @@ func newStaticAssetsHandler(dir string, baseHRef string) func(http.ResponseWrite } fileRequest := r.URL.Path != "/index.html" && fileExists(path.Join(dir, r.URL.Path)) + // Set X-Frame-Options according to configuration + if server.XFrameOptions != "" { + w.Header().Set("X-Frame-Options", server.XFrameOptions) + } + // serve index.html for non file requests to support HTML5 History API if acceptHTML && !fileRequest && (r.Method == "GET" || r.Method == "HEAD") { for k, v := range noCacheHeaders { diff --git a/server/server_test.go b/server/server_test.go index 18efbb91f5..50cb51c740 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io/ioutil" + "net/http" "net/http/httptest" "strings" "testing" @@ -37,11 +38,13 @@ func fakeServer() *ArgoCDServer { appClientSet := apps.NewSimpleClientset() argoCDOpts := ArgoCDServerOpts{ - Namespace: test.FakeArgoCDNamespace, - KubeClientset: kubeclientset, - AppClientset: appClientSet, - Insecure: true, - DisableAuth: true, + Namespace: test.FakeArgoCDNamespace, + KubeClientset: kubeclientset, + AppClientset: appClientSet, + Insecure: true, + DisableAuth: true, + StaticAssetsDir: "../test/testdata/static", + XFrameOptions: "sameorigin", } return NewServer(context.Background(), argoCDOpts) } @@ -465,6 +468,97 @@ func TestAuthenticate(t *testing.T) { } } +func Test_StaticHeaders(t *testing.T) { + // Test default policy "sameorigin" + { + s := fakeServer() + cancelInformer := test.StartInformer(s.projInformer) + defer cancelInformer() + port, err := test.GetFreePort() + assert.NoError(t, err) + metricsPort, err := test.GetFreePort() + assert.NoError(t, err) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.Run(ctx, port, metricsPort) + defer func() { time.Sleep(3 * time.Second) }() + + err = test.WaitForPortListen(fmt.Sprintf("127.0.0.1:%d", port), 10*time.Second) + assert.NoError(t, err) + + // Allow server startup + time.Sleep(1 * time.Second) + + client := http.Client{} + url := fmt.Sprintf("http://127.0.0.1:%d/test.html", port) + req, err := http.NewRequest("GET", url, nil) + assert.NoError(t, err) + resp, err := client.Do(req) + assert.NoError(t, err) + assert.Equal(t, "sameorigin", resp.Header.Get("X-Frame-Options")) + } + + // Test custom policy + { + s := fakeServer() + s.XFrameOptions = "deny" + cancelInformer := test.StartInformer(s.projInformer) + defer cancelInformer() + port, err := test.GetFreePort() + assert.NoError(t, err) + metricsPort, err := test.GetFreePort() + assert.NoError(t, err) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.Run(ctx, port, metricsPort) + defer func() { time.Sleep(3 * time.Second) }() + + err = test.WaitForPortListen(fmt.Sprintf("127.0.0.1:%d", port), 10*time.Second) + assert.NoError(t, err) + + // Allow server startup + time.Sleep(1 * time.Second) + + client := http.Client{} + url := fmt.Sprintf("http://127.0.0.1:%d/test.html", port) + req, err := http.NewRequest("GET", url, nil) + assert.NoError(t, err) + resp, err := client.Do(req) + assert.NoError(t, err) + assert.Equal(t, "deny", resp.Header.Get("X-Frame-Options")) + } + + // Test disabled + { + s := fakeServer() + s.XFrameOptions = "" + cancelInformer := test.StartInformer(s.projInformer) + defer cancelInformer() + port, err := test.GetFreePort() + assert.NoError(t, err) + metricsPort, err := test.GetFreePort() + assert.NoError(t, err) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.Run(ctx, port, metricsPort) + defer func() { time.Sleep(3 * time.Second) }() + + err = test.WaitForPortListen(fmt.Sprintf("127.0.0.1:%d", port), 10*time.Second) + assert.NoError(t, err) + + // Allow server startup + time.Sleep(1 * time.Second) + + client := http.Client{} + url := fmt.Sprintf("http://127.0.0.1:%d/test.html", port) + req, err := http.NewRequest("GET", url, nil) + assert.NoError(t, err) + resp, err := client.Do(req) + assert.NoError(t, err) + assert.Empty(t, resp.Header.Get("X-Frame-Options")) + } +} + func Test_getToken(t *testing.T) { token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c" t.Run("Empty", func(t *testing.T) { diff --git a/test/testdata/static/test.html b/test/testdata/static/test.html new file mode 100644 index 0000000000..7718ed203d --- /dev/null +++ b/test/testdata/static/test.html @@ -0,0 +1,8 @@ + + + Static test asset page + + +

This is a static page.

+ + \ No newline at end of file