Skip to content

Commit

Permalink
server/ui: move UI endpoint registration below SQL initialization
Browse files Browse the repository at this point in the history
HTTP requests are passed through an authentication mux, which checks
that the user is authenticated based on the cookies on the request. To
check authentication, the mux runs SQL to look up a session in the
system.web_sessions table.

Prior to this change, it was possible to hit the cluster with an HTTP
request which would cause the SQL to be run, before the SQL system was
fully initialized -- specifically before the cluster version setting was
initialized. This would crash the node.

This change delays installation of the authentication mux until after
the SQL system has been initialized, removing the possibility of this
crash.

Release note: None
  • Loading branch information
Pete Vilter committed Oct 1, 2018
1 parent 9e977cd commit b351cf5
Showing 1 changed file with 27 additions and 25 deletions.
52 changes: 27 additions & 25 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1234,21 +1234,6 @@ func (s *Server) Start(ctx context.Context) error {
// endpoints.
s.mux.Handle(debug.Endpoint, debug.NewServer(s.st))

// 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, 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
// endpoint working even before the node has fully initialized.
jsonpb := &protoutil.JSONPb{
Expand Down Expand Up @@ -1537,16 +1522,6 @@ func (s *Server) Start(ctx context.Context) error {

s.serveMode.set(modeOperational)

s.mux.Handle(adminPrefix, authHandler)
// Exempt the health check endpoint from authentication.
s.mux.Handle("/_admin/v1/health", gwMux)
s.mux.Handle(ts.URLPrefix, authHandler)
s.mux.Handle(statusPrefix, authHandler)
s.mux.Handle(loginPath, gwMux)
s.mux.Handle(logoutPath, authHandler)
s.mux.Handle(statusVars, http.HandlerFunc(s.status.handleVars))
log.Event(ctx, "added http endpoints")

log.Infof(ctx, "starting %s server at %s (use: %s)",
s.cfg.HTTPRequestScheme(), s.cfg.HTTPAddr, s.cfg.HTTPAdvertiseAddr)
log.Infof(ctx, "starting grpc/postgres server at %s", s.cfg.Addr)
Expand Down Expand Up @@ -1609,6 +1584,33 @@ func (s *Server) Start(ctx context.Context) error {
log.Info(ctx, "serving sql connections")
// Start servicing SQL connections.

// 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,
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)

s.mux.Handle(adminPrefix, authHandler)
// Exempt the health check endpoint from authentication.
s.mux.Handle("/_admin/v1/health", gwMux)
s.mux.Handle(ts.URLPrefix, authHandler)
s.mux.Handle(statusPrefix, authHandler)
s.mux.Handle(loginPath, gwMux)
s.mux.Handle(logoutPath, authHandler)
s.mux.Handle(statusVars, http.HandlerFunc(s.status.handleVars))
log.Event(ctx, "added http endpoints")

// Attempt to upgrade cluster version.
s.startAttemptUpgrade(ctx)

Expand Down

0 comments on commit b351cf5

Please sign in to comment.