diff --git a/pkg/auth/context/requestcontext.go b/pkg/auth/context/requestcontext.go deleted file mode 100644 index 152974397f65..000000000000 --- a/pkg/auth/context/requestcontext.go +++ /dev/null @@ -1,36 +0,0 @@ -package context - -import ( - "net/http" - "sync" -) - -type RequestContextMap struct { - objs map[*http.Request]interface{} - lock sync.Mutex -} - -func NewRequestContextMap() *RequestContextMap { - return &RequestContextMap{ - objs: make(map[*http.Request]interface{}), - } -} - -func (c *RequestContextMap) Set(req *http.Request, obj interface{}) { - c.lock.Lock() - defer c.lock.Unlock() - c.objs[req] = obj -} - -func (c *RequestContextMap) Remove(req *http.Request) { - c.lock.Lock() - defer c.lock.Unlock() - delete(c.objs, req) -} - -func (c *RequestContextMap) Get(req *http.Request) (interface{}, bool) { - c.lock.Lock() - defer c.lock.Unlock() - obj, ok := c.objs[req] - return obj, ok -} diff --git a/pkg/authorization/authorizer/authorizer.go b/pkg/authorization/authorizer/authorizer.go index 4a28a63974d1..f35e9011212c 100644 --- a/pkg/authorization/authorizer/authorizer.go +++ b/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{ diff --git a/pkg/cmd/server/kubernetes/master.go b/pkg/cmd/server/kubernetes/master.go index 307cb4d238fd..df30be544754 100644 --- a/pkg/cmd/server/kubernetes/master.go +++ b/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, diff --git a/pkg/cmd/server/origin/auth.go b/pkg/cmd/server/origin/auth.go index e7781822089a..1fd43e390d3e 100644 --- a/pkg/cmd/server/origin/auth.go +++ b/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) }) diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go index 618f02bc60fb..5dda133d67d4 100644 --- a/pkg/cmd/server/origin/master.go +++ b/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 diff --git a/pkg/cmd/server/start.go b/pkg/cmd/server/start.go index 005acc29f32f..c8dcf4da8e6e 100644 --- a/pkg/cmd/server/start.go +++ b/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 } diff --git a/pkg/user/registry/user/filteruser.go b/pkg/user/registry/user/filteruser.go deleted file mode 100644 index 1cecdfd57233..000000000000 --- a/pkg/user/registry/user/filteruser.go +++ /dev/null @@ -1,63 +0,0 @@ -package user - -import ( - "net/http" - "path" -) - -type Info interface { - GetName() string - GetUID() string -} - -// mux is an object that can register http handlers. -type mux interface { - Handle(pattern string, handler http.Handler) - HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request)) -} - -type Context interface { - Get(*http.Request) (Info, bool) -} - -type ContextFunc func(*http.Request) (Info, bool) - -func (f ContextFunc) Get(req *http.Request) (Info, bool) { - return f(req) -} - -func NewCurrentContextFilter(requestPath string, context Context, handler http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - if req.URL.Path != requestPath { - handler.ServeHTTP(w, req) - return - } - - user, found := context.Get(req) - if !found { - http.Error(w, "Need to be authenticated to access this method", http.StatusUnauthorized) - return - } - - base := path.Dir(req.URL.Path) - req.URL.Path = path.Join(base, user.GetName()) - handler.ServeHTTP(w, req) - }) -} - -// InstallThisUser registers the APIServer log support function into a mux. -func InstallThisUser(mux mux, endpoint string, requestsToUsers Context, apiHandler http.Handler) { - mux.HandleFunc(endpoint, - func(w http.ResponseWriter, req *http.Request) { - user, found := requestsToUsers.Get(req) - if !found { - http.Error(w, "Need to be authenticated to access this method", http.StatusUnauthorized) - return - } - - base := path.Dir(req.URL.Path) - req.URL.Path = path.Join(base, user.GetName()) - apiHandler.ServeHTTP(w, req) - }, - ) -} diff --git a/pkg/user/registry/user/rest.go b/pkg/user/registry/user/rest.go index 25a8c0077767..d560558424e8 100644 --- a/pkg/user/registry/user/rest.go +++ b/pkg/user/registry/user/rest.go @@ -1,7 +1,10 @@ package user import ( + "errors" + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kerrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" @@ -25,5 +28,13 @@ func (s *REST) New() runtime.Object { // Get retrieves an UserIdentityMapping by id. func (s *REST) Get(ctx kapi.Context, id string) (runtime.Object, error) { + // "~" means the currently authenticated user + if id == "~" { + user, ok := kapi.UserFrom(ctx) + if !ok || user.GetName() == "" { + return nil, kerrs.NewForbidden("user", "~", errors.New("Requests to ~ must be authenticated")) + } + id = user.GetName() + } return s.registry.GetUser(id) } diff --git a/test/integration/userclient_test.go b/test/integration/userclient_test.go index c4f10ed93898..0ea94d1270d8 100644 --- a/test/integration/userclient_test.go +++ b/test/integration/userclient_test.go @@ -20,7 +20,6 @@ import ( "github.com/openshift/origin/pkg/api/latest" "github.com/openshift/origin/pkg/api/v1beta1" oapauth "github.com/openshift/origin/pkg/auth/authenticator/password/oauthpassword/registry" - "github.com/openshift/origin/pkg/auth/context" "github.com/openshift/origin/pkg/client" "github.com/openshift/origin/pkg/user" "github.com/openshift/origin/pkg/user/api" @@ -154,26 +153,36 @@ func TestUserLookup(t *testing.T) { userInfo := &kuser.DefaultInfo{ Name: ":test", } - userContext := context.NewRequestContextMap() - userContextFunc := userregistry.ContextFunc(func(req *http.Request) (userregistry.Info, bool) { - obj, found := userContext.Get(req) - if user, ok := obj.(kuser.Info); found && ok { - return user, true - } - return nil, false - }) + contextMapper := kapi.NewRequestContextMapper() storage := map[string]apiserver.RESTStorage{ "userIdentityMappings": useridentitymapping.NewREST(userRegistry), "users": userregistry.NewREST(userRegistry), } - apihandler := apiserver.Handle(storage, interfaces.Codec, "/osapi", "v1beta1", interfaces.MetadataAccessor, admit.NewAlwaysAdmit(), kapi.NewRequestContextMapper(), latest.RESTMapper) - apihandler = userregistry.NewCurrentContextFilter("/osapi/v1beta1/users/~", userContextFunc, apihandler) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - userContext.Set(req, userInfo) + apihandler := apiserver.Handle(storage, interfaces.Codec, "/osapi", "v1beta1", interfaces.MetadataAccessor, admit.NewAlwaysAdmit(), contextMapper, latest.RESTMapper) + + // Wrap with authenticator + authenticatedHandler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + ctx, ok := contextMapper.Get(req) + if !ok { + t.Fatalf("No context on request") + return + } + if err := contextMapper.Update(req, kapi.WithUser(ctx, userInfo)); err != nil { + t.Fatalf("Could not set user on request") + return + } apihandler.ServeHTTP(w, req) - })) + }) + + // Wrap with contextmapper + contextHandler, err := kapi.NewRequestContextFilter(contextMapper, authenticatedHandler) + if err != nil { + t.Fatalf("Could not create context filter") + } + + server := httptest.NewServer(contextHandler) mapping := api.UserIdentityMapping{ Identity: api.Identity{