diff --git a/pkg/authorization/authorizer/authorizer.go b/pkg/authorization/authorizer/authorizer.go deleted file mode 100644 index 9ee04860a4b2..000000000000 --- a/pkg/authorization/authorizer/authorizer.go +++ /dev/null @@ -1,49 +0,0 @@ -package authorizer - -import ( - "errors" - - "k8s.io/apiserver/pkg/authorization/authorizer" -) - -type openshiftAuthorizer struct { - delegate authorizer.Authorizer - forbiddenMessageMaker ForbiddenMessageMaker -} - -func NewAuthorizer(delegate authorizer.Authorizer, forbiddenMessageMaker ForbiddenMessageMaker) authorizer.Authorizer { - return &openshiftAuthorizer{delegate: delegate, forbiddenMessageMaker: forbiddenMessageMaker} -} - -func (a *openshiftAuthorizer) Authorize(attributes authorizer.Attributes) (authorizer.Decision, string, error) { - if attributes.GetUser() == nil { - return authorizer.DecisionNoOpinion, "", errors.New("no user available on context") - } - authorizationDecision, delegateReason, err := a.delegate.Authorize(attributes) - if authorizationDecision == authorizer.DecisionAllow { - return authorizer.DecisionAllow, reason(attributes), nil - } - // errors are allowed to occur - if err != nil { - return authorizationDecision, "", err - } - - denyReason, err := a.forbiddenMessageMaker.MakeMessage(attributes) - if err != nil { - denyReason = err.Error() - } - if len(delegateReason) > 0 { - denyReason += ": " + delegateReason - } - - return authorizationDecision, denyReason, nil -} - -func reason(attributes authorizer.Attributes) string { - if len(attributes.GetNamespace()) == 0 { - return "allowed by cluster rule" - } - // not 100% accurate, because the rule may have been provided by a cluster rule. we no longer have - // this distinction upstream in practice. - return "allowed by openshift authorizer" -} diff --git a/pkg/authorization/authorizer/interfaces.go b/pkg/authorization/authorizer/interfaces.go deleted file mode 100644 index 04a14eae26c2..000000000000 --- a/pkg/authorization/authorizer/interfaces.go +++ /dev/null @@ -1,10 +0,0 @@ -package authorizer - -import ( - "k8s.io/apiserver/pkg/authorization/authorizer" -) - -// ForbiddenMessageMaker creates a forbidden message from Attributes -type ForbiddenMessageMaker interface { - MakeMessage(attrs authorizer.Attributes) (string, error) -} diff --git a/pkg/authorization/authorizer/messages.go b/pkg/authorization/authorizer/messages.go deleted file mode 100644 index 7db853ee9285..000000000000 --- a/pkg/authorization/authorizer/messages.go +++ /dev/null @@ -1,128 +0,0 @@ -package authorizer - -import ( - "bytes" - "text/template" - - "k8s.io/apiserver/pkg/authorization/authorizer" - - authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" -) - -const DefaultProjectRequestForbidden = "You may not request a new project via this API." - -type ForbiddenMessageResolver struct { - // TODO if these maps were map[string]map[sets.String]ForbiddenMessageMaker, we'd be able to handle cases where sets of resources wanted slightly different messages - // unfortunately, maps don't support keys like that, requiring sets.String serialization and deserialization. - namespacedVerbsToResourcesToForbiddenMessageMaker map[string]map[string]ForbiddenMessageMaker - rootScopedVerbsToResourcesToForbiddenMessageMaker map[string]map[string]ForbiddenMessageMaker - - nonResourceURLForbiddenMessageMaker ForbiddenMessageMaker - defaultForbiddenMessageMaker ForbiddenMessageMaker -} - -func NewForbiddenMessageResolver(projectRequestForbiddenTemplate string) *ForbiddenMessageResolver { - apiGroupIfNotEmpty := "{{if len .GetAPIGroup }}.{{.GetAPIGroup}}{{end}}" - resourceWithSubresourceIfNotEmpty := "{{if len .GetSubresource }}{{.GetResource}}/{{.GetSubresource}}{{else}}{{.GetResource}}{{end}}" - - messageResolver := &ForbiddenMessageResolver{ - namespacedVerbsToResourcesToForbiddenMessageMaker: map[string]map[string]ForbiddenMessageMaker{}, - rootScopedVerbsToResourcesToForbiddenMessageMaker: map[string]map[string]ForbiddenMessageMaker{}, - nonResourceURLForbiddenMessageMaker: newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot "{{.GetVerb}}" on "{{.GetPath}}"`), - defaultForbiddenMessageMaker: newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot "{{.GetVerb}}" "` + resourceWithSubresourceIfNotEmpty + apiGroupIfNotEmpty + `" with name "{{.GetName}}" in project "{{.GetNamespace}}"`), - } - - // general messages - messageResolver.addNamespacedForbiddenMessageMaker("create", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot create `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` in project "{{.GetNamespace}}"`)) - messageResolver.addRootScopedForbiddenMessageMaker("create", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot create `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` at the cluster scope`)) - messageResolver.addNamespacedForbiddenMessageMaker("get", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot get `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` in project "{{.GetNamespace}}"`)) - messageResolver.addRootScopedForbiddenMessageMaker("get", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot get `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` at the cluster scope`)) - messageResolver.addNamespacedForbiddenMessageMaker("list", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot list `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` in project "{{.GetNamespace}}"`)) - messageResolver.addRootScopedForbiddenMessageMaker("list", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot list all `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` in the cluster`)) - messageResolver.addNamespacedForbiddenMessageMaker("watch", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot watch `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` in project "{{.GetNamespace}}"`)) - messageResolver.addRootScopedForbiddenMessageMaker("watch", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot watch all `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` in the cluster`)) - messageResolver.addNamespacedForbiddenMessageMaker("update", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot update `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` in project "{{.GetNamespace}}"`)) - messageResolver.addRootScopedForbiddenMessageMaker("update", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot update `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` at the cluster scope`)) - messageResolver.addNamespacedForbiddenMessageMaker("delete", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot delete `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` in project "{{.GetNamespace}}"`)) - messageResolver.addRootScopedForbiddenMessageMaker("delete", authorizationapi.ResourceAll, newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot delete `+resourceWithSubresourceIfNotEmpty+apiGroupIfNotEmpty+` at the cluster scope`)) - - // project request rejection - projectRequestDeny := projectRequestForbiddenTemplate - if len(projectRequestDeny) == 0 { - projectRequestDeny = DefaultProjectRequestForbidden - } - messageResolver.addRootScopedForbiddenMessageMaker("create", "projectrequests", newTemplateForbiddenMessageMaker(projectRequestDeny)) - - // projects "get" request rejection - messageResolver.addNamespacedForbiddenMessageMaker("get", "projects", newTemplateForbiddenMessageMaker(`User "{{.GetUser.GetName}}" cannot get project "{{.GetNamespace}}"`)) - - return messageResolver -} - -func (m *ForbiddenMessageResolver) addNamespacedForbiddenMessageMaker(verb, resource string, messageMaker ForbiddenMessageMaker) { - m.addForbiddenMessageMaker(m.namespacedVerbsToResourcesToForbiddenMessageMaker, verb, resource, messageMaker) -} - -func (m *ForbiddenMessageResolver) addRootScopedForbiddenMessageMaker(verb, resource string, messageMaker ForbiddenMessageMaker) { - m.addForbiddenMessageMaker(m.rootScopedVerbsToResourcesToForbiddenMessageMaker, verb, resource, messageMaker) -} - -func (m *ForbiddenMessageResolver) addForbiddenMessageMaker(target map[string]map[string]ForbiddenMessageMaker, verb, resource string, messageMaker ForbiddenMessageMaker) { - resourcesToForbiddenMessageMaker, exists := target[verb] - if !exists { - resourcesToForbiddenMessageMaker = map[string]ForbiddenMessageMaker{} - target[verb] = resourcesToForbiddenMessageMaker - } - - resourcesToForbiddenMessageMaker[resource] = messageMaker -} - -func (m *ForbiddenMessageResolver) MakeMessage(attrs authorizer.Attributes) (string, error) { - if !attrs.IsResourceRequest() { - return m.nonResourceURLForbiddenMessageMaker.MakeMessage(attrs) - } - - messageMakerMap := m.namespacedVerbsToResourcesToForbiddenMessageMaker - if len(attrs.GetNamespace()) == 0 { - messageMakerMap = m.rootScopedVerbsToResourcesToForbiddenMessageMaker - } - - resourcesToForbiddenMessageMaker, exists := messageMakerMap[attrs.GetVerb()] - if !exists { - resourcesToForbiddenMessageMaker, exists = messageMakerMap[authorizationapi.VerbAll] - if !exists { - return m.defaultForbiddenMessageMaker.MakeMessage(attrs) - } - } - - messageMaker, exists := resourcesToForbiddenMessageMaker[attrs.GetResource()] - if !exists { - messageMaker, exists = resourcesToForbiddenMessageMaker[authorizationapi.ResourceAll] - if !exists { - return m.defaultForbiddenMessageMaker.MakeMessage(attrs) - } - } - - specificMessage, err := messageMaker.MakeMessage(attrs) - if err != nil { - return m.defaultForbiddenMessageMaker.MakeMessage(attrs) - } - - return specificMessage, nil -} - -type templateForbiddenMessageMaker struct { - parsedTemplate *template.Template -} - -func newTemplateForbiddenMessageMaker(text string) templateForbiddenMessageMaker { - parsedTemplate := template.Must(template.New("").Parse(text)) - - return templateForbiddenMessageMaker{parsedTemplate} -} - -func (m templateForbiddenMessageMaker) MakeMessage(attrs authorizer.Attributes) (string, error) { - buffer := &bytes.Buffer{} - err := m.parsedTemplate.Execute(buffer, attrs) - return buffer.String(), err -} diff --git a/pkg/authorization/authorizer/messages_test.go b/pkg/authorization/authorizer/messages_test.go deleted file mode 100644 index 9581a47ee211..000000000000 --- a/pkg/authorization/authorizer/messages_test.go +++ /dev/null @@ -1,212 +0,0 @@ -package authorizer - -import ( - "testing" - - "k8s.io/apiserver/pkg/authentication/user" - kauthorizer "k8s.io/apiserver/pkg/authorization/authorizer" -) - -func TestDefaultForbiddenMessages(t *testing.T) { - messageResolver := NewForbiddenMessageResolver("") - - apiForbidden, err := messageResolver.defaultForbiddenMessageMaker.MakeMessage( - kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Namespace: "foo", - Verb: "get", - Resource: "pods", - Name: "hammer", - }, - ) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - expectedAPIForbidden := `User "chris" cannot "get" "pods" with name "hammer" in project "foo"` - if expectedAPIForbidden != apiForbidden { - t.Errorf("expected %v, got %v", expectedAPIForbidden, apiForbidden) - } - - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: false, - User: &user.DefaultInfo{Name: "chris"}, - Namespace: "foo", - Verb: "post", - Path: "/anything", - }, - expected: `User "chris" cannot "post" on "/anything"`, - }.run(t) -} - -func TestAttributeRecord(t *testing.T) { - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Namespace: "foo", - Verb: "get", - Resource: "users", - APIGroup: "user.openshift.io", - APIVersion: "v1", - Name: "hammer", - }, - expected: `User "chris" cannot get users.user.openshift.io in project "foo"`, - }.run(t) - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: false, - User: &user.DefaultInfo{Name: "chris"}, - Namespace: "foo", - Verb: "get", - Resource: "users", - APIGroup: "user.openshift.io", - APIVersion: "v1", - Path: "/", - }, - expected: `User "chris" cannot "get" on "/"`, - }.run(t) -} - -func TestProjectRequestForbiddenMessage(t *testing.T) { - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Verb: "create", - Resource: "projectrequests", - }, - expected: DefaultProjectRequestForbidden, - }.run(t) -} - -func TestNamespacedForbiddenMessage(t *testing.T) { - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Namespace: "foo", - Verb: "create", - Resource: "builds", - }, - expected: `User "chris" cannot create builds in project "foo"`, - }.run(t) - - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Namespace: "foo", - Verb: "get", - Resource: "builds", - }, - expected: `User "chris" cannot get builds in project "foo"`, - }.run(t) - - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Namespace: "foo", - Verb: "list", - Resource: "builds", - }, - expected: `User "chris" cannot list builds in project "foo"`, - }.run(t) - - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Namespace: "foo", - Verb: "update", - Resource: "builds", - }, - expected: `User "chris" cannot update builds in project "foo"`, - }.run(t) - - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Namespace: "foo", - Verb: "delete", - Resource: "builds", - }, - expected: `User "chris" cannot delete builds in project "foo"`, - }.run(t) - -} - -func TestRootScopedForbiddenMessage(t *testing.T) { - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Verb: "create", - Resource: "builds", - }, - expected: `User "chris" cannot create builds at the cluster scope`, - }.run(t) - - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Verb: "get", - Resource: "builds", - }, - expected: `User "chris" cannot get builds at the cluster scope`, - }.run(t) - - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Verb: "list", - Resource: "builds", - }, - expected: `User "chris" cannot list all builds in the cluster`, - }.run(t) - - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Verb: "update", - Resource: "builds", - }, - expected: `User "chris" cannot update builds at the cluster scope`, - }.run(t) - - messageTest{ - attributes: kauthorizer.AttributesRecord{ - ResourceRequest: true, - User: &user.DefaultInfo{Name: "chris"}, - Verb: "delete", - Resource: "builds", - }, - expected: `User "chris" cannot delete builds at the cluster scope`, - }.run(t) - -} - -type messageTest struct { - attributes kauthorizer.Attributes - expected string -} - -func (test messageTest) run(t *testing.T) { - messageResolver := NewForbiddenMessageResolver("") - - forbidden, err := messageResolver.MakeMessage(test.attributes) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - - if test.expected != forbidden { - t.Errorf("expected %v, got %v", test.expected, forbidden) - } -} diff --git a/pkg/authorization/authorizer/scope/authorizer.go b/pkg/authorization/authorizer/scope/authorizer.go index 9545f35225b7..08d8e9ff8399 100644 --- a/pkg/authorization/authorizer/scope/authorizer.go +++ b/pkg/authorization/authorizer/scope/authorizer.go @@ -9,17 +9,14 @@ import ( authorizerrbac "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac" authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" - defaultauthorizer "github.com/openshift/origin/pkg/authorization/authorizer" ) type scopeAuthorizer struct { clusterRoleGetter rbaclisters.ClusterRoleLister - - forbiddenMessageMaker defaultauthorizer.ForbiddenMessageMaker } -func NewAuthorizer(clusterRoleGetter rbaclisters.ClusterRoleLister, forbiddenMessageMaker defaultauthorizer.ForbiddenMessageMaker) authorizer.Authorizer { - return &scopeAuthorizer{clusterRoleGetter: clusterRoleGetter, forbiddenMessageMaker: forbiddenMessageMaker} +func NewAuthorizer(clusterRoleGetter rbaclisters.ClusterRoleLister) authorizer.Authorizer { + return &scopeAuthorizer{clusterRoleGetter: clusterRoleGetter} } func (a *scopeAuthorizer) Authorize(attributes authorizer.Attributes) (authorizer.Decision, string, error) { @@ -46,11 +43,6 @@ func (a *scopeAuthorizer) Authorize(attributes authorizer.Attributes) (authorize return authorizer.DecisionNoOpinion, "", nil } - denyReason, err := a.forbiddenMessageMaker.MakeMessage(attributes) - if err != nil { - denyReason = err.Error() - } - // the scope prevent this. We need to authoritatively deny - return authorizer.DecisionDeny, fmt.Sprintf("scopes %v prevent this action; %v", scopes, denyReason), kerrors.NewAggregate(nonFatalErrors) + return authorizer.DecisionDeny, fmt.Sprintf("scopes %v prevent this action", scopes), kerrors.NewAggregate(nonFatalErrors) } diff --git a/pkg/authorization/authorizer/scope/authorizer_test.go b/pkg/authorization/authorizer/scope/authorizer_test.go index a1cb91f92d77..0888ed4e4b77 100644 --- a/pkg/authorization/authorizer/scope/authorizer_test.go +++ b/pkg/authorization/authorizer/scope/authorizer_test.go @@ -8,7 +8,6 @@ import ( kauthorizer "k8s.io/apiserver/pkg/authorization/authorizer" authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" - defaultauthorizer "github.com/openshift/origin/pkg/authorization/authorizer" ) func TestAuthorize(t *testing.T) { @@ -63,7 +62,7 @@ func TestAuthorize(t *testing.T) { Namespace: "ns", }, expectedAllowed: kauthorizer.DecisionDeny, - expectedMsg: `scopes [does-not-exist] prevent this action; User "" cannot "" "" with name "" in project "ns"`, + expectedMsg: `scopes [does-not-exist] prevent this action`, expectedErr: `no scope evaluator found for "does-not-exist"`, }, { @@ -74,7 +73,7 @@ func TestAuthorize(t *testing.T) { Namespace: "ns", }, expectedAllowed: kauthorizer.DecisionDeny, - expectedMsg: `scopes [user:dne] prevent this action; User "" cannot "" "" with name "" in project "ns"`, + expectedMsg: `scopes [user:dne] prevent this action`, expectedErr: `unrecognized scope: user:dne`, }, { @@ -85,7 +84,7 @@ func TestAuthorize(t *testing.T) { Namespace: "ns", Verb: "get", Resource: "users", Name: "harold"}, expectedAllowed: kauthorizer.DecisionDeny, - expectedMsg: `scopes [user:info] prevent this action; User "" cannot get users in project "ns"`, + expectedMsg: `scopes [user:info] prevent this action`, }, { name: "scope covers", @@ -127,7 +126,7 @@ func TestAuthorize(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - authorizer := NewAuthorizer(nil, defaultauthorizer.NewForbiddenMessageResolver("")) + authorizer := NewAuthorizer(nil) actualAllowed, actualMsg, actualErr := authorizer.Authorize(tc.attributes) switch { diff --git a/pkg/cmd/server/origin/authorizer.go b/pkg/cmd/server/origin/authorizer.go index 757879305c6c..08706833c712 100644 --- a/pkg/cmd/server/origin/authorizer.go +++ b/pkg/cmd/server/origin/authorizer.go @@ -12,16 +12,14 @@ import ( rbacauthorizer "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac" kbootstrappolicy "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy" - openshiftauthorizer "github.com/openshift/origin/pkg/authorization/authorizer" "github.com/openshift/origin/pkg/authorization/authorizer/browsersafe" "github.com/openshift/origin/pkg/authorization/authorizer/scope" ) func NewAuthorizer(informers InformerAccess, projectRequestDenyMessage string) authorizer.Authorizer { - messageMaker := openshiftauthorizer.NewForbiddenMessageResolver(projectRequestDenyMessage) rbacInformers := informers.GetKubernetesInformers().Rbac().V1() - scopeLimitedAuthorizer := scope.NewAuthorizer(rbacInformers.ClusterRoles().Lister(), messageMaker) + scopeLimitedAuthorizer := scope.NewAuthorizer(rbacInformers.ClusterRoles().Lister()) kubeAuthorizer := rbacauthorizer.New( &rbacauthorizer.RoleGetter{Lister: rbacInformers.Roles().Lister()}, @@ -48,7 +46,7 @@ func NewAuthorizer(informers InformerAccess, projectRequestDenyMessage string) a authorizerfactory.NewPrivilegedGroups(user.SystemPrivilegedGroup), nodeAuthorizer, // Wrap with an authorizer that detects unsafe requests and modifies verbs/resources appropriately so policy can address them separately - browsersafe.NewBrowserSafeAuthorizer(openshiftauthorizer.NewAuthorizer(kubeAuthorizer, messageMaker), user.AllAuthenticated), + browsersafe.NewBrowserSafeAuthorizer(kubeAuthorizer, user.AllAuthenticated), ) return openshiftAuthorizer diff --git a/test/cmd/authentication.sh b/test/cmd/authentication.sh index 58d0172557f2..aa7a6f9ac922 100755 --- a/test/cmd/authentication.sh +++ b/test/cmd/authentication.sh @@ -47,21 +47,21 @@ os::cmd::expect_success "oc policy can-i --list" whoamitoken="$(oc process -f "${OS_ROOT}/test/testdata/authentication/scoped-token-template.yaml" TOKEN_PREFIX=whoami SCOPE=user:info USER_NAME="${username}" USER_UID="${useruid}" | oc create -f - -o name | awk -F/ '{print $2}')" os::cmd::expect_success_and_text "oc get user/~ --token='${whoamitoken}'" "${username}" os::cmd::expect_success_and_text "oc whoami --token='${whoamitoken}'" "${username}" -os::cmd::expect_failure_and_text "oc get pods --token='${whoamitoken}' -n '${project}'" "prevent this action; User \"scoped-user\" cannot list pods in project \"${project}\"" +os::cmd::expect_failure_and_text "oc get pods --token='${whoamitoken}' -n '${project}'" "pods is forbidden: User \"scoped-user\" cannot list pods in the namespace \"${project}\"" listprojecttoken="$(oc process -f "${OS_ROOT}/test/testdata/authentication/scoped-token-template.yaml" TOKEN_PREFIX=listproject SCOPE=user:list-scoped-projects USER_NAME="${username}" USER_UID="${useruid}" | oc create -f - -o name | awk -F/ '{print $2}')" # this token doesn't have rights to see any projects even though it can hit the list endpoint, so an empty list is correct # we'll add another scope that allows listing all known projects even if this token has no other powers in them. os::cmd::expect_success_and_not_text "oc get projects --token='${listprojecttoken}'" "${project}" -os::cmd::expect_failure_and_text "oc get user/~ --token='${listprojecttoken}'" 'prevent this action; User "scoped-user" cannot get users.user.openshift.io at the cluster scope' -os::cmd::expect_failure_and_text "oc get pods --token='${listprojecttoken}' -n '${project}'" "prevent this action; User \"scoped-user\" cannot list pods in project \"${project}\"" +os::cmd::expect_failure_and_text "oc get user/~ --token='${listprojecttoken}'" 'User "scoped-user" cannot get users.user.openshift.io at the cluster scope' +os::cmd::expect_failure_and_text "oc get pods --token='${listprojecttoken}' -n '${project}'" "User \"scoped-user\" cannot list pods in the namespace \"${project}\"" listprojecttoken="$(oc process -f "${OS_ROOT}/test/testdata/authentication/scoped-token-template.yaml" TOKEN_PREFIX=listallprojects SCOPE=user:list-projects USER_NAME="${username}" USER_UID="${useruid}" | oc create -f - -o name | awk -F/ '{print $2}')" os::cmd::expect_success_and_text "oc get projects --token='${listprojecttoken}'" "${project}" adminnonescalatingpowerstoken="$(oc process -f "${OS_ROOT}/test/testdata/authentication/scoped-token-template.yaml" TOKEN_PREFIX=admin SCOPE=role:admin:* USER_NAME="${username}" USER_UID="${useruid}" | oc create -f - -o name | awk -F/ '{print $2}')" -os::cmd::expect_failure_and_text "oc get user/~ --token='${adminnonescalatingpowerstoken}'" 'prevent this action; User "scoped-user" cannot get users.user.openshift.io at the cluster scope' -os::cmd::expect_failure_and_text "oc get secrets --token='${adminnonescalatingpowerstoken}' -n '${project}'" "prevent this action; User \"scoped-user\" cannot list secrets in project \"${project}\"" +os::cmd::expect_failure_and_text "oc get user/~ --token='${adminnonescalatingpowerstoken}'" 'User "scoped-user" cannot get users.user.openshift.io at the cluster scope' +os::cmd::expect_failure_and_text "oc get secrets --token='${adminnonescalatingpowerstoken}' -n '${project}'" "User \"scoped-user\" cannot list secrets in the namespace \"${project}\"" os::cmd::expect_success_and_text "oc get 'projects/${project}' --token='${adminnonescalatingpowerstoken}' -n '${project}'" "${project}" allescalatingpowerstoken="$(oc process -f "${OS_ROOT}/test/testdata/authentication/scoped-token-template.yaml" TOKEN_PREFIX=clusteradmin SCOPE='role:cluster-admin:*:!' USER_NAME="${username}" USER_UID="${useruid}" | oc create -f - -o name | awk -F/ '{print $2}')" diff --git a/test/cmd/basicresources.sh b/test/cmd/basicresources.sh index 22e12e35a461..d42c5c8df230 100755 --- a/test/cmd/basicresources.sh +++ b/test/cmd/basicresources.sh @@ -235,7 +235,7 @@ project=$(oc project -q) os::cmd::expect_success 'oc policy add-role-to-user view view-user' os::cmd::expect_success 'oc login -u view-user -p anything' os::cmd::try_until_success 'oc project ${project}' -os::cmd::expect_failure_and_text "oc set env dc/test-deployment-config --list --resolve" "cannot get secrets in project" +os::cmd::expect_failure_and_text "oc set env dc/test-deployment-config --list --resolve" "cannot get secrets in the namespace" oc login -u system:admin # clean up os::cmd::expect_success "oc delete dc/test-deployment-config" diff --git a/test/cmd/policy.sh b/test/cmd/policy.sh index 6f277d04e213..0e4bccd6066a 100755 --- a/test/cmd/policy.sh +++ b/test/cmd/policy.sh @@ -234,7 +234,7 @@ os::cmd::expect_success_and_text 'oc policy scc-review -z default -f ${OS_ROOT} os::cmd::expect_success_and_text 'oc policy scc-review -z system:serviceaccount:policy-second:default -f ${OS_ROOT}/test/testdata/job.yaml --no-headers=true' 'Job/hello default lax' os::cmd::expect_success_and_text 'oc policy scc-review -f ${OS_ROOT}/test/extended/testdata/deployments/deployment-simple.yaml --no-headers=true' 'DeploymentConfig/deployment-simple default lax' os::cmd::expect_success_and_text 'oc policy scc-review -f ${OS_ROOT}/test/testdata/nginx_pod.yaml --no-headers=true' '' -os::cmd::expect_failure_and_text 'oc policy scc-review -z default -f ${OS_ROOT}/test/testdata/job.yaml --namespace=no-exist' 'error: unable to compute Pod Security Policy Review for "hello": podsecuritypolicyreviews.security.openshift.io is forbidden: User "bob" cannot create podsecuritypolicyreviews.security.openshift.io in the namespace "no-exist": User "bob" cannot create podsecuritypolicyreviews.security.openshift.io in project "no-exist"' +os::cmd::expect_failure_and_text 'oc policy scc-review -z default -f ${OS_ROOT}/test/testdata/job.yaml --namespace=no-exist' 'error: unable to compute Pod Security Policy Review for "hello": podsecuritypolicyreviews.security.openshift.io is forbidden: User "bob" cannot create podsecuritypolicyreviews.security.openshift.io in the namespace "no-exist"' os::cmd::expect_failure_and_text 'oc policy scc-review -z default -f ${OS_ROOT}/test/testdata/pspreview_unsupported_statefulset.yaml' 'error: StatefulSet "rd" with spec.volumeClaimTemplates currently not supported.' os::cmd::expect_failure_and_text 'oc policy scc-review -z no-exist -f ${OS_ROOT}/test/testdata/job.yaml' 'error: unable to compute Pod Security Policy Review for "hello": unable to retrieve ServiceAccount no-exist: serviceaccount "no-exist" not found' os::cmd::expect_success "oc login -u system:admin -n '${project}'" diff --git a/test/integration/authorization_test.go b/test/integration/authorization_test.go index cd45a39808a1..695c15ae1595 100644 --- a/test/integration/authorization_test.go +++ b/test/integration/authorization_test.go @@ -875,7 +875,7 @@ func TestAuthorizationSubjectAccessReviewAPIGroup(t *testing.T) { kubeAuthInterface: clusterAdminSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: true, - Reason: "allowed by openshift authorizer", + Reason: `RBAC: allowed by RoleBinding "admin/hammer-project" of ClusterRole "admin" to User "harold"`, Namespace: "hammer-project", }, }.run(t) @@ -889,7 +889,7 @@ func TestAuthorizationSubjectAccessReviewAPIGroup(t *testing.T) { kubeAuthInterface: clusterAdminSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "harold" cannot get horizontalpodautoscalers in project "hammer-project"`, + Reason: ``, Namespace: "hammer-project", }, }.run(t) @@ -903,7 +903,7 @@ func TestAuthorizationSubjectAccessReviewAPIGroup(t *testing.T) { kubeAuthInterface: clusterAdminKubeClient.Authorization(), response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "harold" cannot get horizontalpodautoscalers.foo in project "hammer-project"`, + Reason: ``, Namespace: "hammer-project", }, }.run(t) @@ -917,7 +917,7 @@ func TestAuthorizationSubjectAccessReviewAPIGroup(t *testing.T) { kubeAuthInterface: clusterAdminSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "harold" cannot get horizontalpodautoscalers.* in project "hammer-project"`, + Reason: ``, Namespace: "hammer-project", }, }.run(t) @@ -1068,7 +1068,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: clusterAdminLocalSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: true, - Reason: "allowed by openshift authorizer", + Reason: `RBAC: allowed by RoleBinding "view/default" of ClusterRole "view" to User "danny"`, Namespace: "default", }, }.run(t) @@ -1079,7 +1079,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: clusterAdminLocalSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "danny" cannot get projects at the cluster scope`, + Reason: ``, Namespace: "", }, }.run(t) @@ -1133,7 +1133,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: haroldSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: true, - Reason: "allowed by openshift authorizer", + Reason: `RBAC: allowed by RoleBinding "view/hammer-project" of ClusterRole "view" to User "valerie"`, Namespace: "hammer-project", }, }.run(t) @@ -1144,7 +1144,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: markSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "valerie" cannot get project "mallet-project"`, + Reason: ``, Namespace: "mallet-project", }, }.run(t) @@ -1160,7 +1160,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: markSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: true, - Reason: "allowed by openshift authorizer", + Reason: `RBAC: allowed by RoleBinding "edit/mallet-project" of ClusterRole "edit" to User "edgar"`, Namespace: "mallet-project", }, }.run(t) @@ -1214,7 +1214,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: haroldSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: true, - Reason: "allowed by openshift authorizer", + Reason: `RBAC: allowed by RoleBinding "admin/hammer-project" of ClusterRole "admin" to User "harold"`, Namespace: "hammer-project", }, }.run(t) @@ -1230,7 +1230,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: clusterAdminLocalSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: true, - Reason: "allowed by cluster rule", + Reason: `RBAC: allowed by ClusterRoleBinding "cluster-admins" of ClusterRole "cluster-admin" to Group "system:cluster-admins"`, Namespace: "", }, }.run(t) @@ -1253,7 +1253,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: haroldSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: true, - Reason: "allowed by openshift authorizer", + Reason: `RBAC: allowed by RoleBinding "admin/hammer-project" of ClusterRole "admin" to User "harold"`, Namespace: "hammer-project", }, }.run(t) @@ -1264,7 +1264,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: anonymousSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: true, - Reason: "allowed by openshift authorizer", + Reason: `RBAC: allowed by RoleBinding "edit/hammer-project" of ClusterRole "edit" to User "system:anonymous"`, Namespace: "hammer-project", }, }.run(t) @@ -1277,7 +1277,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: haroldSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "harold" cannot create pods in project "mallet-project"`, + Reason: ``, Namespace: "mallet-project", }, }.run(t) @@ -1288,7 +1288,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: anonymousSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "system:anonymous" cannot create pods in project "mallet-project"`, + Reason: ``, Namespace: "mallet-project", }, }.run(t) @@ -1302,7 +1302,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: haroldSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "harold" cannot create pods in project "nonexistent-project"`, + Reason: ``, Namespace: "nonexistent-project", }, }.run(t) @@ -1313,7 +1313,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { kubeAuthInterface: anonymousSARGetter, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "system:anonymous" cannot create pods in project "nonexistent-project"`, + Reason: ``, Namespace: "nonexistent-project", }, }.run(t) @@ -1328,7 +1328,7 @@ func TestAuthorizationSubjectAccessReview(t *testing.T) { localReview: askCanICreatePolicyBindings, response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "harold" cannot create policybindings in project "hammer-project"`, + Reason: ``, Namespace: "hammer-project", }, }.run(t) @@ -1364,14 +1364,6 @@ func TestBrowserSafeAuthorizer(t *testing.T) { proxyVerb := []string{"api", "v1", "proxy", "namespaces", "ns", "pods", "podX1:8080"} proxySubresource := []string{"api", "v1", "namespaces", "ns", "pods", "podX1:8080", "proxy", "appEndPoint"} - isUnsafeErr := func(errProxy error) (matches bool) { - if errProxy == nil { - return false - } - return strings.Contains(errProxy.Error(), `cannot "unsafeproxy" "pods" with name "podX1:8080" in project "ns"`) || - strings.Contains(errProxy.Error(), `cannot get pods/unsafeproxy in project "ns"`) - } - for _, tc := range []struct { name string client rest.Interface @@ -1383,35 +1375,27 @@ func TestBrowserSafeAuthorizer(t *testing.T) { name: "safe to proxy verb", client: userClient.Core().RESTClient(), path: proxyVerb, - - expectUnsafe: false, }, { name: "safe to proxy subresource", client: userClient.Core().RESTClient(), path: proxySubresource, - - expectUnsafe: false, }, { name: "unsafe to proxy verb", client: anonymousClient, path: proxyVerb, - - expectUnsafe: true, }, { name: "unsafe to proxy subresource", client: anonymousClient, path: proxySubresource, - - expectUnsafe: true, }, } { errProxy := tc.client.Get().AbsPath(tc.path...).Do().Error() - if errProxy == nil || !kapierror.IsForbidden(errProxy) || tc.expectUnsafe != isUnsafeErr(errProxy) { - t.Errorf("%s: expected forbidden error on GET %s, got %#v (isForbidden=%v, expectUnsafe=%v, actualUnsafe=%v)", - tc.name, tc.path, errProxy, kapierror.IsForbidden(errProxy), tc.expectUnsafe, isUnsafeErr(errProxy)) + if errProxy == nil || !kapierror.IsForbidden(errProxy) { + t.Errorf("%s: expected forbidden error on GET %s, got %#v (isForbidden=%v)", + tc.name, tc.path, errProxy, kapierror.IsForbidden(errProxy)) } } } diff --git a/test/integration/bootstrap_policy_test.go b/test/integration/bootstrap_policy_test.go index 9cc349c9a56b..ae8e83970753 100644 --- a/test/integration/bootstrap_policy_test.go +++ b/test/integration/bootstrap_policy_test.go @@ -98,7 +98,7 @@ func TestBootstrapPolicySelfSubjectAccessReviews(t *testing.T) { kubeAuthInterface: valerieKubeClient.Authorization(), response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "valerie" cannot create policybindings in project "openshift"`, + Reason: ``, Namespace: "openshift", }, }.run(t) @@ -148,7 +148,7 @@ func TestSelfSubjectAccessReviewsNonExistingNamespace(t *testing.T) { kubeAuthInterface: valerieKubeClient.Authorization(), response: authorizationapi.SubjectAccessReviewResponse{ Allowed: false, - Reason: `User "valerie" cannot create pods in project "foo"`, + Reason: ``, Namespace: "foo", }, }.run(t) diff --git a/test/integration/oauth_cert_fallback_test.go b/test/integration/oauth_cert_fallback_test.go index 7cdd057db97e..3e8a867045be 100644 --- a/test/integration/oauth_cert_fallback_test.go +++ b/test/integration/oauth_cert_fallback_test.go @@ -35,7 +35,7 @@ func TestOAuthCertFallback(t *testing.T) { certUser = "system:admin" unauthorizedError = "Unauthorized" - anonymousError = `users.user.openshift.io "~" is forbidden: User "system:anonymous" cannot get users.user.openshift.io at the cluster scope: User "system:anonymous" cannot get users.user.openshift.io at the cluster scope` + anonymousError = `users.user.openshift.io "~" is forbidden: User "system:anonymous" cannot get users.user.openshift.io at the cluster scope` ) // Build master config