diff --git a/pkg/registry/authorization/selfsubjectaccessreview/rest.go b/pkg/registry/authorization/selfsubjectaccessreview/rest.go index a64a84cabfa40..c2e48ed8f1d7b 100644 --- a/pkg/registry/authorization/selfsubjectaccessreview/rest.go +++ b/pkg/registry/authorization/selfsubjectaccessreview/rest.go @@ -87,6 +87,9 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation } } + // when using a scoped token, set the required scopes to perform the self SAR if any is missing + userToCheck = userWithRequiredScopes(userToCheck) + var authorizationAttributes authorizer.AttributesRecord if selfSAR.Spec.ResourceAttributes != nil { authorizationAttributes = authorizationutil.ResourceAttributesFrom(userToCheck, *selfSAR.Spec.ResourceAttributes) diff --git a/pkg/registry/authorization/selfsubjectaccessreview/rest_patch.go b/pkg/registry/authorization/selfsubjectaccessreview/rest_patch.go new file mode 100644 index 0000000000000..1b13327285e89 --- /dev/null +++ b/pkg/registry/authorization/selfsubjectaccessreview/rest_patch.go @@ -0,0 +1,55 @@ +package selfsubjectaccessreview + +import ( + "reflect" + "sort" + + "k8s.io/apiserver/pkg/authentication/user" + + authorizationv1 "github.com/openshift/api/authorization/v1" + authorizationscope "github.com/openshift/apiserver-library-go/pkg/authorization/scope" +) + +func userWithRequiredScopes(userToCheck user.Info) user.Info { + userExtra := userToCheck.GetExtra() + if userExtra == nil || !scopesNeedUserFull(userExtra[authorizationv1.ScopesKey]) { + return userToCheck + } + + userExtraCopy := make(map[string][]string) + for k, v := range userExtra { + userExtraCopy[k] = v + } + userExtraCopy[authorizationv1.ScopesKey] = append(userExtraCopy[authorizationv1.ScopesKey], authorizationscope.UserFull) + + userWithFullScope := &user.DefaultInfo{ + Name: userToCheck.GetName(), + UID: userToCheck.GetUID(), + Groups: userToCheck.GetGroups(), + Extra: userExtraCopy, + } + + return userWithFullScope +} + +// a self-SAR request must be authorized as if it has either the full user's permissions +// or the permissions of the user's role set on the request (if applicable) in order +// to be able to perform the access review +func scopesNeedUserFull(scopes []string) bool { + if len(scopes) == 0 { + return false + } + + sort.Strings(scopes) + switch { + case + // all scope slices used here must be sorted + reflect.DeepEqual(scopes, []string{authorizationscope.UserAccessCheck}), + reflect.DeepEqual(scopes, []string{authorizationscope.UserAccessCheck, authorizationscope.UserInfo}), + reflect.DeepEqual(scopes, []string{authorizationscope.UserAccessCheck, authorizationscope.UserListAllProjects}), + reflect.DeepEqual(scopes, []string{authorizationscope.UserAccessCheck, authorizationscope.UserInfo, authorizationscope.UserListAllProjects}): + return true + } + + return false +} diff --git a/pkg/registry/authorization/selfsubjectaccessreview/rest_patch_test.go b/pkg/registry/authorization/selfsubjectaccessreview/rest_patch_test.go new file mode 100644 index 0000000000000..05a13d521311f --- /dev/null +++ b/pkg/registry/authorization/selfsubjectaccessreview/rest_patch_test.go @@ -0,0 +1,55 @@ +package selfsubjectaccessreview + +import ( + "testing" + + authorizationscope "github.com/openshift/apiserver-library-go/pkg/authorization/scope" +) + +func TestScopesNeedUserFull(t *testing.T) { + roleScope := "role:testrole:testns" + tests := []struct { + want bool + scopes []string + }{ + {true, []string{authorizationscope.UserAccessCheck}}, + {true, []string{authorizationscope.UserInfo, authorizationscope.UserAccessCheck}}, + {true, []string{authorizationscope.UserListAllProjects, authorizationscope.UserAccessCheck}}, + {true, []string{authorizationscope.UserListAllProjects, authorizationscope.UserInfo, authorizationscope.UserAccessCheck}}, + {false, nil}, + {false, []string{}}, + {false, []string{authorizationscope.UserInfo}}, + {false, []string{authorizationscope.UserListAllProjects}}, + {false, []string{authorizationscope.UserFull}}, + {false, []string{roleScope}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserFull}}, + {false, []string{authorizationscope.UserAccessCheck, roleScope}}, + {false, []string{authorizationscope.UserInfo, authorizationscope.UserListAllProjects}}, + {false, []string{authorizationscope.UserInfo, authorizationscope.UserFull}}, + {false, []string{authorizationscope.UserInfo, roleScope}}, + {false, []string{authorizationscope.UserListAllProjects, authorizationscope.UserFull}}, + {false, []string{authorizationscope.UserListAllProjects, roleScope}}, + {false, []string{authorizationscope.UserFull, roleScope}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserInfo, authorizationscope.UserFull}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserInfo, roleScope}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserListAllProjects, authorizationscope.UserFull}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserListAllProjects, roleScope}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserFull, roleScope}}, + {false, []string{authorizationscope.UserInfo, authorizationscope.UserListAllProjects, authorizationscope.UserFull}}, + {false, []string{authorizationscope.UserInfo, authorizationscope.UserListAllProjects, roleScope}}, + {false, []string{authorizationscope.UserInfo, authorizationscope.UserFull, roleScope}}, + {false, []string{authorizationscope.UserListAllProjects, authorizationscope.UserFull, roleScope}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserInfo, authorizationscope.UserListAllProjects, authorizationscope.UserFull}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserInfo, authorizationscope.UserListAllProjects, roleScope}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserInfo, authorizationscope.UserFull, roleScope}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserListAllProjects, authorizationscope.UserFull, roleScope}}, + {false, []string{authorizationscope.UserInfo, authorizationscope.UserListAllProjects, authorizationscope.UserFull, roleScope}}, + {false, []string{authorizationscope.UserAccessCheck, authorizationscope.UserInfo, authorizationscope.UserListAllProjects, authorizationscope.UserFull, roleScope}}, + } + + for _, tt := range tests { + if got := scopesNeedUserFull(tt.scopes); got != tt.want { + t.Errorf("scopes %v; got %v; want %v", tt.scopes, got, tt.want) + } + } +} diff --git a/pkg/registry/authorization/selfsubjectaccessreview/rest_test.go b/pkg/registry/authorization/selfsubjectaccessreview/rest_test.go new file mode 100644 index 0000000000000..31ac1d5c5e91d --- /dev/null +++ b/pkg/registry/authorization/selfsubjectaccessreview/rest_test.go @@ -0,0 +1,136 @@ +package selfsubjectaccessreview + +import ( + "context" + "reflect" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/authorization/authorizer" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/registry/rest" + + authorizationv1 "github.com/openshift/api/authorization/v1" + authorizationscope "github.com/openshift/apiserver-library-go/pkg/authorization/scope" + + authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" +) + +type fakeAuthorizer struct { + attrs authorizer.Attributes +} + +func (f *fakeAuthorizer) Authorize(ctx context.Context, attrs authorizer.Attributes) (authorizer.Decision, string, error) { + f.attrs = attrs + return authorizer.DecisionNoOpinion, "", nil +} + +func TestCreate(t *testing.T) { + userNilExtra := &user.DefaultInfo{} + + userNoExtra := &user.DefaultInfo{ + Extra: make(map[string][]string), + } + + userNoScopes := &user.DefaultInfo{ + Extra: map[string][]string{ + "extra": {"ex1", "ex2"}, + }, + } + + userWithScopesNoCheckAccess := &user.DefaultInfo{ + Extra: map[string][]string{ + "extra": {"ex1", "ex2"}, + authorizationv1.ScopesKey: { + authorizationscope.UserInfo, + authorizationscope.UserListAllProjects, + }, + }, + } + + userWithScopesWithCheckAccess := &user.DefaultInfo{ + Extra: map[string][]string{ + "extra": {"ex1", "ex2"}, + authorizationv1.ScopesKey: { + authorizationscope.UserAccessCheck, + authorizationscope.UserInfo, + }, + }, + } + + userWithScopeUserFull := &user.DefaultInfo{ + Extra: map[string][]string{ + "extra": {"ex1", "ex2"}, + authorizationv1.ScopesKey: { + authorizationscope.UserAccessCheck, + authorizationscope.UserInfo, + authorizationscope.UserFull, + }, + }, + } + + userWithRoleScope := &user.DefaultInfo{ + Extra: map[string][]string{ + "extra": {"ex1", "ex2"}, + authorizationv1.ScopesKey: { + authorizationscope.UserAccessCheck, + "role:testrole:testns", + }, + }, + } + + testcases := map[string]struct { + user user.Info + expectedUser user.Info + }{ + "nil extra": { + user: userNilExtra, + expectedUser: userNilExtra, + }, + + "no extra": { + user: userNoExtra, + expectedUser: userNoExtra, + }, + + "no scopes": { + user: userNoScopes, + expectedUser: userNoScopes, + }, + + "scopes exclude user:check-access": { + user: userWithScopesNoCheckAccess, + expectedUser: userWithScopesNoCheckAccess, + }, + + "scopes include user:check-access": { + user: userWithScopesWithCheckAccess, + expectedUser: userWithScopeUserFull, + }, + + "scopes include role scope": { + user: userWithRoleScope, + expectedUser: userWithRoleScope, + }, + } + + for k, tc := range testcases { + auth := &fakeAuthorizer{} + storage := NewREST(auth) + spec := authorizationapi.SelfSubjectAccessReviewSpec{ + NonResourceAttributes: &authorizationapi.NonResourceAttributes{Verb: "get", Path: "/mypath"}, + } + + ctx := genericapirequest.WithUser(genericapirequest.NewContext(), tc.user) + _, err := storage.Create(ctx, &authorizationapi.SelfSubjectAccessReview{Spec: spec}, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) + if err != nil { + t.Errorf("%s: %v", k, err) + continue + } + + if !reflect.DeepEqual(auth.attrs.GetUser(), tc.expectedUser) { + t.Errorf("%s: expected\n%#v\ngot\n%#v", k, tc.expectedUser, auth.attrs.GetUser()) + } + } +}