diff --git a/authz/authz.go b/authz/authz.go index f48943c04..42c3bba2f 100644 --- a/authz/authz.go +++ b/authz/authz.go @@ -49,27 +49,23 @@ type Auth struct { } type GroupPermissions struct { - Permissions []Permissions `yaml:"group_permissions"` -} - -type Permissions struct { - GroupId string `yaml:"groupId"` - Index string `yaml:"index"` - Permission string `yaml:"permission"` + Permissions map[string]map[string]string } type Group struct { - ID string `json:"id"` - Name string `json:"displayName"` + UserID string + GroupID string `json:"id"` + GroupName string `json:"displayName"` } func (p *GroupPermissions) ReadPermissionsFile(permsFile io.Reader) (err error) { permsData, err := ioutil.ReadAll(permsFile) + if err != nil { return fmt.Errorf("reading permissions failed with error: %s", err) } - err = yaml.Unmarshal(permsData, p) + err = yaml.UnmarshalStrict(permsData, &p.Permissions) if err != nil { return fmt.Errorf("unmarshalling permissions failed with error: %s", err) } @@ -77,54 +73,33 @@ func (p *GroupPermissions) ReadPermissionsFile(permsFile io.Reader) (err error) return nil } -func (p *GroupPermissions) GetPermissions(groups []Group, index []string) (permission string, err error) { +func (p *GroupPermissions) GetPermissions(groups []Group, index string) (permission string, errors error) { - // get union of groups the user is part of obtained from identity provider and groups in permissions file - var groupMatch []Permissions - for _, group := range groups { - for i := range p.Permissions { - if group.ID == p.Permissions[i].GroupId { - groupMatch = append(groupMatch, p.Permissions[i]) - } - } - } - - if len(groupMatch) == 0 { - return "", fmt.Errorf("the user's groups %s are NOT allowed access to FeatureBase", groups) - } - - // check that user's groups have access to the index user want to access - var indexMatch []Permissions - indexCheck := map[string]bool{} - for _, g := range groupMatch { - for _, idx := range index { - if idx == g.Index { - indexMatch = append(indexMatch, g) - indexCheck[idx] = true - } - } - } - - // check which index user does NOT have access to, and return in error mesg - if len(indexCheck) != len(index) { - var indexNotFound []string - for _, idx := range index { - if !indexCheck[idx] { - indexNotFound = append(indexNotFound, idx) - } - } - return "", fmt.Errorf("user is NOT allowed access to index: %s", indexNotFound) - } - - // check permissions for index user has access to allPermissions := map[string]bool{ "admin": false, "write": false, "read": false, } - for _, g := range indexMatch { - allPermissions[g.Permission] = true + if len(groups) == 0 { + return "", fmt.Errorf("user is not part of any groups in identity provider") + } + + var groupsDenied []string + for _, group := range groups { + if _, ok := p.Permissions[group.GroupID]; ok { + if perm, ok := p.Permissions[group.GroupID][index]; ok { + allPermissions[perm] = true + } else { + return "", fmt.Errorf("User %s is NOT allowed access to index %s", group.UserID, index) + } + } else { + groupsDenied = append(groupsDenied, group.GroupID) + } + } + + if len(groupsDenied) == len(groups) { + return "", fmt.Errorf("group(s) %s are NOT allowed access to FeatureBase", groupsDenied) } if allPermissions["admin"] { diff --git a/authz/authz_test.go b/authz/authz_test.go index b1fbc4d6a..fcec43185 100644 --- a/authz/authz_test.go +++ b/authz/authz_test.go @@ -23,64 +23,43 @@ import ( ) func TestAuth_ReadPermissionsFile(t *testing.T) { - var singleInput = `group_permissions: - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee55906b" - index: "test" - permission: "read"` + singleInput := `"dca35310-ecda-4f23-86cd-876aee55906b": + "test": "read"` - var emptyInput = `group_permissions: - - group: - groupId: "" - index: "" - permission: ""` + multiInput := `"dca35310-ecda-4f23-86cd-876aee55906b": + "test": "read" +"dca35310-ecda-4f23-86cd-876aee559900": + "test": "admin"` - var multiInput = `group_permissions: - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee55906b" - index: "test" - permission: "read" - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee559900" - index: "test" - permission: "admin"` - - singleStruct := authz.GroupPermissions{ - Permissions: []authz.Permissions{ - {"dca35310-ecda-4f23-86cd-876aee55906b", "test", "read"}, - }, + singleStruct := map[string]map[string]string{ + "dca35310-ecda-4f23-86cd-876aee55906b": {"test": "read"}, } - emptyStruct := authz.GroupPermissions{ - Permissions: []authz.Permissions{ - {"", "", ""}, - }, - } - multiStruct := authz.GroupPermissions{ - Permissions: []authz.Permissions{ - {"dca35310-ecda-4f23-86cd-876aee55906b", "test", "read"}, - {"dca35310-ecda-4f23-86cd-876aee559900", "test", "admin"}, - }, + + multiStruct := map[string]map[string]string{ + "dca35310-ecda-4f23-86cd-876aee55906b": {"test": "read"}, + "dca35310-ecda-4f23-86cd-876aee559900": {"test": "admin"}, } tests := []struct { input string - output authz.GroupPermissions + output map[string]map[string]string }{ {singleInput, singleStruct}, - {emptyInput, emptyStruct}, {multiInput, multiStruct}, } for i, test := range tests { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { permFile := strings.NewReader(test.input) + var p authz.GroupPermissions - if err := p.ReadPermissionsFile(permFile); err != nil { + err := p.ReadPermissionsFile(permFile) + if err != nil { t.Fatalf("readPermissionsFile error: %s", err) } - if !reflect.DeepEqual(p, test.output) { - t.Fatalf("expected output %s, but got %s", test.output, p) + if !reflect.DeepEqual(p.Permissions, test.output) { + t.Fatalf("expected output %s, but got %s", test.output, p.Permissions) } }, ) @@ -89,103 +68,84 @@ func TestAuth_ReadPermissionsFile(t *testing.T) { func TestAuth_GetPermissions(t *testing.T) { // initializes different example of permissions file in yaml - var permissions1 = `group_permissions: - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee55906b" - index: "test" - permission: "read"` + permissions1 := `"dca35310-ecda-4f23-86cd-876aee55906b": + "test": "read"` - var permissions2 = `group_permissions: - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee559900" - index: "test" - permission: "read" - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee559900" - index: "test" - permission: "write"` + permissions2 := `"dca35310-ecda-4f23-86cd-876aee559900": + "test": "write"` - var permissions3 = `group_permissions: - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee559900" - index: "test" - permission: "read" - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee559900" - index: "test" - permission: "admin"` + permissions3 := `"dca35310-ecda-4f23-86cd-876aee55906b": + "test": "write" + "test2": "read" +"dca35310-ecda-4f23-86cd-876aee559900": + "test": "admin"` - var permissions4 = `group_permissions: - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee55906b" - index: "test" - permission: "" - - group: - groupId: "dca35310-ecda-4f23-86cd-876aee559900" - index: "test" - permission: "admin"` + permissions4 := `"dca35310-ecda-4f23-86cd-876aee559900": + "test": ""` // initializes groups that are returned from identity provider + groupName := "name" + userId := "user-id" groupsList1 := []authz.Group{} - groupsList2 := []authz.Group{{"dca35310-ecda-4f23-86cd-876aee55906b", "name"}} + groupsList2 := []authz.Group{{userId, "fake-group", groupName}} groupsList3 := []authz.Group{ - {"dca35310-ecda-4f23-86cd-876aee55906b", "name"}, - {"dca35310-ecda-4f23-86cd-876aee559900", "name"}, + {userId, "dca35310-ecda-4f23-86cd-876aee55906b", groupName}, + {userId, "dca35310-ecda-4f23-86cd-876aee559900", groupName}, } tests := []struct { yamlData string groups []authz.Group - index []string + index string userAccess string err string }{ { permissions1, groupsList1, - []string{"test"}, + "test", + "", + "user is not part of any groups in identity provider", + }, + { + permissions1, + groupsList3, + "test1", + "", + "NOT allowed access to index", + }, + { + permissions2, + groupsList2, + "test", "", "NOT allowed access to FeatureBase", }, { permissions1, - groupsList2, - []string{"test1"}, - "", - "NOT allowed access to index", - }, - { - permissions1, - groupsList2, - []string{"test"}, + groupsList3, + "test", "read", "", }, { permissions2, groupsList3, - []string{"test"}, + "test", "write", "", }, { permissions3, groupsList3, - []string{"test"}, + "test", "admin", "", }, - { - permissions2, - groupsList3, - []string{"test"}, - "write", - "", - }, { permissions4, - groupsList2, - []string{"test"}, + groupsList3, + "test", "", "no permissions found", }, @@ -195,8 +155,11 @@ func TestAuth_GetPermissions(t *testing.T) { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { permFile := strings.NewReader(test.yamlData) + var p authz.GroupPermissions - p.ReadPermissionsFile(permFile) + if err := p.ReadPermissionsFile(permFile); err != nil { + t.Errorf("Error: %s", err) + } p1, err := p.GetPermissions(test.groups, test.index) diff --git a/server/config.go b/server/config.go index 1f3b62b4a..866ffb3ff 100644 --- a/server/config.go +++ b/server/config.go @@ -4,6 +4,7 @@ package server import ( "context" "fmt" + "io" "log" "net" "net/url" @@ -598,9 +599,9 @@ func lookupAddr(ctx context.Context, resolver *net.Resolver, host string) (strin return addrs[0].String(), nil } -func (c *Config) ValidateAuth() ([]error, error) { +func (c *Config) ValidateAuth() (errors []error) { if !c.Auth.Enable { - return []error{}, nil + return errors } authConfig := map[string]string{ "ClientId": c.Auth.ClientId, @@ -609,10 +610,8 @@ func (c *Config) ValidateAuth() ([]error, error) { "TokenURL": c.Auth.TokenURL, "GroupEndpointURL": c.Auth.GroupEndpointURL, "ScopeURL": c.Auth.ScopeURL, - "PermissionsFile": c.Auth.PermissionsFile, } - errors := make([]error, 0) for name, value := range authConfig { if value == "" { errors = append(errors, fmt.Errorf("empty string for auth config %s", name)) @@ -626,50 +625,99 @@ func (c *Config) ValidateAuth() ([]error, error) { continue } } + } - if strings.Contains(name, "File") { - fileExt := filepath.Ext(value) - if (fileExt != ".yaml") && (fileExt != ".yml") { - errors = append(errors, fmt.Errorf("invalid file extension for auth config %s: %s", name, value)) + if len(errors) > 0 { + return errors + } + return nil +} + +func (c *Config) ValidatePermissions(permsFile io.Reader) (errors []error) { + + var p authz.GroupPermissions + if err := p.ReadPermissionsFile(permsFile); err != nil { + return append(errors, err) + } + + if len(p.Permissions) == 0 { + return append(errors, fmt.Errorf("no group permissions found in permissions file: %s", c.Auth.PermissionsFile)) + } + + for groupId, indexPerm := range p.Permissions { + if groupId == "" { + errors = append(errors, fmt.Errorf("empty string for group id in permissions file %s", c.Auth.PermissionsFile)) + continue + } + + for index, perm := range indexPerm { + if index == "" { + errors = append(errors, fmt.Errorf("empty string for index for group id %s in permissions file %s ", groupId, c.Auth.PermissionsFile)) + continue + } + + if perm == "" { + errors = append(errors, fmt.Errorf("empty string for permission for group id %s and index %s in permissions file %s", groupId, index, c.Auth.PermissionsFile)) + 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)) continue } } } - if len(errors) > 0 { - return errors, fmt.Errorf("there were errors validating config") + return errors } - return errors, nil -} - -func (c *Config) ValidatePermissions() (err error) { - permsFile, err := os.Open(c.Auth.PermissionsFile) - if err != nil { - return err - } - - var p *authz.GroupPermissions - if err := p.ReadPermissionsFile(permsFile); err != nil { - return err - } - - if len(p.Permissions) == 0 { - return fmt.Errorf("no group permissions found in permissions file: %s", c.Auth.PermissionsFile) - } - - defer permsFile.Close() return nil } +func (c *Config) ValidatePermissionsFile() (err error) { + + if c.Auth.PermissionsFile == "" { + return fmt.Errorf("empty string for auth config permissions file") + } + + fileExt := filepath.Ext(c.Auth.PermissionsFile) + if (fileExt != ".yaml") && (fileExt != ".yml") { + return fmt.Errorf("invalid file extension for auth config permissions file: %s", c.Auth.PermissionsFile) + } + return nil +} + func (c *Config) MustValidateAuth() { - if errors, err := c.ValidateAuth(); err != nil { - for _, e1 := range errors { - log.Println(e1) + + errorsAuth := c.ValidateAuth() + if len(errorsAuth) > 0 { + for _, e := range errorsAuth { + log.Println(e) } - if e2 := c.ValidatePermissions(); e2 != nil { - log.Println(e2) + } + + var errorsPerm []error + errorsPermFile := c.ValidatePermissionsFile() + if errorsPermFile == nil { + permsFile, err := os.Open(c.Auth.PermissionsFile) + if err != nil { + log.Println(err) } - log.Fatal(err) + + defer permsFile.Close() + + errorsPerm = c.ValidatePermissions(permsFile) + if len(errorsPerm) > 0 { + for _, e := range errorsPerm { + log.Println(e) + } + } + + } else { + log.Println(errorsPermFile) + } + + if len(errorsAuth) > 0 || len(errorsPerm) > 0 || errorsPermFile != nil { + log.Fatal(fmt.Errorf("there were errors validating authN/authZ config and/or permissions")) } } diff --git a/server/config_internal_test.go b/server/config_internal_test.go index 4d6dcc71c..48ba081db 100644 --- a/server/config_internal_test.go +++ b/server/config_internal_test.go @@ -281,12 +281,9 @@ func TestConfig_validateAddrsGRPC(t *testing.T) { func TestConfig_validateAuth(t *testing.T) { errorMesgEmpty := "empty string" errorMesgURL := "invalid URL" - errorMesgPermissions := "invalid file extension" validTestURL := "https://url.com/" validClientID := "clientid" validClientSecret := "clientSecret" - validFilename := "permissions.yaml" - invalidFilename := "permissions.txt" invalidURL := "not-a-url" emptyString := "" enable := true @@ -306,7 +303,6 @@ func TestConfig_validateAuth(t *testing.T) { errorMesgEmpty, errorMesgEmpty, errorMesgEmpty, - errorMesgEmpty, }, authz.Auth{ Enable: enable, @@ -316,106 +312,6 @@ func TestConfig_validateAuth(t *testing.T) { TokenURL: emptyString, GroupEndpointURL: emptyString, ScopeURL: emptyString, - PermissionsFile: emptyString, - }, - }, - { - // Auth enabled, some configs are set to empty string - []string{ - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - }, - authz.Auth{ - Enable: enable, - ClientId: validClientID, - ClientSecret: emptyString, - AuthorizeURL: emptyString, - TokenURL: emptyString, - GroupEndpointURL: emptyString, - ScopeURL: emptyString, - PermissionsFile: emptyString, - }, - }, - { - // Auth enabled, some configs are set to empty string - []string{ - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - }, - authz.Auth{ - Enable: enable, - ClientId: emptyString, - ClientSecret: validClientSecret, - AuthorizeURL: emptyString, - TokenURL: emptyString, - GroupEndpointURL: emptyString, - ScopeURL: emptyString, - PermissionsFile: emptyString, - }, - }, - { - // Auth enabled, some configs are set to empty string - []string{ - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - }, - authz.Auth{ - Enable: enable, - ClientId: validClientID, - ClientSecret: validClientSecret, - AuthorizeURL: emptyString, - TokenURL: emptyString, - GroupEndpointURL: emptyString, - ScopeURL: emptyString, - PermissionsFile: emptyString, - }, - }, - { - // Auth enabled, some configs are set to empty string - []string{ - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - }, - authz.Auth{ - Enable: enable, - ClientId: validClientID, - ClientSecret: validClientSecret, - AuthorizeURL: validTestURL, - TokenURL: emptyString, - GroupEndpointURL: emptyString, - ScopeURL: emptyString, - PermissionsFile: emptyString, - }, - }, - { - // Auth enabled, some configs are set to empty string - []string{ - errorMesgEmpty, - errorMesgEmpty, - errorMesgEmpty, - }, - authz.Auth{ - Enable: enable, - ClientId: validClientID, - ClientSecret: validClientSecret, - AuthorizeURL: validTestURL, - TokenURL: validTestURL, - GroupEndpointURL: emptyString, - ScopeURL: emptyString, - PermissionsFile: emptyString, }, }, { @@ -431,40 +327,6 @@ func TestConfig_validateAuth(t *testing.T) { TokenURL: validTestURL, GroupEndpointURL: validTestURL, ScopeURL: validTestURL, - PermissionsFile: validFilename, - }, - }, - { - // Auth enabled, some strings are set to invalid URL - []string{ - errorMesgURL, - errorMesgURL, - }, - authz.Auth{ - Enable: enable, - ClientId: validClientID, - ClientSecret: validClientSecret, - AuthorizeURL: validTestURL, - TokenURL: invalidURL, - GroupEndpointURL: invalidURL, - ScopeURL: validTestURL, - PermissionsFile: validFilename, - }, - }, - { - // Auth enabled, permissions file is set to invalid string - []string{ - errorMesgPermissions, - }, - authz.Auth{ - Enable: enable, - ClientId: validClientID, - ClientSecret: validClientSecret, - AuthorizeURL: validTestURL, - TokenURL: validTestURL, - GroupEndpointURL: validTestURL, - ScopeURL: validTestURL, - PermissionsFile: invalidFilename, }, }, { @@ -478,7 +340,6 @@ func TestConfig_validateAuth(t *testing.T) { TokenURL: validTestURL, GroupEndpointURL: validTestURL, ScopeURL: validTestURL, - PermissionsFile: validFilename, }, }, { @@ -492,7 +353,6 @@ func TestConfig_validateAuth(t *testing.T) { TokenURL: emptyString, GroupEndpointURL: emptyString, ScopeURL: emptyString, - PermissionsFile: emptyString, }, }, } @@ -502,9 +362,9 @@ func TestConfig_validateAuth(t *testing.T) { c := NewConfig() c.Auth = test.input - errors, err := c.ValidateAuth() + errors := c.ValidateAuth() if len(test.expErrs) > 0 { - if err == nil { + if errors == nil { t.Fatal("expected errors, but none were found") } } @@ -522,3 +382,97 @@ func TestConfig_validateAuth(t *testing.T) { }) } } + +func TestConfig_validatePermissions(t *testing.T) { + permissions0 := `` + + permissions1 := `"": + "test": "read"` + + permissions2 := `"dca35310-ecda-4f23-86cd-876aee559900": + "": "write"` + + permissions3 := `"dca35310-ecda-4f23-86cd-876aee559900": + "test": ""` + + permissions4 := `"dca35310-ecda-4f23-86cd-876aee559900": + "test": "readwrite"` + + tests := []struct { + err string + input string + }{ + { + "no group permissions found in permissions file", + permissions0, + }, + { + "empty string for group id", + permissions1, + }, + { + "empty string for index", + permissions2, + }, + { + "empty string for permission", + permissions3, + }, + { + "not a valid permission", + permissions4, + }, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + + c := NewConfig() + c.Auth.PermissionsFile = "test.yaml" + + permFile := strings.NewReader(test.input) + errors := c.ValidatePermissions(permFile) + + if errors == nil { + t.Fatal("expected errors, but none were found") + } + + for _, err := range errors { + if !strings.Contains(err.Error(), test.err) { + t.Errorf("expected error to contain %s, but got %s", test.err, err.Error()) + + } + } + }) + } +} + +func TestConfig_validatePermissionsFilename(t *testing.T) { + + tests := []struct { + err string + input string + }{ + { + "empty string for auth config permissions file", + "", + }, + { + "invalid file extension for auth config permissions file", + "permissions.txt", + }, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + c := NewConfig() + c.Auth.PermissionsFile = test.input + + if err := c.ValidatePermissionsFile(); err != nil { + if !strings.Contains(err.Error(), test.err) { + t.Errorf("expected error to contain %s, but got %s", test.err, err.Error()) + } + } + }) + } +} diff --git a/server/server.go b/server/server.go index b876f3eb6..c2418630b 100644 --- a/server/server.go +++ b/server/server.go @@ -226,18 +226,17 @@ func (m *Command) Start() (err error) { if m.Config.Auth.Enable { m.Config.MustValidateAuth() - // Read permissions file permsFile, err := os.Open(m.Config.Auth.PermissionsFile) if err != nil { return err } + defer permsFile.Close() + var p authz.GroupPermissions - if err := p.ReadPermissionsFile(permsFile); err != nil { + if err = p.ReadPermissionsFile(permsFile); err != nil { return err } - - defer permsFile.Close() } // Initialize server.