[ML] Fix saved object sync check for jobs which are hidden from the user (#160266)

Fixes a bug introduced in PR
https://github.com/elastic/kibana/pull/146155
A user who cannot see all spaces will incorrectly be told that jobs
which only exist in spaces they cannot see are in need of
synchronisation.
The problem was caused by an accident replacement of the
`internalSavedObjectsClient` function (which can see all spaces) with
the cached saved objects client which can only see the user's allowed
spaces.
The fix is to revert to the original code.

This particular scenario was not covered by API tests. The tests have
also been updated in this PR.


- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

x-pack/plugins/ml/server/saved_objects/service.ts

@@ -248,8 +248,7 @@ export function mlSavedObjectServiceFactory(
       filter,
     };
 
-    const jobs = await _savedObjectsClientFindMemo<JobObject>(options);
-    return jobs.saved_objects;
+    return (await internalSavedObjectsClient.find<JobObject>(options)).saved_objects;
   }
 
   async function addDatafeed(datafeedId: string, jobId: string) {

x-pack/test/api_integration/apis/ml/jobs/reset.ts

@@ -84,6 +84,7 @@ export default ({ getService }: FtrProviderContext) => {
         await ml.api.deleteAnomalyDetectionJobES(job.job_id);
       }
       await ml.api.cleanMlIndices();
+      await ml.testResources.cleanMLSavedObjects();
     });
 
     it('succeeds for ML Poweruser and keeps user annotations', async () => {

x-pack/test/api_integration/apis/ml/saved_objects/sync_jobs.ts

@@ -22,10 +22,16 @@ export default ({ getService }: FtrProviderContext) => {
   const adJobId3 = 'fq_single_3';
   const adJobIdES = 'fq_single_es';
   const idSpace1 = 'space1';
+  const idSpace2 = 'space2';
 
-  async function runSyncRequest(user: USER, expectedStatusCode: number) {
+  async function runSyncRequest(
+    user: USER,
+    expectedStatusCode: number,
+    simulate: boolean = false,
+    space: string = idSpace1
+  ) {
     const { body, status } = await supertest
-      .get(`/s/${idSpace1}/api/ml/saved_objects/sync`)
+      .get(`/s/${space}/api/ml/saved_objects/sync?simulate=${simulate}`)
       .auth(user, ml.securityCommon.getPasswordForUser(user))
       .set(getCommonRequestHeader('2023-10-31'));
     ml.api.assertResponseStatusCode(expectedStatusCode, status, body);
@@ -36,10 +42,11 @@ export default ({ getService }: FtrProviderContext) => {
   async function runSyncCheckRequest(
     user: USER,
     mlSavedObjectType: MlSavedObjectType,
-    expectedStatusCode: number
+    expectedStatusCode: number,
+    space: string = idSpace1
   ) {
     const { body, status } = await supertest
-      .post(`/s/${idSpace1}/internal/ml/saved_objects/sync_check`)
+      .post(`/s/${space}/internal/ml/saved_objects/sync_check`)
       .auth(user, ml.securityCommon.getPasswordForUser(user))
       .set(getCommonRequestHeader('1'))
       .send({ mlSavedObjectType });
@@ -52,11 +59,13 @@ export default ({ getService }: FtrProviderContext) => {
     beforeEach(async () => {
       await ml.api.initSavedObjects();
       await spacesService.create({ id: idSpace1, name: 'space_one', disabledFeatures: [] });
+      await spacesService.create({ id: idSpace2, name: 'space_two', disabledFeatures: [] });
       await ml.testResources.setKibanaTimeZoneToUTC();
     });
 
     afterEach(async () => {
       await spacesService.delete(idSpace1);
+      await spacesService.delete(idSpace2);
       await ml.api.cleanMlIndices();
       await ml.testResources.cleanMLSavedObjects();
     });
@@ -131,6 +140,122 @@ export default ({ getService }: FtrProviderContext) => {
       });
     });
 
+    it('should simulate syncing saved objects', async () => {
+      // check to see if a sync is needed
+      const syncNeeded = await runSyncCheckRequest(
+        USER.ML_POWERUSER_ALL_SPACES,
+        'anomaly-detector',
+        200
+      );
+      expect(syncNeeded.result).to.eql(false, 'sync should not be needed');
+
+      // prepare test data
+      await ml.api.createAnomalyDetectionJobES(
+        ml.commonConfig.getADFqSingleMetricJobConfig(adJobId1)
+      );
+      await ml.api.createDatafeedES(ml.commonConfig.getADFqDatafeedConfig(adJobId1));
+
+      // check to see if a sync is needed
+      const syncNeeded2 = await runSyncCheckRequest(
+        USER.ML_POWERUSER_ALL_SPACES,
+        'anomaly-detector',
+        200
+      );
+      expect(syncNeeded2.result).to.eql(true, 'sync should be needed');
+
+      // run the sync request and verify the response
+      const body = await runSyncRequest(USER.ML_POWERUSER_ALL_SPACES, 200, true);
+
+      expect(body).to.eql({
+        datafeedsAdded: {},
+        datafeedsRemoved: {},
+        savedObjectsCreated: {
+          'anomaly-detector': {
+            [adJobId1]: { success: true },
+          },
+        },
+        savedObjectsDeleted: {},
+      });
+
+      const syncNeeded3 = await runSyncCheckRequest(
+        USER.ML_POWERUSER_ALL_SPACES,
+        'anomaly-detector',
+        200
+      );
+      expect(syncNeeded3.result).to.eql(true, 'sync should still be needed');
+
+      const body2 = await runSyncRequest(USER.ML_POWERUSER_ALL_SPACES, 200, true);
+
+      expect(body2).to.eql({
+        datafeedsAdded: {},
+        datafeedsRemoved: {},
+        savedObjectsCreated: {
+          'anomaly-detector': {
+            [adJobId1]: { success: true },
+          },
+        },
+        savedObjectsDeleted: {},
+      });
+    });
+
+    // add a job to space 2, then check that it is not synced from space 1
+    it('should not sync saved objects in different space', async () => {
+      // check to see if a sync is needed
+      const syncNeeded = await runSyncCheckRequest(
+        USER.ML_POWERUSER_ALL_SPACES,
+        'anomaly-detector',
+        200
+      );
+      expect(syncNeeded.result).to.eql(false, 'sync should not be needed');
+
+      // prepare test data
+      await ml.api.createAnomalyDetectionJob(
+        ml.commonConfig.getADFqSingleMetricJobConfig(adJobId1),
+        idSpace2
+      );
+      await ml.api.createDatafeed(ml.commonConfig.getADFqDatafeedConfig(adJobId1), idSpace2);
+
+      // check to see if a sync is needed
+      const syncNeeded2 = await runSyncCheckRequest(
+        USER.ML_POWERUSER_ALL_SPACES,
+        'anomaly-detector',
+        200
+      );
+      expect(syncNeeded2.result).to.eql(
+        false,
+        'sync should be needed for a user who can see all spaces'
+      );
+
+      const syncNeeded3 = await runSyncCheckRequest(
+        USER.ML_POWERUSER_SPACE1,
+        'anomaly-detector',
+        200
+      );
+      expect(syncNeeded3.result).to.eql(
+        false,
+        'sync should be needed for a user who can only see space 1'
+      );
+
+      // check sync outputs
+      const body = await runSyncRequest(USER.ML_POWERUSER_ALL_SPACES, 200, true);
+
+      expect(body).to.eql({
+        datafeedsAdded: {},
+        datafeedsRemoved: {},
+        savedObjectsCreated: {},
+        savedObjectsDeleted: {},
+      });
+
+      const body2 = await runSyncRequest(USER.ML_POWERUSER_SPACE1, 200, true);
+
+      expect(body2).to.eql({
+        datafeedsAdded: {},
+        datafeedsRemoved: {},
+        savedObjectsCreated: {},
+        savedObjectsDeleted: {},
+      });
+    });
+
     it('should sync datafeeds after recreation in ES with different name', async () => {
       // check to see if a sync is needed
       const syncNeeded = await runSyncCheckRequest(