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

@@ -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

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 {

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

test/testdata/static/test.html

@@ -0,0 +1,8 @@
+<html>
+  <head>
+    <title>Static test asset page</title>
+  </head>
+  <body>
+    <h1>This is a static page.</h1>
+  </body>
+</html>