allocator: fix the debug UI
The debug UI is broken because the handler now expects both the instance
and mount name (and associated HMAC) to be present in the request.
However, links generated by the debug browser code only fill in the
mount name parameter. Rather than change the debug browser code to pass
the instance name, this CL figures out ownership based on the mount name
and the email from the login cookie. It expands the cache to hold all
instances belonging to a given email, and uses that cache both to look
up ownership by instance name or by mount name. One advantage is that
once a given email's cache entry is refreshed, all their instances will
result in quick cache hits.
Independently, increase cookie validity to 1 week.
Change-Id: Id92caad1496d4519022d661aadb680dff53c554f
diff --git a/services/allocator/allocatord/cache.go b/services/allocator/allocatord/cache.go
index bd5807f..042024c 100644
--- a/services/allocator/allocatord/cache.go
+++ b/services/allocator/allocatord/cache.go
@@ -11,31 +11,62 @@
"v.io/v23/context"
"v.io/v23/verror"
+ "v.io/x/ref/services/allocator"
)
var (
- ownerCache *lru.Cache
- ownerCacheMutex sync.Mutex
+ // instanceCache maps email to list of instances.
+ instanceCache *lru.Cache
+ instanceCacheMutex sync.Mutex
)
-func checkOwner(ctx *context.T, email, kName string) error {
- ownerCacheMutex.Lock()
- if ownerCache == nil {
- ownerCache = lru.New(maxInstancesFlag)
- } else if v, ok := ownerCache.Get(kName); ok {
- ownerCacheMutex.Unlock()
- if email != v {
- return verror.New(verror.ErrNoExistOrNoAccess, nil)
- }
- return nil
- }
- ownerCacheMutex.Unlock()
+func checkOwnerOfInstance(ctx *context.T, email, handle string) error {
+ return checkOwner(ctx, email, func(instance allocator.Instance) bool {
+ return instance.Handle == handle
+ })
+}
- if err := isOwnerOfInstance(ctx, email, kName); err != nil {
+func checkOwnerOfMountName(ctx *context.T, email, mName string) error {
+ return checkOwner(ctx, email, func(instance allocator.Instance) bool {
+ return instance.MountName == mName
+ })
+}
+
+// checkOwner returns the mount name of the instance if the given email has
+// access to it, or an error otherwise.
+func checkOwner(ctx *context.T, email string, predicate func(allocator.Instance) bool) error {
+ instanceCacheMutex.Lock()
+ if instanceCache == nil {
+ instanceCache = lru.New(maxInstancesFlag)
+ instanceCacheMutex.Unlock()
+ } else if v, ok := instanceCache.Get(email); ok {
+ instanceCacheMutex.Unlock()
+ if instances, ok := v.([]allocator.Instance); !ok {
+ // Our cache code is broken. Proceed to refresh entry.
+ ctx.Errorf("invalid cache entry type %T for email %v", v, email)
+ } else {
+ for _, instance := range instances {
+ if predicate(instance) {
+ return nil
+ }
+ }
+ }
+ }
+
+ instances, err := serverInstances(ctx, email)
+ if err != nil {
return err
}
- ownerCacheMutex.Lock()
- ownerCache.Add(kName, email)
- ownerCacheMutex.Unlock()
- return nil
+ // Regardless of whether an instance matches or not, update the email's
+ // cache entry since the user is using the system.
+ instanceCacheMutex.Lock()
+ instanceCache.Add(email, instances)
+ instanceCacheMutex.Unlock()
+
+ for _, instance := range instances {
+ if predicate(instance) {
+ return nil
+ }
+ }
+ return verror.New(verror.ErrNoExistOrNoAccess, nil)
}
diff --git a/services/allocator/allocatord/dashboard.go b/services/allocator/allocatord/dashboard.go
index 087f5c5..3063c49 100644
--- a/services/allocator/allocatord/dashboard.go
+++ b/services/allocator/allocatord/dashboard.go
@@ -61,7 +61,7 @@
if instance == "" {
return fmt.Errorf("parameter %q required for instance name", paramInstance)
}
- if err := checkOwner(ctx, rs.email, instance); err != nil {
+ if err := checkOwnerOfInstance(ctx, rs.email, instance); err != nil {
return err
}
@@ -110,7 +110,7 @@
if instance == "" {
return fmt.Errorf("parameter %q required for instance name", paramInstance)
}
- if err := checkOwner(ctx, rs.email, instance); err != nil {
+ if err := checkOwnerOfInstance(ctx, rs.email, instance); err != nil {
return err
}
diff --git a/services/allocator/allocatord/handlers.go b/services/allocator/allocatord/handlers.go
index 5bbbb24..6f59f0d 100644
--- a/services/allocator/allocatord/handlers.go
+++ b/services/allocator/allocatord/handlers.go
@@ -5,9 +5,6 @@
package main
import (
- "crypto/hmac"
- "encoding/base64"
- "errors"
"fmt"
"net/http"
@@ -51,13 +48,7 @@
SuspendURL: makeURL(ctx, routeSuspend, params{paramInstance: instance.Handle, paramCSRF: rs.csrfToken}),
ResumeURL: makeURL(ctx, routeResume, params{paramInstance: instance.Handle, paramCSRF: rs.csrfToken}),
DashboardURL: makeURL(ctx, routeDashboard, params{paramInstance: instance.Handle}),
- DebugURL: makeURL(
- ctx,
- routeDebug+"/",
- params{
- paramInstance: instance.Handle,
- paramMountName: instance.MountName,
- paramHMAC: base64.URLEncoding.EncodeToString(computeHMAC(instance.Handle, instance.MountName))}),
+ DebugURL: makeURL(ctx, routeDebug+"/", params{paramMountName: instance.MountName}),
})
}
if err := ss.args.assets.executeTemplate(rs.w, homeTmpl, tmplArgs); err != nil {
@@ -134,26 +125,12 @@
}
func handleDebug(ss *serverState, rs *requestState, debugBrowserServeMux *http.ServeMux) error {
- instance := rs.r.FormValue(paramInstance)
- if instance == "" {
- return fmt.Errorf("parameter %q required for instance name", paramInstance)
- }
+ ctx := ss.ctx
mountName := rs.r.FormValue(paramMountName)
if mountName == "" {
return fmt.Errorf("parameter %q required for instance mount name", paramMountName)
}
- hmacBase64 := rs.r.FormValue(paramHMAC)
- if hmacBase64 == "" {
- return fmt.Errorf("parameter %q required for name signature", paramHMAC)
- }
- hmacSignature, err := base64.URLEncoding.DecodeString(hmacBase64)
- if err != nil {
- return fmt.Errorf("invalid signature: %v", err)
- }
- if !hmac.Equal(hmacSignature, computeHMAC(instance, mountName)) {
- return errors.New("mismatching signature")
- }
- if err := checkOwner(ss.ctx, rs.email, instance); err != nil {
+ if err := checkOwnerOfMountName(ctx, rs.email, mountName); err != nil {
return err
}
http.StripPrefix(routeDebug, debugBrowserServeMux).ServeHTTP(rs.w, rs.r)
diff --git a/services/allocator/allocatord/http.go b/services/allocator/allocatord/http.go
index 502d4a4..db47fbe 100644
--- a/services/allocator/allocatord/http.go
+++ b/services/allocator/allocatord/http.go
@@ -41,7 +41,7 @@
// paramMountName has to match the name parameter in debug browser.
paramMountName = "n"
- cookieValidity = 10 * time.Minute
+ cookieValidity = 7 * 24 * time.Hour
)
type param struct {
@@ -131,10 +131,10 @@
newHandler := func(f handlerFunc, mutating, forceLogin bool) *handler {
return &handler{
ss: &serverState{
- ctx: ctx,
- args: args,
+ ctx: ctx,
+ args: args,
+ baker: baker,
},
- baker: baker,
f: f,
mutating: mutating,
forceLogin: forceLogin,
@@ -196,8 +196,9 @@
}
type serverState struct {
- ctx *context.T
- args httpArgs
+ ctx *context.T
+ args httpArgs
+ baker cookieBaker
}
type requestState struct {
@@ -213,7 +214,6 @@
// the oauth flow if the user is not logged in yet).
type handler struct {
ss *serverState
- baker cookieBaker
f handlerFunc
mutating bool
forceLogin bool
@@ -231,12 +231,12 @@
err error
)
if !h.forceLogin {
- if email, csrfToken, err = checkSession(h.baker, r, h.mutating); err != nil {
+ if email, csrfToken, err = checkSession(h.ss.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 {
+ if email, csrfToken, err = requireSession(ctx, oauthCfg, h.ss.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
