added reviewer's suggestions

auth/auth.go

@@ -56,9 +56,9 @@ func ReadPermissionsFile(filePath string) (yamlData []byte) {
 }
 
 func (p *GroupPermissions) CreatePermissionsStruct(data []byte) {
-	err := yaml.Unmarshal([]byte(data), &p)
+	err := yaml.Unmarshal([]byte(data), p)
 	if err != nil {
-		log.Fatalf("Error %s", err)
+		log.Fatalf("error %s", err)
 	}
 }
 
@@ -87,7 +87,7 @@ func (p *GroupPermissions) ResolvePermissions(groups []map[string]string, index
 	}
 
 	if len(groupMatch) == 0 {
-		return "", fmt.Errorf("User is NOT allowed access to FeatureBase")
+		return "", fmt.Errorf("user is NOT allowed access to FeatureBase")
 	}
 
 	// check that user's groups have access to the index user want to access
@@ -102,16 +102,15 @@ func (p *GroupPermissions) ResolvePermissions(groups []map[string]string, index
 		}
 	}
 
-	// check that user has access to every index
-	indexCount := 0
-	for _, value := range indexCheck {
-		if value {
-			indexCount += 1
+	// 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)
+			}
 		}
-	}
-
-	if indexCount != len(index) {
-		return "", fmt.Errorf("User is not allowed access to index: %s", index)
+		return "", fmt.Errorf("user is not allowed access to index: %s", indexNotFound)
 	}
 
 	// check permissions for index user has access to
@@ -122,18 +121,16 @@ func (p *GroupPermissions) ResolvePermissions(groups []map[string]string, index
 	}
 
 	for _, g := range indexMatch {
-		if !allPermissions[g.Permission] {
-			allPermissions[g.Permission] = true
-		}
+		allPermissions[g.Permission] = true
 	}
 
 	if allPermissions["admin"] {
-		return "admin", error(nil)
+		return "admin", nil
 	} else if allPermissions["write"] {
-		return "write", error(nil)
+		return "write", nil
 	} else if allPermissions["read"] {
-		return "read", error(nil)
+		return "read", nil
 	} else {
-		return "", fmt.Errorf("No permissions found")
+		return "", fmt.Errorf("no permissions found")
 	}
 }

auth/auth_test.go

@@ -22,19 +22,6 @@ import (
 	"github.com/molecula/featurebase/v2/auth"
 )
 
-func createStruct(inputs [][]string) (permissions auth.GroupPermissions) {
-	var sliceStruct []auth.Permissions
-	for _, i := range inputs {
-		groupId := i[0]
-		index := i[1]
-		permission := i[2]
-		p := auth.Permissions{groupId, index, permission}
-		sliceStruct = append(sliceStruct, p)
-	}
-	permissions = auth.GroupPermissions{Permissions: sliceStruct}
-	return permissions
-}
-
 func TestAuth_CreatePermissionsStruct(t *testing.T) {
 	var singleInput = []byte(`group_permissions:
   - group:
@@ -58,21 +45,22 @@ func TestAuth_CreatePermissionsStruct(t *testing.T) {
     index: "test"
     permission: "admin"`)
 
-	var slice1 [][]string
-	var slice2 [][]string
-	var slice3 [][]string
-	var subslice1 []string
-	var subslice2 []string
-	var subslice3 []string
-	subslice1 = append(subslice1, "dca35310-ecda-4f23-86cd-876aee55906b", "test", "read")
-	subslice2 = append(subslice2, "", "", "")
-	subslice3 = append(subslice3, "dca35310-ecda-4f23-86cd-876aee559900", "test", "admin")
-	slice1 = append(slice1, subslice1)
-	slice2 = append(slice2, subslice2)
-	slice3 = append(slice3, subslice1, subslice3)
-	singleStruct := createStruct(slice1)
-	emptyStruct := createStruct(slice2)
-	multiStruct := createStruct(slice3)
+	singleStruct := auth.GroupPermissions{
+		Permissions: []auth.Permissions{
+			{"dca35310-ecda-4f23-86cd-876aee55906b", "test", "read"},
+		},
+	}
+	emptyStruct := auth.GroupPermissions{
+		Permissions: []auth.Permissions{
+			{"", "", ""},
+		},
+	}
+	multiStruct := auth.GroupPermissions{
+		Permissions: []auth.Permissions{
+			{"dca35310-ecda-4f23-86cd-876aee55906b", "test", "read"},
+			{"dca35310-ecda-4f23-86cd-876aee559900", "test", "admin"},
+		},
+	}
 
 	tests := []struct {
 		input  []byte
@@ -89,7 +77,7 @@ func TestAuth_CreatePermissionsStruct(t *testing.T) {
 			p.CreatePermissionsStruct(test.input)
 
 			if !reflect.DeepEqual(p, test.output) {
-				t.Fatalf("Expected output %s, but got %s", test.output, p)
+				t.Fatalf("expected output %s, but got %s", test.output, p)
 			}
 		},
 		)
@@ -162,14 +150,14 @@ func TestAuth_ResolvePermissions(t *testing.T) {
 			createGroupMaps(groupsList1),
 			[]string{"test"},
 			"",
-			"User is NOT allowed access to FeatureBase",
+			"user is NOT allowed access to FeatureBase",
 		},
 		{
 			permissions1,
 			createGroupMaps(groupsList2),
 			[]string{"test1"},
 			"",
-			"User is not allowed access to index",
+			"user is not allowed access to index",
 		},
 		{
 			permissions1,
@@ -204,7 +192,7 @@ func TestAuth_ResolvePermissions(t *testing.T) {
 			createGroupMaps(groupsList2),
 			[]string{"test"},
 			"",
-			"No permissions found",
+			"no permissions found",
 		},
 	}
 
@@ -217,12 +205,12 @@ func TestAuth_ResolvePermissions(t *testing.T) {
 			p1, err := p.ResolvePermissions(test.groups, test.index)
 
 			if p1 != test.userAccess {
-				t.Errorf("Expected permission to be %s, but got %s", test.userAccess, p1)
+				t.Errorf("expected permission to be %s, but got %s", test.userAccess, p1)
 			}
 
 			if err != nil {
 				if !strings.Contains(err.Error(), test.err) {
-					t.Errorf("Expected error to contain %s, but got %s", test.err, err.Error())
+					t.Errorf("expected error to contain %s, but got %s", test.err, err.Error())
 				}
 			}
 

server/config.go

@@ -614,14 +614,14 @@ func (c *Config) ValidateAuth() ([]error, error) {
 	errors := make([]error, 0)
 	for name, value := range authConfig {
 		if value == "" {
-			errors = append(errors, fmt.Errorf("Empty string for auth config %s", name))
+			errors = append(errors, fmt.Errorf("empty string for auth config %s", name))
 			continue
 		}
 
 		if strings.Contains(name, "URL") {
 			_, err := url.ParseRequestURI(value)
 			if err != nil {
-				errors = append(errors, fmt.Errorf("Invalid URL for auth config %s: %s", name, err))
+				errors = append(errors, fmt.Errorf("invalid URL for auth config %s: %s", name, err))
 				continue
 			}
 		}
@@ -629,14 +629,14 @@ func (c *Config) ValidateAuth() ([]error, error) {
 		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))
+				errors = append(errors, fmt.Errorf("invalid file extension for auth config %s: %s", name, value))
 				continue
 			}
 		}
 	}
 
 	if len(errors) > 0 {
-		return errors, fmt.Errorf("There were errors validating config")
+		return errors, fmt.Errorf("there were errors validating config")
 	}
 	return errors, nil
 }
@@ -647,7 +647,7 @@ func (c *Config) ValidatePermissions() (err error) {
 	var p auth.GroupPermissions
 	p.CreatePermissionsStruct(yamlData)
 	if len(p.Permissions) == 0 {
-		return fmt.Errorf("No group permissions found in permissions file: %s", c.Auth.PermissionsFile)
+		return fmt.Errorf("no group permissions found in permissions file: %s", c.Auth.PermissionsFile)
 	}
 	return nil
 }

server/config_internal_test.go

@@ -279,9 +279,9 @@ func TestConfig_validateAddrsGRPC(t *testing.T) {
 }
 
 func TestConfig_validateAuth(t *testing.T) {
-	errorMesgEmpty := "Empty string"
-	errorMesgURL := "Invalid URL"
-	errorMesgPermissions := "Invalid file extension"
+	errorMesgEmpty := "empty string"
+	errorMesgURL := "invalid URL"
+	errorMesgPermissions := "invalid file extension"
 	validTestURL := "https://url.com/"
 	validClientID := "clientid"
 	validClientSecret := "clientSecret"