UPSTREAM: <carry>: selfsubjectaccessreview: grant user:full scope to …

…self-SARs that have user:check-access

Otherwise, the request will inherit any scopes that an access token might have
and the scopeAuthorizer will deny the access review if the scopes do not include
user:full

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)

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
+}

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)
+		}
+	}
+}

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())
+		}
+	}
+}