diff --git a/pkg/build/info.go b/pkg/build/info.go index 7da423d5a64e..6f05b4f454e8 100644 --- a/pkg/build/info.go +++ b/pkg/build/info.go @@ -92,6 +92,15 @@ func (b Info) Short() string { b.Distribution, b.Tag, plat, b.Time, b.GoVersion) } +// GoTime parses the utcTime string and returns a time.Time. +func (b Info) GoTime() time.Time { + val, err := time.Parse(TimeFormat, b.Time) + if err != nil { + return time.Time{} + } + return val +} + // Timestamp parses the utcTime string and returns the number of seconds since epoch. func (b Info) Timestamp() (int64, error) { val, err := time.Parse(TimeFormat, b.Time) diff --git a/pkg/server/authentication.go b/pkg/server/authentication.go index 824560a352d7..067e5f6fe31b 100644 --- a/pkg/server/authentication.go +++ b/pkg/server/authentication.go @@ -356,19 +356,17 @@ const webSessionIDKeyStr = "webSessionID" func (am *authenticationMux) ServeHTTP(w http.ResponseWriter, req *http.Request) { username, cookie, err := am.getSession(w, req) - if err != nil && !am.allowAnonymous { + if err == nil { + ctx := req.Context() + ctx = context.WithValue(ctx, webSessionUserKey{}, username) + ctx = context.WithValue(ctx, webSessionIDKey{}, cookie.ID) + req = req.WithContext(ctx) + } else if !am.allowAnonymous { log.Infof(req.Context(), "Web session error: %s", err) http.Error(w, "a valid authentication cookie is required", http.StatusUnauthorized) return } - - newCtx := context.WithValue(req.Context(), webSessionUserKey{}, username) - if cookie != nil { - newCtx = context.WithValue(newCtx, webSessionIDKey{}, cookie.ID) - } - newReq := req.WithContext(newCtx) - - am.inner.ServeHTTP(w, newReq) + am.inner.ServeHTTP(w, req) } func encodeSessionCookie(sessionCookie *serverpb.SessionCookie) (*http.Cookie, error) { diff --git a/pkg/server/server.go b/pkg/server/server.go index f9f7b5c7d435..f5ca06749936 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -18,9 +18,7 @@ import ( "compress/gzip" "context" "crypto/tls" - "encoding/json" "fmt" - "html/template" "io" "io/ioutil" "math" @@ -33,7 +31,6 @@ import ( "sync/atomic" "time" - assetfs "github.com/elazarl/go-bindata-assetfs" raven "github.com/getsentry/raven-go" gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime" opentracing "github.com/opentracing/opentracing-go" @@ -42,7 +39,6 @@ import ( "github.com/cockroachdb/cmux" "github.com/cockroachdb/cockroach/pkg/base" - "github.com/cockroachdb/cockroach/pkg/build" "github.com/cockroachdb/cockroach/pkg/gossip" "github.com/cockroachdb/cockroach/pkg/internal/client" "github.com/cockroachdb/cockroach/pkg/jobs" @@ -1236,17 +1232,19 @@ func (s *Server) Start(ctx context.Context) error { // endpoints. s.mux.Handle(debug.Endpoint, debug.NewServer(s.st)) - fileServer := http.FileServer(&assetfs.AssetFS{ - Asset: ui.Asset, - AssetDir: ui.AssetDir, - AssetInfo: ui.AssetInfo, - }) - // Serve UI assets. This needs to be before the gRPC handlers are registered, otherwise // the `s.mux.Handle("/", ...)` would cover all URLs, allowing anonymous access. maybeAuthMux := newAuthenticationMuxAllowAnonymous( - s.authentication, serveUIAssets(fileServer, s.cfg), - ) + s.authentication, ui.Handler(ui.Config{ + ExperimentalUseLogin: s.cfg.EnableWebSessionAuthentication, + LoginEnabled: s.cfg.RequireWebSession(), + GetUser: func(ctx context.Context) *string { + if u, ok := ctx.Value(webSessionUserKey{}).(string); ok { + return &u + } + return nil + }, + })) s.mux.Handle("/", maybeAuthMux) // Initialize grpc-gateway mux and context in order to get the /health @@ -1896,6 +1894,8 @@ func (s *Server) PGServer() *pgwire.Server { return s.pgServer } +// TODO(benesch): Use https://github.com/NYTimes/gziphandler instead. +// gzipResponseWriter reinvents the wheel and is not as robust. type gzipResponseWriter struct { gz gzip.Writer http.ResponseWriter @@ -1918,6 +1918,11 @@ func (w *gzipResponseWriter) Reset(rw http.ResponseWriter) { } func (w *gzipResponseWriter) Write(b []byte) (int, error) { + // The underlying http.ResponseWriter can't sniff gzipped data properly, so we + // do our own sniffing on the uncompressed data. + if w.Header().Get("Content-Type") == "" { + w.Header().Set("Content-Type", http.DetectContentType(b)) + } return w.gz.Write(b) } @@ -1944,43 +1949,6 @@ func (w *gzipResponseWriter) Close() error { return err } -func serveUIAssets(fileServer http.Handler, cfg Config) http.Handler { - return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { - if request.URL.Path != "/" { - fileServer.ServeHTTP(writer, request) - return - } - - // Construct arguments for template. - tmplArgs := ui.IndexHTMLArgs{ - ExperimentalUseLogin: cfg.EnableWebSessionAuthentication, - LoginEnabled: cfg.RequireWebSession(), - Tag: build.GetInfo().Tag, - Version: build.VersionPrefix(), - } - loggedInUser, ok := request.Context().Value(webSessionUserKey{}).(string) - if ok && loggedInUser != "" { - tmplArgs.LoggedInUser = &loggedInUser - } - - argsJSON, err := json.Marshal(tmplArgs) - if err != nil { - http.Error(writer, err.Error(), 500) - } - - // Execute the template. - writer.Header().Add("Content-Type", "text/html") - if err := ui.IndexHTMLTemplate.Execute(writer, map[string]template.JS{ - "DataFromServer": template.JS(string(argsJSON)), - }); err != nil { - wrappedErr := errors.Wrap(err, "templating index.html") - http.Error(writer, wrappedErr.Error(), 500) - log.Error(request.Context(), wrappedErr) - return - } - }) -} - func init() { tracing.RegisterTagRemapping("n", "node") } diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 211f0a345440..4739ce682a2c 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -23,6 +23,7 @@ import ( "io/ioutil" "net/http" "net/url" + "os" "path/filepath" "reflect" "testing" @@ -43,6 +44,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/storage/engine" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/ui" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/httputil" "github.com/cockroachdb/cockroach/pkg/util/leaktest" @@ -858,6 +860,17 @@ func TestServeIndexHTML(t *testing.T) { ` + linkInFakeUI := func() { + ui.Asset = func(string) (_ []byte, _ error) { return } + ui.AssetDir = func(name string) (_ []string, _ error) { return } + ui.AssetInfo = func(name string) (_ os.FileInfo, _ error) { return } + } + unlinkFakeUI := func() { + ui.Asset = nil + ui.AssetDir = nil + ui.AssetInfo = nil + } + t.Run("Insecure mode", func(t *testing.T) { s, _, _ := serverutils.StartServer(t, base.TestServerArgs{ Insecure: true, @@ -874,32 +887,62 @@ func TestServeIndexHTML(t *testing.T) { t.Fatal(err) } - resp, err := client.Get(s.AdminURL()) - if err != nil { - t.Fatal(err) - } - if resp.StatusCode != 200 { - t.Fatalf("expected status code 200; got %d", resp.StatusCode) - } - respBytes, err := ioutil.ReadAll(resp.Body) - if err != nil { - t.Fatal(err) - } - respString := string(respBytes) - expected := fmt.Sprintf( - htmlTemplate, - fmt.Sprintf( - `{"ExperimentalUseLogin":false,"LoginEnabled":false,"LoggedInUser":null,"Tag":"%s","Version":"%s"}`, - build.GetInfo().Tag, - build.VersionPrefix(), - ), - ) - if respString != expected { - t.Fatalf("expected %s; got %s", expected, respString) - } + t.Run("short build", func(t *testing.T) { + resp, err := client.Get(s.AdminURL()) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != 200 { + t.Fatalf("expected status code 200; got %d", resp.StatusCode) + } + respBytes, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + respString := string(respBytes) + expected := fmt.Sprintf(` +CockroachDB +Binary built without web UI. +
+%s`, + build.GetInfo().Short()) + if respString != expected { + t.Fatalf("expected %s; got %s", expected, respString) + } + }) + + t.Run("non-short build", func(t *testing.T) { + linkInFakeUI() + defer unlinkFakeUI() + resp, err := client.Get(s.AdminURL()) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != 200 { + t.Fatalf("expected status code 200; got %d", resp.StatusCode) + } + respBytes, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + respString := string(respBytes) + expected := fmt.Sprintf( + htmlTemplate, + fmt.Sprintf( + `{"ExperimentalUseLogin":false,"LoginEnabled":false,"LoggedInUser":null,"Tag":"%s","Version":"%s"}`, + build.GetInfo().Tag, + build.VersionPrefix(), + ), + ) + if respString != expected { + t.Fatalf("expected %s; got %s", expected, respString) + } + }) }) t.Run("Secure mode", func(t *testing.T) { + linkInFakeUI() + defer unlinkFakeUI() s, _, _ := serverutils.StartServer(t, base.TestServerArgs{}) defer s.Stopper().Stop(context.TODO()) tsrv := s.(*TestServer) diff --git a/pkg/ui/ui.go b/pkg/ui/ui.go index 15fa7913666f..511fc4c9ed93 100644 --- a/pkg/ui/ui.go +++ b/pkg/ui/ui.go @@ -21,27 +21,22 @@ package ui import ( + "bytes" + "context" "fmt" "html/template" + "net/http" "os" "github.com/cockroachdb/cockroach/pkg/build" + "github.com/cockroachdb/cockroach/pkg/util/log" + assetfs "github.com/elazarl/go-bindata-assetfs" + "github.com/pkg/errors" ) -var indexHTML = []byte(fmt.Sprintf(` -CockroachDB -Binary built without web UI. -
-%s`, build.GetInfo().Short())) - // Asset loads and returns the asset for the given name. It returns an error if // the asset could not be found or could not be loaded. -var Asset = func(name string) ([]byte, error) { - if name == "index.html" { - return indexHTML, nil - } - return nil, os.ErrNotExist -} +var Asset func(name string) ([]byte, error) // AssetDir returns the file names below a certain directory in the embedded // filesystem. @@ -58,33 +53,22 @@ var Asset = func(name string) ([]byte, error) { // AssetDir("data") returns []string{"foo.txt", "img"} // AssetDir("data/img") returns []string{"a.png", "b.png"} // AssetDir("foo.txt") and AssetDir("notexist") return errors -var AssetDir = func(name string) ([]string, error) { - if name == "" { - return []string{"index.html"}, nil - } - return nil, os.ErrNotExist -} +var AssetDir func(name string) ([]string, error) // AssetInfo loads and returns metadata for the asset with the given name. It // returns an error if the asset could not be found or could not be loaded. var AssetInfo func(name string) (os.FileInfo, error) -// IndexHTMLTemplate takes arguments about the current session and returns HTML which -// includes the UI JavaScript bundles, plus a script tag which sets the currently logged in user -// so that the UI JavaScript can decide whether to show a login page. -var IndexHTMLTemplate *template.Template - -// IndexHTMLArgs are the arguments to IndexHTMLTemplate. -type IndexHTMLArgs struct { - ExperimentalUseLogin bool - LoginEnabled bool - LoggedInUser *string - Tag string - Version string +// haveUI returns whether the admin UI has been linked into the binary. +func haveUI() bool { + return Asset != nil && AssetDir != nil && AssetInfo != nil } -func init() { - t, err := template.New("index").Parse(` +// indexTemplate takes arguments about the current session and returns HTML +// which includes the UI JavaScript bundles, plus a script tag which sets the +// currently logged in user so that the UI JavaScript can decide whether to show +// a login page. +var indexHTMLTemplate = template.Must(template.New("index").Parse(` Cockroach Console @@ -95,7 +79,7 @@ func init() {
@@ -103,9 +87,61 @@ func init() { -`) - if err != nil { - panic(fmt.Sprintf("can't parse template: %s", err)) - } - IndexHTMLTemplate = t +`)) + +type indexHTMLArgs struct { + ExperimentalUseLogin bool + LoginEnabled bool + LoggedInUser *string + Tag string + Version string +} + +// bareIndexHTML is used in place of indexHTMLTemplate when the binary is built +// without the web UI. +var bareIndexHTML = []byte(fmt.Sprintf(` +CockroachDB +Binary built without web UI. +
+%s`, build.GetInfo().Short())) + +// Config contains the configuration parameters for Handler. +type Config struct { + ExperimentalUseLogin bool + LoginEnabled bool + GetUser func(ctx context.Context) *string +} + +// Handler returns an http.Handler that serves the UI. +func Handler(cfg Config) http.Handler { + fileServer := http.FileServer(&assetfs.AssetFS{ + Asset: Asset, + AssetDir: AssetDir, + AssetInfo: AssetInfo, + }) + buildInfo := build.GetInfo() + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !haveUI() { + http.ServeContent(w, r, "index.html", buildInfo.GoTime(), bytes.NewReader(bareIndexHTML)) + return + } + + if r.URL.Path != "/" { + fileServer.ServeHTTP(w, r) + return + } + + if err := indexHTMLTemplate.Execute(w, indexHTMLArgs{ + ExperimentalUseLogin: cfg.ExperimentalUseLogin, + LoginEnabled: cfg.LoginEnabled, + LoggedInUser: cfg.GetUser(r.Context()), + Tag: buildInfo.Tag, + Version: build.VersionPrefix(), + }); err != nil { + err = errors.Wrap(err, "templating index.html") + http.Error(w, err.Error(), 500) + log.Error(r.Context(), err) + } + }) }