Put user in context, map requests to context above resthandler layer

pkg/auth/handlers/handlers.go

@@ -18,39 +18,35 @@ package handlers
 
 import (
 	"net/http"
-	"sync"
 
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/auth/authenticator"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user"
 	"github.com/golang/glog"
 )
 
-// RequestContext is the interface used to associate a user with an http Request.
-type RequestContext interface {
-	Set(*http.Request, user.Info)
-	Get(req *http.Request) (user.Info, bool)
-	Remove(*http.Request)
-}
-
 // NewRequestAuthenticator creates an http handler that tries to authenticate the given request as a user, and then
 // stores any such user found onto the provided context for the request. If authentication fails or returns an error
 // the failed handler is used. On success, handler is invoked to serve the request.
-func NewRequestAuthenticator(context RequestContext, auth authenticator.Request, failed http.Handler, handler http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		user, ok, err := auth.AuthenticateRequest(req)
-		if err != nil || !ok {
-			if err != nil {
-				glog.Errorf("Unable to authenticate the request due to an error: %v", err)
+func NewRequestAuthenticator(mapper api.RequestContextMapper, auth authenticator.Request, failed http.Handler, handler http.Handler) (http.Handler, error) {
+	return api.NewRequestContextFilter(
+		mapper,
+		http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+			user, ok, err := auth.AuthenticateRequest(req)
+			if err != nil || !ok {
+				if err != nil {
+					glog.Errorf("Unable to authenticate the request due to an error: %v", err)
+				}
+				failed.ServeHTTP(w, req)
+				return
 			}
-			failed.ServeHTTP(w, req)
-			return
-		}
 
-		context.Set(req, user)
-		defer context.Remove(req)
+			if ctx, ok := mapper.Get(req); ok {
+				mapper.Update(req, api.WithUser(ctx, user))
+			}
 
-		handler.ServeHTTP(w, req)
-	})
+			handler.ServeHTTP(w, req)
+		}),
+	)
 }
 
 var Unauthorized http.HandlerFunc = unauthorized
@@ -59,38 +55,3 @@ var Unauthorized http.HandlerFunc = unauthorized
 func unauthorized(w http.ResponseWriter, req *http.Request) {
 	http.Error(w, "Unauthorized", http.StatusUnauthorized)
 }
-
-// UserRequestContext allows different levels of a call stack to store/retrieve info about the
-// current user associated with an http.Request.
-type UserRequestContext struct {
-	requests map[*http.Request]user.Info
-	lock     sync.Mutex
-}
-
-// NewUserRequestContext provides a map for storing and retrieving users associated with requests.
-// Be sure to pair each `context.Set(req, user)` call with a `defer context.Remove(req)` call or
-// you will leak requests. It implements the RequestContext interface.
-func NewUserRequestContext() *UserRequestContext {
-	return &UserRequestContext{
-		requests: make(map[*http.Request]user.Info),
-	}
-}
-
-func (c *UserRequestContext) Get(req *http.Request) (user.Info, bool) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-	user, ok := c.requests[req]
-	return user, ok
-}
-
-func (c *UserRequestContext) Set(req *http.Request, user user.Info) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-	c.requests[req] = user
-}
-
-func (c *UserRequestContext) Remove(req *http.Request) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-	delete(c.requests, req)
-}

pkg/auth/handlers/handlers_test.go

@@ -22,15 +22,16 @@ import (
 	"net/http/httptest"
 	"testing"
 
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/auth/authenticator"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user"
 )
 
 func TestAuthenticateRequest(t *testing.T) {
 	success := make(chan struct{})
-	context := NewUserRequestContext()
-	auth := NewRequestAuthenticator(
-		context,
+	contextMapper := api.NewRequestContextMapper()
+	auth, err := NewRequestAuthenticator(
+		contextMapper,
 		authenticator.RequestFunc(func(req *http.Request) (user.Info, bool, error) {
 			return &user.DefaultInfo{Name: "user"}, true, nil
 		}),
@@ -38,8 +39,13 @@ func TestAuthenticateRequest(t *testing.T) {
 			t.Errorf("unexpected call to failed")
 		}),
 		http.HandlerFunc(func(_ http.ResponseWriter, req *http.Request) {
-			if user, ok := context.Get(req); user == nil || !ok {
-				t.Errorf("no user stored on context: %#v", context)
+			ctx, ok := contextMapper.Get(req)
+			if ctx == nil || !ok {
+				t.Errorf("no context stored on contextMapper: %#v", contextMapper)
+			}
+			user, ok := api.UserFrom(ctx)
+			if user == nil || !ok {
+				t.Errorf("no user stored in context: %#v", ctx)
 			}
 			close(success)
 		}),
@@ -48,16 +54,20 @@ func TestAuthenticateRequest(t *testing.T) {
 	auth.ServeHTTP(httptest.NewRecorder(), &http.Request{})
 
 	<-success
-	if len(context.requests) > 0 {
-		t.Errorf("context should have no stored requests: %v", context)
+	empty, err := api.IsEmpty(contextMapper)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !empty {
+		t.Fatalf("contextMapper should have no stored requests: %v", contextMapper)
 	}
 }
 
 func TestAuthenticateRequestFailed(t *testing.T) {
 	failed := make(chan struct{})
-	context := NewUserRequestContext()
-	auth := NewRequestAuthenticator(
-		context,
+	contextMapper := api.NewRequestContextMapper()
+	auth, err := NewRequestAuthenticator(
+		contextMapper,
 		authenticator.RequestFunc(func(req *http.Request) (user.Info, bool, error) {
 			return nil, false, nil
 		}),
@@ -72,16 +82,20 @@ func TestAuthenticateRequestFailed(t *testing.T) {
 	auth.ServeHTTP(httptest.NewRecorder(), &http.Request{})
 
 	<-failed
-	if len(context.requests) > 0 {
-		t.Errorf("context should have no stored requests: %v", context)
+	empty, err := api.IsEmpty(contextMapper)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !empty {
+		t.Fatalf("contextMapper should have no stored requests: %v", contextMapper)
 	}
 }
 
 func TestAuthenticateRequestError(t *testing.T) {
 	failed := make(chan struct{})
-	context := NewUserRequestContext()
-	auth := NewRequestAuthenticator(
-		context,
+	contextMapper := api.NewRequestContextMapper()
+	auth, err := NewRequestAuthenticator(
+		contextMapper,
 		authenticator.RequestFunc(func(req *http.Request) (user.Info, bool, error) {
 			return nil, false, errors.New("failure")
 		}),
@@ -96,7 +110,11 @@ func TestAuthenticateRequestError(t *testing.T) {
 	auth.ServeHTTP(httptest.NewRecorder(), &http.Request{})
 
 	<-failed
-	if len(context.requests) > 0 {
-		t.Errorf("context should have no stored requests: %v", context)
+	empty, err := api.IsEmpty(contextMapper)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !empty {
+		t.Fatalf("contextMapper should have no stored requests: %v", contextMapper)
 	}
 }