diff --git a/authz/authorization.go b/authz/authorization.go index f89a52ea6..77bd67ade 100644 --- a/authz/authorization.go +++ b/authz/authorization.go @@ -49,7 +49,8 @@ type Auth struct { } type GroupPermissions struct { - Permissions map[string]map[string]string + Permissions map[string]map[string]string `yaml:"user-groups"` + Admin string `yaml:"admin"` } type Group struct { @@ -65,7 +66,7 @@ func (p *GroupPermissions) ReadPermissionsFile(permsFile io.Reader) (err error) return fmt.Errorf("reading permissions failed with error: %s", err) } - err = yaml.UnmarshalStrict(permsData, &p.Permissions) + err = yaml.UnmarshalStrict(permsData, &p) if err != nil { return fmt.Errorf("unmarshalling permissions failed with error: %s", err) } @@ -75,8 +76,11 @@ func (p *GroupPermissions) ReadPermissionsFile(permsFile io.Reader) (err error) func (p *GroupPermissions) GetPermissions(groups []Group, index string) (permission string, errors error) { + if admin := p.IsAdmin(groups); admin { + return "admin", nil + } + allPermissions := map[string]bool{ - "admin": false, "write": false, "read": false, } @@ -102,9 +106,7 @@ func (p *GroupPermissions) GetPermissions(groups []Group, index string) (permiss return "", fmt.Errorf("group(s) %s does not have permission to FeatureBase", groupsDenied) } - if allPermissions["admin"] { - return "admin", nil - } else if allPermissions["write"] { + if allPermissions["write"] { return "write", nil } else if allPermissions["read"] { return "read", nil @@ -115,26 +117,29 @@ func (p *GroupPermissions) GetPermissions(groups []Group, index string) (permiss func (p *GroupPermissions) IsAdmin(groups []Group) bool { for _, group := range groups { - if _, ok := p.Permissions[group.GroupID]; ok { - for _, permission := range p.Permissions[group.GroupID] { - if permission == "admin" { - return true - } - } + if p.Admin == group.GroupID { + return true } } return false } func (p *GroupPermissions) GetAuthorizedIndexList(groups []Group, desiredPermission string) (indexList []string) { + // if user is admin, find all indexes in permissions file and return them + if admin := p.IsAdmin(groups); admin { + for groupId := range p.Permissions { + for index := range p.Permissions[groupId] { + indexList = append(indexList, index) + } + } + return indexList + } for _, group := range groups { if _, ok := p.Permissions[group.GroupID]; ok { for index, permission := range p.Permissions[group.GroupID] { if permission == desiredPermission { indexList = append(indexList, index) - } else if permission == "admin" { - indexList = append(indexList, index) } else if permission == "write" && desiredPermission == "read" { indexList = append(indexList, index) } diff --git a/authz/authorization_test.go b/authz/authorization_test.go index cd985f973..dab33dbe1 100644 --- a/authz/authorization_test.go +++ b/authz/authorization_test.go @@ -25,29 +25,39 @@ import ( func TestAuth_ReadPermissionsFile(t *testing.T) { - singleInput := `"dca35310-ecda-4f23-86cd-876aee55906b": - "test": "read"` + singleInput := `user-groups: + "dca35310-ecda-4f23-86cd-876aee55906b": + "test": "read" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` - multiInput := `"dca35310-ecda-4f23-86cd-876aee55906b": - "test": "read" -"dca35310-ecda-4f23-86cd-876aee559900": - "test": "admin"` + multiInput := `user-groups: + "dca35310-ecda-4f23-86cd-876aee55906b": + "test": "read" + "test2": "write" + "dca35310-ecda-4f23-86cd-876aee559900": + "test": "write" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` - singleStruct := map[string]map[string]string{ - "dca35310-ecda-4f23-86cd-876aee55906b": {"test": "read"}, + singlePermission := authz.GroupPermissions{ + Permissions: map[string]map[string]string{ + "dca35310-ecda-4f23-86cd-876aee55906b": {"test": "read"}, + }, + Admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe", } - multiStruct := map[string]map[string]string{ - "dca35310-ecda-4f23-86cd-876aee55906b": {"test": "read"}, - "dca35310-ecda-4f23-86cd-876aee559900": {"test": "admin"}, + multiPermission := authz.GroupPermissions{ + Permissions: map[string]map[string]string{ + "dca35310-ecda-4f23-86cd-876aee55906b": {"test": "read", "test2": "write"}, + "dca35310-ecda-4f23-86cd-876aee559900": {"test": "write"}}, + Admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe", } tests := []struct { input string - output map[string]map[string]string + output authz.GroupPermissions }{ - {singleInput, singleStruct}, - {multiInput, multiStruct}, + {singleInput, singlePermission}, + {multiInput, multiPermission}, } for i, test := range tests { @@ -60,8 +70,8 @@ func TestAuth_ReadPermissionsFile(t *testing.T) { t.Fatalf("readPermissionsFile error: %s", err) } - if !reflect.DeepEqual(p.Permissions, test.output) { - t.Fatalf("expected output %s, but got %s", test.output, p.Permissions) + if !reflect.DeepEqual(p, test.output) { + t.Fatalf("expected output %s, but got %s", test.output, p) } }, ) @@ -71,20 +81,28 @@ func TestAuth_ReadPermissionsFile(t *testing.T) { func TestAuth_GetPermissions(t *testing.T) { // initializes different example of permissions file in yaml - permissions1 := `"dca35310-ecda-4f23-86cd-876aee55906b": - "test": "read"` + permissions1 := `"user-groups": + "dca35310-ecda-4f23-86cd-876aee55906b": + "test": "read" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` - permissions2 := `"dca35310-ecda-4f23-86cd-876aee559900": - "test": "write"` + permissions2 := `"user-groups": + "dca35310-ecda-4f23-86cd-876aee559900": + "test": "write" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` - permissions3 := `"dca35310-ecda-4f23-86cd-876aee55906b": - "test": "write" - "test2": "read" -"dca35310-ecda-4f23-86cd-876aee559900": - "test": "admin"` + permissions3 := `"user-groups": + "dca35310-ecda-4f23-86cd-876aee55906b": + "test": "write" + "test2": "read" + "dca35310-ecda-4f23-86cd-876aee559900": + "test": "read" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` - permissions4 := `"dca35310-ecda-4f23-86cd-876aee559900": - "test": ""` + permissions4 := `"user-groups": + "dca35310-ecda-4f23-86cd-876aee559900": + "test": "" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` // initializes groups that are returned from identity provider groupName := "name" @@ -95,6 +113,7 @@ func TestAuth_GetPermissions(t *testing.T) { {userId, "dca35310-ecda-4f23-86cd-876aee55906b", groupName}, {userId, "dca35310-ecda-4f23-86cd-876aee559900", groupName}, } + groupsList4 := []authz.Group{{userId, "ac97c9e2-346b-42a2-b6da-18bcb61a32fe", groupName}} tests := []struct { yamlData string @@ -140,7 +159,7 @@ func TestAuth_GetPermissions(t *testing.T) { }, { permissions3, - groupsList3, + groupsList4, "test", "admin", "", @@ -182,34 +201,37 @@ func TestAuth_GetPermissions(t *testing.T) { func TestAuth_IsAdmin(t *testing.T) { - group := []authz.Group{ - {"user-is", "dca35310-ecda-4f23-86cd-876aee55906b", "group-name"}, + group1 := []authz.Group{ + {"admin-user-id", "ac97c9e2-346b-42a2-b6da-18bcb61a32fe", "admin-group"}, } - groupPermissions1 := map[string]map[string]string{ - "dca35310-ecda-4f23-86cd-876aee55906b": {"test": "admin"}, + group2 := []authz.Group{ + {"user-id", "dca35310-ecda-4f23-86cd-876aee55906b", "group-name"}, } - groupPermissions2 := map[string]map[string]string{ - "dca35310-ecda-4f23-86cd-876aee55906b": {"test": "read"}, + groupPermissions := authz.GroupPermissions{ + Permissions: map[string]map[string]string{ + "dca35310-ecda-4f23-86cd-876aee55906b": {"test": "write"}, + }, + Admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe", } tests := []struct { groups []authz.Group - groupPermissions map[string]map[string]string + groupPermissions authz.GroupPermissions output bool }{ { - group, groupPermissions1, true, + group1, groupPermissions, true, }, { - group, groupPermissions2, false, + group2, groupPermissions, false, }, } for i, test := range tests { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - p := authz.GroupPermissions{test.groupPermissions} + p := test.groupPermissions resp := p.IsAdmin(test.groups) if resp != test.output { t.Errorf("expected %t, but got %t", test.output, resp) @@ -220,17 +242,30 @@ func TestAuth_IsAdmin(t *testing.T) { func TestAuth_GetAuthorizedIndexList(t *testing.T) { - group := []authz.Group{ - {"user-is", "dca35310-ecda-4f23-86cd-876aee55906b", "group-name"}, + group1 := []authz.Group{ + {"user-id", "dca35310-ecda-4f23-86cd-876aee55906b", "group-name"}, } - p := authz.GroupPermissions{map[string]map[string]string{ - "dca35310-ecda-4f23-86cd-876aee55906b": { - "test1": "admin", - "test2": "read", - "test3": "write", + group2 := []authz.Group{ + {"admin-user-id", "ac97c9e2-346b-42a2-b6da-18bcb61a32fe", "admin-group"}, + } + + group3 := []authz.Group{ + {"user-id", "dca35310-ecda-4f23-86cd-876aee559900", "group-name"}, + } + + p := authz.GroupPermissions{ + Permissions: map[string]map[string]string{ + "dca35310-ecda-4f23-86cd-876aee55906b": { + "test1": "read", + "test2": "write", + }, + "dca35310-ecda-4f23-86cd-876aee559900": { + "test3": "read", + }, }, - }} + Admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe", + } tests := []struct { groups []authz.Group @@ -238,19 +273,29 @@ func TestAuth_GetAuthorizedIndexList(t *testing.T) { output []string }{ { - group, + group1, + "read", + []string{"test1", "test2"}, + }, + { + group1, + "write", + []string{"test2"}, + }, + { + group3, + "write", + nil, + }, + { + group2, "read", []string{"test1", "test2", "test3"}, }, { - group, - "admin", - []string{"test1"}, - }, - { - group, + group2, "write", - []string{"test1", "test3"}, + []string{"test1", "test2", "test3"}, }, } diff --git a/server/config.go b/server/config.go index 5f4f0573c..807e19b7f 100644 --- a/server/config.go +++ b/server/config.go @@ -657,12 +657,17 @@ func (c *Config) ValidatePermissions(permsFile io.Reader) (errors []error) { continue } - if !((perm == "admin") || (perm == "write") || (perm == "read")) { - errors = append(errors, fmt.Errorf("not a valid permission %s for group id %s and index %s in permissions file %s", perm, groupId, index, c.Auth.PermissionsFile)) + if !((perm == "write") || (perm == "read")) { + errors = append(errors, fmt.Errorf("not a valid permission %s for group id %s and index %s in permissions file %s; expected permissions are read or write", perm, groupId, index, c.Auth.PermissionsFile)) continue } } } + + if p.Admin == "" { + errors = append(errors, fmt.Errorf("empty string for admin in permissions file: %s", c.Auth.PermissionsFile)) + } + return errors } diff --git a/server/config_internal_test.go b/server/config_internal_test.go index 48ba081db..11148dfad 100644 --- a/server/config_internal_test.go +++ b/server/config_internal_test.go @@ -386,17 +386,29 @@ func TestConfig_validateAuth(t *testing.T) { func TestConfig_validatePermissions(t *testing.T) { permissions0 := `` - permissions1 := `"": - "test": "read"` + permissions1 := `user-groups: + "": + "test": "read" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` - permissions2 := `"dca35310-ecda-4f23-86cd-876aee559900": - "": "write"` + permissions2 := `user-groups: + "dca35310-ecda-4f23-86cd-876aee559900": + "": "write" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` - permissions3 := `"dca35310-ecda-4f23-86cd-876aee559900": - "test": ""` + permissions3 := `user-groups: + "dca35310-ecda-4f23-86cd-876aee559900": + "test": "" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` - permissions4 := `"dca35310-ecda-4f23-86cd-876aee559900": - "test": "readwrite"` + permissions4 := `user-groups: + "dca35310-ecda-4f23-86cd-876aee559900": + "test": "readwrite" +admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` + + permissions5 := `user-groups: + "dca35310-ecda-4f23-86cd-876aee559900": + "test": "read"` tests := []struct { err string @@ -422,6 +434,10 @@ func TestConfig_validatePermissions(t *testing.T) { "not a valid permission", permissions4, }, + { + "empty string for admin in permissions file", + permissions5, + }, } for i, test := range tests {