add context namespacing filter (Bug 1196022)

pkg/authorization/authorizer/attributes_builder.go

@@ -36,12 +36,6 @@ func (a *openshiftAuthorizationAttributeBuilder) GetAttributes(req *http.Request
 		return nil, err
 	}
 
-	// TODO reconsider special casing this.  Having the special case hereallow us to fully share the kube
-	// APIRequestInfoResolver without any modification or customization.
-	if (requestInfo.Resource == "projects") && (len(requestInfo.Name) > 0) {
-		requestInfo.Namespace = requestInfo.Name
-	}
-
 	return DefaultAuthorizationAttributes{
 		Verb:              requestInfo.Verb,
 		Resource:          requestInfo.Resource,

pkg/cmd/experimental/policy/remove_group.go

@@ -10,17 +10,17 @@ import (
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
 )
 
-type removeGroupOptions struct {
-	roleNamespace    string
-	roleName         string
-	bindingNamespace string
-	client           client.Interface
+type RemoveGroupOptions struct {
+	RoleNamespace    string
+	RoleName         string
+	BindingNamespace string
+	Client           client.Interface
 
-	groups []string
+	Groups []string
 }
 
 func NewCmdRemoveGroup(f *clientcmd.Factory) *cobra.Command {
-	options := &removeGroupOptions{}
+	options := &RemoveGroupOptions{}
 
 	cmd := &cobra.Command{
 		Use:   "remove-group <role> <group> [group]...",
@@ -32,48 +32,48 @@ func NewCmdRemoveGroup(f *clientcmd.Factory) *cobra.Command {
 			}
 
 			var err error
-			if options.client, _, err = f.Clients(cmd); err != nil {
+			if options.Client, _, err = f.Clients(cmd); err != nil {
 				glog.Fatalf("Error getting client: %v", err)
 			}
-			if options.bindingNamespace, err = f.DefaultNamespace(cmd); err != nil {
+			if options.BindingNamespace, err = f.DefaultNamespace(cmd); err != nil {
 				glog.Fatalf("Error getting client: %v", err)
 			}
-			if err := options.run(); err != nil {
+			if err := options.Run(); err != nil {
 				glog.Fatal(err)
 			}
 		},
 	}
 
-	cmd.Flags().StringVar(&options.roleNamespace, "role-namespace", "master", "namespace where the role is located.")
+	cmd.Flags().StringVar(&options.RoleNamespace, "role-namespace", "master", "namespace where the role is located.")
 
 	return cmd
 }
 
-func (o *removeGroupOptions) complete(cmd *cobra.Command) bool {
+func (o *RemoveGroupOptions) complete(cmd *cobra.Command) bool {
 	args := cmd.Flags().Args()
 	if len(args) < 2 {
 		cmd.Help()
 		return false
 	}
 
-	o.roleName = args[0]
-	o.groups = args[1:]
+	o.RoleName = args[0]
+	o.Groups = args[1:]
 	return true
 }
 
-func (o *removeGroupOptions) run() error {
-	roleBindings, err := getExistingRoleBindingsForRole(o.roleNamespace, o.roleName, o.client.PolicyBindings(o.bindingNamespace))
+func (o *RemoveGroupOptions) Run() error {
+	roleBindings, err := getExistingRoleBindingsForRole(o.RoleNamespace, o.RoleName, o.Client.PolicyBindings(o.BindingNamespace))
 	if err != nil {
 		return err
 	}
 	if len(roleBindings) == 0 {
-		return fmt.Errorf("unable to locate RoleBinding for %v::%v in %v", o.roleNamespace, o.roleName, o.bindingNamespace)
+		return fmt.Errorf("unable to locate RoleBinding for %v::%v in %v", o.RoleNamespace, o.RoleName, o.BindingNamespace)
 	}
 
 	for _, roleBinding := range roleBindings {
-		roleBinding.Groups.Delete(o.groups...)
+		roleBinding.Groups.Delete(o.Groups...)
 
-		_, err = o.client.RoleBindings(o.bindingNamespace).Update(roleBinding)
+		_, err = o.Client.RoleBindings(o.BindingNamespace).Update(roleBinding)
 		if err != nil {
 			return err
 		}

pkg/cmd/experimental/project/new_project.go

@@ -15,20 +15,20 @@ import (
 	projectapi "github.com/openshift/origin/pkg/project/api"
 )
 
-type newProjectOptions struct {
-	projectName string
-	displayName string
-	description string
+type NewProjectOptions struct {
+	ProjectName string
+	DisplayName string
+	Description string
 
-	client client.Interface
+	Client client.Interface
 
-	adminRole             string
-	masterPolicyNamespace string
-	adminUser             string
+	AdminRole             string
+	MasterPolicyNamespace string
+	AdminUser             string
 }
 
 func NewCmdNewProject(f *clientcmd.Factory, parentName, name string) *cobra.Command {
-	options := &newProjectOptions{}
+	options := &NewProjectOptions{}
 
 	cmd := &cobra.Command{
 		Use:   name + " <project-name>",
@@ -40,68 +40,68 @@ func NewCmdNewProject(f *clientcmd.Factory, parentName, name string) *cobra.Comm
 			}
 
 			var err error
-			if options.client, _, err = f.Clients(cmd); err != nil {
+			if options.Client, _, err = f.Clients(cmd); err != nil {
 				glog.Fatalf("Error getting client: %v", err)
 			}
-			if err := options.run(); err != nil {
+			if err := options.Run(); err != nil {
 				glog.Fatal(err)
 			}
 		},
 	}
 
 	// TODO remove once we have global policy objects
-	cmd.Flags().StringVar(&options.masterPolicyNamespace, "master-policy-namespace", "master", "master policy namespace")
-	cmd.Flags().StringVar(&options.adminRole, "admin-role", "admin", "project admin role name in the master policy namespace")
-	cmd.Flags().StringVar(&options.adminUser, "admin", "", "project admin username")
-	cmd.Flags().StringVar(&options.displayName, "display-name", "", "project display name")
-	cmd.Flags().StringVar(&options.description, "description", "", "project description")
+	cmd.Flags().StringVar(&options.MasterPolicyNamespace, "master-policy-namespace", "master", "master policy namespace")
+	cmd.Flags().StringVar(&options.AdminRole, "admin-role", "admin", "project admin role name in the master policy namespace")
+	cmd.Flags().StringVar(&options.AdminUser, "admin", "", "project admin username")
+	cmd.Flags().StringVar(&options.DisplayName, "display-name", "", "project display name")
+	cmd.Flags().StringVar(&options.Description, "description", "", "project description")
 
 	return cmd
 }
 
-func (o *newProjectOptions) complete(cmd *cobra.Command) bool {
+func (o *NewProjectOptions) complete(cmd *cobra.Command) bool {
 	args := cmd.Flags().Args()
 	if len(args) != 1 {
 		cmd.Help()
 		return false
 	}
 
-	o.projectName = args[0]
+	o.ProjectName = args[0]
 
 	return true
 }
 
-func (o *newProjectOptions) run() error {
-	if _, err := o.client.Projects().Get(o.projectName); err != nil {
+func (o *NewProjectOptions) Run() error {
+	if _, err := o.Client.Projects().Get(o.ProjectName); err != nil {
 		if !kerrors.IsNotFound(err) {
 			return err
 		}
 	} else {
-		return fmt.Errorf("project %v already exists", o.projectName)
+		return fmt.Errorf("project %v already exists", o.ProjectName)
 	}
 
 	project := &projectapi.Project{}
-	project.Name = o.projectName
-	project.DisplayName = o.displayName
+	project.Name = o.ProjectName
+	project.DisplayName = o.DisplayName
 	project.Annotations = make(map[string]string)
-	project.Annotations["description"] = o.description
-	project, err := o.client.Projects().Create(project)
+	project.Annotations["description"] = o.Description
+	project, err := o.Client.Projects().Create(project)
 	if err != nil {
 		return err
 	}
 
-	if len(o.adminUser) != 0 {
+	if len(o.AdminUser) != 0 {
 		adminRoleBinding := &authorizationapi.RoleBinding{}
 
 		adminRoleBinding.Name = "admins"
-		adminRoleBinding.RoleRef.Namespace = o.masterPolicyNamespace
-		adminRoleBinding.RoleRef.Name = o.adminRole
-		adminRoleBinding.Users = util.NewStringSet(o.adminUser)
+		adminRoleBinding.RoleRef.Namespace = o.MasterPolicyNamespace
+		adminRoleBinding.RoleRef.Name = o.AdminRole
+		adminRoleBinding.Users = util.NewStringSet(o.AdminUser)
 
-		_, err := o.client.RoleBindings(project.Name).Create(adminRoleBinding)
+		_, err := o.Client.RoleBindings(project.Name).Create(adminRoleBinding)
 		if err != nil {
-			fmt.Printf("The project %v was created, but %v could not be added to the %v role.\n", o.projectName, o.adminUser, o.adminRole)
-			fmt.Printf("To add the user to the existing project, run\n\n\topenshift ex policy add-user --namespace=%v --role-namespace=%v %v %v\n", o.projectName, o.masterPolicyNamespace, o.adminRole, o.adminUser)
+			fmt.Printf("The project %v was created, but %v could not be added to the %v role.\n", o.ProjectName, o.AdminUser, o.AdminRole)
+			fmt.Printf("To add the user to the existing project, run\n\n\topenshift ex policy add-user --namespace=%v --role-namespace=%v %v %v\n", o.ProjectName, o.MasterPolicyNamespace, o.AdminRole, o.AdminUser)
 			return err
 		}
 	}

pkg/cmd/server/command.go

@@ -57,7 +57,7 @@ func NewCommandStartServer(name string) (*cobra.Command, *Config) {
 
 			cfg.Complete(args)
 
-			if err := start(*cfg, args); err != nil {
+			if err := cfg.Start(args); err != nil {
 				glog.Fatal(err)
 			}
 		},

pkg/cmd/server/origin/master.go

@@ -304,6 +304,7 @@ func (c *MasterConfig) Run(protected []APIInstaller, unprotected []APIInstaller)
 	}
 	handler := c.authorizationFilter(safe)
 	handler = authenticationHandlerFilter(handler, c.Authenticator, c.getRequestContextMapper())
+	handler = namespacingFilter(handler, c.getRequestContextMapper())
 
 	// unprotected resources
 	unprotected = append(unprotected, APIInstallFunc(c.InstallUnprotectedAPI))
@@ -693,3 +694,38 @@ type clientDeploymentInterface struct {
 func (c clientDeploymentInterface) GetDeployment(ctx api.Context, name string) (*api.ReplicationController, error) {
 	return c.KubeClient.ReplicationControllers(api.NamespaceValue(ctx)).Get(name)
 }
+
+// namespacingFilter adds a filter that adds the namespace of the request to the context.  Not all requests will have namespaces,
+// but any that do will have the appropriate value added.
+func namespacingFilter(handler http.Handler, contextMapper kapi.RequestContextMapper) http.Handler {
+	infoResolver := &apiserver.APIRequestInfoResolver{util.NewStringSet("api", "osapi"), latest.RESTMapper}
+
+	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		ctx, ok := contextMapper.Get(req)
+		if !ok {
+			http.Error(w, "Unable to find request context", http.StatusInternalServerError)
+			return
+		}
+
+		if _, exists := kapi.NamespaceFrom(ctx); !exists {
+			if requestInfo, err := infoResolver.GetAPIRequestInfo(req); err == nil {
+				// only set the namespace if the apiRequestInfo was resolved
+				// keep in mind that GetAPIRequestInfo will fail on non-api requests, so don't fail the entire http request on that
+				// kind of failure.
+
+				// TODO reconsider special casing this.  Having the special case hereallow us to fully share the kube
+				// APIRequestInfoResolver without any modification or customization.
+				namespace := requestInfo.Namespace
+				if (requestInfo.Resource == "projects") && (len(requestInfo.Name) > 0) {
+					namespace = requestInfo.Name
+				}
+
+				ctx = kapi.WithNamespace(ctx, namespace)
+				contextMapper.Update(req, ctx)
+				glog.V(2).Infof("set namespace on context to %v", requestInfo.Namespace)
+			}
+		}
+
+		handler.ServeHTTP(w, req)
+	})
+}

pkg/cmd/server/start.go

@@ -107,7 +107,7 @@ func (cfg Config) startMaster() error {
 }
 
 // run launches the appropriate startup modes or returns an error.
-func start(cfg Config, args []string) error {
+func (cfg Config) Start(args []string) error {
 	if cfg.WriteConfigOnly {
 		return nil
 	}

test/integration/authorization_test.go

@@ -0,0 +1,88 @@
+// +build integration,!no-etcd
+
+package integration
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
+
+	policy "github.com/openshift/origin/pkg/cmd/experimental/policy"
+)
+
+func TestRestrictedAccessForProjectAdmins(t *testing.T) {
+	startConfig, err := StartTestMaster()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	openshiftClient, openshiftClientConfig, err := startConfig.GetOpenshiftClient()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	// TODO remove once bootstrap authorization rules are tightened
+	removeInsecureOptions := &policy.RemoveGroupOptions{
+		RoleNamespace:    "master",
+		RoleName:         "cluster-admin",
+		BindingNamespace: "master",
+		Client:           openshiftClient,
+		Groups:           []string{"system:authenticated", "system:unauthenticated"},
+	}
+	if err := removeInsecureOptions.Run(); err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	haroldClient, err := CreateNewProject(openshiftClient, *openshiftClientConfig, "hammer-project", "harold")
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	markClient, err := CreateNewProject(openshiftClient, *openshiftClientConfig, "mallet-project", "mark")
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	_, err = haroldClient.Deployments("hammer-project").List(labels.Everything(), labels.Everything())
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	// TODO make kube and origin authorization failures cause a kapierror.Forbidden
+	_, err = markClient.Deployments("hammer-project").List(labels.Everything(), labels.Everything())
+	if (err == nil) || (!strings.Contains(err.Error(), "Forbidden")) {
+		t.Errorf("expected forbidden error, but didn't get one")
+	}
+
+	// projects are a special case where a get of a project actually sets a namespace.  Make sure that
+	// the namespace is properly special cased and set for authorization rules
+	_, err = haroldClient.Projects().Get("hammer-project")
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	// TODO make kube and origin authorization failures cause a kapierror.Forbidden
+	_, err = markClient.Projects().Get("hammer-project")
+	if (err == nil) || (!strings.Contains(err.Error(), "Forbidden")) {
+		t.Errorf("expected forbidden error, but didn't get one")
+	}
+
+	// wait for the project authorization cache to catch the change.  It is on a one second period
+	time.Sleep(2 * time.Second)
+
+	haroldProjects, err := haroldClient.Projects().List(labels.Everything(), labels.Everything())
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if !((len(haroldProjects.Items) == 1) && (haroldProjects.Items[0].Name == "hammer-project")) {
+		t.Errorf("expected hammer-project, got %#v", haroldProjects.Items)
+	}
+
+	markProjects, err := markClient.Projects().List(labels.Everything(), labels.Everything())
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if !((len(markProjects.Items) == 1) && (markProjects.Items[0].Name == "mallet-project")) {
+		t.Errorf("expected mallet-project, got %#v", markProjects.Items)
+	}
+}