From 7d4c6412972e4c88be0af326c1488108e7060869 Mon Sep 17 00:00:00 2001 From: lemonScaletech Date: Fri, 24 Nov 2023 13:52:17 +0530 Subject: [PATCH 1/6] fix: * removed fmt.Println --- server/env/env.go | 1 - 1 file changed, 1 deletion(-) diff --git a/server/env/env.go b/server/env/env.go index 2aa08b3..bd713c3 100644 --- a/server/env/env.go +++ b/server/env/env.go @@ -848,7 +848,6 @@ func InitAllEnv() error { envData[constants.EnvKeyDisableTOTPLogin] = boolValue } } - fmt.Println("=> final value", envData[constants.EnvKeyDisableTOTPLogin]) if _, ok := envData[constants.EnvKeyDisableMailOTPLogin]; !ok { envData[constants.EnvKeyDisableMailOTPLogin] = osDisableMailOTPLogin == "true" From 5cb94a782032289d51b08960fc37d9c0adda48d6 Mon Sep 17 00:00:00 2001 From: lemonScaletech Date: Thu, 7 Dec 2023 19:33:59 +0530 Subject: [PATCH 2/6] fix: * added logic if role is deleted then also be deleted from user side if role is assigned to that user. * default role should be subset of roles --- server/resolvers/update_env.go | 61 ++++++++++++++++++++++++++++++---- server/utils/common.go | 40 ++++++++++++++++++++++ 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/server/resolvers/update_env.go b/server/resolvers/update_env.go index e7ceb70..4bda560 100644 --- a/server/resolvers/update_env.go +++ b/server/resolvers/update_env.go @@ -7,6 +7,7 @@ import ( "fmt" "reflect" "strings" + "time" log "github.com/sirupsen/logrus" @@ -93,6 +94,42 @@ func clearSessionIfRequired(currentData, updatedData map[string]interface{}) { } } +func updateRoles(ctx context.Context, deletedRoles []string) error { + data, err := db.Provider.ListUsers(ctx, &model.Pagination{ + Limit: 1, + Offset: 1, + }) + if err != nil { + return err + } + + allData, err := db.Provider.ListUsers(ctx, &model.Pagination{ + Limit: data.Pagination.Total, + }) + if err != nil { + return err + } + + for i := range allData.Users { + now := time.Now().Unix() + allData.Users[i].Roles = utils.DeleteFromArray(allData.Users[i].Roles, deletedRoles) + allData.Users[i].UpdatedAt = &now + } + + for i := range allData.Users { + updatedValues := map[string]interface{}{ + "roles": strings.Join(allData.Users[i].Roles, ","), + "updated_at": time.Now().Unix(), + } + id := []string{allData.Users[i].ID} + err = db.Provider.UpdateUsers(ctx, updatedValues, id) + if err != nil { + return err + } + } + return nil +} + // UpdateEnvResolver is a resolver for update config mutation // This is admin only mutation func UpdateEnvResolver(ctx context.Context, params model.UpdateEnvInput) (*model.Response, error) { @@ -291,12 +328,17 @@ func UpdateEnvResolver(ctx context.Context, params model.UpdateEnvInput) (*model }, nil) } + previousRoles := strings.Split(currentData[constants.EnvKeyRoles].(string), ",") + updatedRoles := strings.Split(updatedData[constants.EnvKeyRoles].(string), ",") + updatedDefaultRoles := strings.Split(updatedData[constants.EnvKeyDefaultRoles].(string), ",") + updatedProtectedRoles := strings.Split(updatedData[constants.EnvKeyProtectedRoles].(string), ",") + // check the roles change - if len(params.Roles) > 0 { - if len(params.DefaultRoles) > 0 { + if len(updatedRoles) > 0 { + if len(updatedDefaultRoles) > 0 { // should be subset of roles - for _, role := range params.DefaultRoles { - if !utils.StringSliceContains(params.Roles, role) { + for _, role := range updatedDefaultRoles { + if !utils.StringSliceContains(updatedRoles, role) { log.Debug("Default roles should be subset of roles") return res, fmt.Errorf("default role %s is not in roles", role) } @@ -304,15 +346,20 @@ func UpdateEnvResolver(ctx context.Context, params model.UpdateEnvInput) (*model } } - if len(params.ProtectedRoles) > 0 { - for _, role := range params.ProtectedRoles { - if utils.StringSliceContains(params.Roles, role) || utils.StringSliceContains(params.DefaultRoles, role) { + if len(updatedProtectedRoles) > 0 { + for _, role := range updatedProtectedRoles { + if utils.StringSliceContains(updatedRoles, role) || utils.StringSliceContains(updatedDefaultRoles, role) { log.Debug("Protected roles should not be in roles or default roles") return res, fmt.Errorf("protected role %s found roles or default roles", role) } } } + deletedRoles := utils.FindDeletedValues(previousRoles, updatedRoles) + if len(deletedRoles) > 0 { + go updateRoles(ctx, deletedRoles) + } + // Update local store memorystore.Provider.UpdateEnvStore(updatedData) jwk, err := crypto.GenerateJWKBasedOnEnv() diff --git a/server/utils/common.go b/server/utils/common.go index 642d859..b5f81b6 100644 --- a/server/utils/common.go +++ b/server/utils/common.go @@ -95,3 +95,43 @@ func GetInviteVerificationURL(verificationURL, token, redirectURI string) string func GetEmailVerificationURL(token, hostname, redirectURI string) string { return hostname + "/verify_email?token=" + token + "&redirect_uri=" + redirectURI } + +// FindDeletedValues find deleted values between original and updated one +func FindDeletedValues(original, updated []string) []string { + deletedValues := make([]string, 0) + + // Create a map to store elements of the updated array for faster lookups + updatedMap := make(map[string]bool) + for _, value := range updated { + updatedMap[value] = true + } + + // Check for deleted values in the original array + for _, value := range original { + if _, found := updatedMap[value]; !found { + deletedValues = append(deletedValues, value) + } + } + + return deletedValues +} + +// DeleteFromArray will delete array from an array +func DeleteFromArray(original, valuesToDelete []string) []string { + result := make([]string, 0) + + // Create a map to store values to delete for faster lookups + valuesToDeleteMap := make(map[string]bool) + for _, value := range valuesToDelete { + valuesToDeleteMap[value] = true + } + + // Check if each element in the original array should be deleted + for _, value := range original { + if _, found := valuesToDeleteMap[value]; !found { + result = append(result, value) + } + } + + return result +} From b8c2ab4cf8f8e8beaaaee96ee2810f961ddee3a7 Mon Sep 17 00:00:00 2001 From: lemonScaletech Date: Fri, 8 Dec 2023 10:38:09 +0530 Subject: [PATCH 3/6] refactoring: * removed extra for loop * commenting on functions --- server/resolvers/update_env.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/server/resolvers/update_env.go b/server/resolvers/update_env.go index 4bda560..68087b4 100644 --- a/server/resolvers/update_env.go +++ b/server/resolvers/update_env.go @@ -94,6 +94,8 @@ func clearSessionIfRequired(currentData, updatedData map[string]interface{}) { } } +// updateRoles will update DB for user roles, if a role is deleted by admin +// then this function will those roles from user roles if exists func updateRoles(ctx context.Context, deletedRoles []string) error { data, err := db.Provider.ListUsers(ctx, &model.Pagination{ Limit: 1, @@ -111,20 +113,17 @@ func updateRoles(ctx context.Context, deletedRoles []string) error { } for i := range allData.Users { - now := time.Now().Unix() - allData.Users[i].Roles = utils.DeleteFromArray(allData.Users[i].Roles, deletedRoles) - allData.Users[i].UpdatedAt = &now - } - - for i := range allData.Users { - updatedValues := map[string]interface{}{ - "roles": strings.Join(allData.Users[i].Roles, ","), - "updated_at": time.Now().Unix(), - } - id := []string{allData.Users[i].ID} - err = db.Provider.UpdateUsers(ctx, updatedValues, id) - if err != nil { - return err + roles := utils.DeleteFromArray(allData.Users[i].Roles, deletedRoles) + if len(allData.Users[i].Roles) != len(roles) { + updatedValues := map[string]interface{}{ + "roles": strings.Join(roles, ","), + "updated_at": time.Now().Unix(), + } + id := []string{allData.Users[i].ID} + err = db.Provider.UpdateUsers(ctx, updatedValues, id) + if err != nil { + return err + } } } return nil From 47f26103b0d732dc399f9ee24294edff9bdcb9c1 Mon Sep 17 00:00:00 2001 From: lemonScaletech Date: Fri, 8 Dec 2023 18:22:24 +0530 Subject: [PATCH 4/6] test: * added integration test for role deletion functionality --- server/test/integration_test.go | 1 + server/test/role_deletion_test.go | 98 +++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 server/test/role_deletion_test.go diff --git a/server/test/integration_test.go b/server/test/integration_test.go index 587235c..2374aaf 100644 --- a/server/test/integration_test.go +++ b/server/test/integration_test.go @@ -122,6 +122,7 @@ func TestResolvers(t *testing.T) { updateEmailTemplateTest(t, s) emailTemplatesTest(t, s) deleteEmailTemplateTest(t, s) + RoleDeletionTest(t, s) // user resolvers tests loginTests(t, s) diff --git a/server/test/role_deletion_test.go b/server/test/role_deletion_test.go new file mode 100644 index 0000000..ed0ed90 --- /dev/null +++ b/server/test/role_deletion_test.go @@ -0,0 +1,98 @@ +package test + +import ( + "fmt" + "github.com/authorizerdev/authorizer/server/crypto" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/authorizerdev/authorizer/server/constants" + "github.com/authorizerdev/authorizer/server/graph/model" + "github.com/authorizerdev/authorizer/server/memorystore" + "github.com/authorizerdev/authorizer/server/refs" + "github.com/authorizerdev/authorizer/server/resolvers" +) + +func RoleDeletionTest(t *testing.T, s TestSetup) { + t.Helper() + t.Run(`should complete role deletion`, func(t *testing.T) { + // login as admin + req, ctx := createContext(s) + + _, err := resolvers.AdminLoginResolver(ctx, model.AdminLoginInput{ + AdminSecret: "admin_test", + }) + assert.NotNil(t, err) + + adminSecret, err := memorystore.Provider.GetStringStoreEnvVariable(constants.EnvKeyAdminSecret) + assert.Nil(t, err) + _, err = resolvers.AdminLoginResolver(ctx, model.AdminLoginInput{ + AdminSecret: adminSecret, + }) + assert.Nil(t, err) + + h, err := crypto.EncryptPassword(adminSecret) + assert.Nil(t, err) + req.Header.Set("Cookie", fmt.Sprintf("%s=%s", constants.AdminCookieName, h)) + + // add new default role to get role, if not present in roles + originalDefaultRoles, err := memorystore.Provider.GetStringStoreEnvVariable(constants.EnvKeyDefaultRoles) + assert.Nil(t, err) + originalDefaultRolesSlice := strings.Split(originalDefaultRoles, ",") + + data := model.UpdateEnvInput{ + DefaultRoles: append(originalDefaultRolesSlice, "abc"), + } + _, err = resolvers.UpdateEnvResolver(ctx, data) + assert.Error(t, err) + + // add new role + originalRoles, err := memorystore.Provider.GetStringStoreEnvVariable(constants.EnvKeyRoles) + assert.Nil(t, err) + originalRolesSlice := strings.Split(originalRoles, ",") + roleToBeAdded := "abc" + newRoles := append(originalRolesSlice, roleToBeAdded) + data = model.UpdateEnvInput{ + Roles: newRoles, + } + _, err = resolvers.UpdateEnvResolver(ctx, data) + assert.Nil(t, err) + + // register a user with all roles + email := "update_user." + s.TestInfo.Email + _, err = resolvers.SignupResolver(ctx, model.SignUpInput{ + Email: refs.NewStringRef(email), + Password: s.TestInfo.Password, + ConfirmPassword: s.TestInfo.Password, + Roles: newRoles, + }) + assert.Nil(t, err) + + regUserDetails, _ := resolvers.UserResolver(ctx, model.GetUserRequest{ + Email: refs.NewStringRef(email), + }) + + // update env by removing role "abc" + var newRolesAfterDeletion []string + for _, value := range newRoles { + if value != roleToBeAdded { + newRolesAfterDeletion = append(newRolesAfterDeletion, value) + } + } + data = model.UpdateEnvInput{ + Roles: newRolesAfterDeletion, + } + _, err = resolvers.UpdateEnvResolver(ctx, data) + assert.Nil(t, err) + + // check user if role still exist + userDetails, err := resolvers.UserResolver(ctx, model.GetUserRequest{ + Email: refs.NewStringRef(email), + }) + assert.Nil(t, err) + assert.Equal(t, newRolesAfterDeletion, userDetails.Roles) + assert.NotEqual(t, newRolesAfterDeletion, regUserDetails.Roles) + }) +} From 7bcd5a70c30c1efbacc388595b6bb862430c8e01 Mon Sep 17 00:00:00 2001 From: lemonScaletech Date: Tue, 2 Jan 2024 11:50:26 +0530 Subject: [PATCH 5/6] feat: * PR suggested changes --- server/resolvers/update_env.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/server/resolvers/update_env.go b/server/resolvers/update_env.go index 68087b4..5b27a11 100644 --- a/server/resolvers/update_env.go +++ b/server/resolvers/update_env.go @@ -328,19 +328,17 @@ func UpdateEnvResolver(ctx context.Context, params model.UpdateEnvInput) (*model } previousRoles := strings.Split(currentData[constants.EnvKeyRoles].(string), ",") + previousProtectedRoles := strings.Split(currentData[constants.EnvKeyProtectedRoles].(string), ",") updatedRoles := strings.Split(updatedData[constants.EnvKeyRoles].(string), ",") updatedDefaultRoles := strings.Split(updatedData[constants.EnvKeyDefaultRoles].(string), ",") updatedProtectedRoles := strings.Split(updatedData[constants.EnvKeyProtectedRoles].(string), ",") - // check the roles change - if len(updatedRoles) > 0 { - if len(updatedDefaultRoles) > 0 { - // should be subset of roles - for _, role := range updatedDefaultRoles { - if !utils.StringSliceContains(updatedRoles, role) { - log.Debug("Default roles should be subset of roles") - return res, fmt.Errorf("default role %s is not in roles", role) - } + if len(updatedRoles) > 0 && len(updatedDefaultRoles) > 0 { + // should be subset of roles + for _, role := range updatedDefaultRoles { + if !utils.StringSliceContains(updatedRoles, role) { + log.Debug("Default roles should be subset of roles") + return res, fmt.Errorf("default role %s is not in roles", role) } } } @@ -359,6 +357,11 @@ func UpdateEnvResolver(ctx context.Context, params model.UpdateEnvInput) (*model go updateRoles(ctx, deletedRoles) } + deletedProtectedRoles := utils.FindDeletedValues(previousProtectedRoles, updatedProtectedRoles) + if len(deletedProtectedRoles) > 0 { + go updateRoles(ctx, deletedProtectedRoles) + } + // Update local store memorystore.Provider.UpdateEnvStore(updatedData) jwk, err := crypto.GenerateJWKBasedOnEnv() From 40a0a2fbcc511ea0c69080119cfebe8206a5ec94 Mon Sep 17 00:00:00 2001 From: lemonScaletech Date: Wed, 10 Jan 2024 12:56:30 +0530 Subject: [PATCH 6/6] feat: * added chunks of 1000 for deletion of role --- server/resolvers/update_env.go | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/server/resolvers/update_env.go b/server/resolvers/update_env.go index 5b27a11..62bd33e 100644 --- a/server/resolvers/update_env.go +++ b/server/resolvers/update_env.go @@ -112,17 +112,29 @@ func updateRoles(ctx context.Context, deletedRoles []string) error { return err } - for i := range allData.Users { - roles := utils.DeleteFromArray(allData.Users[i].Roles, deletedRoles) - if len(allData.Users[i].Roles) != len(roles) { - updatedValues := map[string]interface{}{ - "roles": strings.Join(roles, ","), - "updated_at": time.Now().Unix(), - } - id := []string{allData.Users[i].ID} - err = db.Provider.UpdateUsers(ctx, updatedValues, id) - if err != nil { - return err + chunkSize := 1000 + totalUsers := len(allData.Users) + + for start := 0; start < totalUsers; start += chunkSize { + end := start + chunkSize + if end > totalUsers { + end = totalUsers + } + + chunkUsers := allData.Users[start:end] + + for i := range chunkUsers { + roles := utils.DeleteFromArray(chunkUsers[i].Roles, deletedRoles) + if len(chunkUsers[i].Roles) != len(roles) { + updatedValues := map[string]interface{}{ + "roles": strings.Join(roles, ","), + "updated_at": time.Now().Unix(), + } + id := []string{chunkUsers[i].ID} + err = db.Provider.UpdateUsers(ctx, updatedValues, id) + if err != nil { + return err + } } } }