Merge pull request #1407 from influxdata/1369-fix/router_basepath

Fix OAuth when using a Basepath

CHANGELOG.md

@@ -7,6 +7,7 @@
   1. [#1399](https://github.com/influxdata/chronograf/pull/1399): User can no longer create a blank template variable by clicking outside a newly added one
   1. [#1406](https://github.com/influxdata/chronograf/pull/1406): Ensure thresholds for Kapacitor Rule Alerts appear on page load
   1. [#1412](https://github.com/influxdata/chronograf/pull/1412): Check kapacitor status on configuration update
+  1. [#1407](https://github.com/influxdata/chronograf/pull/1407): Fix Authentication when using Chronograf with a basepath set
 
 ### Features
   1. [#1382](https://github.com/influxdata/chronograf/pull/1382): Add line-protocol proxy for InfluxDB data sources

docs/auth.md

@@ -18,6 +18,8 @@ export GH_ORGS=biffs-gang                                         # Restrict to
 To use authentication in Chronograf, both the OAuth provider and JWT signature
 need to be configured.
 
+**Note:** If you're using the `--basepath` option when starting Chronograf, you will need to add the same basepath to the callback URL of any OAuth provider that you configure.
+
 #### Configuring JWT signature
 
 Set a [JWT](https://tools.ietf.org/html/rfc7519) signature to a random string. This is needed for all OAuth2 providers that you choose to configure. *Keep this random string around!*

oauth2/mux.go

@@ -2,6 +2,7 @@ package oauth2
 
 import (
 	"net/http"
+	"path"
 	"time"
 
 	"github.com/influxdata/chronograf"
@@ -15,15 +16,15 @@ var _ Mux = &AuthMux{}
 const TenMinutes = 10 * time.Minute
 
 // NewAuthMux constructs a Mux handler that checks a cookie against the authenticator
-func NewAuthMux(p Provider, a Authenticator, t Tokenizer, l chronograf.Logger) *AuthMux {
+func NewAuthMux(p Provider, a Authenticator, t Tokenizer, basepath string, l chronograf.Logger) *AuthMux {
 	return &AuthMux{
 		Provider:   p,
 		Auth:       a,
 		Tokens:     t,
-		Logger:     l,
+		SuccessURL: path.Join(basepath, "/"),
-		SuccessURL: "/",
+		FailureURL: path.Join(basepath, "/login"),
-		FailureURL: "/login",
+		Now:    DefaultNowTime,
-		Now:        DefaultNowTime,
+		Logger: l,
 	}
 }
 

oauth2/mux_test.go

@@ -37,7 +37,7 @@ func setupMuxTest(selector func(*AuthMux) http.Handler) (*http.Client, *httptest
 		Tokens:     mt,
 	}
 
-	jm := NewAuthMux(mp, auth, mt, clog.New(clog.ParseLevel("debug")))
+	jm := NewAuthMux(mp, auth, mt, "", clog.New(clog.ParseLevel("debug")))
 	ts := httptest.NewServer(selector(jm))
 	jar, _ := cookiejar.New(nil)
 	hc := http.Client{

server/logger.go

@@ -2,20 +2,44 @@ package server
 
 import (
 	"net/http"
+	"time"
 
 	"github.com/influxdata/chronograf"
 )
 
+type logResponseWriter struct {
+	http.ResponseWriter
+
+	responseCode int
+}
+
+func (l *logResponseWriter) WriteHeader(status int) {
+	l.responseCode = status
+	l.ResponseWriter.WriteHeader(status)
+}
+
 // Logger is middleware that logs the request
 func Logger(logger chronograf.Logger, next http.Handler) http.Handler {
 	fn := func(w http.ResponseWriter, r *http.Request) {
+		now := time.Now()
 		logger.
 			WithField("component", "server").
 			WithField("remote_addr", r.RemoteAddr).
 			WithField("method", r.Method).
 			WithField("url", r.URL).
 			Info("Request")
-		next.ServeHTTP(w, r)
+
+		lrr := &logResponseWriter{w, 0}
+		next.ServeHTTP(lrr, r)
+		later := time.Now()
+		elapsed := later.Sub(now)
+
+		logger.
+			WithField("component", "server").
+			WithField("remote_addr", r.RemoteAddr).
+			WithField("response_time", elapsed.String()).
+			WithField("code", lrr.responseCode).
+			Info("Response: ", http.StatusText(lrr.responseCode))
 	}
 	return http.HandlerFunc(fn)
 }

server/logout.go

@@ -1,19 +1,22 @@
 package server
 
-import "net/http"
+import (
+	"net/http"
+	"path"
+)
 
 // Logout chooses the correct provider logout route and redirects to it
-func Logout(nextURL string, routes AuthRoutes) http.HandlerFunc {
+func Logout(nextURL, basepath string, routes AuthRoutes) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		ctx := r.Context()
 		principal, err := getPrincipal(ctx)
 		if err != nil {
-			http.Redirect(w, r, nextURL, http.StatusTemporaryRedirect)
+			http.Redirect(w, r, path.Join(basepath, nextURL), http.StatusTemporaryRedirect)
 			return
 		}
 		route, ok := routes.Lookup(principal.Issuer)
 		if !ok {
-			http.Redirect(w, r, nextURL, http.StatusTemporaryRedirect)
+			http.Redirect(w, r, path.Join(basepath, nextURL), http.StatusTemporaryRedirect)
 			return
 		}
 		http.Redirect(w, r, route.Logout, http.StatusTemporaryRedirect)

server/mountable_router.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"net/http"
+	libpath "path"
 
 	"github.com/influxdata/chronograf"
 )
@@ -18,37 +19,37 @@ type MountableRouter struct {
 // DELETE defines a route responding to a DELETE request that will be prefixed
 // with the configured route prefix
 func (mr *MountableRouter) DELETE(path string, handler http.HandlerFunc) {
-	mr.Delegate.DELETE(mr.Prefix+path, handler)
+	mr.Delegate.DELETE(libpath.Join(mr.Prefix, path), handler)
 }
 
 // GET defines a route responding to a GET request that will be prefixed
 // with the configured route prefix
 func (mr *MountableRouter) GET(path string, handler http.HandlerFunc) {
-	mr.Delegate.GET(mr.Prefix+path, handler)
+	mr.Delegate.GET(libpath.Join(mr.Prefix, path), handler)
 }
 
 // POST defines a route responding to a POST request that will be prefixed
 // with the configured route prefix
 func (mr *MountableRouter) POST(path string, handler http.HandlerFunc) {
-	mr.Delegate.POST(mr.Prefix+path, handler)
+	mr.Delegate.POST(libpath.Join(mr.Prefix, path), handler)
 }
 
 // PUT defines a route responding to a PUT request that will be prefixed
 // with the configured route prefix
 func (mr *MountableRouter) PUT(path string, handler http.HandlerFunc) {
-	mr.Delegate.PUT(mr.Prefix+path, handler)
+	mr.Delegate.PUT(libpath.Join(mr.Prefix, path), handler)
 }
 
 // PATCH defines a route responding to a PATCH request that will be prefixed
 // with the configured route prefix
 func (mr *MountableRouter) PATCH(path string, handler http.HandlerFunc) {
-	mr.Delegate.PATCH(mr.Prefix+path, handler)
+	mr.Delegate.PATCH(libpath.Join(mr.Prefix, path), handler)
 }
 
 // Handler defines a prefixed route responding to a request type specified in
 // the method parameter
 func (mr *MountableRouter) Handler(method string, path string, handler http.Handler) {
-	mr.Delegate.Handler(method, mr.Prefix+path, handler)
+	mr.Delegate.Handler(method, libpath.Join(mr.Prefix, path), handler)
 }
 
 // ServeHTTP is an implementation of http.Handler which delegates to the

server/mux.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"path"
 	"strconv"
 	"strings"
 
@@ -178,10 +179,15 @@ func NewMux(opts MuxOpts, service Service) http.Handler {
 
 	router.PUT("/chronograf/v1/sources/:id/dbs/:dbid/rps/:rpid", service.UpdateRetentionPolicy)
 	router.DELETE("/chronograf/v1/sources/:id/dbs/:dbid/rps/:rpid", service.DropRetentionPolicy)
-
 	var authRoutes AuthRoutes
 	var out http.Handler
+
 	/* Authentication */
+	logout := "/oauth/logout"
+	basepath := ""
+	if opts.PrefixRoutes {
+		basepath = opts.Basepath
+	}
 	if opts.UseAuth {
 		// Encapsulate the router with OAuth2
 		var auth http.Handler
@@ -189,15 +195,13 @@ func NewMux(opts MuxOpts, service Service) http.Handler {
 
 		// Create middleware to redirect to the appropriate provider logout
 		targetURL := "/"
-		router.GET("/oauth/logout", Logout(targetURL, authRoutes))
+		router.GET(logout, Logout(targetURL, basepath, authRoutes))
-
+		out = Logger(opts.Logger, PrefixedRedirect(opts.Basepath, auth))
-		out = Logger(opts.Logger, auth)
 	} else {
-		out = Logger(opts.Logger, router)
+		out = Logger(opts.Logger, PrefixedRedirect(opts.Basepath, router))
 	}
 
-	router.GET("/chronograf/v1/", AllRoutes(authRoutes, opts.Logger))
+	router.GET("/chronograf/v1/", AllRoutes(authRoutes, path.Join(opts.Basepath, logout), opts.Logger))
-	router.GET("/chronograf/v1", AllRoutes(authRoutes, opts.Logger))
 
 	return out
 }
@@ -208,26 +212,41 @@ func AuthAPI(opts MuxOpts, router chronograf.Router) (http.Handler, AuthRoutes)
 	for _, pf := range opts.ProviderFuncs {
 		pf(func(p oauth2.Provider, m oauth2.Mux) {
 			urlName := PathEscape(strings.ToLower(p.Name()))
-			loginPath := fmt.Sprintf("%s/oauth/%s/login", opts.Basepath, urlName)
+
-			logoutPath := fmt.Sprintf("%s/oauth/%s/logout", opts.Basepath, urlName)
+			loginPath := path.Join("/oauth", urlName, "login")
-			callbackPath := fmt.Sprintf("%s/oauth/%s/callback", opts.Basepath, urlName)
+			logoutPath := path.Join("/oauth", urlName, "logout")
+			callbackPath := path.Join("/oauth", urlName, "callback")
+
 			router.Handler("GET", loginPath, m.Login())
 			router.Handler("GET", logoutPath, m.Logout())
 			router.Handler("GET", callbackPath, m.Callback())
 			routes = append(routes, AuthRoute{
-				Name:     p.Name(),
+				Name:  p.Name(),
-				Label:    strings.Title(p.Name()),
+				Label: strings.Title(p.Name()),
-				Login:    loginPath,
+				// AuthRoutes are content served to the page. When Basepath is set, it
-				Logout:   logoutPath,
+				// says that all content served to the page will be prefixed with the
-				Callback: callbackPath,
+				// basepath. Since these routes are consumed by JS, it will need the
+				// basepath set to traverse a proxy correctly
+				Login:    path.Join(opts.Basepath, loginPath),
+				Logout:   path.Join(opts.Basepath, logoutPath),
+				Callback: path.Join(opts.Basepath, callbackPath),
 			})
 		})
 	}
 
+	rootPath := "/chronograf/v1"
+	logoutPath := "/oauth/logout"
+
+	if opts.PrefixRoutes {
+		rootPath = path.Join(opts.Basepath, rootPath)
+		logoutPath = path.Join(opts.Basepath, logoutPath)
+	}
+
 	tokenMiddleware := AuthorizedToken(opts.Auth, opts.Logger, router)
 	// Wrap the API with token validation middleware.
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if strings.HasPrefix(r.URL.Path, "/chronograf/v1/") || r.URL.Path == "/oauth/logout" {
+		cleanPath := path.Clean(r.URL.Path) // compare ignoring path garbage, trailing slashes, etc.
+		if (strings.HasPrefix(cleanPath, rootPath) && len(cleanPath) > len(rootPath)) || cleanPath == logoutPath {
 			tokenMiddleware.ServeHTTP(w, r)
 			return
 		}

server/prefixing_redirector.go

@@ -0,0 +1,34 @@
+package server
+
+import (
+	"net/http"
+	"net/url"
+	"path"
+	"strings"
+)
+
+type interceptingResponseWriter struct {
+	http.ResponseWriter
+	Prefix string
+}
+
+func (i *interceptingResponseWriter) WriteHeader(status int) {
+	if status >= 300 && status < 400 {
+		location := i.ResponseWriter.Header().Get("Location")
+		if u, err := url.Parse(location); err == nil && !u.IsAbs() {
+			if !strings.HasPrefix(location, i.Prefix) {
+				i.ResponseWriter.Header().Set("Location", path.Join(i.Prefix, location)+"/")
+			}
+		}
+	}
+	i.ResponseWriter.WriteHeader(status)
+}
+
+// PrefixingRedirector alters the Location header of downstream http.Handlers
+// to include a specified prefix
+func PrefixedRedirect(prefix string, next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		iw := &interceptingResponseWriter{w, prefix}
+		next.ServeHTTP(iw, r)
+	})
+}

server/routes.go

@@ -29,16 +29,17 @@ func (r *AuthRoutes) Lookup(provider string) (AuthRoute, bool) {
 }
 
 type getRoutesResponse struct {
-	Layouts    string      `json:"layouts"`    // Location of the layouts endpoint
+	Layouts    string      `json:"layouts"`          // Location of the layouts endpoint
-	Mappings   string      `json:"mappings"`   // Location of the application mappings endpoint
+	Mappings   string      `json:"mappings"`         // Location of the application mappings endpoint
-	Sources    string      `json:"sources"`    // Location of the sources endpoint
+	Sources    string      `json:"sources"`          // Location of the sources endpoint
-	Me         string      `json:"me"`         // Location of the me endpoint
+	Me         string      `json:"me"`               // Location of the me endpoint
-	Dashboards string      `json:"dashboards"` // Location of the dashboards endpoint
+	Dashboards string      `json:"dashboards"`       // Location of the dashboards endpoint
-	Auth       []AuthRoute `json:"auth"`       // Location of all auth routes.
+	Auth       []AuthRoute `json:"auth"`             // Location of all auth routes.
+	Logout     *string     `json:"logout,omitempty"` // Location of the logout route for all auth routes
 }
 
 // AllRoutes returns all top level routes within chronograf
-func AllRoutes(authRoutes []AuthRoute, logger chronograf.Logger) http.HandlerFunc {
+func AllRoutes(authRoutes []AuthRoute, logout string, logger chronograf.Logger) http.HandlerFunc {
 	routes := getRoutesResponse{
 		Sources:    "/chronograf/v1/sources",
 		Layouts:    "/chronograf/v1/layouts",
@@ -47,6 +48,9 @@ func AllRoutes(authRoutes []AuthRoute, logger chronograf.Logger) http.HandlerFun
 		Dashboards: "/chronograf/v1/dashboards",
 		Auth:       make([]AuthRoute, len(authRoutes)),
 	}
+	if logout != "" {
+		routes.Logout = &logout
+	}
 
 	for i, route := range authRoutes {
 		routes.Auth[i] = route

server/routes_test.go

@@ -11,7 +11,7 @@ import (
 
 func TestAllRoutes(t *testing.T) {
 	logger := log.New(log.DebugLevel)
-	handler := AllRoutes([]AuthRoute{}, logger)
+	handler := AllRoutes([]AuthRoute{}, "", logger)
 	req := httptest.NewRequest("GET", "http://docbrowns-inventions.com", nil)
 	w := httptest.NewRecorder()
 	handler(w, req)

server/server.go

@@ -124,7 +124,7 @@ func (s *Server) githubOAuth(logger chronograf.Logger, auth oauth2.Authenticator
 		Logger:       logger,
 	}
 	jwt := oauth2.NewJWT(s.TokenSecret)
-	ghMux := oauth2.NewAuthMux(&gh, auth, jwt, logger)
+	ghMux := oauth2.NewAuthMux(&gh, auth, jwt, s.Basepath, logger)
 	return &gh, ghMux, s.UseGithub
 }
 
@@ -138,7 +138,7 @@ func (s *Server) googleOAuth(logger chronograf.Logger, auth oauth2.Authenticator
 		Logger:       logger,
 	}
 	jwt := oauth2.NewJWT(s.TokenSecret)
-	goMux := oauth2.NewAuthMux(&google, auth, jwt, logger)
+	goMux := oauth2.NewAuthMux(&google, auth, jwt, s.Basepath, logger)
 	return &google, goMux, s.UseGoogle
 }
 
@@ -150,7 +150,7 @@ func (s *Server) herokuOAuth(logger chronograf.Logger, auth oauth2.Authenticator
 		Logger:        logger,
 	}
 	jwt := oauth2.NewJWT(s.TokenSecret)
-	hMux := oauth2.NewAuthMux(&heroku, auth, jwt, logger)
+	hMux := oauth2.NewAuthMux(&heroku, auth, jwt, s.Basepath, logger)
 	return &heroku, hMux, s.UseHeroku
 }
 
@@ -167,7 +167,7 @@ func (s *Server) genericOAuth(logger chronograf.Logger, auth oauth2.Authenticato
 		Logger:         logger,
 	}
 	jwt := oauth2.NewJWT(s.TokenSecret)
-	genMux := oauth2.NewAuthMux(&gen, auth, jwt, logger)
+	genMux := oauth2.NewAuthMux(&gen, auth, jwt, s.Basepath, logger)
 	return &gen, genMux, s.UseGenericOAuth2
 }
 
@@ -236,6 +236,11 @@ func (s *Server) Serve(ctx context.Context) error {
 	}
 	service := openService(ctx, s.BoltPath, layoutBuilder, sourcesBuilder, kapacitorBuilder, logger, s.useAuth())
 	basepath = s.Basepath
+	if basepath != "" && s.PrefixRoutes == false {
+		logger.
+			WithField("component", "server").
+			Info("Note: you may want to use --prefix-routes with --basepath. Try `./chronograf --help` for more info.")
+	}
 
 	providerFuncs := []func(func(oauth2.Provider, oauth2.Mux)){}
 

ui/src/index.js

@@ -1,8 +1,8 @@
 import React from 'react'
 import {render} from 'react-dom'
 import {Provider} from 'react-redux'
-import {Router, Route, useRouterHistory} from 'react-router'
+import {Router, Route} from 'react-router'
-import {createHistory} from 'history'
+import {createHistory, useBasename} from 'history'
 import {syncHistoryWithStore} from 'react-router-redux'
 
 import App from 'src/App'
@@ -32,6 +32,7 @@ import {
   authReceived,
   meRequested,
   meReceived,
+  logoutLinkReceived,
 } from 'shared/actions/auth'
 import {errorThrown} from 'shared/actions/errors'
 
@@ -41,18 +42,11 @@ import {HEARTBEAT_INTERVAL} from 'shared/constants'
 
 const rootNode = document.getElementById('react-root')
 
-let browserHistory
+const basepath = rootNode.dataset.basepath || ''
-const basepath = rootNode.dataset.basepath
 window.basepath = basepath
-if (basepath) {
+const browserHistory = useBasename(createHistory)({
-  browserHistory = useRouterHistory(createHistory)({
+  basename: basepath, // basepath is written in when available by the URL prefixer middleware
-    basename: basepath, // this is written in when available by the URL prefixer middleware
+})
-  })
-} else {
-  browserHistory = useRouterHistory(createHistory)({
-    basename: '',
-  })
-}
 
 const store = configureStore(loadLocalStorage(), browserHistory)
 const {dispatch} = store
@@ -86,10 +80,11 @@ const Root = React.createClass({
 
   async startHeartbeat({shouldDispatchResponse}) {
     try {
-      const {data: me, auth} = await getMe()
+      const {data: me, auth, logoutLink} = await getMe()
       if (shouldDispatchResponse) {
         dispatch(authReceived(auth))
         dispatch(meReceived(me))
+        dispatch(logoutLinkReceived(logoutLink))
       }
 
       setTimeout(() => {
@@ -107,12 +102,12 @@ const Root = React.createClass({
       <Provider store={store}>
         <Router history={history}>
           <Route path="/" component={UserIsAuthenticated(CheckSources)} />
-          <Route path="login" component={UserIsNotAuthenticated(Login)} />
+          <Route path="/login" component={UserIsNotAuthenticated(Login)} />
           <Route
-            path="sources/new"
+            path="/sources/new"
             component={UserIsAuthenticated(CreateSource)}
           />
-          <Route path="sources/:sourceID" component={UserIsAuthenticated(App)}>
+          <Route path="/sources/:sourceID" component={UserIsAuthenticated(App)}>
             <Route component={CheckSources}>
               <Route path="manage-sources" component={ManageSources} />
               <Route path="manage-sources/new" component={SourcePage} />

ui/src/shared/actions/auth.js

@@ -26,3 +26,10 @@ export const meReceived = me => ({
     me,
   },
 })
+
+export const logoutLinkReceived = logoutLink => ({
+  type: 'LOGOUT_LINK_RECEIVED',
+  payload: {
+    logoutLink,
+  },
+})

ui/src/shared/reducers/auth.js

@@ -3,6 +3,7 @@ const getInitialState = () => ({
   me: null,
   isMeLoading: false,
   isAuthLoading: false,
+  logoutLink: null,
 })
 
 export const initialState = getInitialState()
@@ -27,6 +28,10 @@ const authReducer = (state = initialState, action) => {
       const {me} = action.payload
       return {...state, me, isMeLoading: false}
     }
+    case 'LOGOUT_LINK_RECEIVED': {
+      const {logoutLink} = action.payload
+      return {...state, logoutLink}
+    }
   }
 
   return state

ui/src/side_nav/containers/SideNav.js

@@ -23,6 +23,7 @@ const SideNav = React.createClass({
       email: string,
     }),
     isHidden: bool.isRequired,
+    logoutLink: string,
   },
 
   render() {
@@ -31,6 +32,7 @@ const SideNav = React.createClass({
       params: {sourceID},
       location: {pathname: location},
       isHidden,
+      logoutLink,
     } = this.props
 
     const sourcePrefix = `/sources/${sourceID}`
@@ -79,7 +81,7 @@ const SideNav = React.createClass({
           </NavBlock>
           {showLogout
             ? <NavBlock icon="user-outline" className="sidebar__square-last">
-                <a className="sidebar__menu-item" href="/oauth/logout">
+                <a className="sidebar__menu-item" href={logoutLink}>
                   Logout
                 </a>
               </NavBlock>
@@ -89,11 +91,12 @@ const SideNav = React.createClass({
 })
 
 const mapStateToProps = ({
-  auth: {me},
+  auth: {me, logoutLink},
   app: {ephemeral: {inPresentationMode}},
 }) => ({
   me,
   isHidden: inPresentationMode,
+  logoutLink,
 })
 
 export default connect(mapStateToProps)(withRouter(SideNav))

ui/src/utils/ajax.js

@@ -25,8 +25,6 @@ export default async function AJAX({
       links = linksRes.data
     }
 
-    const {auth} = links
-
     if (resource) {
       url = id
         ? `${basepath}${links[resource]}/${id}`
@@ -41,14 +39,17 @@ export default async function AJAX({
       headers,
     })
 
+    const {auth} = links
+
     return {
-      auth,
       ...response,
+      auth: {links: auth},
+      logoutLink: links.logout,
     }
   } catch (error) {
     const {response} = error
 
     const {auth} = links
-    throw {...response, auth: {links: auth}} // eslint-disable-line no-throw-literal
+    throw {...response, auth: {links: auth}, logout: links.logout} // eslint-disable-line no-throw-literal
   }
 }