[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 <kalle.persson@grafana.com>
(cherry picked from commit c872cad879)

Co-authored-by: Misi <mgyongyosi@users.noreply.github.com>

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

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,