From a606bd030a09ac65cbc8cedba80533650f710260 Mon Sep 17 00:00:00 2001
From: Souhaila Noor <souhaila.noor@molecula.com>
Date: Fri, 17 Dec 2021 16:46:07 -0600
Subject: [PATCH] addressed reviewer's feedback

---
 authz/authorization.go      |  8 ++++++--
 authz/authorization_test.go |  8 +++++---
 server/config.go            | 16 ++++------------
 server/server.go            | 31 -------------------------------
 4 files changed, 15 insertions(+), 48 deletions(-)

diff --git a/authz/authorization.go b/authz/authorization.go
index 7f311899d..f89a52ea6 100644
--- a/authz/authorization.go
+++ b/authz/authorization.go
@@ -70,7 +70,7 @@ func (p *GroupPermissions) ReadPermissionsFile(permsFile io.Reader) (err error)
 		return fmt.Errorf("unmarshalling permissions failed with error: %s", err)
 	}
 
-	return nil
+	return
 }
 
 func (p *GroupPermissions) GetPermissions(groups []Group, index string) (permission string, errors error) {
@@ -91,7 +91,7 @@ func (p *GroupPermissions) GetPermissions(groups []Group, index string) (permiss
 			if perm, ok := p.Permissions[group.GroupID][index]; ok {
 				allPermissions[perm] = true
 			} else {
-				return "", fmt.Errorf("User %s does not have permission to index %s", group.UserID, index)
+				return "", fmt.Errorf("user %s does not have permission to index %s", group.UserID, index)
 			}
 		} else {
 			groupsDenied = append(groupsDenied, group.GroupID)
@@ -133,6 +133,10 @@ func (p *GroupPermissions) GetAuthorizedIndexList(groups []Group, desiredPermiss
 			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 84db21ab5..cd985f973 100644
--- a/authz/authorization_test.go
+++ b/authz/authorization_test.go
@@ -16,6 +16,7 @@ package authz_test
 import (
 	"fmt"
 	"reflect"
+	"sort"
 	"strings"
 	"testing"
 
@@ -227,7 +228,7 @@ func TestAuth_GetAuthorizedIndexList(t *testing.T) {
 		"dca35310-ecda-4f23-86cd-876aee55906b": {
 			"test1": "admin",
 			"test2": "read",
-			"test3": "read",
+			"test3": "write",
 		},
 	}}
 
@@ -239,7 +240,7 @@ func TestAuth_GetAuthorizedIndexList(t *testing.T) {
 		{
 			group,
 			"read",
-			[]string{"test2", "test3"},
+			[]string{"test1", "test2", "test3"},
 		},
 		{
 			group,
@@ -249,7 +250,7 @@ func TestAuth_GetAuthorizedIndexList(t *testing.T) {
 		{
 			group,
 			"write",
-			nil,
+			[]string{"test1", "test3"},
 		},
 	}
 
@@ -257,6 +258,7 @@ func TestAuth_GetAuthorizedIndexList(t *testing.T) {
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
 
 			indexList := p.GetAuthorizedIndexList(test.groups, test.permission)
+			sort.Strings(indexList)
 
 			if !reflect.DeepEqual(indexList, test.output) {
 				t.Errorf("expected %s, but got %s", test.output, indexList)
diff --git a/server/config.go b/server/config.go
index 866ffb3ff..5f4f0573c 100644
--- a/server/config.go
+++ b/server/config.go
@@ -601,7 +601,7 @@ func lookupAddr(ctx context.Context, resolver *net.Resolver, host string) (strin
 
 func (c *Config) ValidateAuth() (errors []error) {
 	if !c.Auth.Enable {
-		return errors
+		return
 	}
 	authConfig := map[string]string{
 		"ClientId":         c.Auth.ClientId,
@@ -626,11 +626,7 @@ func (c *Config) ValidateAuth() (errors []error) {
 			}
 		}
 	}
-
-	if len(errors) > 0 {
-		return errors
-	}
-	return nil
+	return errors
 }
 
 func (c *Config) ValidatePermissions(permsFile io.Reader) (errors []error) {
@@ -667,11 +663,7 @@ func (c *Config) ValidatePermissions(permsFile io.Reader) (errors []error) {
 			}
 		}
 	}
-	if len(errors) > 0 {
-		return errors
-	}
-
-	return nil
+	return errors
 }
 
 func (c *Config) ValidatePermissionsFile() (err error) {
@@ -684,7 +676,7 @@ func (c *Config) ValidatePermissionsFile() (err error) {
 	if (fileExt != ".yaml") && (fileExt != ".yml") {
 		return fmt.Errorf("invalid file extension for auth config permissions file: %s", c.Auth.PermissionsFile)
 	}
-	return nil
+	return
 }
 
 func (c *Config) MustValidateAuth() {
diff --git a/server/server.go b/server/server.go
index 17ea28933..c2418630b 100644
--- a/server/server.go
+++ b/server/server.go
@@ -237,37 +237,6 @@ func (m *Command) Start() (err error) {
 		if err = p.ReadPermissionsFile(permsFile); err != nil {
 			return err
 		}
-
-		groups := []authz.Group{
-			{
-				UserID:    "user-id",
-				GroupID:   "dca35310-ecda-4f23-86cd-876aee55906b",
-				GroupName: "group-name",
-			},
-			// {
-			// 	UserID:    "user-id",
-			// 	GroupID:   "dca35310-ecda-4f23-86cd-876aee559900",
-			// 	GroupName: "group-name",
-			// },
-		}
-
-		index := "test"
-
-		perm, err := p.GetPermissions(groups, index)
-		fmt.Printf("\nuser has %s access to index %s\n", perm, index)
-		if err != nil {
-			fmt.Printf("\np: %s, err: %s\n", perm, err.Error())
-		}
-
-		adminAccess := p.IsAdmin(groups)
-		fmt.Printf("\nAdminAccess: %t\n", adminAccess)
-
-		accessList := []string{"read", "write", "admin"}
-		for _, a := range accessList {
-			indexList := p.GetAuthorizedIndexList(groups, a)
-			fmt.Printf("\nPermission requested: %s, Index List: %s\n", a, indexList)
-		}
-
 	}
 
 	// Initialize server.