From c7e6c1dc32df8e43e3f5b9844b8d2404dfdc6a16 Mon Sep 17 00:00:00 2001 From: Misi Date: Thu, 10 Oct 2024 16:20:12 +0200 Subject: [PATCH] [v11.1.x] OrgSync: Do not set default Organization for a user to a non-existent Organization (#94551) OrgSync: Do not set default Organization for a user to a non-existent Organization (#94537) Do not set default org for a user to a missing org Co-authored-by: Karl Persson (cherry picked from commit c872cad879a0f4ac558012c3b924827140c8fc94) --- pkg/services/authn/authnimpl/sync/org_sync.go | 11 ++++++-- .../authn/authnimpl/sync/org_sync_test.go | 25 +++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/pkg/services/authn/authnimpl/sync/org_sync.go b/pkg/services/authn/authnimpl/sync/org_sync.go index 38429a07704..53117a1c861 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync.go +++ b/pkg/services/authn/authnimpl/sync/org_sync.go @@ -82,18 +82,25 @@ func (s *OrgSync) SyncOrgRolesHook(ctx context.Context, id *authn.Identity, _ *a orgIDs := make([]int64, 0, len(id.OrgRoles)) // add any new org roles for orgId, orgRole := range id.OrgRoles { - orgIDs = append(orgIDs, orgId) if _, exists := handledOrgIds[orgId]; exists { + orgIDs = append(orgIDs, orgId) continue } // add role cmd := &org.AddOrgUserCommand{UserID: userID, Role: orgRole, OrgID: orgId} err := s.orgService.AddOrgUser(ctx, cmd) - if err != nil && !errors.Is(err, org.ErrOrgNotFound) { + + if errors.Is(err, org.ErrOrgNotFound) { + continue + } + + if err != nil { ctxLogger.Error("Failed to update active org for user", "error", err) return err } + + orgIDs = append(orgIDs, orgId) } // delete any removed org roles diff --git a/pkg/services/authn/authnimpl/sync/org_sync_test.go b/pkg/services/authn/authnimpl/sync/org_sync_test.go index f8b774696d9..a2383c2b05a 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/org_sync_test.go @@ -22,7 +22,8 @@ import ( ) func TestOrgSync_SyncOrgRolesHook(t *testing.T) { - orgService := &orgtest.FakeOrgService{ExpectedUserOrgDTO: []*org.UserOrgDTO{ + orgService := &orgtest.MockService{} + orgService.On("GetUserOrgList", mock.Anything, mock.Anything).Return([]*org.UserOrgDTO{ { OrgID: 1, Role: org.RoleEditor, @@ -31,14 +32,16 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { OrgID: 3, Role: org.RoleViewer, }, - }, - ExpectedOrgListResponse: orgtest.OrgListResponse{ - { - OrgID: 3, - Response: nil, - }, - }, - } + }, nil) + orgService.On("RemoveOrgUser", mock.Anything, mock.MatchedBy(func(cmd *org.RemoveOrgUserCommand) bool { + return cmd.OrgID == 3 && cmd.UserID == 1 + })).Return(nil) + orgService.On("UpdateOrgUser", mock.Anything, mock.MatchedBy(func(cmd *org.UpdateOrgUserCommand) bool { + return cmd.OrgID == 1 && cmd.UserID == 1 && cmd.Role == org.RoleAdmin + })).Return(nil) + orgService.On("AddOrgUser", mock.Anything, mock.MatchedBy(func(cmd *org.AddOrgUserCommand) bool { + return cmd.OrgID == 2 && cmd.UserID == 1 && cmd.Role == org.RoleEditor + })).Return(org.ErrOrgNotFound) acService := &actest.FakeService{} userService := &usertest.FakeUserService{ExpectedUser: &user.User{ ID: 1, @@ -65,7 +68,7 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { wantID *authn.Identity }{ { - name: "add user to multiple orgs", + name: "add user to multiple orgs, should not set the user's default orgID to an org that does not exist", fields: fields{ userService: userService, orgService: orgService, @@ -96,7 +99,7 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { Name: "test", Email: "test", OrgRoles: map[int64]roletype.RoleType{1: org.RoleAdmin, 2: org.RoleEditor}, - OrgID: 1, //set using org + OrgID: 1, // set using org IsGrafanaAdmin: ptrBool(false), ClientParams: authn.ClientParams{ SyncOrgRoles: true,