From 432e6ecdaec7a972af03f295efdd9ae0d59a9ae3 Mon Sep 17 00:00:00 2001 From: deads2k Date: Thu, 18 Aug 2016 07:46:51 -0400 Subject: [PATCH] allow impersonating user.Info.Extra --- pkg/apis/authentication/types.go | 6 ++ pkg/apiserver/handler_impersonation.go | 83 ++++++++++++---- pkg/apiserver/handler_impersonation_test.go | 103 ++++++++++++++++++-- pkg/auth/user/user.go | 2 + 4 files changed, 168 insertions(+), 26 deletions(-) diff --git a/pkg/apis/authentication/types.go b/pkg/apis/authentication/types.go index 8a474ccf9d7..480e8af012e 100644 --- a/pkg/apis/authentication/types.go +++ b/pkg/apis/authentication/types.go @@ -28,6 +28,12 @@ const ( // ImpersonateGroupHeader is used to impersonate a particular group during an API server request. // It can be repeated multipled times for multiple groups. ImpersonateGroupHeader = "Impersonate-Group" + + // ImpersonateUserExtraHeaderPrefix is a prefix for any header used to impersonate an entry in the + // extra map[string][]string for user.Info. The key will be every after the prefix. + // It can be repeated multipled times for multiple map keys and the same key can be repeated multiple + // times to have multiple elements in the slice under a single key + ImpersonateUserExtraHeaderPrefix = "Impersonate-Extra-" ) // +genclient=true diff --git a/pkg/apiserver/handler_impersonation.go b/pkg/apiserver/handler_impersonation.go index 9aedf237eb1..bda8d11c28d 100644 --- a/pkg/apiserver/handler_impersonation.go +++ b/pkg/apiserver/handler_impersonation.go @@ -17,7 +17,9 @@ limitations under the License. package apiserver import ( + "fmt" "net/http" + "strings" "github.com/golang/glog" @@ -32,20 +34,17 @@ import ( // WithImpersonation is a filter that will inspect and check requests that attempt to change the user.Info for their requests func WithImpersonation(handler http.Handler, requestContextMapper api.RequestContextMapper, a authorizer.Authorizer) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - requestedUser := req.Header.Get(authenticationapi.ImpersonateUserHeader) - if len(requestedUser) == 0 { - if len(req.Header[authenticationapi.ImpersonateGroupHeader]) > 0 { - glog.V(4).Infof("attempt to impersonate groups without impersonating a user: %v", req.Header[authenticationapi.ImpersonateGroupHeader]) - forbidden(w, req) - return - } - + impersonationRequests, err := buildImpersonationRequests(req.Header) + if err != nil { + glog.V(4).Infof("%v", err) + forbidden(w, req) + return + } + if len(impersonationRequests) == 0 { handler.ServeHTTP(w, req) return } - impersonationRequests := buildImpersonationRequests(requestedUser, req.Header[authenticationapi.ImpersonateGroupHeader]) - ctx, exists := requestContextMapper.Get(req) if !exists { forbidden(w, req) @@ -65,6 +64,7 @@ func WithImpersonation(handler http.Handler, requestContextMapper api.RequestCon // and group information username := "" groups := []string{} + userExtra := map[string][]string{} for _, impersonationRequest := range impersonationRequests { actingAsAttributes := &authorizer.AttributesRecord{ User: requestor, @@ -92,8 +92,15 @@ func WithImpersonation(handler http.Handler, requestContextMapper api.RequestCon actingAsAttributes.Resource = "groups" groups = append(groups, impersonationRequest.Name) + case authenticationapi.Kind("UserExtra"): + extraKey := impersonationRequest.FieldPath + extraValue := impersonationRequest.Name + actingAsAttributes.Resource = "userextras" + actingAsAttributes.Subresource = extraKey + userExtra[extraKey] = append(userExtra[extraKey], extraValue) + default: - glog.V(4).Infof("unknown impersonation request type: %v", impersonationRequest) + glog.V(4).Infof("unknown impersonation request type: %v\n", impersonationRequest) forbidden(w, req) return } @@ -109,7 +116,7 @@ func WithImpersonation(handler http.Handler, requestContextMapper api.RequestCon newUser := &user.DefaultInfo{ Name: username, Groups: groups, - Extra: map[string][]string{}, + Extra: userExtra, } requestContextMapper.Update(req, api.WithUser(ctx, newUser)) @@ -121,19 +128,55 @@ func WithImpersonation(handler http.Handler, requestContextMapper api.RequestCon } // buildImpersonationRequests returns a list of objectreferences that represent the different things we're requesting to impersonate. -// Each request must be authorized against the current user before switching contexts -func buildImpersonationRequests(requestedUser string, requestedGroups []string) []api.ObjectReference { +// Also includes a map[string][]string representing user.Info.Extra +// Each request must be authorized against the current user before switching contexts. +func buildImpersonationRequests(headers http.Header) ([]api.ObjectReference, error) { impersonationRequests := []api.ObjectReference{} - if namespace, name, err := serviceaccount.SplitUsername(requestedUser); err == nil { - impersonationRequests = append(impersonationRequests, api.ObjectReference{Kind: "ServiceAccount", Namespace: namespace, Name: name}) - } else { - impersonationRequests = append(impersonationRequests, api.ObjectReference{Kind: "User", Name: requestedUser}) + requestedUser := headers.Get(authenticationapi.ImpersonateUserHeader) + hasUser := len(requestedUser) > 0 + if hasUser { + if namespace, name, err := serviceaccount.SplitUsername(requestedUser); err == nil { + impersonationRequests = append(impersonationRequests, api.ObjectReference{Kind: "ServiceAccount", Namespace: namespace, Name: name}) + } else { + impersonationRequests = append(impersonationRequests, api.ObjectReference{Kind: "User", Name: requestedUser}) + } } - for _, group := range requestedGroups { + hasGroups := false + for _, group := range headers[authenticationapi.ImpersonateGroupHeader] { + hasGroups = true impersonationRequests = append(impersonationRequests, api.ObjectReference{Kind: "Group", Name: group}) } - return impersonationRequests + hasUserExtra := false + for headerName, values := range headers { + if !strings.HasPrefix(headerName, authenticationapi.ImpersonateUserExtraHeaderPrefix) { + continue + } + + hasUserExtra = true + extraKey := strings.ToLower(headerName[len(authenticationapi.ImpersonateUserExtraHeaderPrefix):]) + + // make a separate request for each extra value they're trying to set + for _, value := range values { + impersonationRequests = append(impersonationRequests, + api.ObjectReference{ + Kind: "UserExtra", + // we only parse out a group above, but the parsing will fail if there isn't SOME version + // using the internal version will help us fail if anyone starts using it + APIVersion: authenticationapi.SchemeGroupVersion.String(), + Name: value, + // ObjectReference doesn't have a subresource field. FieldPath is close and available, so we'll use that + // TODO fight the good fight for ObjectReference to refer to resources and subresources + FieldPath: extraKey, + }) + } + } + + if (hasGroups || hasUserExtra) && !hasUser { + return nil, fmt.Errorf("requested %v without impersonating a user", impersonationRequests) + } + + return impersonationRequests, nil } diff --git a/pkg/apiserver/handler_impersonation_test.go b/pkg/apiserver/handler_impersonation_test.go index 228405afc54..50cce24be1e 100644 --- a/pkg/apiserver/handler_impersonation_test.go +++ b/pkg/apiserver/handler_impersonation_test.go @@ -62,17 +62,31 @@ func (impersonateAuthorizer) Authorize(a authorizer.Attributes) (authorized bool return true, "", nil } + if len(user.GetGroups()) > 1 && user.GetGroups()[1] == "extra-setter-scopes" && a.GetVerb() == "impersonate" && a.GetResource() == "userextras" && a.GetSubresource() == "scopes" { + return true, "", nil + } + + if len(user.GetGroups()) > 1 && user.GetGroups()[1] == "extra-setter-particular-scopes" && + a.GetVerb() == "impersonate" && a.GetResource() == "userextras" && a.GetSubresource() == "scopes" && a.GetName() == "scope-a" { + return true, "", nil + } + + if len(user.GetGroups()) > 1 && user.GetGroups()[1] == "extra-setter-project" && a.GetVerb() == "impersonate" && a.GetResource() == "userextras" && a.GetSubresource() == "project" { + return true, "", nil + } + return false, "deny by default", nil } func TestImpersonationFilter(t *testing.T) { testCases := []struct { - name string - user user.Info - impersonationUser string - impersonationGroups []string - expectedUser user.Info - expectedCode int + name string + user user.Info + impersonationUser string + impersonationGroups []string + impersonationUserExtras map[string][]string + expectedUser user.Info + expectedCode int }{ { name: "not-impersonating", @@ -106,6 +120,17 @@ func TestImpersonationFilter(t *testing.T) { }, expectedCode: http.StatusForbidden, }, + { + name: "impersonating-extra-without-user", + user: &user.DefaultInfo{ + Name: "tester", + }, + impersonationUserExtras: map[string][]string{"scopes": {"scope-a"}}, + expectedUser: &user.DefaultInfo{ + Name: "tester", + }, + expectedCode: http.StatusForbidden, + }, { name: "disallowed-group", user: &user.DefaultInfo{ @@ -135,6 +160,66 @@ func TestImpersonationFilter(t *testing.T) { }, expectedCode: http.StatusOK, }, + { + name: "disallowed-userextra-1", + user: &user.DefaultInfo{ + Name: "dev", + Groups: []string{"wheel"}, + }, + impersonationUser: "system:admin", + impersonationGroups: []string{"some-group"}, + impersonationUserExtras: map[string][]string{"scopes": {"scope-a"}}, + expectedUser: &user.DefaultInfo{ + Name: "dev", + Groups: []string{"wheel"}, + }, + expectedCode: http.StatusForbidden, + }, + { + name: "disallowed-userextra-2", + user: &user.DefaultInfo{ + Name: "dev", + Groups: []string{"wheel", "extra-setter-project"}, + }, + impersonationUser: "system:admin", + impersonationGroups: []string{"some-group"}, + impersonationUserExtras: map[string][]string{"scopes": {"scope-a"}}, + expectedUser: &user.DefaultInfo{ + Name: "dev", + Groups: []string{"wheel", "extra-setter-project"}, + }, + expectedCode: http.StatusForbidden, + }, + { + name: "disallowed-userextra-3", + user: &user.DefaultInfo{ + Name: "dev", + Groups: []string{"wheel", "extra-setter-particular-scopes"}, + }, + impersonationUser: "system:admin", + impersonationGroups: []string{"some-group"}, + impersonationUserExtras: map[string][]string{"scopes": {"scope-a", "scope-b"}}, + expectedUser: &user.DefaultInfo{ + Name: "dev", + Groups: []string{"wheel", "extra-setter-particular-scopes"}, + }, + expectedCode: http.StatusForbidden, + }, + { + name: "allowed-userextras", + user: &user.DefaultInfo{ + Name: "dev", + Groups: []string{"wheel", "extra-setter-scopes"}, + }, + impersonationUser: "system:admin", + impersonationUserExtras: map[string][]string{"scopes": {"scope-a", "scope-b"}}, + expectedUser: &user.DefaultInfo{ + Name: "system:admin", + Groups: []string{}, + Extra: map[string][]string{"scopes": {"scope-a", "scope-b"}}, + }, + expectedCode: http.StatusOK, + }, { name: "allowed-users-impersonation", user: &user.DefaultInfo{ @@ -238,6 +323,12 @@ func TestImpersonationFilter(t *testing.T) { for _, group := range tc.impersonationGroups { req.Header.Add(authenticationapi.ImpersonateGroupHeader, group) } + for extraKey, values := range tc.impersonationUserExtras { + for _, value := range values { + req.Header.Add(authenticationapi.ImpersonateUserExtraHeaderPrefix+extraKey, value) + } + } + resp, err := http.DefaultClient.Do(req) if err != nil { t.Errorf("%s: unexpected error: %v", tc.name, err) diff --git a/pkg/auth/user/user.go b/pkg/auth/user/user.go index 99261965d81..7e7cc16f68b 100644 --- a/pkg/auth/user/user.go +++ b/pkg/auth/user/user.go @@ -36,6 +36,8 @@ type Info interface { // This is a map[string][]string because it needs to be serializeable into // a SubjectAccessReviewSpec.authorization.k8s.io for proper authorization // delegation flows + // In order to faithfully round-trip through an impersonation flow, these keys + // MUST be lowercase. GetExtra() map[string][]string }