Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,15 @@ export const performFind = async <T = unknown, A = unknown>(
// If the user is unauthorized to find *anything* they requested, return an empty response
return SavedObjectsUtils.createEmptyFindResponse<T, A>(options);
}
if (authorizationResult?.status === 'partially_authorized') {
if (
authorizationResult?.status === 'fully_authorized' ||
Copy link
Member Author

@afharo afharo Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only ensures the same path is followed for both authorization levels, although, functionally, it achieves the same result if we don't add this clause in the if.

Happy to remove this line if we think that performance might be a problem.

I'd appreciate @azasypkin's thoughts on this.

authorizationResult?.status === 'partially_authorized'
) {
typeToNamespacesMap = new Map<string, string[]>();
for (const [objType, entry] of authorizationResult.typeMap) {
if (!entry.find) continue;
// Discard the types that the SO repository doesn't know about (typically hidden objects).
if (!allowedTypes.includes(objType)) continue;
Comment on lines +180 to +181
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixing the user's authorization and the client's scope here (which can be odd).

However, typeToNamespacesMap doesn't really imply that it's bound to only one.

// This ensures that the query DSL can filter only for object types that the user is authorized to access for a given space
const { authorizedSpaces, isGloballyAuthorized } = entry.find;
typeToNamespacesMap.set(objType, isGloballyAuthorized ? namespaces : authorizedSpaces);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import {
generateIndexPatternSearchResults,
setupAuthorizeFunc,
setupAuthorizeFind,
HIDDEN_TYPE,
} from '../test_helpers/repository.test.common';
import { savedObjectsExtensionsMock } from '../mocks/saved_objects_extensions.mock';
import { arrayMapsAreEqual } from '@kbn/core-saved-objects-utils-server';
Expand Down Expand Up @@ -845,14 +846,108 @@ describe('SavedObjectsRepository Security Extension', () => {
});
});

test(`returns empty authorization map for partially authorized if the authorized types are not part of the query`, async () => {
setupAuthorizeFind(mockSecurityExt, 'partially_authorized');
setupRedactPassthrough(mockSecurityExt);

await findSuccess(
client,
repository,
{
type: [type, NAMESPACE_AGNOSTIC_TYPE],
namespaces: [namespace, 'ns-1'],
}, // include multiple types and spaces
namespace
);

expect(mockGetSearchDsl.mock.calls[0].length).toBe(3); // Find success verifies this is called once, this should always pass
const {
typeToNamespacesMap: actualMap,
}: { typeToNamespacesMap: Map<string, string[] | undefined> } =
mockGetSearchDsl.mock.calls[0][2];

expect(actualMap).not.toBeUndefined();
const expectedMap = new Map<string, string[] | undefined>();

expect(arrayMapsAreEqual(actualMap, expectedMap)).toBeTruthy();
});

test(`returns empty authorization map for fully authorized if the authorized types are not part of the query`, async () => {
setupAuthorizeFind(mockSecurityExt, 'fully_authorized');
setupRedactPassthrough(mockSecurityExt);

await findSuccess(
client,
repository,
{
type: [type, NAMESPACE_AGNOSTIC_TYPE],
namespaces: [namespace, 'ns-1'],
}, // include multiple types and spaces
namespace
);

expect(mockGetSearchDsl.mock.calls[0].length).toBe(3); // Find success verifies this is called once, this should always pass
const {
typeToNamespacesMap: actualMap,
}: { typeToNamespacesMap: Map<string, string[] | undefined> } =
mockGetSearchDsl.mock.calls[0][2];

expect(actualMap).not.toBeUndefined();
const expectedMap = new Map<string, string[] | undefined>();

expect(arrayMapsAreEqual(actualMap, expectedMap)).toBeTruthy();
});

test(`uses the authorization map when partially authorized`, async () => {
setupAuthorizeFind(mockSecurityExt, 'partially_authorized');
setupRedactPassthrough(mockSecurityExt);

await findSuccess(
client,
repository,
{ type: [type, NAMESPACE_AGNOSTIC_TYPE], namespaces: [namespace, 'ns-1'] }, // include multiple types and spaces
{
type: [
type,
'foo',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add foo to the requested type so that it is kept in the auth map.

// Explicitly request the hidden type despite the repository not having access to it to confirm that it's not authorized.
HIDDEN_TYPE,
NAMESPACE_AGNOSTIC_TYPE,
],
namespaces: [namespace, 'ns-1'],
}, // include multiple types and spaces
namespace
);

expect(mockGetSearchDsl.mock.calls[0].length).toBe(3); // Find success verifies this is called once, this should always pass
const {
typeToNamespacesMap: actualMap,
}: { typeToNamespacesMap: Map<string, string[] | undefined> } =
mockGetSearchDsl.mock.calls[0][2];

expect(actualMap).not.toBeUndefined();
const expectedMap = new Map<string, string[] | undefined>();
expectedMap.set('foo', ['bar']); // this is what is hard-coded in authMap

expect(arrayMapsAreEqual(actualMap, expectedMap)).toBeTruthy();
});

test(`uses the authorization map when fully authorized`, async () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just repeating the test above, but for fully authorized.

setupAuthorizeFind(mockSecurityExt, 'fully_authorized');
setupRedactPassthrough(mockSecurityExt);

await findSuccess(
client,
repository,
{
type: [
type,
'foo',
// Explicitly request the hidden type despite the repository not having access to it to confirm that it's not authorized.
HIDDEN_TYPE,
NAMESPACE_AGNOSTIC_TYPE,
],
namespaces: [namespace, 'ns-1'],
}, // include multiple types and spaces
namespace
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@ export interface TypeIdTuple {

export const mappings: SavedObjectsTypeMappingDefinition = {
properties: {
foo: {
properties: {
// No fields indexed
},
},
config: {
properties: {
otherField: {
Expand Down Expand Up @@ -233,7 +238,13 @@ export const mappings: SavedObjectsTypeMappingDefinition = {
export const authRecord: Record<string, AuthorizationTypeEntry> = {
find: { authorizedSpaces: ['bar'] },
};
export const authMap = Object.freeze(new Map([['foo', authRecord]]));
export const authMap = Object.freeze(
new Map([
['foo', authRecord],
// The user is authorized to read hidden types but tests will confirm that repositories without that hidden type listed, won't show this as authorized.
[HIDDEN_TYPE, authRecord],
])
);

export const checkAuthError = SavedObjectsErrorHelpers.createBadRequestError(
'Failed to check authorization'
Expand Down Expand Up @@ -344,6 +355,7 @@ export const createType = (

export const createRegistry = () => {
const registry = new SavedObjectTypeRegistry();
registry.registerType(createType('foo'));
registry.registerType(createType('config'));
registry.registerType(createType('index-pattern'));
registry.registerType(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,12 @@ export const swapReferencesRoute =
},
router.handleLegacyErrors(
handleErrors(async (ctx, req, res) => {
const savedObjectsClient = (await ctx.core).savedObjects.client;
const [core] = await getStartServices();
const types = core.savedObjects.getTypeRegistry().getAllTypes();
const savedObjectsContract = (await ctx.core).savedObjects;
const types = savedObjectsContract.typeRegistry.getAllTypes();
const savedObjectsClient = savedObjectsContract.getClient({
// Make sure to search through all the hidden types as well.
includedHiddenTypes: types.filter((t) => t.hidden).map((t) => t.name),
Comment on lines +119 to +120
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that we want to search references to the Data View across all types. Is my assumption correct?

});
const type = req.body.fromType || DATA_VIEW_SAVED_OBJECT_TYPE;
const searchId =
!Array.isArray(req.body.forId) && req.body.forId !== undefined
Expand Down
Loading