forked from kubernetes/kubernetes
-
Notifications
You must be signed in to change notification settings - Fork 102
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
1 parent
c327802
commit 8ecfb54
Showing
4 changed files
with
249 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
55 changes: 55 additions & 0 deletions
55
pkg/registry/authorization/selfsubjectaccessreview/rest_patch.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} |
55 changes: 55 additions & 0 deletions
55
pkg/registry/authorization/selfsubjectaccessreview/rest_patch_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) | ||
} | ||
} | ||
} |
136 changes: 136 additions & 0 deletions
136
pkg/registry/authorization/selfsubjectaccessreview/rest_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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()) | ||
} | ||
} | ||
} |