From f6f1ab34aa475ea34c66c7ce3d0308f2df5b59d0 Mon Sep 17 00:00:00 2001 From: deads2k Date: Tue, 12 Jul 2016 13:25:07 -0400 Subject: [PATCH] authorize based on user.Info --- pkg/auth/authorizer/abac/abac.go | 11 +++++++++-- pkg/auth/authorizer/interfaces.go | 18 ++++-------------- plugin/pkg/auth/authorizer/rbac/rbac.go | 10 ++-------- plugin/pkg/auth/authorizer/rbac/rbac_test.go | 5 +++-- plugin/pkg/auth/authorizer/webhook/webhook.go | 13 ++++++++----- test/integration/auth/auth_test.go | 10 +++++----- .../serviceaccount/service_account_test.go | 5 ++++- 7 files changed, 35 insertions(+), 37 deletions(-) diff --git a/pkg/auth/authorizer/abac/abac.go b/pkg/auth/authorizer/abac/abac.go index ebd53ae46b..ca5ef6c633 100644 --- a/pkg/auth/authorizer/abac/abac.go +++ b/pkg/auth/authorizer/abac/abac.go @@ -133,12 +133,19 @@ func matches(p api.Policy, a authorizer.Attributes) bool { func subjectMatches(p api.Policy, a authorizer.Attributes) bool { matched := false + username := "" + groups := []string{} + if user := a.GetUser(); user != nil { + username = user.GetName() + groups = user.GetGroups() + } + // If the policy specified a user, ensure it matches if len(p.Spec.User) > 0 { if p.Spec.User == "*" { matched = true } else { - matched = p.Spec.User == a.GetUserName() + matched = p.Spec.User == username if !matched { return false } @@ -151,7 +158,7 @@ func subjectMatches(p api.Policy, a authorizer.Attributes) bool { matched = true } else { matched = false - for _, group := range a.GetGroups() { + for _, group := range groups { if p.Spec.Group == group { matched = true } diff --git a/pkg/auth/authorizer/interfaces.go b/pkg/auth/authorizer/interfaces.go index 0b7bdc0e1b..c44fd4a2b3 100644 --- a/pkg/auth/authorizer/interfaces.go +++ b/pkg/auth/authorizer/interfaces.go @@ -25,14 +25,8 @@ import ( // Attributes is an interface used by an Authorizer to get information about a request // that is used to make an authorization decision. type Attributes interface { - // The user string which the request was authenticated as, or empty if - // no authentication occurred and the request was allowed to proceed. - GetUserName() string - - // The list of group names the authenticated user is a member of. Can be - // empty if the authenticated user is not in any groups, or if no - // authentication occurred. - GetGroups() []string + // GetUser returns the user.Info object to authorize + GetUser() user.Info // GetVerb returns the kube verb associated with API requests (this includes get, list, watch, create, update, patch, delete, and proxy), // or the lowercased HTTP verb associated with non-API requests (this includes get, put, post, patch, and delete) @@ -101,12 +95,8 @@ type AttributesRecord struct { Path string } -func (a AttributesRecord) GetUserName() string { - return a.User.GetName() -} - -func (a AttributesRecord) GetGroups() []string { - return a.User.GetGroups() +func (a AttributesRecord) GetUser() user.Info { + return a.User } func (a AttributesRecord) GetVerb() string { diff --git a/plugin/pkg/auth/authorizer/rbac/rbac.go b/plugin/pkg/auth/authorizer/rbac/rbac.go index aee5cbd90a..087e564a98 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac.go @@ -22,7 +22,6 @@ import ( "k8s.io/kubernetes/pkg/apis/rbac" "k8s.io/kubernetes/pkg/apis/rbac/validation" "k8s.io/kubernetes/pkg/auth/authorizer" - "k8s.io/kubernetes/pkg/auth/user" "k8s.io/kubernetes/pkg/registry/clusterrole" "k8s.io/kubernetes/pkg/registry/clusterrolebinding" "k8s.io/kubernetes/pkg/registry/role" @@ -36,16 +35,11 @@ type RBACAuthorizer struct { } func (r *RBACAuthorizer) Authorize(attr authorizer.Attributes) error { - if r.superUser != "" && attr.GetUserName() == r.superUser { + if r.superUser != "" && attr.GetUser() != nil && attr.GetUser().GetName() == r.superUser { return nil } - userInfo := &user.DefaultInfo{ - Name: attr.GetUserName(), - Groups: attr.GetGroups(), - } - - ctx := api.WithNamespace(api.WithUser(api.NewContext(), userInfo), attr.GetNamespace()) + ctx := api.WithNamespace(api.WithUser(api.NewContext(), attr.GetUser()), attr.GetNamespace()) // Frame the authorization request as a privilege escalation check. var requestedRule rbac.PolicyRule diff --git a/plugin/pkg/auth/authorizer/rbac/rbac_test.go b/plugin/pkg/auth/authorizer/rbac/rbac_test.go index ec327a7a75..7aaf104dc5 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac_test.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac_test.go @@ -99,8 +99,9 @@ func (d *defaultAttributes) String() string { d.user, strings.Split(d.groups, ","), d.verb, d.resource, d.namespace, d.apiGroup) } -func (d *defaultAttributes) GetUserName() string { return d.user } -func (d *defaultAttributes) GetGroups() []string { return strings.Split(d.groups, ",") } +func (d *defaultAttributes) GetUser() user.Info { + return &user.DefaultInfo{Name: d.user, Groups: strings.Split(d.groups, ",")} +} func (d *defaultAttributes) GetVerb() string { return d.verb } func (d *defaultAttributes) IsReadOnly() bool { return d.verb == "get" || d.verb == "watch" } func (d *defaultAttributes) GetNamespace() string { return d.namespace } diff --git a/plugin/pkg/auth/authorizer/webhook/webhook.go b/plugin/pkg/auth/authorizer/webhook/webhook.go index e6044e48d2..621c72ec9c 100644 --- a/plugin/pkg/auth/authorizer/webhook/webhook.go +++ b/plugin/pkg/auth/authorizer/webhook/webhook.go @@ -129,12 +129,15 @@ func newWithBackoff(kubeConfigFile string, authorizedTTL, unauthorizedTTL, initi // } // func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (err error) { - r := &v1beta1.SubjectAccessReview{ - Spec: v1beta1.SubjectAccessReviewSpec{ - User: attr.GetUserName(), - Groups: attr.GetGroups(), - }, + r := &v1beta1.SubjectAccessReview{} + if user := attr.GetUser(); user != nil { + r.Spec = v1beta1.SubjectAccessReviewSpec{ + User: user.GetName(), + Groups: user.GetGroups(), + Extra: user.GetExtra(), + } } + if attr.IsResourceRequest() { r.Spec.ResourceAttributes = &v1beta1.ResourceAttributes{ Namespace: attr.GetNamespace(), diff --git a/test/integration/auth/auth_test.go b/test/integration/auth/auth_test.go index d75493aa05..d0e23fc2ad 100644 --- a/test/integration/auth/auth_test.go +++ b/test/integration/auth/auth_test.go @@ -537,7 +537,7 @@ func TestAuthModeAlwaysDeny(t *testing.T) { type allowAliceAuthorizer struct{} func (allowAliceAuthorizer) Authorize(a authorizer.Attributes) error { - if a.GetUserName() == "alice" { + if a.GetUser() != nil && a.GetUser().GetName() == "alice" { return nil } return errors.New("I can't allow that. Go ask alice.") @@ -705,18 +705,18 @@ type impersonateAuthorizer struct{} // alice can't act as anyone and bob can't do anything but act-as someone func (impersonateAuthorizer) Authorize(a authorizer.Attributes) error { // alice can impersonate service accounts and do other actions - if a.GetUserName() == "alice" && a.GetVerb() == "impersonate" && a.GetResource() == "serviceaccounts" { + if a.GetUser() != nil && a.GetUser().GetName() == "alice" && a.GetVerb() == "impersonate" && a.GetResource() == "serviceaccounts" { return nil } - if a.GetUserName() == "alice" && a.GetVerb() != "impersonate" { + if a.GetUser() != nil && a.GetUser().GetName() == "alice" && a.GetVerb() != "impersonate" { return nil } // bob can impersonate anyone, but that it - if a.GetUserName() == "bob" && a.GetVerb() == "impersonate" { + if a.GetUser() != nil && a.GetUser().GetName() == "bob" && a.GetVerb() == "impersonate" { return nil } // service accounts can do everything - if strings.HasPrefix(a.GetUserName(), serviceaccount.ServiceAccountUsernamePrefix) { + if a.GetUser() != nil && strings.HasPrefix(a.GetUser().GetName(), serviceaccount.ServiceAccountUsernamePrefix) { return nil } diff --git a/test/integration/serviceaccount/service_account_test.go b/test/integration/serviceaccount/service_account_test.go index dcd7f5e220..f6a38824c0 100644 --- a/test/integration/serviceaccount/service_account_test.go +++ b/test/integration/serviceaccount/service_account_test.go @@ -370,7 +370,10 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie // 2. ServiceAccounts named "ro" are allowed read-only operations in their namespace // 3. ServiceAccounts named "rw" are allowed any operation in their namespace authorizer := authorizer.AuthorizerFunc(func(attrs authorizer.Attributes) error { - username := attrs.GetUserName() + username := "" + if user := attrs.GetUser(); user != nil { + username = user.GetName() + } ns := attrs.GetNamespace() // If the user is "root"...