Delete legacy URL aliases when objects are deleted or unshared (#117056) (#117461)

Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>

src/core/server/saved_objects/object_types/registration.ts

@@ -16,8 +16,9 @@ const legacyUrlAliasType: SavedObjectsType = {
     dynamic: false,
     properties: {
       sourceId: { type: 'keyword' },
-      targetType: { type: 'keyword' },
       targetNamespace: { type: 'keyword' },
+      targetType: { type: 'keyword' },
+      targetId: { type: 'keyword' },
       resolveCounter: { type: 'long' },
       disabled: { type: 'boolean' },
       // other properties exist, but we aren't querying or aggregating on those, so we don't need to specify them (because we use `dynamic: false` above)

src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.mock.ts

@@ -6,14 +6,14 @@
  * Side Public License, v 1.
  */
 
-import type { findLegacyUrlAliases } from './find_legacy_url_aliases';
+import type { findLegacyUrlAliases } from './legacy_url_aliases';
 import type * as InternalUtils from './internal_utils';
 
 export const mockFindLegacyUrlAliases = jest.fn() as jest.MockedFunction<
   typeof findLegacyUrlAliases
 >;
 
-jest.mock('./find_legacy_url_aliases', () => {
+jest.mock('./legacy_url_aliases', () => {
   return { findLegacyUrlAliases: mockFindLegacyUrlAliases };
 });
 

src/core/server/saved_objects/service/lib/collect_multi_namespace_references.ts

@@ -9,7 +9,7 @@
 import type { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry';
 import type { SavedObjectsSerializer } from '../../serialization';
 import type { SavedObject, SavedObjectsBaseOptions } from '../../types';
-import { findLegacyUrlAliases } from './find_legacy_url_aliases';
+import { findLegacyUrlAliases } from './legacy_url_aliases';
 import { getRootFields } from './included_fields';
 import {
   getObjectKey,

src/core/server/saved_objects/service/lib/legacy_url_aliases/delete_legacy_url_aliases.test.mock.ts

@@ -0,0 +1,23 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import type { getErrorMessage } from '../../../../elasticsearch';
+
+export const mockGetEsErrorMessage = jest.fn() as jest.MockedFunction<typeof getErrorMessage>;
+
+jest.mock('../../../../elasticsearch', () => {
+  return { getErrorMessage: mockGetEsErrorMessage };
+});
+
+// Mock these functions to return empty results, as this simplifies test cases and we don't need to exercise alternate code paths for these
+jest.mock('@kbn/es-query', () => {
+  return { nodeTypes: { function: { buildNode: jest.fn() } } };
+});
+jest.mock('../search_dsl', () => {
+  return { getSearchDsl: jest.fn() };
+});

src/core/server/saved_objects/service/lib/legacy_url_aliases/delete_legacy_url_aliases.test.ts

@@ -0,0 +1,152 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { mockGetEsErrorMessage } from './delete_legacy_url_aliases.test.mock'; // Note: importing this file applies default mocks for other functions too
+
+import { errors as EsErrors } from '@elastic/elasticsearch';
+
+import { elasticsearchClientMock } from '../../../../elasticsearch/client/mocks';
+import { typeRegistryMock } from '../../../saved_objects_type_registry.mock';
+import { ALL_NAMESPACES_STRING } from '../utils';
+import { deleteLegacyUrlAliases } from './delete_legacy_url_aliases';
+import type { DeleteLegacyUrlAliasesParams } from './delete_legacy_url_aliases';
+
+type SetupParams = Pick<
+  DeleteLegacyUrlAliasesParams,
+  'type' | 'id' | 'namespaces' | 'deleteBehavior'
+>;
+
+describe('deleteLegacyUrlAliases', () => {
+  function setup(setupParams: SetupParams) {
+    return {
+      mappings: { properties: {} }, // doesn't matter, only used as an argument to getSearchDsl which is mocked
+      registry: typeRegistryMock.create(), // doesn't matter, only used as an argument to getSearchDsl which is mocked
+      client: elasticsearchClientMock.createElasticsearchClient(),
+      getIndexForType: jest.fn(), // doesn't matter
+      ...setupParams,
+    };
+  }
+
+  const type = 'obj-type';
+  const id = 'obj-id';
+
+  it('throws an error if namespaces includes the "all namespaces" string', async () => {
+    const namespaces = [ALL_NAMESPACES_STRING];
+    const params = setup({ type, id, namespaces, deleteBehavior: 'inclusive' });
+
+    await expect(() => deleteLegacyUrlAliases(params)).rejects.toThrowError(
+      `Failed to delete legacy URL aliases for ${type}/${id}: "namespaces" cannot include the * string`
+    );
+    expect(params.client.updateByQuery).not.toHaveBeenCalled();
+  });
+
+  it('throws an error if updateByQuery fails', async () => {
+    const namespaces = ['space-a', 'space-b'];
+    const params = setup({ type, id, namespaces, deleteBehavior: 'inclusive' });
+    const esError = new EsErrors.ResponseError(
+      elasticsearchClientMock.createApiResponse({
+        statusCode: 500,
+        body: { error: { type: 'es_type', reason: 'es_reason' } },
+      })
+    );
+    params.client.updateByQuery.mockResolvedValueOnce(
+      elasticsearchClientMock.createErrorTransportRequestPromise(esError)
+    );
+    mockGetEsErrorMessage.mockClear();
+    mockGetEsErrorMessage.mockReturnValue('Oh no!');
+
+    await expect(() => deleteLegacyUrlAliases(params)).rejects.toThrowError(
+      `Failed to delete legacy URL aliases for ${type}/${id}: Oh no!`
+    );
+    expect(params.client.updateByQuery).toHaveBeenCalledTimes(1);
+    expect(mockGetEsErrorMessage).toHaveBeenCalledTimes(1);
+    expect(mockGetEsErrorMessage).toHaveBeenCalledWith(esError);
+  });
+
+  describe('deleteBehavior "inclusive"', () => {
+    const deleteBehavior = 'inclusive' as const;
+
+    it('when filtered namespaces is not empty, returns early', async () => {
+      const namespaces = ['default'];
+      const params = setup({ type, id, namespaces, deleteBehavior });
+
+      await deleteLegacyUrlAliases(params);
+      expect(params.client.updateByQuery).not.toHaveBeenCalled();
+    });
+
+    it('when filtered namespaces is not empty, calls updateByQuery with expected script params', async () => {
+      const namespaces = ['space-a', 'default', 'space-b'];
+      const params = setup({ type, id, namespaces, deleteBehavior });
+
+      await deleteLegacyUrlAliases(params);
+      expect(params.client.updateByQuery).toHaveBeenCalledTimes(1);
+      expect(params.client.updateByQuery).toHaveBeenCalledWith(
+        expect.objectContaining({
+          body: expect.objectContaining({
+            script: expect.objectContaining({
+              params: {
+                namespaces: ['space-a', 'space-b'], // 'default' is filtered out
+                matchTargetNamespaceOp: 'delete',
+                notMatchTargetNamespaceOp: 'noop',
+              },
+            }),
+          }),
+        }),
+        expect.anything()
+      );
+    });
+  });
+
+  describe('deleteBehavior "exclusive"', () => {
+    const deleteBehavior = 'exclusive' as const;
+
+    it('when filtered namespaces is empty, calls updateByQuery with expected script params', async () => {
+      const namespaces = ['default'];
+      const params = setup({ type, id, namespaces, deleteBehavior });
+
+      await deleteLegacyUrlAliases(params);
+      expect(params.client.updateByQuery).toHaveBeenCalledTimes(1);
+      expect(params.client.updateByQuery).toHaveBeenCalledWith(
+        expect.objectContaining({
+          body: expect.objectContaining({
+            script: expect.objectContaining({
+              params: {
+                namespaces: [], // 'default' is filtered out
+                matchTargetNamespaceOp: 'noop',
+                notMatchTargetNamespaceOp: 'delete',
+              },
+            }),
+          }),
+        }),
+        expect.anything()
+      );
+    });
+
+    it('when filtered namespaces is not empty, calls updateByQuery with expected script params', async () => {
+      const namespaces = ['space-a', 'default', 'space-b'];
+      const params = setup({ type, id, namespaces, deleteBehavior });
+
+      await deleteLegacyUrlAliases(params);
+      expect(params.client.updateByQuery).toHaveBeenCalledTimes(1);
+      expect(params.client.updateByQuery).toHaveBeenCalledWith(
+        expect.objectContaining({
+          body: expect.objectContaining({
+            script: expect.objectContaining({
+              params: {
+                namespaces: ['space-a', 'space-b'], // 'default' is filtered out
+                matchTargetNamespaceOp: 'noop',
+                notMatchTargetNamespaceOp: 'delete',
+              },
+            }),
+          }),
+        }),
+        expect.anything()
+      );
+    });
+  });
+});

src/core/server/saved_objects/service/lib/legacy_url_aliases/delete_legacy_url_aliases.ts

@@ -0,0 +1,113 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import * as esKuery from '@kbn/es-query';
+
+import { getErrorMessage as getEsErrorMessage } from '../../../../elasticsearch';
+import type { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
+import type { IndexMapping } from '../../../mappings';
+import { LEGACY_URL_ALIAS_TYPE } from '../../../object_types';
+import type { RepositoryEsClient } from '../repository_es_client';
+import { getSearchDsl } from '../search_dsl';
+import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils';
+
+/** @internal */
+export interface DeleteLegacyUrlAliasesParams {
+  mappings: IndexMapping;
+  registry: ISavedObjectTypeRegistry;
+  client: RepositoryEsClient;
+  getIndexForType: (type: string) => string;
+  /** The object type. */
+  type: string;
+  /** The object ID. */
+  id: string;
+  /**
+   * The namespaces to include or exclude when searching for legacy URL alias targets (depends on the `deleteBehavior` parameter).
+   * Note that using `namespaces: [], deleteBehavior: 'exclusive'` will delete all aliases for this object in all spaces.
+   */
+  namespaces: string[];
+  /**
+   * If this is equal to 'inclusive', all aliases with a `targetNamespace` in the `namespaces` array will be deleted.
+   * If this is equal to 'exclusive', all aliases with a `targetNamespace` _not_ in the `namespaces` array will be deleted.
+   */
+  deleteBehavior: 'inclusive' | 'exclusive';
+}
+
+/**
+ * Deletes legacy URL aliases that point to a given object.
+ *
+ * Note that aliases are only created when an object is converted to become share-capable, and each targetId is deterministically generated
+ * with uuidv5 -- this means that the chances of there actually being _multiple_ legacy URL aliases that target a given type/ID are slim to
+ * none. However, we don't always know exactly what space an alias could be in (if an object exists in multiple spaces, or in all spaces),
+ * so the most straightforward way for us to ensure that aliases are reliably deleted is to use updateByQuery, which is what this function
+ * does.
+ *
+ * @internal
+ */
+export async function deleteLegacyUrlAliases(params: DeleteLegacyUrlAliasesParams) {
+  const { mappings, registry, client, getIndexForType, type, id, namespaces, deleteBehavior } =
+    params;
+
+  if (namespaces.includes(ALL_NAMESPACES_STRING)) {
+    throwError(type, id, '"namespaces" cannot include the * string');
+  }
+
+  // Legacy URL aliases cannot exist in the default space; filter that out
+  const filteredNamespaces = namespaces.filter(
+    (namespace) => namespace !== DEFAULT_NAMESPACE_STRING
+  );
+  if (!filteredNamespaces.length && deleteBehavior === 'inclusive') {
+    // nothing to do, return early
+    return;
+  }
+
+  const { buildNode } = esKuery.nodeTypes.function;
+  const match1 = buildNode('is', `${LEGACY_URL_ALIAS_TYPE}.targetType`, type);
+  const match2 = buildNode('is', `${LEGACY_URL_ALIAS_TYPE}.targetId`, id);
+  const kueryNode = buildNode('and', [match1, match2]);
+
+  try {
+    await client.updateByQuery(
+      {
+        index: getIndexForType(LEGACY_URL_ALIAS_TYPE),
+        refresh: false, // This could be called many times in succession, intentionally do not wait for a refresh
+        body: {
+          ...getSearchDsl(mappings, registry, {
+            type: LEGACY_URL_ALIAS_TYPE,
+            kueryNode,
+          }),
+          script: {
+            // Intentionally use one script source with variable params to take advantage of ES script caching
+            source: `
+              if (params['namespaces'].indexOf(ctx._source['${LEGACY_URL_ALIAS_TYPE}']['targetNamespace']) > -1) {
+                ctx.op = params['matchTargetNamespaceOp'];
+              } else {
+                ctx.op = params['notMatchTargetNamespaceOp'];
+              }
+            `,
+            params: {
+              namespaces: filteredNamespaces,
+              matchTargetNamespaceOp: deleteBehavior === 'inclusive' ? 'delete' : 'noop',
+              notMatchTargetNamespaceOp: deleteBehavior === 'inclusive' ? 'noop' : 'delete',
+            },
+            lang: 'painless',
+          },
+          conflicts: 'proceed',
+        },
+      },
+      { ignore: [404] }
+    );
+  } catch (err) {
+    const errorMessage = getEsErrorMessage(err);
+    throwError(type, id, `${errorMessage}`);
+  }
+}
+
+function throwError(type: string, id: string, message: string) {
+  throw new Error(`Failed to delete legacy URL aliases for ${type}/${id}: ${message}`);
+}

src/core/server/saved_objects/service/lib/legacy_url_aliases/find_legacy_url_aliases.test.ts

@@ -8,12 +8,12 @@
 
 import type { DeeplyMockedKeys } from '@kbn/utility-types/jest';
 
-import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../object_types';
+import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../../object_types';
+import type { CreatePointInTimeFinderFn, PointInTimeFinder } from '../point_in_time_finder';
+import { savedObjectsPointInTimeFinderMock } from '../point_in_time_finder.mock';
+import type { ISavedObjectsRepository } from '../repository';
+import { savedObjectsRepositoryMock } from '../repository.mock';
 import { findLegacyUrlAliases } from './find_legacy_url_aliases';
-import type { CreatePointInTimeFinderFn, PointInTimeFinder } from './point_in_time_finder';
-import { savedObjectsPointInTimeFinderMock } from './point_in_time_finder.mock';
-import type { ISavedObjectsRepository } from './repository';
-import { savedObjectsRepositoryMock } from './repository.mock';
 
 describe('findLegacyUrlAliases', () => {
   let savedObjectsMock: jest.Mocked<ISavedObjectsRepository>;

src/core/server/saved_objects/service/lib/legacy_url_aliases/find_legacy_url_aliases.ts

@@ -7,9 +7,9 @@
  */
 
 import * as esKuery from '@kbn/es-query';
-import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../object_types';
-import { getObjectKey } from './internal_utils';
-import type { CreatePointInTimeFinderFn } from './point_in_time_finder';
+import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../../object_types';
+import { getObjectKey } from '../internal_utils';
+import type { CreatePointInTimeFinderFn } from '../point_in_time_finder';
 
 interface FindLegacyUrlAliasesObject {
   type: string;
@@ -68,6 +68,7 @@ export async function findLegacyUrlAliases(
 
 function createAliasKueryFilter(objects: Array<{ type: string; id: string }>) {
   const { buildNode } = esKuery.nodeTypes.function;
+  // Note: these nodes include '.attributes' for type-level fields because these are eventually passed to `validateConvertFilterToKueryNode`, which requires it
   const kueryNodes = objects.reduce<unknown[]>((acc, { type, id }) => {
     const match1 = buildNode('is', `${LEGACY_URL_ALIAS_TYPE}.attributes.targetType`, type);
     const match2 = buildNode('is', `${LEGACY_URL_ALIAS_TYPE}.attributes.sourceId`, id);

src/core/server/saved_objects/service/lib/legacy_url_aliases/index.ts

@@ -0,0 +1,12 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+export { findLegacyUrlAliases } from './find_legacy_url_aliases';
+
+export { deleteLegacyUrlAliases } from './delete_legacy_url_aliases';
+export type { DeleteLegacyUrlAliasesParams } from './delete_legacy_url_aliases';

src/core/server/saved_objects/service/lib/preflight_check_for_create.test.mock.ts

@@ -6,14 +6,14 @@
  * Side Public License, v 1.
  */
 
-import type { findLegacyUrlAliases } from './find_legacy_url_aliases';
+import type { findLegacyUrlAliases } from './legacy_url_aliases';
 import type * as InternalUtils from './internal_utils';
 
 export const mockFindLegacyUrlAliases = jest.fn() as jest.MockedFunction<
   typeof findLegacyUrlAliases
 >;
 
-jest.mock('./find_legacy_url_aliases', () => {
+jest.mock('./legacy_url_aliases', () => {
   return { findLegacyUrlAliases: mockFindLegacyUrlAliases };
 });
 

src/core/server/saved_objects/service/lib/preflight_check_for_create.ts

@@ -13,7 +13,7 @@ import type {
   SavedObjectsRawDocSource,
   SavedObjectsSerializer,
 } from '../../serialization';
-import { findLegacyUrlAliases } from './find_legacy_url_aliases';
+import { findLegacyUrlAliases } from './legacy_url_aliases';
 import { Either, rawDocExistsInNamespaces } from './internal_utils';
 import { getObjectKey, isLeft, isRight } from './internal_utils';
 import type { CreatePointInTimeFinderFn } from './point_in_time_finder';

src/core/server/saved_objects/service/lib/repository.test.js

@@ -14,6 +14,7 @@ import {
   mockUpdateObjectsSpaces,
   mockGetCurrentTime,
   mockPreflightCheckForCreate,
+  mockDeleteLegacyUrlAliases,
 } from './repository.test.mock';
 
 import { SavedObjectsRepository } from './repository';
@@ -2394,9 +2395,11 @@ describe('SavedObjectsRepository', () => {
     const id = 'logstash-*';
     const namespace = 'foo-namespace';
 
-    const deleteSuccess = async (type, id, options) => {
+    const deleteSuccess = async (type, id, options, internalOptions = {}) => {
+      const { mockGetResponseValue } = internalOptions;
       if (registry.isMultiNamespace(type)) {
-        const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace);
+        const mockGetResponse =
+          mockGetResponseValue ?? getMockGetResponse({ type, id }, options?.namespace);
         client.get.mockResolvedValueOnce(
           elasticsearchClientMock.createSuccessTransportRequestPromise(mockGetResponse)
         );
@@ -2409,6 +2412,11 @@ describe('SavedObjectsRepository', () => {
       return result;
     };
 
+    beforeEach(() => {
+      mockDeleteLegacyUrlAliases.mockClear();
+      mockDeleteLegacyUrlAliases.mockResolvedValue();
+    });
+
     describe('client calls', () => {
       it(`should use the ES delete action when not using a multi-namespace type`, async () => {
         await deleteSuccess(type, id);
@@ -2482,6 +2490,64 @@ describe('SavedObjectsRepository', () => {
       });
     });
 
+    describe('legacy URL aliases', () => {
+      it(`doesn't delete legacy URL aliases for single-namespace object types`, async () => {
+        await deleteSuccess(type, id, { namespace });
+        expect(mockDeleteLegacyUrlAliases).not.toHaveBeenCalled();
+      });
+
+      // We intentionally do not include a test case for a multi-namespace object with a "not found" preflight result, because that throws
+      // an error (without deleting aliases) and we already have a test case for that
+
+      it(`deletes legacy URL aliases for multi-namespace object types (all spaces)`, async () => {
+        const internalOptions = {
+          mockGetResponseValue: getMockGetResponse(
+            { type: MULTI_NAMESPACE_TYPE, id },
+            ALL_NAMESPACES_STRING
+          ),
+        };
+        await deleteSuccess(MULTI_NAMESPACE_TYPE, id, { namespace, force: true }, internalOptions);
+        expect(mockDeleteLegacyUrlAliases).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: MULTI_NAMESPACE_TYPE,
+            id,
+            namespaces: [],
+            deleteBehavior: 'exclusive',
+          })
+        );
+      });
+
+      it(`deletes legacy URL aliases for multi-namespace object types (specific spaces)`, async () => {
+        await deleteSuccess(MULTI_NAMESPACE_TYPE, id, { namespace }); // this function mocks a preflight response with the given namespace by default
+        expect(mockDeleteLegacyUrlAliases).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: MULTI_NAMESPACE_TYPE,
+            id,
+            namespaces: [namespace],
+            deleteBehavior: 'inclusive',
+          })
+        );
+      });
+
+      it(`logs a message when deleteLegacyUrlAliases returns an error`, async () => {
+        client.get.mockResolvedValueOnce(
+          elasticsearchClientMock.createSuccessTransportRequestPromise(
+            getMockGetResponse({ type: MULTI_NAMESPACE_ISOLATED_TYPE, id, namespace })
+          )
+        );
+        client.delete.mockResolvedValueOnce(
+          elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'deleted' })
+        );
+        mockDeleteLegacyUrlAliases.mockRejectedValueOnce(new Error('Oh no!'));
+        await savedObjectsRepository.delete(MULTI_NAMESPACE_ISOLATED_TYPE, id, { namespace });
+        expect(client.get).toHaveBeenCalledTimes(1);
+        expect(logger.error).toHaveBeenCalledTimes(1);
+        expect(logger.error).toHaveBeenCalledWith(
+          'Unable to delete aliases when deleting an object: Oh no!'
+        );
+      });
+    });
+
     describe('errors', () => {
       const expectNotFoundError = async (type, id, options) => {
         await expect(savedObjectsRepository.delete(type, id, options)).rejects.toThrowError(

src/core/server/saved_objects/service/lib/repository.test.mock.ts

@@ -11,6 +11,7 @@ import type { internalBulkResolve } from './internal_bulk_resolve';
 import type * as InternalUtils from './internal_utils';
 import type { preflightCheckForCreate } from './preflight_check_for_create';
 import type { updateObjectsSpaces } from './update_objects_spaces';
+import type { deleteLegacyUrlAliases } from './legacy_url_aliases';
 
 export const mockCollectMultiNamespaceReferences = jest.fn() as jest.MockedFunction<
   typeof collectMultiNamespaceReferences
@@ -60,3 +61,10 @@ export const pointInTimeFinderMock = jest.fn();
 jest.doMock('./point_in_time_finder', () => ({
   PointInTimeFinder: pointInTimeFinderMock,
 }));
+
+export const mockDeleteLegacyUrlAliases = jest.fn() as jest.MockedFunction<
+  typeof deleteLegacyUrlAliases
+>;
+jest.mock('./legacy_url_aliases', () => ({
+  deleteLegacyUrlAliases: mockDeleteLegacyUrlAliases,
+}));

src/core/server/saved_objects/service/lib/repository.ts

@@ -100,6 +100,7 @@ import {
   preflightCheckForCreate,
   PreflightCheckForCreateObject,
 } from './preflight_check_for_create';
+import { deleteLegacyUrlAliases } from './legacy_url_aliases';
 
 // BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository
 // so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient.
@@ -717,6 +718,25 @@ export class SavedObjectsRepository {
 
     const deleted = body.result === 'deleted';
     if (deleted) {
+      const namespaces = preflightResult?.savedObjectNamespaces;
+      if (namespaces) {
+        // This is a multi-namespace object type, and it might have legacy URL aliases that need to be deleted.
+        await deleteLegacyUrlAliases({
+          mappings: this._mappings,
+          registry: this._registry,
+          client: this.client,
+          getIndexForType: this.getIndexForType.bind(this),
+          type,
+          id,
+          ...(namespaces.includes(ALL_NAMESPACES_STRING)
+            ? { namespaces: [], deleteBehavior: 'exclusive' } // delete legacy URL aliases for this type/ID for all spaces
+            : { namespaces, deleteBehavior: 'inclusive' }), // delete legacy URL aliases for this type/ID for these specific spaces
+        }).catch((err) => {
+          // The object has already been deleted, but we caught an error when attempting to delete aliases.
+          // A consumer cannot attempt to delete the object again, so just log the error and swallow it.
+          this._logger.error(`Unable to delete aliases when deleting an object: ${err.message}`);
+        });
+      }
       return {};
     }
 
@@ -1344,10 +1364,12 @@ export class SavedObjectsRepository {
     options?: SavedObjectsUpdateObjectsSpacesOptions
   ) {
     return updateObjectsSpaces({
+      mappings: this._mappings,
       registry: this._registry,
       allowedTypes: this._allowedTypes,
       client: this.client,
       serializer: this._serializer,
+      logger: this._logger,
       getIndexForType: this.getIndexForType.bind(this),
       objects,
       spacesToAdd,

src/core/server/saved_objects/service/lib/update_objects_spaces.test.mock.ts

@@ -7,6 +7,7 @@
  */
 
 import type * as InternalUtils from './internal_utils';
+import type { deleteLegacyUrlAliases } from './legacy_url_aliases';
 
 export const mockGetBulkOperationError = jest.fn() as jest.MockedFunction<
   typeof InternalUtils['getBulkOperationError']
@@ -27,3 +28,10 @@ jest.mock('./internal_utils', () => {
     rawDocExistsInNamespace: mockRawDocExistsInNamespace,
   };
 });
+
+export const mockDeleteLegacyUrlAliases = jest.fn() as jest.MockedFunction<
+  typeof deleteLegacyUrlAliases
+>;
+jest.mock('./legacy_url_aliases', () => ({
+  deleteLegacyUrlAliases: mockDeleteLegacyUrlAliases,
+}));

src/core/server/saved_objects/service/lib/update_objects_spaces.test.ts

@@ -10,12 +10,14 @@ import {
   mockGetBulkOperationError,
   mockGetExpectedVersionProperties,
   mockRawDocExistsInNamespace,
+  mockDeleteLegacyUrlAliases,
 } from './update_objects_spaces.test.mock';
 
 import type { DeeplyMockedKeys } from '@kbn/utility-types/jest';
 import type { ElasticsearchClient } from 'src/core/server/elasticsearch';
 import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks';
 
+import { loggerMock } from '../../../logging/logger.mock';
 import { typeRegistryMock } from '../../saved_objects_type_registry.mock';
 import { SavedObjectsSerializer } from '../../serialization';
 import type {
@@ -23,6 +25,7 @@ import type {
   UpdateObjectsSpacesParams,
 } from './update_objects_spaces';
 import { updateObjectsSpaces } from './update_objects_spaces';
+import { ALL_NAMESPACES_STRING } from './utils';
 
 type SetupParams = Partial<
   Pick<UpdateObjectsSpacesParams, 'objects' | 'spacesToAdd' | 'spacesToRemove' | 'options'>
@@ -53,6 +56,8 @@ beforeEach(() => {
   mockGetExpectedVersionProperties.mockReturnValue(EXPECTED_VERSION_PROPS);
   mockRawDocExistsInNamespace.mockReset();
   mockRawDocExistsInNamespace.mockReturnValue(true); // return true by default
+  mockDeleteLegacyUrlAliases.mockReset();
+  mockDeleteLegacyUrlAliases.mockResolvedValue(); // resolve an empty promise by default
 });
 
 afterAll(() => {
@@ -71,10 +76,12 @@ describe('#updateObjectsSpaces', () => {
     client = elasticsearchClientMock.createElasticsearchClient();
     const serializer = new SavedObjectsSerializer(registry);
     return {
+      mappings: { properties: {} }, // doesn't matter, only used as an argument to deleteLegacyUrlAliases which is mocked
       registry,
       allowedTypes: [SHAREABLE_OBJ_TYPE, NON_SHAREABLE_OBJ_TYPE], // SHAREABLE_HIDDEN_OBJ_TYPE is excluded
       client,
       serializer,
+      logger: loggerMock.create(),
       getIndexForType: (type: string) => `index-for-${type}`,
       objects,
       spacesToAdd,
@@ -382,6 +389,149 @@ describe('#updateObjectsSpaces', () => {
         expect(client.bulk).not.toHaveBeenCalled();
       });
     });
+
+    describe('legacy URL aliases', () => {
+      it('does not delete aliases for objects that were not removed from any spaces', async () => {
+        const space1 = 'space-to-add';
+        const space2 = 'space-to-remove';
+        const space3 = 'other-space';
+        const obj1 = { type: SHAREABLE_OBJ_TYPE, id: 'id-1', spaces: [space1] }; // will not be changed
+        const obj2 = { type: SHAREABLE_OBJ_TYPE, id: 'id-2', spaces: [space3] }; // will be updated
+
+        const objects = [obj1, obj2];
+        const spacesToAdd = [space1];
+        const spacesToRemove = [space2];
+        const params = setup({ objects, spacesToAdd, spacesToRemove });
+        // this test case does not call mget
+        mockBulkResults({ error: false }); // result for obj2
+
+        await updateObjectsSpaces(params);
+        expect(client.bulk).toHaveBeenCalledTimes(1);
+        expectBulkArgs({ action: 'update', object: { ...obj2, namespaces: [space3, space1] } });
+        expect(mockDeleteLegacyUrlAliases).not.toHaveBeenCalled();
+        expect(params.logger.error).not.toHaveBeenCalled();
+      });
+
+      it('does not delete aliases for objects that were removed from spaces but were also added to All Spaces (*)', async () => {
+        const space2 = 'space-to-remove';
+        const obj1 = { type: SHAREABLE_OBJ_TYPE, id: 'id-1', spaces: [space2] };
+
+        const objects = [obj1];
+        const spacesToAdd = [ALL_NAMESPACES_STRING];
+        const spacesToRemove = [space2];
+        const params = setup({ objects, spacesToAdd, spacesToRemove });
+        // this test case does not call mget
+        mockBulkResults({ error: false }); // result for obj1
+
+        await updateObjectsSpaces(params);
+        expect(client.bulk).toHaveBeenCalledTimes(1);
+        expectBulkArgs({
+          action: 'update',
+          object: { ...obj1, namespaces: [ALL_NAMESPACES_STRING] },
+        });
+        expect(mockDeleteLegacyUrlAliases).not.toHaveBeenCalled();
+        expect(params.logger.error).not.toHaveBeenCalled();
+      });
+
+      it('deletes aliases for objects that were removed from specific spaces using "deleteBehavior: exclusive"', async () => {
+        const space1 = 'space-to-remove';
+        const space2 = 'another-space-to-remove';
+        const space3 = 'other-space';
+        const obj1 = { type: SHAREABLE_OBJ_TYPE, id: 'id-1', spaces: [space3] }; // will not be changed
+        const obj2 = { type: SHAREABLE_OBJ_TYPE, id: 'id-1', spaces: [space1, space2, space3] }; // will be updated
+        const obj3 = { type: SHAREABLE_OBJ_TYPE, id: 'id-1', spaces: [space1] }; // will be deleted
+
+        const objects = [obj1, obj2, obj3];
+        const spacesToRemove = [space1, space2];
+        const params = setup({ objects, spacesToRemove });
+        // this test case does not call mget
+        mockBulkResults({ error: false }, { error: false }); // result2 for obj2 and obj3
+
+        await updateObjectsSpaces(params);
+        expect(client.bulk).toHaveBeenCalledTimes(1);
+        expectBulkArgs(
+          { action: 'update', object: { ...obj2, namespaces: [space3] } },
+          { action: 'delete', object: obj3 }
+        );
+        expect(mockDeleteLegacyUrlAliases).toHaveBeenCalledTimes(2);
+        expect(mockDeleteLegacyUrlAliases).toHaveBeenNthCalledWith(
+          1, // the first call resulted in an error which generated a log message (see assertion below)
+          expect.objectContaining({
+            type: obj2.type,
+            id: obj2.id,
+            namespaces: [space1, space2],
+            deleteBehavior: 'inclusive',
+          })
+        );
+        expect(mockDeleteLegacyUrlAliases).toHaveBeenNthCalledWith(
+          2,
+          expect.objectContaining({
+            type: obj3.type,
+            id: obj3.id,
+            namespaces: [space1],
+            deleteBehavior: 'inclusive',
+          })
+        );
+        expect(params.logger.error).not.toHaveBeenCalled();
+      });
+
+      it('deletes aliases for objects that were removed from all spaces using "deleteBehavior: inclusive"', async () => {
+        const space1 = 'space-to-add';
+        const space2 = 'other-space';
+        const obj1 = { type: SHAREABLE_OBJ_TYPE, id: 'id-1', spaces: [space2] }; // will be updated to add space1
+        const obj2 = {
+          type: SHAREABLE_OBJ_TYPE,
+          id: 'id-2',
+          spaces: [space2, ALL_NAMESPACES_STRING], // will be updated to add space1 and remove *
+        };
+
+        const objects = [obj1, obj2];
+        const spacesToAdd = [space1];
+        const spacesToRemove = [ALL_NAMESPACES_STRING];
+        const params = setup({ objects, spacesToAdd, spacesToRemove });
+        // this test case does not call mget
+        mockBulkResults({ error: false }); // result for obj1
+
+        await updateObjectsSpaces(params);
+        expect(client.bulk).toHaveBeenCalledTimes(1);
+        expectBulkArgs(
+          { action: 'update', object: { ...obj1, namespaces: [space2, space1] } },
+          { action: 'update', object: { ...obj2, namespaces: [space2, space1] } }
+        );
+        expect(mockDeleteLegacyUrlAliases).toHaveBeenCalledTimes(1);
+        expect(mockDeleteLegacyUrlAliases).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: obj2.type,
+            id: obj2.id,
+            namespaces: [space2, space1],
+            deleteBehavior: 'exclusive',
+          })
+        );
+        expect(params.logger.error).not.toHaveBeenCalled();
+      });
+
+      it('logs a message when deleteLegacyUrlAliases returns an error', async () => {
+        const space1 = 'space-to-remove';
+        const space2 = 'other-space';
+        const obj1 = { type: SHAREABLE_OBJ_TYPE, id: 'id-1', spaces: [space1, space2] }; // will be updated
+
+        const objects = [obj1];
+        const spacesToRemove = [space1];
+        const params = setup({ objects, spacesToRemove });
+        // this test case does not call mget
+        mockBulkResults({ error: false }); // result for obj1
+        mockDeleteLegacyUrlAliases.mockRejectedValueOnce(new Error('Oh no!')); // result for deleting aliases for obj1
+
+        await updateObjectsSpaces(params);
+        expect(client.bulk).toHaveBeenCalledTimes(1);
+        expectBulkArgs({ action: 'update', object: { ...obj1, namespaces: [space2] } });
+        expect(mockDeleteLegacyUrlAliases).toHaveBeenCalledTimes(1); // don't assert deleteLegacyUrlAliases args, we have tests for that above
+        expect(params.logger.error).toHaveBeenCalledTimes(1);
+        expect(params.logger.error).toHaveBeenCalledWith(
+          'Unable to delete aliases when unsharing an object: Oh no!'
+        );
+      });
+    });
   });
 
   describe('returns expected results', () => {

src/core/server/saved_objects/service/lib/update_objects_spaces.ts

@@ -6,9 +6,12 @@
  * Side Public License, v 1.
  */
 
+import pMap from 'p-map';
 import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
 import intersection from 'lodash/intersection';
 
+import type { Logger } from '../../../logging';
+import type { IndexMapping } from '../../mappings';
 import type { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry';
 import type { SavedObjectsRawDocSource, SavedObjectsSerializer } from '../../serialization';
 import type {
@@ -28,6 +31,9 @@ import {
 } from './internal_utils';
 import { DEFAULT_REFRESH_SETTING } from './repository';
 import type { RepositoryEsClient } from './repository_es_client';
+import { ALL_NAMESPACES_STRING } from './utils';
+import type { DeleteLegacyUrlAliasesParams } from './legacy_url_aliases';
+import { deleteLegacyUrlAliases } from './legacy_url_aliases';
 
 /**
  * An object that should have its spaces updated.
@@ -94,10 +100,12 @@ export interface SavedObjectsUpdateObjectsSpacesResponseObject {
  * @internal
  */
 export interface UpdateObjectsSpacesParams {
+  mappings: IndexMapping;
   registry: ISavedObjectTypeRegistry;
   allowedTypes: string[];
   client: RepositoryEsClient;
   serializer: SavedObjectsSerializer;
+  logger: Logger;
   getIndexForType: (type: string) => string;
   objects: SavedObjectsUpdateObjectsSpacesObject[];
   spacesToAdd: string[];
@@ -105,15 +113,24 @@ export interface UpdateObjectsSpacesParams {
   options?: SavedObjectsUpdateObjectsSpacesOptions;
 }
 
+type ObjectToDeleteAliasesFor = Pick<
+  DeleteLegacyUrlAliasesParams,
+  'type' | 'id' | 'namespaces' | 'deleteBehavior'
+>;
+
+const MAX_CONCURRENT_ALIAS_DELETIONS = 10;
+
 /**
  * Gets all references and transitive references of the given objects. Ignores any object and/or reference that is not a multi-namespace
  * type.
  */
 export async function updateObjectsSpaces({
+  mappings,
   registry,
   allowedTypes,
   client,
   serializer,
+  logger,
   getIndexForType,
   objects,
   spacesToAdd,
@@ -190,8 +207,12 @@ export async function updateObjectsSpaces({
   const time = new Date().toISOString();
   let bulkOperationRequestIndexCounter = 0;
   const bulkOperationParams: estypes.BulkOperationContainer[] = [];
+  const objectsToDeleteAliasesFor: ObjectToDeleteAliasesFor[] = [];
   const expectedBulkOperationResults: Array<
-    Either<SavedObjectsUpdateObjectsSpacesResponseObject, Record<string, any>>
+    Either<
+      SavedObjectsUpdateObjectsSpacesResponseObject,
+      { type: string; id: string; updatedSpaces: string[]; esRequestIndex?: number }
+    >
   > = expectedBulkGetResults.map((expectedBulkGetResult) => {
     if (isLeft(expectedBulkGetResult)) {
       return expectedBulkGetResult;
@@ -225,7 +246,7 @@ export async function updateObjectsSpaces({
       versionProperties = getExpectedVersionProperties(version);
     }
 
-    const { newSpaces, isUpdateRequired } = getNewSpacesArray(
+    const { updatedSpaces, removedSpaces, isUpdateRequired } = analyzeSpaceChanges(
       currentSpaces,
       spacesToAdd,
       spacesToRemove
@@ -233,7 +254,7 @@ export async function updateObjectsSpaces({
     const expectedResult = {
       type,
       id,
-      newSpaces,
+      updatedSpaces,
       ...(isUpdateRequired && { esRequestIndex: bulkOperationRequestIndexCounter++ }),
     };
 
@@ -243,13 +264,24 @@ export async function updateObjectsSpaces({
         _index: getIndexForType(type),
         ...versionProperties,
       };
-      if (newSpaces.length) {
-        const documentToSave = { updated_at: time, namespaces: newSpaces };
+      if (updatedSpaces.length) {
+        const documentToSave = { updated_at: time, namespaces: updatedSpaces };
         // @ts-expect-error BulkOperation.retry_on_conflict, BulkOperation.routing. BulkOperation.version, and BulkOperation.version_type are optional
         bulkOperationParams.push({ update: documentMetadata }, { doc: documentToSave });
       } else {
         bulkOperationParams.push({ delete: documentMetadata });
       }
+
+      if (removedSpaces.length && !updatedSpaces.includes(ALL_NAMESPACES_STRING)) {
+        // The object is being removed from at least one space; make sure to delete aliases appropriately
+        objectsToDeleteAliasesFor.push({
+          type,
+          id,
+          ...(removedSpaces.includes(ALL_NAMESPACES_STRING)
+            ? { namespaces: updatedSpaces, deleteBehavior: 'exclusive' }
+            : { namespaces: removedSpaces, deleteBehavior: 'inclusive' }),
+        });
+      }
     }
 
     return { tag: 'Right', value: expectedResult };
@@ -260,6 +292,24 @@ export async function updateObjectsSpaces({
     ? await client.bulk({ refresh, body: bulkOperationParams, require_alias: true })
     : undefined;
 
+  // Delete aliases if necessary, ensuring we don't have too many concurrent operations running.
+  const mapper = async ({ type, id, namespaces, deleteBehavior }: ObjectToDeleteAliasesFor) =>
+    deleteLegacyUrlAliases({
+      mappings,
+      registry,
+      client,
+      getIndexForType,
+      type,
+      id,
+      namespaces,
+      deleteBehavior,
+    }).catch((err) => {
+      // The object has already been unshared, but we caught an error when attempting to delete aliases.
+      // A consumer cannot attempt to unshare the object again, so just log the error and swallow it.
+      logger.error(`Unable to delete aliases when unsharing an object: ${err.message}`);
+    });
+  await pMap(objectsToDeleteAliasesFor, mapper, { concurrency: MAX_CONCURRENT_ALIAS_DELETIONS });
+
   return {
     objects: expectedBulkOperationResults.map<SavedObjectsUpdateObjectsSpacesResponseObject>(
       (expectedResult) => {
@@ -267,7 +317,7 @@ export async function updateObjectsSpaces({
           return expectedResult.value;
         }
 
-        const { type, id, newSpaces, esRequestIndex } = expectedResult.value;
+        const { type, id, updatedSpaces, esRequestIndex } = expectedResult.value;
         if (esRequestIndex !== undefined) {
           const response = bulkOperationResponse?.body.items[esRequestIndex] ?? {};
           const rawResponse = Object.values(response)[0] as any;
@@ -277,7 +327,7 @@ export async function updateObjectsSpaces({
           }
         }
 
-        return { id, type, spaces: newSpaces };
+        return { id, type, spaces: updatedSpaces };
       }
     ),
   };
@@ -289,17 +339,22 @@ function errorContent(error: DecoratedError) {
 }
 
 /** Gets the remaining spaces for an object after adding new ones and removing old ones. */
-function getNewSpacesArray(
+function analyzeSpaceChanges(
   existingSpaces: string[],
   spacesToAdd: string[],
   spacesToRemove: string[]
 ) {
   const addSet = new Set(spacesToAdd);
   const removeSet = new Set(spacesToRemove);
-  const newSpaces = existingSpaces
+  const removedSpaces: string[] = [];
+  const updatedSpaces = existingSpaces
     .filter((x) => {
       addSet.delete(x);
-      return !removeSet.delete(x);
+      const removed = removeSet.delete(x);
+      if (removed) {
+        removedSpaces.push(x);
+      }
+      return !removed;
     })
     .concat(Array.from(addSet));
 
@@ -307,5 +362,5 @@ function getNewSpacesArray(
   const isAnySpaceRemoved = removeSet.size < spacesToRemove.length;
   const isUpdateRequired = isAnySpaceAdded || isAnySpaceRemoved;
 
-  return { newSpaces, isUpdateRequired };
+  return { updatedSpaces, removedSpaces, isUpdateRequired };
 }

x-pack/test/saved_object_api_integration/common/suites/delete.ts

@@ -6,7 +6,9 @@
  */
 
 import { SuperTest } from 'supertest';
+import type { Client } from '@elastic/elasticsearch';
 import expect from '@kbn/expect';
+import type { SearchTotalHits } from '@elastic/elasticsearch/lib/api/types';
 import { SAVED_OBJECT_TEST_CASES as CASES } from '../lib/saved_object_test_cases';
 import { SPACES } from '../lib/spaces';
 import { expectResponses, getUrlPrefix, getTestTitle } from '../lib/saved_object_test_utils';
@@ -21,9 +23,13 @@ export interface DeleteTestCase extends TestCase {
   failure?: 400 | 403 | 404;
 }
 
+const ALIAS_DELETE_INCLUSIVE = Object.freeze({ type: 'resolvetype', id: 'alias-match-newid' }); // exists in three specific spaces; deleting this should also delete the alias that targets it in space 1
+const ALIAS_DELETE_EXCLUSIVE = Object.freeze({ type: 'resolvetype', id: 'all_spaces' }); // exists in all spaces; deleting this should also delete the alias that targets it in space 1
 const DOES_NOT_EXIST = Object.freeze({ type: 'dashboard', id: 'does-not-exist' });
 export const TEST_CASES: Record<string, DeleteTestCase> = Object.freeze({
   ...CASES,
+  ALIAS_DELETE_INCLUSIVE,
+  ALIAS_DELETE_EXCLUSIVE,
   DOES_NOT_EXIST,
 });
 
@@ -32,7 +38,7 @@ export const TEST_CASES: Record<string, DeleteTestCase> = Object.freeze({
  */
 const createRequest = ({ type, id, force }: DeleteTestCase) => ({ type, id, force });
 
-export function deleteTestSuiteFactory(esArchiver: any, supertest: SuperTest<any>) {
+export function deleteTestSuiteFactory(es: Client, esArchiver: any, supertest: SuperTest<any>) {
   const expectSavedObjectForbidden = expectResponses.forbiddenTypes('delete');
   const expectResponseBody =
     (testCase: DeleteTestCase): ExpectResponseBody =>
@@ -47,6 +53,25 @@ export function deleteTestSuiteFactory(esArchiver: any, supertest: SuperTest<any
         } else {
           // the success response for `delete` is an empty object
           expect(object).to.eql({});
+
+          // if we deleted an object that had an alias pointing to it, the alias should have been deleted as well
+          await es.indices.refresh({ index: '.kibana' }); // alias deletion uses refresh: false, so we need to manually refresh the index before searching
+          const searchResponse = await es.search({
+            index: '.kibana',
+            body: {
+              size: 0,
+              query: { terms: { type: ['legacy-url-alias'] } },
+              track_total_hits: true,
+            },
+          });
+          const expectAliasWasDeleted = !![ALIAS_DELETE_INCLUSIVE, ALIAS_DELETE_EXCLUSIVE].find(
+            ({ type, id }) => testCase.type === type && testCase.id === id
+          );
+          expect((searchResponse.hits.total as SearchTotalHits).value).to.eql(
+            // Five aliases exist but only one should be deleted in each case (for the "inclusive" case, this asserts that the aliases
+            // targeting that object in space x and space y were *not* deleted)
+            expectAliasWasDeleted ? 4 : 5
+          );
         }
       }
     };

x-pack/test/saved_object_api_integration/security_and_spaces/apis/delete.ts

@@ -51,6 +51,8 @@ const createTestCases = (spaceId: string) => {
     },
     { ...CASES.MULTI_NAMESPACE_ISOLATED_ONLY_SPACE_1, ...fail404(spaceId !== SPACE_1_ID) },
     CASES.NAMESPACE_AGNOSTIC,
+    { ...CASES.ALIAS_DELETE_INCLUSIVE, force: true },
+    { ...CASES.ALIAS_DELETE_EXCLUSIVE, force: true },
     { ...CASES.DOES_NOT_EXIST, ...fail404() },
   ];
   const hiddenType = [{ ...CASES.HIDDEN, ...fail404() }];
@@ -61,8 +63,9 @@ const createTestCases = (spaceId: string) => {
 export default function ({ getService }: FtrProviderContext) {
   const supertest = getService('supertestWithoutAuth');
   const esArchiver = getService('esArchiver');
+  const es = getService('es');
 
-  const { addTests, createTestDefinitions } = deleteTestSuiteFactory(esArchiver, supertest);
+  const { addTests, createTestDefinitions } = deleteTestSuiteFactory(es, esArchiver, supertest);
   const createTests = (spaceId: string) => {
     const { normalTypes, hiddenType, allTypes } = createTestCases(spaceId);
     return {

x-pack/test/saved_object_api_integration/spaces_only/apis/delete.ts

@@ -45,6 +45,8 @@ const createTestCases = (spaceId: string) => [
   },
   { ...CASES.MULTI_NAMESPACE_ISOLATED_ONLY_SPACE_1, ...fail404(spaceId !== SPACE_1_ID) },
   CASES.NAMESPACE_AGNOSTIC,
+  { ...CASES.ALIAS_DELETE_INCLUSIVE, force: true },
+  { ...CASES.ALIAS_DELETE_EXCLUSIVE, force: true },
   { ...CASES.HIDDEN, ...fail404() },
   { ...CASES.DOES_NOT_EXIST, ...fail404() },
 ];
@@ -52,8 +54,9 @@ const createTestCases = (spaceId: string) => [
 export default function ({ getService }: FtrProviderContext) {
   const supertest = getService('supertest');
   const esArchiver = getService('esArchiver');
+  const es = getService('es');
 
-  const { addTests, createTestDefinitions } = deleteTestSuiteFactory(esArchiver, supertest);
+  const { addTests, createTestDefinitions } = deleteTestSuiteFactory(es, esArchiver, supertest);
   const createTests = (spaceId: string) => {
     const testCases = createTestCases(spaceId);
     return createTestDefinitions(testCases, false, { spaceId });

x-pack/test/spaces_api_integration/common/fixtures/es_archiver/saved_objects/spaces/data.json

@@ -582,6 +582,94 @@
   }
 }
 
+{
+  "type": "doc",
+  "value": {
+    "id": "sharedtype:alias_delete_inclusive",
+    "index": ".kibana",
+    "source": {
+      "sharedtype": {
+        "title": "This is used to test that when an object is unshared from a space, inbound aliases for just those spaces are removed"
+      },
+      "type": "sharedtype",
+      "namespaces": ["default", "space_1", "space_2"],
+      "updated_at": "2017-09-21T18:59:16.270Z"
+    },
+    "type": "doc"
+  }
+}
+
+{
+  "type": "doc",
+  "value": {
+    "id": "legacy-url-alias:space_1:sharedtype:doesnt-matter",
+    "index": ".kibana",
+    "source": {
+      "type": "legacy-url-alias",
+      "updated_at": "2017-09-21T18:51:23.794Z",
+      "legacy-url-alias": {
+        "sourceId": "doesnt-matter",
+        "targetNamespace": "space_1",
+        "targetType": "sharedtype",
+        "targetId": "alias_delete_inclusive"
+      }
+    }
+  }
+}
+
+{
+  "type": "doc",
+  "value": {
+    "id": "legacy-url-alias:space_2:sharedtype:doesnt-matter",
+    "index": ".kibana",
+    "source": {
+      "type": "legacy-url-alias",
+      "updated_at": "2017-09-21T18:51:23.794Z",
+      "legacy-url-alias": {
+        "sourceId": "doesnt-matter",
+        "targetNamespace": "space_2",
+        "targetType": "sharedtype",
+        "targetId": "alias_delete_inclusive"
+      }
+    }
+  }
+}
+
+{
+  "type": "doc",
+  "value": {
+    "id": "sharedtype:alias_delete_exclusive",
+    "index": ".kibana",
+    "source": {
+      "sharedtype": {
+        "title": "This is used to test that when an object is unshared from all space, inbound aliases for all spaces are removed"
+      },
+      "type": "sharedtype",
+      "namespaces": ["*"],
+      "updated_at": "2017-09-21T18:59:16.270Z"
+    },
+    "type": "doc"
+  }
+}
+
+{
+  "type": "doc",
+  "value": {
+    "id": "legacy-url-alias:other_space:sharedtype:doesnt-matter",
+    "index": ".kibana",
+    "source": {
+      "type": "legacy-url-alias",
+      "updated_at": "2017-09-21T18:51:23.794Z",
+      "legacy-url-alias": {
+        "sourceId": "doesnt-matter",
+        "targetNamespace": "other_space",
+        "targetType": "sharedtype",
+        "targetId": "alias_delete_exclusive"
+      }
+    }
+  }
+}
+
 {
   "type": "doc",
   "value": {

x-pack/test/spaces_api_integration/common/lib/saved_object_test_cases.ts

@@ -38,6 +38,14 @@ export const MULTI_NAMESPACE_SAVED_OBJECT_TEST_CASES = Object.freeze({
     id: 'all_spaces',
     existingNamespaces: ['*'], // all current and future spaces
   }),
+  ALIAS_DELETE_INCLUSIVE: Object.freeze({
+    id: 'alias_delete_inclusive',
+    existingNamespaces: ['default', 'space_1', 'space_2'], // each individual space
+  }),
+  ALIAS_DELETE_EXCLUSIVE: Object.freeze({
+    id: 'alias_delete_exclusive',
+    existingNamespaces: ['*'], // all current and future spaces
+  }),
   DOES_NOT_EXIST: Object.freeze({
     id: 'does_not_exist',
     existingNamespaces: [] as string[],

x-pack/test/spaces_api_integration/common/suites/delete.ts

@@ -53,8 +53,8 @@ export function deleteTestSuiteFactory(es: Client, esArchiver: any, supertest: S
     // @ts-expect-error @elastic/elasticsearch doesn't defined `count.buckets`.
     const buckets = response.aggregations?.count.buckets;
 
-    // The test fixture contains three legacy URL aliases:
-    // (1) one for "space_1", (2) one for "space_2", and (3) one for "other_space", which is a non-existent space.
+    // The test fixture contains six legacy URL aliases:
+    // (1) two for "space_1", (2) two for "space_2", and (3) two for "other_space", which is a non-existent space.
     // Each test deletes "space_2", so the agg buckets should reflect that aliases (1) and (3) still exist afterwards.
 
     // Space 2 deleted, all others should exist
@@ -75,26 +75,26 @@ export function deleteTestSuiteFactory(es: Client, esArchiver: any, supertest: S
         },
       },
       {
-        doc_count: 6,
+        doc_count: 7,
         key: 'space_1',
         countByType: {
           doc_count_error_upper_bound: 0,
           sum_other_doc_count: 0,
           buckets: [
             { key: 'visualization', doc_count: 3 },
+            { key: 'legacy-url-alias', doc_count: 2 }, // aliases (1)
             { key: 'dashboard', doc_count: 1 },
             { key: 'index-pattern', doc_count: 1 },
-            { key: 'legacy-url-alias', doc_count: 1 }, // alias (1)
           ],
         },
       },
       {
-        doc_count: 1,
+        doc_count: 2,
         key: 'other_space',
         countByType: {
           doc_count_error_upper_bound: 0,
           sum_other_doc_count: 0,
-          buckets: [{ key: 'legacy-url-alias', doc_count: 1 }], // alias (3)
+          buckets: [{ key: 'legacy-url-alias', doc_count: 2 }], // aliases (3)
         },
       },
     ];
@@ -110,8 +110,8 @@ export function deleteTestSuiteFactory(es: Client, esArchiver: any, supertest: S
       body: { query: { terms: { type: ['sharedtype'] } } },
     });
     const docs = multiNamespaceResponse.hits.hits;
-    // Just 12 results, since spaces_2_only, conflict_1_space_2 and conflict_2_space_2 got deleted.
-    expect(docs).length(12);
+    // Just 14 results, since spaces_2_only, conflict_1_space_2 and conflict_2_space_2 got deleted.
+    expect(docs).length(14);
     docs.forEach((doc) => () => {
       const containsSpace2 = doc?._source?.namespaces.includes('space_2');
       expect(containsSpace2).to.eql(false);

x-pack/test/spaces_api_integration/common/suites/update_objects_spaces.ts

@@ -6,6 +6,8 @@
  */
 
 import expect from '@kbn/expect';
+import type { Client } from '@elastic/elasticsearch';
+import type { SearchTotalHits } from '@elastic/elasticsearch/lib/api/types';
 import { without, uniq } from 'lodash';
 import { SuperTest } from 'supertest';
 import {
@@ -35,6 +37,7 @@ export interface UpdateObjectsSpacesTestCase {
   objects: Array<{
     id: string;
     existingNamespaces: string[];
+    expectAliasDifference?: number;
     failure?: 400 | 404;
   }>;
   spacesToAdd: string[];
@@ -54,7 +57,11 @@ const getTestTitle = ({ objects, spacesToAdd, spacesToRemove }: UpdateObjectsSpa
   return `{objects: [${objStr}], spacesToAdd: [${addStr}], spacesToRemove: [${remStr}]}`;
 };
 
-export function updateObjectsSpacesTestSuiteFactory(esArchiver: any, supertest: SuperTest<any>) {
+export function updateObjectsSpacesTestSuiteFactory(
+  es: Client,
+  esArchiver: any,
+  supertest: SuperTest<any>
+) {
   const expectForbidden = expectResponses.forbiddenTypes('share_to_space');
   const expectResponseBody =
     (
@@ -68,7 +75,10 @@ export function updateObjectsSpacesTestSuiteFactory(esArchiver: any, supertest:
       } else {
         const { objects, spacesToAdd, spacesToRemove } = testCase;
         const apiResponse = response.body as SavedObjectsUpdateObjectsSpacesResponse;
-        objects.forEach(({ id, existingNamespaces, failure }, i) => {
+
+        let hasRefreshed = false;
+        for (let i = 0; i < objects.length; i++) {
+          const { id, existingNamespaces, expectAliasDifference, failure } = objects[i];
           const object = apiResponse.objects[i];
           if (failure === 404) {
             const error = SavedObjectsErrorHelpers.createGenericNotFoundError(TYPE, id);
@@ -84,8 +94,28 @@ export function updateObjectsSpacesTestSuiteFactory(esArchiver: any, supertest:
             expect(result.type).to.eql(TYPE);
             expect(result.id).to.eql(id);
             expect(result.spaces.sort()).to.eql(expectedSpaces.sort());
+
+            if (expectAliasDifference !== undefined) {
+              // if we deleted an object that had an alias pointing to it, the alias should have been deleted as well
+              if (!hasRefreshed) {
+                await es.indices.refresh({ index: '.kibana' }); // alias deletion uses refresh: false, so we need to manually refresh the index before searching
+                hasRefreshed = true;
+              }
+              const searchResponse = await es.search({
+                index: '.kibana',
+                body: {
+                  size: 0,
+                  query: { terms: { type: ['legacy-url-alias'] } },
+                  track_total_hits: true,
+                },
+              });
+              expect((searchResponse.hits.total as SearchTotalHits).value).to.eql(
+                // Six aliases exist in the test fixtures
+                6 + expectAliasDifference
+              );
+            }
           }
-        });
+        }
       }
     };
   const createTestDefinitions = (

x-pack/test/spaces_api_integration/security_and_spaces/apis/update_objects_spaces.ts

@@ -28,6 +28,8 @@ const { fail404 } = testCaseFailures;
 
 const createTestCases = (spaceId: string): UpdateObjectsSpacesTestCase[] => {
   const eachSpace = [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID];
+  // Note: we intentionally exclude ALIAS_DELETION test cases because they are already covered in spaces_only test suite, and there is no
+  // authZ-specific logic that affects alias deletion, all of that happens at the Saved Objects Repository level.
   return [
     // Test case to check adding and removing all spaces ("*") to a saved object
     {
@@ -125,8 +127,10 @@ const calculateSingleSpaceAuthZ = (testCases: UpdateObjectsSpacesTestCase[], spa
 export default function ({ getService }: FtrProviderContext) {
   const supertest = getService('supertestWithoutAuth');
   const esArchiver = getService('esArchiver');
+  const es = getService('es');
 
   const { addTests, createTestDefinitions } = updateObjectsSpacesTestSuiteFactory(
+    es,
     esArchiver,
     supertest
   );

x-pack/test/spaces_api_integration/spaces_only/apis/update_objects_spaces.ts

@@ -11,6 +11,7 @@ import {
   getTestScenarios,
 } from '../../../saved_object_api_integration/common/lib/saved_object_test_utils';
 import { MULTI_NAMESPACE_SAVED_OBJECT_TEST_CASES as CASES } from '../../common/lib/saved_object_test_cases';
+import type { UpdateObjectsSpacesTestCase } from '../../common/suites/update_objects_spaces';
 import { updateObjectsSpacesTestSuiteFactory } from '../../common/suites/update_objects_spaces';
 import { FtrProviderContext } from '../../common/ftr_provider_context';
 
@@ -51,7 +52,55 @@ const createSinglePartTestCases = (spaceId: string) => {
 const createMultiPartTestCases = () => {
   const nonExistentSpace = 'does_not_exist'; // space that doesn't exist
   const eachSpace = [DEFAULT_SPACE_ID, SPACE_1_ID, SPACE_2_ID];
-  const group1 = [
+  const group1: UpdateObjectsSpacesTestCase[] = [
+    // These test cases ensure that aliases are deleted when objects are unshared.
+    // For simplicity these are done separately, before the others.
+    {
+      objects: [
+        {
+          id: CASES.ALIAS_DELETE_INCLUSIVE.id,
+          existingNamespaces: eachSpace,
+          expectAliasDifference: -1, // one alias should have been deleted from space_2
+        },
+      ],
+      spacesToAdd: [],
+      spacesToRemove: [SPACE_2_ID],
+    },
+    {
+      objects: [
+        {
+          id: CASES.ALIAS_DELETE_INCLUSIVE.id,
+          existingNamespaces: [DEFAULT_SPACE_ID, SPACE_1_ID],
+          expectAliasDifference: -2, // one alias should have been deleted from space_1
+        },
+      ],
+      spacesToAdd: [],
+      spacesToRemove: [SPACE_1_ID],
+    },
+    {
+      objects: [
+        {
+          id: CASES.ALIAS_DELETE_INCLUSIVE.id,
+          existingNamespaces: [DEFAULT_SPACE_ID],
+          expectAliasDifference: -2, // no aliases can exist in the default space, so no aliases were deleted
+        },
+      ],
+      spacesToAdd: [],
+      spacesToRemove: [DEFAULT_SPACE_ID],
+    },
+    {
+      objects: [
+        {
+          id: CASES.ALIAS_DELETE_EXCLUSIVE.id,
+          existingNamespaces: [SPACE_1_ID],
+          expectAliasDifference: -3, // one alias should have been deleted from other_space
+        },
+      ],
+      spacesToAdd: [SPACE_1_ID],
+      spacesToRemove: ['*'],
+    },
+  ];
+  const group2 = [
     // first, add this object to each space and remove it from nonExistentSpace
     // this will succeed even though the object already exists in the default space and it doesn't exist in nonExistentSpace
     { objects: [CASES.DEFAULT_ONLY], spacesToAdd: eachSpace, spacesToRemove: [nonExistentSpace] },
@@ -87,7 +136,7 @@ const createMultiPartTestCases = () => {
       spacesToRemove: [SPACE_1_ID],
     },
   ];
-  const group2 = [
+  const group3 = [
     // first, add this object to space_2 and remove it from space_1
     {
       objects: [CASES.DEFAULT_AND_SPACE_1],
@@ -111,15 +160,17 @@ const createMultiPartTestCases = () => {
       spacesToRemove: [],
     },
   ];
-  return [...group1, ...group2];
+  return [...group1, ...group2, ...group3];
 };
 
 // eslint-disable-next-line import/no-default-export
 export default function ({ getService }: FtrProviderContext) {
   const supertest = getService('supertest');
   const esArchiver = getService('esArchiver');
+  const es = getService('es');
 
   const { addTests, createTestDefinitions } = updateObjectsSpacesTestSuiteFactory(
+    es,
     esArchiver,
     supertest
   );