diff --git a/pkg/authorization/authorizer/attributes_builder.go b/pkg/authorization/authorizer/attributes_builder.go index 333ab2dd6991..a6eff957eb96 100644 --- a/pkg/authorization/authorizer/attributes_builder.go +++ b/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, diff --git a/pkg/cmd/experimental/policy/remove_group.go b/pkg/cmd/experimental/policy/remove_group.go index 6f39482b2ecb..4be19f1cd833 100644 --- a/pkg/cmd/experimental/policy/remove_group.go +++ b/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 [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 } diff --git a/pkg/cmd/experimental/project/new_project.go b/pkg/cmd/experimental/project/new_project.go index 08f2fef2cf91..da90cd1b4514 100644 --- a/pkg/cmd/experimental/project/new_project.go +++ b/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 + " ", @@ -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 } } diff --git a/pkg/cmd/server/command.go b/pkg/cmd/server/command.go index ec65cc838093..d9c115a4a46d 100644 --- a/pkg/cmd/server/command.go +++ b/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) } }, diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go index fab013de6478..b37dc35fd3fd 100644 --- a/pkg/cmd/server/origin/master.go +++ b/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) + }) +} diff --git a/pkg/cmd/server/start.go b/pkg/cmd/server/start.go index 4be26ff9f0a8..917ba58a6677 100644 --- a/pkg/cmd/server/start.go +++ b/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 } diff --git a/test/integration/authorization_test.go b/test/integration/authorization_test.go new file mode 100644 index 000000000000..30bf76d24c43 --- /dev/null +++ b/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) + } +} diff --git a/test/integration/test_server.go b/test/integration/test_server.go new file mode 100644 index 000000000000..2dfbf44c04ea --- /dev/null +++ b/test/integration/test_server.go @@ -0,0 +1,133 @@ +// +build integration,!no-etcd + +package integration + +import ( + "fmt" + "net/http/httptest" + "os" + "path" + "time" + + kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + + "github.com/openshift/origin/pkg/client" + newproject "github.com/openshift/origin/pkg/cmd/experimental/project" + start "github.com/openshift/origin/pkg/cmd/server" + cmdutil "github.com/openshift/origin/pkg/cmd/util" + "github.com/openshift/origin/pkg/cmd/util/tokencmd" +) + +func init() { + requireEtcd() +} + +func StartTestServer(args ...string) (start.Config, error) { + deleteAllEtcdKeys() + + startConfig := start.NewDefaultConfig() + + basedir := path.Join(os.TempDir(), "openshift-integration-tests") + + startConfig.VolumeDir = path.Join(basedir, "volume") + startConfig.EtcdDir = path.Join(basedir, "etcd") + startConfig.CertDir = path.Join(basedir, "cert") + + masterAddr := httptest.NewUnstartedServer(nil).Listener.Addr().String() + fmt.Printf("masterAddr: %#v\n", masterAddr) + + startConfig.MasterAddr.Set(masterAddr) + startConfig.BindAddr.Set(masterAddr) + startConfig.EtcdAddr.Set(getEtcdURL()) + + startConfig.Complete(args) + + var startError error + go func() { + err := startConfig.Start(args) + if err != nil { + startError = err + fmt.Printf("ERROR STARTING SERVER! %v", err) + } + }() + + // wait for the server to come up: 35 seconds + if err := cmdutil.WaitForSuccessfulDial(true, "tcp", masterAddr, 100*time.Millisecond, 100*time.Millisecond, 175); err != nil { + return *startConfig, err + } + + stopChannel := make(chan struct{}) + util.Until( + func() { + if startError != nil { + close(stopChannel) + return + } + + // confirm that we can actually query from the api server + client, _, err := startConfig.GetOpenshiftClient() + if err != nil { + return + } + if _, err := client.Policies("master").List(labels.Everything(), labels.Everything()); err == nil { + close(stopChannel) + } + }, 100*time.Millisecond, stopChannel) + + return *startConfig, startError +} + +// StartTestMaster starts up a test master and returns back the startConfig so you can get clients and certs +func StartTestMaster() (start.Config, error) { + return StartTestServer("master") +} + +// StartTestNode starts up a test node and returns back the startConfig so you can get clients and certs +func StartTestNode() (start.Config, error) { + return StartTestServer("node") +} + +// StartTestAllInOne starts up a test all-in-one and returns back the startConfig so you can get clients and certs +func StartTestAllInOne() (start.Config, error) { + return StartTestServer() +} + +// CreateNewProject creates a new project using the clusterAdminClient, then gets a token for the adminUser and returns +// back a client for the admin user +func CreateNewProject(clusterAdminClient *client.Client, clientConfig kclient.Config, projectName, adminUser string) (*client.Client, error) { + qualifiedUser := "anypassword:" + adminUser + newProjectOptions := &newproject.NewProjectOptions{ + Client: clusterAdminClient, + ProjectName: projectName, + AdminRole: "admin", + MasterPolicyNamespace: "master", + AdminUser: qualifiedUser, + } + + if err := newProjectOptions.Run(); err != nil { + return nil, err + } + + token, err := tokencmd.RequestToken(&clientConfig, nil, adminUser, "password") + if err != nil { + return nil, err + } + + adminClientConfig := clientConfig + adminClientConfig.BearerToken = token + adminClientConfig.Username = "" + adminClientConfig.Password = "" + adminClientConfig.TLSClientConfig.CertFile = "" + adminClientConfig.TLSClientConfig.KeyFile = "" + adminClientConfig.TLSClientConfig.CertData = nil + adminClientConfig.TLSClientConfig.KeyData = nil + + adminClient, err := client.New(&adminClientConfig) + if err != nil { + return nil, err + } + + return adminClient, nil +} diff --git a/test/integration/utils.go b/test/integration/utils.go index b99ab6165d36..738411ac1218 100644 --- a/test/integration/utils.go +++ b/test/integration/utils.go @@ -22,14 +22,18 @@ const ( ) func newEtcdClient() *etcd.Client { - etcdServers := []string{"http://127.0.0.1:4001"} + etcdServers := []string{getEtcdURL()} + return etcd.NewClient(etcdServers) +} + +func getEtcdURL() string { etcdFromEnv := os.Getenv("ETCD_SERVER") if len(etcdFromEnv) > 0 { - etcdServers = []string{etcdFromEnv} + return etcdFromEnv } - return etcd.NewClient(etcdServers) + return "http://127.0.0.1:4001" } func init() {