Initial addition of groups to user/policy

duda · Dec 18, 2014 · 9d8d313 · 9d8d313
1 parent 3f926e9
commit 9d8d313
Show file tree

Hide file tree

Showing 4 changed files with 162 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
+}