Allow anonymous users to check their access

openshift · Jul 19, 2016 · 9f5bf70 · 9f5bf70
1 parent affbf42
commit 9f5bf70
Show file tree

Hide file tree

Showing 5 changed files with 192 additions and 32 deletions.
diff --git a/pkg/cmd/server/bootstrappolicy/constants.go b/pkg/cmd/server/bootstrappolicy/constants.go
@@ -51,15 +51,16 @@ const (
 
 // Roles
 const (
-	ClusterAdminRoleName    = "cluster-admin"
-	SudoerRoleName          = "sudoer"
-	ClusterReaderRoleName   = "cluster-reader"
-	AdminRoleName           = "admin"
-	EditRoleName            = "edit"
-	ViewRoleName            = "view"
-	SelfProvisionerRoleName = "self-provisioner"
-	BasicUserRoleName       = "basic-user"
-	StatusCheckerRoleName   = "cluster-status"
+	ClusterAdminRoleName       = "cluster-admin"
+	SudoerRoleName             = "sudoer"
+	ClusterReaderRoleName      = "cluster-reader"
+	AdminRoleName              = "admin"
+	EditRoleName               = "edit"
+	ViewRoleName               = "view"
+	SelfProvisionerRoleName    = "self-provisioner"
+	BasicUserRoleName          = "basic-user"
+	StatusCheckerRoleName      = "cluster-status"
+	SelfAccessReviewerRoleName = "self-access-reviewer"
 
 	RegistryAdminRoleName  = "registry-admin"
 	RegistryViewerRoleName = "registry-viewer"
@@ -97,29 +98,30 @@ const (
 
 // RoleBindings
 const (
-	SelfProvisionerRoleBindingName   = SelfProvisionerRoleName + "s"
-	DeployerRoleBindingName          = DeployerRoleName + "s"
-	ClusterAdminRoleBindingName      = ClusterAdminRoleName + "s"
-	ClusterReaderRoleBindingName     = ClusterReaderRoleName + "s"
-	BasicUserRoleBindingName         = BasicUserRoleName + "s"
-	OAuthTokenDeleterRoleBindingName = OAuthTokenDeleterRoleName + "s"
-	StatusCheckerRoleBindingName     = StatusCheckerRoleName + "-binding"
-	ImagePullerRoleBindingName       = ImagePullerRoleName + "s"
-	ImageBuilderRoleBindingName      = ImageBuilderRoleName + "s"
-	RouterRoleBindingName            = RouterRoleName + "s"
-	RegistryRoleBindingName          = RegistryRoleName + "s"
-	MasterRoleBindingName            = MasterRoleName + "s"
-	NodeRoleBindingName              = NodeRoleName + "s"
-	NodeProxierRoleBindingName       = NodeProxierRoleName + "s"
-	NodeAdminRoleBindingName         = NodeAdminRoleName + "s"
-	NodeReaderRoleBindingName        = NodeReaderRoleName + "s"
-	SDNReaderRoleBindingName         = SDNReaderRoleName + "s"
-	SDNManagerRoleBindingName        = SDNManagerRoleName + "s"
-	WebHooksRoleBindingName          = WebHooksRoleName + "s"
-	DiscoveryRoleBindingName         = DiscoveryRoleName + "-binding"
-	RegistryAdminRoleBindingName     = RegistryAdminRoleName + "s"
-	RegistryViewerRoleBindingName    = RegistryViewerRoleName + "s"
-	RegistryEditorRoleBindingName    = RegistryEditorRoleName + "s"
+	SelfAccessReviewerRoleBindingName = SelfAccessReviewerRoleName + "s"
+	SelfProvisionerRoleBindingName    = SelfProvisionerRoleName + "s"
+	DeployerRoleBindingName           = DeployerRoleName + "s"
+	ClusterAdminRoleBindingName       = ClusterAdminRoleName + "s"
+	ClusterReaderRoleBindingName      = ClusterReaderRoleName + "s"
+	BasicUserRoleBindingName          = BasicUserRoleName + "s"
+	OAuthTokenDeleterRoleBindingName  = OAuthTokenDeleterRoleName + "s"
+	StatusCheckerRoleBindingName      = StatusCheckerRoleName + "-binding"
+	ImagePullerRoleBindingName        = ImagePullerRoleName + "s"
+	ImageBuilderRoleBindingName       = ImageBuilderRoleName + "s"
+	RouterRoleBindingName             = RouterRoleName + "s"
+	RegistryRoleBindingName           = RegistryRoleName + "s"
+	MasterRoleBindingName             = MasterRoleName + "s"
+	NodeRoleBindingName               = NodeRoleName + "s"
+	NodeProxierRoleBindingName        = NodeProxierRoleName + "s"
+	NodeAdminRoleBindingName          = NodeAdminRoleName + "s"
+	NodeReaderRoleBindingName         = NodeReaderRoleName + "s"
+	SDNReaderRoleBindingName          = SDNReaderRoleName + "s"
+	SDNManagerRoleBindingName         = SDNManagerRoleName + "s"
+	WebHooksRoleBindingName           = WebHooksRoleName + "s"
+	DiscoveryRoleBindingName          = DiscoveryRoleName + "-binding"
+	RegistryAdminRoleBindingName      = RegistryAdminRoleName + "s"
+	RegistryViewerRoleBindingName     = RegistryViewerRoleName + "s"
+	RegistryEditorRoleBindingName     = RegistryEditorRoleName + "s"
 
 	BuildStrategyDockerRoleBindingName          = BuildStrategyDockerRoleName + "-binding"
 	BuildStrategyCustomRoleBindingName          = BuildStrategyCustomRoleName + "-binding"

diff --git a/pkg/cmd/server/bootstrappolicy/policy.go b/pkg/cmd/server/bootstrappolicy/policy.go
@@ -359,6 +359,15 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole {
 				{Verbs: sets.NewString("create"), APIGroups: []string{authzGroup}, Resources: sets.NewString("subjectaccessreviews", "localsubjectaccessreviews"), AttributeRestrictions: &authorizationapi.IsPersonalSubjectAccessReview{}},
 			},
 		},
+		{
+			ObjectMeta: kapi.ObjectMeta{
+				Name: SelfAccessReviewerRoleName,
+			},
+			Rules: []authorizationapi.PolicyRule{
+				authorizationapi.NewRule("create").Groups(authzGroup).Resources("selfsubjectrulesreviews").RuleOrDie(),
+				{Verbs: sets.NewString("create"), APIGroups: []string{authzGroup}, Resources: sets.NewString("subjectaccessreviews", "localsubjectaccessreviews"), AttributeRestrictions: &authorizationapi.IsPersonalSubjectAccessReview{}},
+			},
+		},
 		{
 			ObjectMeta: kapi.ObjectMeta{
 				Name: SelfProvisionerRoleName,
@@ -772,6 +781,18 @@ func GetBootstrapClusterRoleBindings() []authorizationapi.ClusterRoleBinding {
 			},
 			Subjects: []kapi.ObjectReference{{Kind: authorizationapi.SystemGroupKind, Name: AuthenticatedGroup}},
 		},
+		{
+			ObjectMeta: kapi.ObjectMeta{
+				Name: SelfAccessReviewerRoleBindingName,
+			},
+			RoleRef: kapi.ObjectReference{
+				Name: SelfAccessReviewerRoleName,
+			},
+			Subjects: []kapi.ObjectReference{
+				{Kind: authorizationapi.SystemGroupKind, Name: AuthenticatedGroup},
+				{Kind: authorizationapi.SystemGroupKind, Name: UnauthenticatedGroup},
+			},
+		},
 		{
 			ObjectMeta: kapi.ObjectMeta{
 				Name: SelfProvisionerRoleBindingName,

diff --git a/test/integration/authorization_test.go b/test/integration/authorization_test.go
@@ -21,6 +21,7 @@ import (
 	"github.com/openshift/origin/pkg/client"
 	policy "github.com/openshift/origin/pkg/cmd/admin/policy"
 	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
+	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
 	deployapi "github.com/openshift/origin/pkg/deploy/api"
 	imageapi "github.com/openshift/origin/pkg/image/api"
 	oauthapi "github.com/openshift/origin/pkg/oauth/api"
@@ -732,6 +733,22 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) {
 		t.Fatalf("error requesting token: %v", err)
 	}
 
+	anonymousConfig := clientcmd.AnonymousClientConfig(clusterAdminClientConfig)
+	anonymousClient, err := client.New(&anonymousConfig)
+	if err != nil {
+		t.Fatalf("error getting anonymous client: %v", err)
+	}
+
+	addAnonymous := &policy.RoleModificationOptions{
+		RoleNamespace:       "",
+		RoleName:            bootstrappolicy.EditRoleName,
+		RoleBindingAccessor: policy.NewLocalRoleBindingAccessor("hammer-project", clusterAdminClient),
+		Users:               []string{"system:anonymous"},
+	}
+	if err := addAnonymous.AddRole(); err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	addDanny := &policy.RoleModificationOptions{
 		RoleNamespace:       "",
 		RoleName:            bootstrappolicy.ViewRoleName,
@@ -774,6 +791,12 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) {
 		clusterReview:    askCanDannyGetProject,
 		err:              `User "danny" cannot create subjectaccessreviews at the cluster scope`,
 	}.run(t)
+	subjectAccessReviewTest{
+		description:      "as anonymous, can I make cluster subject access reviews",
+		clusterInterface: anonymousClient.SubjectAccessReviews(),
+		clusterReview:    askCanDannyGetProject,
+		err:              `User "system:anonymous" cannot create subjectaccessreviews at the cluster scope`,
+	}.run(t)
 
 	addValerie := &policy.RoleModificationOptions{
 		RoleNamespace:       "",
@@ -834,12 +857,32 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) {
 			Namespace: "mallet-project",
 		},
 	}.run(t)
+	// ensure unprivileged users cannot check other users' access
 	subjectAccessReviewTest{
 		description:    "harold denied ability to run subject access review in project mallet-project",
 		localInterface: haroldClient.LocalSubjectAccessReviews("mallet-project"),
 		localReview:    askCanEdgarDeletePods,
 		err:            `User "harold" cannot create localsubjectaccessreviews in project "mallet-project"`,
 	}.run(t)
+	subjectAccessReviewTest{
+		description:    "system:anonymous denied ability to run subject access review in project mallet-project",
+		localInterface: anonymousClient.LocalSubjectAccessReviews("mallet-project"),
+		localReview:    askCanEdgarDeletePods,
+		err:            `User "system:anonymous" cannot create localsubjectaccessreviews in project "mallet-project"`,
+	}.run(t)
+	// ensure message does not leak whether the namespace exists or not
+	subjectAccessReviewTest{
+		description:    "harold denied ability to run subject access review in project nonexistent-project",
+		localInterface: haroldClient.LocalSubjectAccessReviews("nonexistent-project"),
+		localReview:    askCanEdgarDeletePods,
+		err:            `User "harold" cannot create localsubjectaccessreviews in project "nonexistent-project"`,
+	}.run(t)
+	subjectAccessReviewTest{
+		description:    "system:anonymous denied ability to run subject access review in project nonexistent-project",
+		localInterface: anonymousClient.LocalSubjectAccessReviews("nonexistent-project"),
+		localReview:    askCanEdgarDeletePods,
+		err:            `User "system:anonymous" cannot create localsubjectaccessreviews in project "nonexistent-project"`,
+	}.run(t)
 
 	askCanHaroldUpdateProject := &authorizationapi.LocalSubjectAccessReview{
 		User:   "harold",
@@ -890,6 +933,61 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) {
 			Namespace: "hammer-project",
 		},
 	}.run(t)
+	subjectAccessReviewTest{
+		description:    "system:anonymous told he can create pods in project hammer-project",
+		localInterface: anonymousClient.LocalSubjectAccessReviews("hammer-project"),
+		localReview:    askCanICreatePods,
+		response: authorizationapi.SubjectAccessReviewResponse{
+			Allowed:   true,
+			Reason:    "allowed by rule in hammer-project",
+			Namespace: "hammer-project",
+		},
+	}.run(t)
+
+	// test checking self permissions when denied
+	subjectAccessReviewTest{
+		description:    "harold told he cannot create pods in project mallet-project",
+		localInterface: haroldClient.LocalSubjectAccessReviews("mallet-project"),
+		localReview:    askCanICreatePods,
+		response: authorizationapi.SubjectAccessReviewResponse{
+			Allowed:   false,
+			Reason:    `User "harold" cannot create pods in project "mallet-project"`,
+			Namespace: "mallet-project",
+		},
+	}.run(t)
+	subjectAccessReviewTest{
+		description:    "system:anonymous told he cannot create pods in project mallet-project",
+		localInterface: anonymousClient.LocalSubjectAccessReviews("mallet-project"),
+		localReview:    askCanICreatePods,
+		response: authorizationapi.SubjectAccessReviewResponse{
+			Allowed:   false,
+			Reason:    `User "system:anonymous" cannot create pods in project "mallet-project"`,
+			Namespace: "mallet-project",
+		},
+	}.run(t)
+
+	// test checking self-permissions doesn't leak whether namespace exists or not
+	subjectAccessReviewTest{
+		description:    "harold told he cannot create pods in project nonexistent-project",
+		localInterface: haroldClient.LocalSubjectAccessReviews("nonexistent-project"),
+		localReview:    askCanICreatePods,
+		response: authorizationapi.SubjectAccessReviewResponse{
+			Allowed:   false,
+			Reason:    `User "harold" cannot create pods in project "nonexistent-project"`,
+			Namespace: "nonexistent-project",
+		},
+	}.run(t)
+	subjectAccessReviewTest{
+		description:    "system:anonymous told he cannot create pods in project nonexistent-project",
+		localInterface: anonymousClient.LocalSubjectAccessReviews("nonexistent-project"),
+		localReview:    askCanICreatePods,
+		response: authorizationapi.SubjectAccessReviewResponse{
+			Allowed:   false,
+			Reason:    `User "system:anonymous" cannot create pods in project "nonexistent-project"`,
+			Namespace: "nonexistent-project",
+		},
+	}.run(t)
+
 	askCanICreatePolicyBindings := &authorizationapi.LocalSubjectAccessReview{
 		Action: authorizationapi.AuthorizationAttributes{Verb: "create", Resource: "policybindings"},
 	}

diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_role_bindings.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_role_bindings.yaml
@@ -71,6 +71,22 @@ items:
   - kind: SystemGroup
     name: system:authenticated
   userNames: null
+- apiVersion: v1
+  groupNames:
+  - system:authenticated
+  - system:unauthenticated
+  kind: ClusterRoleBinding
+  metadata:
+    creationTimestamp: null
+    name: self-access-reviewers
+  roleRef:
+    name: self-access-reviewer
+  subjects:
+  - kind: SystemGroup
+    name: system:authenticated
+  - kind: SystemGroup
+    name: system:unauthenticated
+  userNames: null
 - apiVersion: v1
   groupNames:
   - system:authenticated:oauth

diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml
@@ -1297,6 +1297,29 @@ items:
     - subjectaccessreviews
     verbs:
     - create
+- apiVersion: v1
+  kind: ClusterRole
+  metadata:
+    creationTimestamp: null
+    name: self-access-reviewer
+  rules:
+  - apiGroups:
+    - ""
+    attributeRestrictions: null
+    resources:
+    - selfsubjectrulesreviews
+    verbs:
+    - create
+  - apiGroups:
+    - ""
+    attributeRestrictions:
+      apiVersion: v1
+      kind: IsPersonalSubjectAccessReview
+    resources:
+    - localsubjectaccessreviews
+    - subjectaccessreviews
+    verbs:
+    - create
 - apiVersion: v1
   kind: ClusterRole
   metadata: