diff --git a/pkg/auth/authorizer/abac/abac.go b/pkg/auth/authorizer/abac/abac.go index 03b704c2eec..cba7a796e78 100644 --- a/pkg/auth/authorizer/abac/abac.go +++ b/pkg/auth/authorizer/abac/abac.go @@ -37,8 +37,8 @@ import ( // body.Namespace, if we want to add that feature, without affecting the // meta.Namespace. type policy struct { - User string `json:"user,omitempty"` - // TODO: add support for groups as well as users. + User string `json:"user,omitempty"` + Group string `json:"group,omitempty"` // TODO: add support for robot accounts as well as human user accounts. // TODO: decide how to namespace user names when multiple authentication // providers are in use. Either add "Realm", or assume "user@example.com" @@ -98,7 +98,7 @@ func NewFromFile(path string) (policyList, error) { } func (p policy) matches(a authorizer.Attributes) bool { - if p.User == "" || p.User == a.GetUserName() { + if p.subjectMatches(a) { if p.Readonly == false || (p.Readonly == a.IsReadOnly()) { if p.Kind == "" || (p.Kind == a.GetKind()) { if p.Namespace == "" || (p.Namespace == a.GetNamespace()) { @@ -110,6 +110,27 @@ func (p policy) matches(a authorizer.Attributes) bool { return false } +func (p policy) subjectMatches(a authorizer.Attributes) bool { + if p.User != "" { + // Require user match + if p.User != a.GetUserName() { + return false + } + } + + if p.Group != "" { + // Require group match + for _, group := range a.GetGroups() { + if p.Group == group { + return true + } + } + return false + } + + return true +} + // Authorizer implements authorizer.Authorize func (pl policyList) Authorize(a authorizer.Attributes) error { for _, p := range pl { diff --git a/pkg/auth/authorizer/abac/abac_test.go b/pkg/auth/authorizer/abac/abac_test.go index ded50a87daf..06855586a87 100644 --- a/pkg/auth/authorizer/abac/abac_test.go +++ b/pkg/auth/authorizer/abac/abac_test.go @@ -131,6 +131,126 @@ func NotTestAuthorize(t *testing.T) { } } +func TestSubjectMatches(t *testing.T) { + testCases := map[string]struct { + User user.DefaultInfo + PolicyUser string + PolicyGroup string + ExpectMatch bool + }{ + "empty policy matches unauthed user": { + User: user.DefaultInfo{}, + PolicyUser: "", + PolicyGroup: "", + ExpectMatch: true, + }, + "empty policy matches authed user": { + User: user.DefaultInfo{Name: "Foo"}, + PolicyUser: "", + PolicyGroup: "", + ExpectMatch: true, + }, + "empty policy matches authed user with groups": { + User: user.DefaultInfo{Name: "Foo", Groups: []string{"a", "b"}}, + PolicyUser: "", + PolicyGroup: "", + ExpectMatch: true, + }, + + "user policy does not match unauthed user": { + User: user.DefaultInfo{}, + PolicyUser: "Foo", + PolicyGroup: "", + ExpectMatch: false, + }, + "user policy does not match different user": { + User: user.DefaultInfo{Name: "Bar"}, + PolicyUser: "Foo", + PolicyGroup: "", + ExpectMatch: false, + }, + "user policy is case-sensitive": { + User: user.DefaultInfo{Name: "foo"}, + PolicyUser: "Foo", + PolicyGroup: "", + ExpectMatch: false, + }, + "user policy does not match substring": { + User: user.DefaultInfo{Name: "FooBar"}, + PolicyUser: "Foo", + PolicyGroup: "", + ExpectMatch: false, + }, + "user policy matches username": { + User: user.DefaultInfo{Name: "Foo"}, + PolicyUser: "Foo", + PolicyGroup: "", + ExpectMatch: true, + }, + + "group policy does not match unauthed user": { + User: user.DefaultInfo{}, + PolicyUser: "", + PolicyGroup: "Foo", + ExpectMatch: false, + }, + "group policy does not match user in different group": { + User: user.DefaultInfo{Name: "FooBar", Groups: []string{"B"}}, + PolicyUser: "", + PolicyGroup: "A", + ExpectMatch: false, + }, + "group policy is case-sensitive": { + User: user.DefaultInfo{Name: "Foo", Groups: []string{"A", "B", "C"}}, + PolicyUser: "", + PolicyGroup: "b", + ExpectMatch: false, + }, + "group policy does not match substring": { + User: user.DefaultInfo{Name: "Foo", Groups: []string{"A", "BBB", "C"}}, + PolicyUser: "", + PolicyGroup: "B", + ExpectMatch: false, + }, + "group policy matches user in group": { + User: user.DefaultInfo{Name: "Foo", Groups: []string{"A", "B", "C"}}, + PolicyUser: "", + PolicyGroup: "B", + ExpectMatch: true, + }, + + "user and group policy requires user match": { + User: user.DefaultInfo{Name: "Bar", Groups: []string{"A", "B", "C"}}, + PolicyUser: "Foo", + PolicyGroup: "B", + ExpectMatch: false, + }, + "user and group policy requires group match": { + User: user.DefaultInfo{Name: "Foo", Groups: []string{"A", "B", "C"}}, + PolicyUser: "Foo", + PolicyGroup: "D", + ExpectMatch: false, + }, + "user and group policy matches": { + User: user.DefaultInfo{Name: "Foo", Groups: []string{"A", "B", "C"}}, + PolicyUser: "Foo", + PolicyGroup: "B", + ExpectMatch: true, + }, + } + + for k, tc := range testCases { + attr := authorizer.AttributesRecord{ + User: &tc.User, + } + actualMatch := policy{User: tc.PolicyUser, Group: tc.PolicyGroup}.subjectMatches(attr) + if tc.ExpectMatch != actualMatch { + t.Errorf("%v: Expected actorMatches=%v but actually got=%v", + k, tc.ExpectMatch, actualMatch) + } + } +} + func newWithContents(t *testing.T, contents string) (authorizer.Authorizer, error) { f, err := ioutil.TempFile("", "abac_test") if err != nil { diff --git a/pkg/auth/authorizer/interfaces.go b/pkg/auth/authorizer/interfaces.go index 5913244141e..30d897d0f41 100644 --- a/pkg/auth/authorizer/interfaces.go +++ b/pkg/auth/authorizer/interfaces.go @@ -26,7 +26,11 @@ type Attributes interface { // The user string which the request was authenticated as, or empty if // no authentication occured and the request was allowed to proceed. GetUserName() string - // TODO: add groups, e.g. GetGroups() []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 // When IsReadOnly() == true, the request has no side effects, other than // caching, logging, and other incidentals. @@ -58,6 +62,10 @@ func (a AttributesRecord) GetUserName() string { return a.User.GetName() } +func (a AttributesRecord) GetGroups() []string { + return a.User.GetGroups() +} + func (a AttributesRecord) IsReadOnly() bool { return a.ReadOnly } diff --git a/pkg/auth/user/user.go b/pkg/auth/user/user.go index e38da660ef6..36407fb0cf4 100644 --- a/pkg/auth/user/user.go +++ b/pkg/auth/user/user.go @@ -25,13 +25,16 @@ type Info interface { // if the user is removed from the system and another user is added with // the same name. GetUID() string + // GetGroups returns the names of the groups the user is a member of + GetGroups() []string } // DefaultInfo provides a simple user information exchange object // for components that implement the UserInfo interface. type DefaultInfo struct { - Name string - UID string + Name string + UID string + Groups []string } func (i *DefaultInfo) GetName() string { @@ -41,3 +44,7 @@ func (i *DefaultInfo) GetName() string { func (i *DefaultInfo) GetUID() string { return i.UID } + +func (i *DefaultInfo) GetGroups() []string { + return i.Groups +}