Switch to using request context mapper

pkg/authorization/authorizer/authorizer.go

@@ -13,7 +13,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors"
 
-	authcontext "github.com/openshift/origin/pkg/auth/context"
 	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
 	policyregistry "github.com/openshift/origin/pkg/authorization/registry/policy"
 	policybindingregistry "github.com/openshift/origin/pkg/authorization/registry/policybinding"
@@ -58,12 +57,12 @@ type DefaultAuthorizationAttributes struct {
 }
 
 type openshiftAuthorizationAttributeBuilder struct {
-	requestsToUsers *authcontext.RequestContextMap
-	infoResolver    *APIRequestInfoResolver
+	contextMapper kapi.RequestContextMapper
+	infoResolver  *APIRequestInfoResolver
 }
 
-func NewAuthorizationAttributeBuilder(requestsToUsers *authcontext.RequestContextMap, infoResolver *APIRequestInfoResolver) AuthorizationAttributeBuilder {
-	return &openshiftAuthorizationAttributeBuilder{requestsToUsers, infoResolver}
+func NewAuthorizationAttributeBuilder(contextMapper kapi.RequestContextMapper, infoResolver *APIRequestInfoResolver) AuthorizationAttributeBuilder {
+	return &openshiftAuthorizationAttributeBuilder{contextMapper, infoResolver}
 }
 
 func doesApplyToUser(ruleUsers, ruleGroups []string, user user.Info) bool {
@@ -386,13 +385,13 @@ func (a *openshiftAuthorizationAttributeBuilder) GetAttributes(req *http.Request
 		requestInfo.Namespace = requestInfo.Name
 	}
 
-	userInterface, ok := a.requestsToUsers.Get(req)
+	ctx, ok := a.contextMapper.Get(req)
 	if !ok {
-		return nil, errors.New("could not get user")
+		return nil, errors.New("could not get request context")
 	}
-	userInfo, ok := userInterface.(user.Info)
+	userInfo, ok := kapi.UserFrom(ctx)
 	if !ok {
-		return nil, errors.New("wrong type returned for user")
+		return nil, errors.New("could not get user")
 	}
 
 	return DefaultAuthorizationAttributes{

pkg/cmd/server/kubernetes/master.go

@@ -37,6 +37,8 @@ type MasterConfig struct {
 	NodeHosts  []string
 	PortalNet  *net.IPNet
 
+	RequestContextMapper kapi.RequestContextMapper
+
 	EtcdHelper tools.EtcdHelper
 	KubeClient *kclient.Client
 
@@ -79,6 +81,8 @@ func (c *MasterConfig) InstallAPI(container *restful.Container) []string {
 
 		PortalNet: c.PortalNet,
 
+		RequestContextMapper: c.RequestContextMapper,
+
 		RestfulContainer: container,
 		KubeletClient:    kubeletClient,
 		APIPrefix:        KubeAPIPrefix,

pkg/cmd/server/origin/auth.go

@@ -27,7 +27,6 @@ import (
 	"github.com/openshift/origin/pkg/auth/authenticator/request/headerrequest"
 	"github.com/openshift/origin/pkg/auth/authenticator/request/unionrequest"
 	"github.com/openshift/origin/pkg/auth/authenticator/token/filetoken"
-	authcontext "github.com/openshift/origin/pkg/auth/context"
 	"github.com/openshift/origin/pkg/auth/oauth/external"
 	"github.com/openshift/origin/pkg/auth/oauth/external/github"
 	"github.com/openshift/origin/pkg/auth/oauth/external/google"
@@ -50,7 +49,6 @@ import (
 	"github.com/openshift/origin/pkg/oauth/server/osinserver/registrystorage"
 	"github.com/openshift/origin/pkg/user"
 	useretcd "github.com/openshift/origin/pkg/user/registry/etcd"
-	userregistry "github.com/openshift/origin/pkg/user/registry/user"
 )
 
 const (
@@ -607,46 +605,26 @@ func (redirectSuccessHandler) AuthenticationSucceeded(user kuser.Info, then stri
 	return true, nil
 }
 
-// currentUserContextFilter replaces the the last segment of the provided URL with the current user's name
-func currentUserContextFilter(requestsToUsers *authcontext.RequestContextMap) restful.FilterFunction {
-	return func(req *restful.Request, res *restful.Response, chain *restful.FilterChain) {
-		name := path.Base(req.Request.URL.Path)
-		if name != "~" {
-			chain.ProcessFilter(req, res)
-			return
-		}
-
-		val, found := requestsToUsers.Get(req.Request)
-		if !found {
-			http.Error(res.ResponseWriter, "Need to be authenticated to access this method", http.StatusUnauthorized)
-			return
-		}
-		user, ok := val.(userregistry.Info)
-		if !ok {
-			http.Error(res.ResponseWriter, "Unable to convert internal object", http.StatusInternalServerError)
-			return
-		}
-
-		base := path.Dir(req.Request.URL.Path)
-		req.Request.URL.Path = path.Join(base, user.GetName())
-
-		chain.ProcessFilter(req, res)
-	}
-}
-
 // authenticationHandlerFilter creates a filter object that will enforce authentication directly
-func authenticationHandlerFilter(handler http.Handler, authenticator authenticator.Request, requestsToUsers *authcontext.RequestContextMap) http.Handler {
+func authenticationHandlerFilter(handler http.Handler, authenticator authenticator.Request, contextMapper kapi.RequestContextMapper) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		user, ok, err := authenticator.AuthenticateRequest(req)
 		if err != nil || !ok {
-			w.WriteHeader(http.StatusUnauthorized)
-			w.Write([]byte("Unauthorized"))
+			http.Error(w, "Unauthorized", http.StatusUnauthorized)
 			return
 		}
 		glog.V(4).Infof("user %v -> %v", user, req.URL)
 
-		requestsToUsers.Set(req, user)
-		defer requestsToUsers.Remove(req)
+		ctx, ok := contextMapper.Get(req)
+		if !ok {
+			http.Error(w, "Unable to find request context", http.StatusInternalServerError)
+			return
+		}
+		if err := contextMapper.Update(req, kapi.WithUser(ctx, user)); err != nil {
+			glog.V(4).Infof("Error setting authenticated context: %v", err)
+			http.Error(w, "Unable to set authenticated request context", http.StatusInternalServerError)
+			return
+		}
 
 		handler.ServeHTTP(w, req)
 	})

pkg/cmd/server/origin/master.go

@@ -31,7 +31,6 @@ import (
 	"github.com/openshift/origin/pkg/api/v1beta1"
 	"github.com/openshift/origin/pkg/assets"
 	"github.com/openshift/origin/pkg/auth/authenticator"
-	authcontext "github.com/openshift/origin/pkg/auth/context"
 	buildclient "github.com/openshift/origin/pkg/build/client"
 	buildcontrollerfactory "github.com/openshift/origin/pkg/build/controller/factory"
 	buildstrategy "github.com/openshift/origin/pkg/build/controller/strategy"
@@ -113,9 +112,10 @@ type MasterConfig struct {
 	Authenticator                 authenticator.Request
 	Authorizer                    authorizer.Authorizer
 	AuthorizationAttributeBuilder authorizer.AuthorizationAttributeBuilder
-	// RequestsToUsers is used by both authentication and authorization.  This is a shared, in-memory map, so they must use exactly the same instance
-	RequestsToUsers              *authcontext.RequestContextMap
-	MasterAuthorizationNamespace string
+	MasterAuthorizationNamespace  string
+
+	// Map requests to contexts
+	RequestContextMapper kapi.RequestContextMapper
 
 	EtcdHelper tools.EtcdHelper
 
@@ -311,35 +311,19 @@ func (c *MasterConfig) InstallProtectedAPI(container *restful.Container) []strin
 
 	admissionControl := admit.NewAlwaysAdmit()
 
-	if err := apiserver.NewAPIGroupVersion(storage, v1beta1.Codec, OpenShiftAPIPrefix, OpenShiftAPIV1Beta1, latest.SelfLinker, admissionControl, kapi.NewRequestContextMapper(), latest.RESTMapper).InstallREST(container, OpenShiftAPIPrefix, "v1beta1"); err != nil {
+	if err := apiserver.NewAPIGroupVersion(storage, v1beta1.Codec, OpenShiftAPIPrefix, OpenShiftAPIV1Beta1, latest.SelfLinker, admissionControl, c.getRequestContextMapper(), latest.RESTMapper).InstallREST(container, OpenShiftAPIPrefix, "v1beta1"); err != nil {
 		glog.Fatalf("Unable to initialize API: %v", err)
 	}
 
 	var root *restful.WebService
-	userRoutesChanged := 0
 	for _, svc := range container.RegisteredWebServices() {
 		switch svc.RootPath() {
 		case "/":
 			root = svc
 		case OpenShiftAPIPrefixV1Beta1:
 			svc.Doc("OpenShift REST API, version v1beta1").ApiVersion("v1beta1")
-
-			// add the current user filter
-			// TODO: factor this better
-			filter := currentUserContextFilter(c.getRequestsToUsers())
-			routes := svc.Routes()
-			for i := range routes {
-				route := &routes[i]
-				if route.Method == "GET" && (route.Path == OpenShiftAPIPrefixV1Beta1+"/users/{name}") {
-					route.Filters = append(route.Filters, filter)
-					userRoutesChanged++
-				}
-			}
 		}
 	}
-	if userRoutesChanged != 1 {
-		glog.Fatalf("Could not find user route to install the current user filter.")
-	}
 	if root == nil {
 		root = new(restful.WebService)
 		container.Add(root)
@@ -396,7 +380,7 @@ func (c *MasterConfig) Run(protected []APIInstaller, unprotected []APIInstaller)
 		extra = append(extra, i.InstallAPI(safe)...)
 	}
 	handler := c.authorizationFilter(safe)
-	handler = authenticationHandlerFilter(handler, c.Authenticator, c.getRequestsToUsers())
+	handler = authenticationHandlerFilter(handler, c.Authenticator, c.getRequestContextMapper())
 
 	// unprotected resources
 	unprotected = append(unprotected, APIInstallFunc(c.InstallUnprotectedAPI))
@@ -420,6 +404,13 @@ func (c *MasterConfig) Run(protected []APIInstaller, unprotected []APIInstaller)
 		handler = apiserver.CORS(handler, origins, nil, nil, "true")
 	}
 
+	// Make the outermost filter the requestContextMapper to ensure all components share the same context
+	if contextHandler, err := kapi.NewRequestContextFilter(c.getRequestContextMapper(), handler); err != nil {
+		glog.Fatalf("Error setting up request context filter: %v", err)
+	} else {
+		handler = contextHandler
+	}
+
 	server := &http.Server{
 		Addr:           c.MasterBindAddr,
 		Handler:        handler,
@@ -451,12 +442,12 @@ func (c *MasterConfig) Run(protected []APIInstaller, unprotected []APIInstaller)
 	cmdutil.WaitForSuccessfulDial("tcp", c.MasterBindAddr, 100*time.Millisecond, 100*time.Millisecond, 100)
 }
 
-// getRequestsToUsers returns the shared user context
-func (c *MasterConfig) getRequestsToUsers() *authcontext.RequestContextMap {
-	if c.RequestsToUsers == nil {
-		c.RequestsToUsers = authcontext.NewRequestContextMap()
+// getRequestContextMapper returns a mapper from requests to contexts, initializing it if needed
+func (c *MasterConfig) getRequestContextMapper() kapi.RequestContextMapper {
+	if c.RequestContextMapper == nil {
+		c.RequestContextMapper = kapi.NewRequestContextMapper()
 	}
-	return c.RequestsToUsers
+	return c.RequestContextMapper
 }
 
 // ensureComponentAuthorizationRules initializes the global policies

pkg/cmd/server/start.go

@@ -37,7 +37,6 @@ import (
 	"github.com/openshift/origin/pkg/auth/authenticator/request/paramtoken"
 	"github.com/openshift/origin/pkg/auth/authenticator/request/unionrequest"
 	"github.com/openshift/origin/pkg/auth/authenticator/request/x509request"
-	authcontext "github.com/openshift/origin/pkg/auth/context"
 	"github.com/openshift/origin/pkg/authorization/authorizer"
 	authorizationetcd "github.com/openshift/origin/pkg/authorization/registry/etcd"
 
@@ -308,7 +307,7 @@ func start(cfg *config, args []string) error {
 			return fmt.Errorf("Error setting up Kubernetes server storage: %v", err)
 		}
 
-		requestsToUsers := authcontext.NewRequestContextMap()
+		requestContextMapper := kapi.NewRequestContextMapper()
 		masterAuthorizationNamespace := "master"
 
 		// determine whether public API addresses were specified
@@ -360,9 +359,9 @@ func start(cfg *config, args []string) error {
 
 			AdmissionControl:              admit.NewAlwaysAdmit(),
 			Authorizer:                    newAuthorizer(etcdHelper, masterAuthorizationNamespace),
-			AuthorizationAttributeBuilder: newAuthorizationAttributeBuilder(requestsToUsers),
+			AuthorizationAttributeBuilder: newAuthorizationAttributeBuilder(requestContextMapper),
 			MasterAuthorizationNamespace:  masterAuthorizationNamespace,
-			RequestsToUsers:               requestsToUsers,
+			RequestContextMapper:          requestContextMapper,
 
 			UseLocalImages: useLocalImages,
 			ImageFor:       imageResolverFn,
@@ -555,14 +554,15 @@ func start(cfg *config, args []string) error {
 			}
 
 			kmaster := &kubernetes.MasterConfig{
-				MasterIP:         masterIP,
-				MasterPort:       cfg.MasterAddr.Port,
-				NodeHosts:        cfg.NodeList,
-				PortalNet:        &portalNet,
-				EtcdHelper:       ketcdHelper,
-				KubeClient:       osmaster.KubeClient(),
-				Authorizer:       apiserver.NewAlwaysAllowAuthorizer(),
-				AdmissionControl: admit.NewAlwaysAdmit(),
+				MasterIP:             masterIP,
+				MasterPort:           cfg.MasterAddr.Port,
+				NodeHosts:            cfg.NodeList,
+				PortalNet:            &portalNet,
+				RequestContextMapper: requestContextMapper,
+				EtcdHelper:           ketcdHelper,
+				KubeClient:           osmaster.KubeClient(),
+				Authorizer:           apiserver.NewAlwaysAllowAuthorizer(),
+				AdmissionControl:     admit.NewAlwaysAdmit(),
 			}
 			kmaster.EnsurePortalFlags()
 
@@ -642,8 +642,8 @@ func newAuthorizer(etcdHelper tools.EtcdHelper, masterAuthorizationNamespace str
 	return authorizer
 }
 
-func newAuthorizationAttributeBuilder(requestsToUsers *authcontext.RequestContextMap) authorizer.AuthorizationAttributeBuilder {
-	authorizationAttributeBuilder := authorizer.NewAuthorizationAttributeBuilder(requestsToUsers, &authorizer.APIRequestInfoResolver{kutil.NewStringSet("api", "osapi"), latest.RESTMapper})
+func newAuthorizationAttributeBuilder(requestContextMapper kapi.RequestContextMapper) authorizer.AuthorizationAttributeBuilder {
+	authorizationAttributeBuilder := authorizer.NewAuthorizationAttributeBuilder(requestContextMapper, &authorizer.APIRequestInfoResolver{kutil.NewStringSet("api", "osapi"), latest.RESTMapper})
 	return authorizationAttributeBuilder
 }