allocatord: prevent stats handler from triggering oauth flow
Otherwise, the handler will set the csrf cookie, but will fail to
complete the flow via the oauth handler since the stats request is made
via ajax (so the redirect never happens).
This CL makes the distinction between handlers that want the oauth flow,
and handlers that only want to check if the user is logged in (but
proceed with an empty email address if not). The stats handler will
return a new NotLoggedIn field in the JSON result in this case.
Change-Id: I6058d85d4d4c896c1b3751477f8bcda6e08982de
diff --git a/services/allocator/allocatord/dashboard.go b/services/allocator/allocatord/dashboard.go
index fff3715..b8dd17c 100644
--- a/services/allocator/allocatord/dashboard.go
+++ b/services/allocator/allocatord/dashboard.go
@@ -51,6 +51,8 @@
MinTime int64
MaxTime int64
+
+ NotLoggedIn bool
}
func handleDashboard(ss *serverState, rs *requestState) error {
@@ -78,11 +80,32 @@
return nil
}
+// TODO(jingjin): Returning an error from handleStats will cause an error page
+// to be rendered, which is not what we want when we consume the HTTP response
+// via Ajax.
+
// handleStats responds to /stats request. It retrieves time series data
// for the given syncbase instance from GCM.
func handleStats(ss *serverState, rs *requestState) error {
ctx := ss.ctx
+ var result statsResult
+ writeResult := func() error {
+ // Convert result to json and return it.
+ b, err := json.MarshalIndent(&result, "", " ")
+ if err != nil {
+ return err
+ }
+ rs.w.Header().Set("Content-Type", "application/json")
+ rs.w.Write(b)
+ return nil
+ }
+
+ if rs.email == "" {
+ result.NotLoggedIn = true
+ return writeResult()
+ }
+
mountedName := rs.r.FormValue(paramDashboardName)
if mountedName == "" {
return fmt.Errorf("parameter %q required for instance name", paramDashboardName)
@@ -157,7 +180,6 @@
}
// Process data and put it into statsResult.
- result := statsResult{}
minTime := int64(math.MaxInt64)
maxTime := int64(0)
for metricName, pts := range tsMap {
@@ -188,15 +210,7 @@
result.MinTime = minTime
result.MaxTime = maxTime
- // Convert result to json and return it.
- b, err := json.MarshalIndent(&result, "", " ")
- if err != nil {
- return err
- }
- rs.w.Header().Set("Content-Type", "application/json")
- rs.w.Write(b)
-
- return nil
+ return writeResult()
}
func getAlignmentPeriodInSeconds(duration time.Duration) int {
diff --git a/services/allocator/allocatord/http.go b/services/allocator/allocatord/http.go
index f86d55f..d56df93 100644
--- a/services/allocator/allocatord/http.go
+++ b/services/allocator/allocatord/http.go
@@ -124,15 +124,16 @@
// handlers, any re-authentication should result in redirection to the
// home page (to foil CSRF attacks that trick the user into launching
// actions with consequences).
- newHandler := func(f handlerFunc, mutating bool) *handler {
+ newHandler := func(f handlerFunc, mutating, forceLogin bool) *handler {
return &handler{
ss: &serverState{
ctx: ctx,
args: args,
},
- baker: baker,
- f: f,
- mutating: mutating,
+ baker: baker,
+ f: f,
+ mutating: mutating,
+ forceLogin: forceLogin,
}
}
@@ -153,19 +154,19 @@
ctx.Infof("%s[%s] : error %v", r.Method, r.URL, err)
}
})
- http.Handle(routeHome, newHandler(handleHome, false))
- http.Handle(routeCreate, newHandler(handleCreate, true))
- http.Handle(routeDashboard, newHandler(handleDashboard, false))
+ http.Handle(routeHome, newHandler(handleHome, false, true))
+ http.Handle(routeCreate, newHandler(handleCreate, true, true))
+ http.Handle(routeDashboard, newHandler(handleDashboard, false, true))
http.Handle(routeDebug+"/", newHandler(
func(ss *serverState, rs *requestState) error {
return handleDebug(ss, rs, debugBrowserServeMux)
- }, false))
- http.Handle(routeDestroy, newHandler(handleDestroy, true))
+ }, false, true))
+ http.Handle(routeDestroy, newHandler(handleDestroy, true, true))
http.HandleFunc(routeOauth, func(w http.ResponseWriter, r *http.Request) {
handleOauth(ctx, args, baker, w, r)
})
http.Handle(routeStatic, http.StripPrefix(routeStatic, args.assets))
- http.Handle(routeStats, newHandler(handleStats, false))
+ http.Handle(routeStats, newHandler(handleStats, false, false))
http.HandleFunc(routeHealth, func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})
@@ -204,27 +205,39 @@
// Vanadium context, configuration args, and user's email address (performing
// the oauth flow if the user is not logged in yet).
type handler struct {
- ss *serverState
- baker cookieBaker
- f handlerFunc
- mutating bool
+ ss *serverState
+ baker cookieBaker
+ f handlerFunc
+ mutating bool
+ forceLogin bool
}
-// ServeHTTP verifies that the user is logged in, and redirects to the oauth
-// flow if not. If the user is logged in, it extracts the email address from
-// the cookie and passes it to the handler function.
+// ServeHTTP verifies that the user is logged in. If the user is logged in, it
+// extracts the email address from the cookie and passes it to the handler
+// function. If the user is not logged in, and forceLogin is true, it redirects
+// to the oauth flow; otherwise, it leaves the email field blank when invoking
+// the handler function.
func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
ctx := h.ss.ctx
- oauthCfg := oauthConfig(h.ss.args.externalURL, h.ss.args.oauthCreds)
- email, csrfToken, err := requireSession(ctx, oauthCfg, h.baker, w, r, h.mutating)
- if err != nil {
- h.ss.args.assets.errorOccurred(ctx, w, r, routeHome, err)
- ctx.Infof("%s[%s] : error %v", r.Method, r.URL, err)
- return
- }
- if email == "" {
- ctx.Infof("%s[%s] -> login", r.Method, r.URL)
- return
+ var (
+ email, csrfToken, sessionBlurb string
+ err error
+ )
+ if !h.forceLogin {
+ if email, csrfToken, err = checkSession(h.baker, r, h.mutating); err != nil {
+ sessionBlurb = fmt.Sprintf("no session (%v)", err)
+ }
+ } else {
+ oauthCfg := oauthConfig(h.ss.args.externalURL, h.ss.args.oauthCreds)
+ if email, csrfToken, err = requireSession(ctx, oauthCfg, h.baker, w, r, h.mutating); err != nil {
+ h.ss.args.assets.errorOccurred(ctx, w, r, routeHome, err)
+ ctx.Infof("%s[%s] : error %v", r.Method, r.URL, err)
+ return
+ }
+ if email == "" {
+ ctx.Infof("%s[%s] -> login", r.Method, r.URL)
+ return
+ }
}
rs := &requestState{
email: email,
@@ -234,8 +247,8 @@
}
if err := h.f(h.ss, rs); err != nil {
h.ss.args.assets.errorOccurred(ctx, w, r, routeHome, err)
- ctx.Infof("%s[%s] : error %v", r.Method, r.URL, err)
+ ctx.Infof("%s[%s] %s : error %v", r.Method, r.URL, sessionBlurb, err)
return
}
- ctx.Infof("%s[%s] : OK", r.Method, r.URL)
+ ctx.Infof("%s[%s] %s : OK", r.Method, r.URL, sessionBlurb)
}
diff --git a/services/allocator/allocatord/oauth.go b/services/allocator/allocatord/oauth.go
index cd2b933..6887ff3 100644
--- a/services/allocator/allocatord/oauth.go
+++ b/services/allocator/allocatord/oauth.go
@@ -160,13 +160,13 @@
return cookieToken == csrfCookieValue && cookieCSRFToken == csrfToken
}
-func requireSession(ctx *context.T, oauthCfg *oauth2.Config, baker cookieBaker, w http.ResponseWriter, r *http.Request, mutating bool) (string, string, error) {
+func checkSession(baker cookieBaker, r *http.Request, mutating bool) (string, string, error) {
email, csrfToken, err := baker.get(r, cookieName)
switch {
case err == nil && email != "" && email != csrfCookieValue:
// The user is already logged in.
if mutating && r.FormValue(paramCSRF) != csrfToken {
- ctx.Info("Re-authenticating. Bad CSRF token for mutating request.")
+ return "", "", errors.New("bad CSRF token for mutating request")
} else {
return email, csrfToken, nil
}
@@ -180,14 +180,20 @@
// request's CSRF token will no longer match on the retry).
return "", "", errOauthInProgress
case err == nil:
- // Proceed with the oauth flow.
- ctx.Infof("Authenticating. Missing email cookie.")
- case err != nil:
- // Proceed with the oauth flow.
- ctx.Infof("Re-authenticating. Bad email cookie: %v", err)
+ return "", "", errors.New("missing cookie")
+ default:
+ return "", "", fmt.Errorf("bad cookie: %v", err)
}
- // Do the oauth.
- csrfToken = generateCSRFToken(ctx)
+}
+
+func requireSession(ctx *context.T, oauthCfg *oauth2.Config, baker cookieBaker, w http.ResponseWriter, r *http.Request, mutating bool) (string, string, error) {
+ if email, csrfToken, err := checkSession(baker, r, mutating); err == nil || err == errOauthInProgress {
+ return email, csrfToken, err
+ } else {
+ ctx.Infof("Re-authenticating: %v", err)
+ }
+ // Proceed with the oauth flow.
+ csrfToken := generateCSRFToken(ctx)
if csrfToken == "" {
return "", "", errors.New("failed to generate CSRF token")
}
