From e8ef26196c314e552c97514c4b2133084c1679f0 Mon Sep 17 00:00:00 2001 From: Arya Khochare <91268931+Aryakoste@users.noreply.github.com> Date: Tue, 25 Feb 2025 15:19:28 +0530 Subject: [PATCH] Fixed errcheck issues in server/channels/app/permissions.go (#29064) Co-authored-by: Ben Schumacher --- server/.golangci.yml | 1 - server/channels/app/permissions.go | 107 ---------------- server/channels/app/permissions_test.go | 162 ------------------------ 3 files changed, 270 deletions(-) diff --git a/server/.golangci.yml b/server/.golangci.yml index 550538d4f3..a819583e7e 100644 --- a/server/.golangci.yml +++ b/server/.golangci.yml @@ -89,7 +89,6 @@ issues: channels/app/helper_test.go|\ channels/app/import_functions.go|\ channels/app/integration_action.go|\ - channels/app/permissions.go|\ channels/app/permissions_test.go|\ channels/app/platform/helper_test.go|\ channels/app/platform/license.go|\ diff --git a/server/channels/app/permissions.go b/server/channels/app/permissions.go index 4557464b99..dab4839997 100644 --- a/server/channels/app/permissions.go +++ b/server/channels/app/permissions.go @@ -4,10 +4,8 @@ package app import ( - "bufio" "context" "encoding/json" - "fmt" "io" "net/http" @@ -152,108 +150,3 @@ func (a *App) ExportPermissions(w io.Writer) error { _, err = w.Write(schemeExport) return err } - -func (a *App) ImportPermissions(jsonl io.Reader) error { - createdSchemeIDs := []string{} - - scanner := bufio.NewScanner(jsonl) - - for scanner.Scan() { - var schemeConveyor *model.SchemeConveyor - err := json.Unmarshal(scanner.Bytes(), &schemeConveyor) - if err != nil { - rollback(a, createdSchemeIDs) - return err - } - - if schemeConveyor.Name == systemSchemeName { - for _, roleIn := range schemeConveyor.Roles { - dbRole, err := a.GetRoleByName(context.Background(), roleIn.Name) - if err != nil { - rollback(a, createdSchemeIDs) - return errors.New(err.Message) - } - _, err = a.PatchRole(dbRole, &model.RolePatch{ - Permissions: &roleIn.Permissions, - }) - if err != nil { - rollback(a, createdSchemeIDs) - return err - } - } - continue - } - - // Create the new Scheme. The new Roles are created automatically. - var appErr *model.AppError - schemeCreated, appErr := a.CreateScheme(schemeConveyor.Scheme()) - if appErr != nil { - rollback(a, createdSchemeIDs) - return errors.New(appErr.Message) - } - createdSchemeIDs = append(createdSchemeIDs, schemeCreated.Id) - - schemeIn := schemeConveyor.Scheme() - roleNameTuples := [][]string{ - {schemeCreated.DefaultTeamAdminRole, schemeIn.DefaultTeamAdminRole}, - {schemeCreated.DefaultTeamUserRole, schemeIn.DefaultTeamUserRole}, - {schemeCreated.DefaultTeamGuestRole, schemeIn.DefaultTeamGuestRole}, - {schemeCreated.DefaultChannelAdminRole, schemeIn.DefaultChannelAdminRole}, - {schemeCreated.DefaultChannelUserRole, schemeIn.DefaultChannelUserRole}, - {schemeCreated.DefaultChannelGuestRole, schemeIn.DefaultChannelGuestRole}, - } - for _, roleNameTuple := range roleNameTuples { - if roleNameTuple[0] == "" || roleNameTuple[1] == "" { - continue - } - - err = updateRole(a, schemeConveyor, roleNameTuple[0], roleNameTuple[1]) - if err != nil { - // Delete the new Schemes. The new Roles are deleted automatically. - rollback(a, createdSchemeIDs) - return err - } - } - } - - if err := scanner.Err(); err != nil { - rollback(a, createdSchemeIDs) - return err - } - - return nil -} - -func rollback(a *App, createdSchemeIDs []string) { - for _, schemeID := range createdSchemeIDs { - a.DeleteScheme(schemeID) - } -} - -func updateRole(a *App, sc *model.SchemeConveyor, roleCreatedName, defaultRoleName string) error { - var err *model.AppError - - roleCreated, err := a.GetRoleByName(context.Background(), roleCreatedName) - if err != nil { - return errors.New(err.Message) - } - - var roleIn *model.Role - for _, role := range sc.Roles { - if role.Name == defaultRoleName { - roleIn = role - break - } - } - - roleCreated.DisplayName = roleIn.DisplayName - roleCreated.Description = roleIn.Description - roleCreated.Permissions = roleIn.Permissions - - _, err = a.UpdateRole(roleCreated) - if err != nil { - return fmt.Errorf("failed to update role: %w", err) - } - - return nil -} diff --git a/server/channels/app/permissions_test.go b/server/channels/app/permissions_test.go index bd94ba6563..5eeb585058 100644 --- a/server/channels/app/permissions_test.go +++ b/server/channels/app/permissions_test.go @@ -6,8 +6,6 @@ package app import ( "context" "encoding/json" - "fmt" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -91,166 +89,6 @@ func TestExportPermissions(t *testing.T) { } } -func TestImportPermissions(t *testing.T) { - th := Setup(t) - defer th.TearDown() - - name := model.NewId() - displayName := model.NewId() - description := "my test description" - scope := model.SchemeScopeChannel - roleName1 := model.NewId() - roleName2 := model.NewId() - - var results []*model.Scheme - var beforeCount int - withMigrationMarkedComplete(th, func() { - var appErr *model.AppError - results, appErr = th.App.GetSchemes(scope, 0, 100) - if appErr != nil { - panic(appErr) - } - beforeCount = len(results) - - json := fmt.Sprintf(`{"display_name":"%v","name":"%v","description":"%v","scope":"%v","default_team_admin_role":"","default_team_user_role":"","default_channel_admin_role":"%v","default_channel_user_role":"%v","roles":[{"id":"yzfx3g9xjjfw8cqo6bpn33xr7o","name":"%v","display_name":"Channel Admin Role for Scheme my_scheme_1526475590","description":"","create_at":1526475589687,"update_at":1526475589687,"delete_at":0,"permissions":["manage_channel_roles"],"scheme_managed":true,"built_in":false},{"id":"a7s3cp4n33dfxbsrmyh9djao3a","name":"%v","display_name":"Channel User Role for Scheme my_scheme_1526475590","description":"","create_at":1526475589688,"update_at":1526475589688,"delete_at":0,"permissions":["read_channel","add_reaction","remove_reaction","manage_public_channel_members","upload_file","get_public_link","create_post","manage_private_channel_members","delete_post","edit_post"],"scheme_managed":true,"built_in":false}]}`, displayName, name, description, scope, roleName1, roleName2, roleName1, roleName2) - r := strings.NewReader(json) - - err := th.App.ImportPermissions(r) - if err != nil { - t.Error(err) - } - results, appErr = th.App.GetSchemes(scope, 0, 100) - if appErr != nil { - panic(appErr) - } - }) - - actual := len(results) - expected := beforeCount + 1 - if actual != expected { - t.Errorf("Expected %v roles but got %v.", expected, actual) - } - - newScheme := results[0] - - channelAdminRole, appErr := th.App.GetRoleByName(context.Background(), newScheme.DefaultChannelAdminRole) - if appErr != nil { - t.Error(appErr) - } - - channelUserRole, appErr := th.App.GetRoleByName(context.Background(), newScheme.DefaultChannelUserRole) - if appErr != nil { - t.Error(appErr) - } - - channelGuestRole, appErr := th.App.GetRoleByName(context.Background(), newScheme.DefaultChannelGuestRole) - if appErr != nil { - t.Error(appErr) - } - - expectations := map[string]string{ - newScheme.DisplayName: displayName, - newScheme.Name: name, - newScheme.Description: description, - newScheme.Scope: scope, - newScheme.DefaultTeamAdminRole: "", - newScheme.DefaultTeamUserRole: "", - newScheme.DefaultTeamGuestRole: "", - channelAdminRole.Name: newScheme.DefaultChannelAdminRole, - channelUserRole.Name: newScheme.DefaultChannelUserRole, - channelGuestRole.Name: newScheme.DefaultChannelGuestRole, - } - - for actual, expected := range expectations { - if actual != expected { - t.Errorf("Expected %v but got %v.", expected, actual) - } - } -} - -func TestImportPermissions_idempotentScheme(t *testing.T) { - th := Setup(t) - defer th.TearDown() - - name := model.NewId() - displayName := model.NewId() - description := "my test description" - scope := model.SchemeScopeChannel - roleName1 := model.NewId() - roleName2 := model.NewId() - - json := fmt.Sprintf(`{"display_name":"%v","name":"%v","description":"%v","scope":"%v","default_team_admin_role":"","default_team_user_role":"","default_channel_admin_role":"%v","default_channel_user_role":"%v","roles":[{"id":"yzfx3g9xjjfw8cqo6bpn33xr7o","name":"%v","display_name":"Channel Admin Role for Scheme my_scheme_1526475590","description":"","create_at":1526475589687,"update_at":1526475589687,"delete_at":0,"permissions":["manage_channel_roles"],"scheme_managed":true,"built_in":false},{"id":"a7s3cp4n33dfxbsrmyh9djao3a","name":"%v","display_name":"Channel User Role for Scheme my_scheme_1526475590","description":"","create_at":1526475589688,"update_at":1526475589688,"delete_at":0,"permissions":["read_channel","add_reaction","remove_reaction","manage_public_channel_members","upload_file","get_public_link","create_post","manage_private_channel_members","delete_post","edit_post"],"scheme_managed":true,"built_in":false}]}`, displayName, name, description, scope, roleName1, roleName2, roleName1, roleName2) - jsonl := strings.Repeat(json+"\n", 4) - r := strings.NewReader(jsonl) - - var results []*model.Scheme - var expected int - withMigrationMarkedComplete(th, func() { - var appErr *model.AppError - results, appErr = th.App.GetSchemes(model.SchemeScopeChannel, 0, 100) - if appErr != nil { - panic(appErr) - } - expected = len(results) - - err := th.App.ImportPermissions(r) - if err == nil { - t.Error(err) - } - - results, appErr = th.App.GetSchemes(model.SchemeScopeChannel, 0, 100) - if appErr != nil { - panic(appErr) - } - }) - actual := len(results) - - if expected != actual { - t.Errorf("Expected count to be %v but got %v", expected, actual) - } -} - -func TestImportPermissions_schemeDeletedOnRoleFailure(t *testing.T) { - th := Setup(t) - defer th.TearDown() - - name := model.NewId() - displayName := model.NewId() - description := "my test description" - scope := "invalid scope" - roleName1 := model.NewId() - roleName2 := model.NewId() - - jsonl := fmt.Sprintf(`{"display_name":"%v","name":"%v","description":"%v","scope":"%v","default_team_admin_role":"","default_team_user_role":"","default_channel_admin_role":"%v","default_channel_user_role":"%v","roles":[{"id":"yzfx3g9xjjfw8cqo6bpn33xr7o","name":"%v","display_name":"Channel Admin Role for Scheme my_scheme_1526475590","description":"","create_at":1526475589687,"update_at":1526475589687,"delete_at":0,"permissions":["manage_channel_roles"],"scheme_managed":true,"built_in":false},{"id":"a7s3cp4n33dfxbsrmyh9djao3a","name":"%v","display_name":"Channel User Role for Scheme my_scheme_1526475590","description":"","create_at":1526475589688,"update_at":1526475589688,"delete_at":0,"permissions":["read_channel","add_reaction","remove_reaction","manage_public_channel_members","upload_file","get_public_link","create_post","manage_private_channel_members","delete_post","edit_post"],"scheme_managed":true,"built_in":false}]}`, displayName, name, description, scope, roleName1, roleName2, roleName1, roleName2) - r := strings.NewReader(jsonl) - - var results []*model.Scheme - var expected int - withMigrationMarkedComplete(th, func() { - var appErr *model.AppError - results, appErr = th.App.GetSchemes(model.SchemeScopeChannel, 0, 100) - if appErr != nil { - panic(appErr) - } - expected = len(results) - - err := th.App.ImportPermissions(r) - if err == nil { - t.Error(err) - } - - results, appErr = th.App.GetSchemes(model.SchemeScopeChannel, 0, 100) - if appErr != nil { - panic(appErr) - } - }) - actual := len(results) - - if expected != actual { - t.Errorf("Expected count to be %v but got %v", expected, actual) - } -} - func TestMigration(t *testing.T) { th := Setup(t) defer th.TearDown()