From 2a7d8279a2b4698e6bee09e8c09348512bb620b0 Mon Sep 17 00:00:00 2001 From: "grafana-delivery-bot[bot]" <132647405+grafana-delivery-bot[bot]@users.noreply.github.com> Date: Fri, 11 Oct 2024 15:44:17 +0300 Subject: [PATCH] [v11.3.x] OrgSync: Do not set default Organization for a user to a non-existent Organization (#94613) 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) Co-authored-by: Misi --- 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 87d79215ce5..8a50d4cd73f 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync.go +++ b/pkg/services/authn/authnimpl/sync/org_sync.go @@ -87,18 +87,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 b62bcbcbf00..246aec07399 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/org_sync_test.go @@ -24,7 +24,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, @@ -33,14 +34,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, @@ -67,7 +70,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, @@ -100,7 +103,7 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { Name: "test", Email: "test", OrgRoles: map[int64]identity.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,