Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce capabilities provider and switcher to file upload plugin #96593

Merged
merged 5 commits into from May 14, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
261 changes: 261 additions & 0 deletions x-pack/plugins/file_upload/server/capabilities.test.ts
@@ -0,0 +1,261 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { setupCapabilities } from './capabilities';
import { coreMock, httpServerMock } from '../../../../src/core/server/mocks';
import { Capabilities, CoreStart } from 'kibana/server';
import { securityMock } from '../../security/server/mocks';

describe('setupCapabilities', () => {
it('registers a capabilities provider for the file upload feature', () => {
const coreSetup = coreMock.createSetup();
setupCapabilities(coreSetup);

expect(coreSetup.capabilities.registerProvider).toHaveBeenCalledTimes(1);
const [provider] = coreSetup.capabilities.registerProvider.mock.calls[0];
expect(provider()).toMatchInlineSnapshot(`
Object {
"fileUpload": Object {
"show": true,
},
}
`);
});

it('registers a capabilities switcher that returns unaltered capabilities when security is disabled', async () => {
const coreSetup = coreMock.createSetup();
setupCapabilities(coreSetup);

expect(coreSetup.capabilities.registerSwitcher).toHaveBeenCalledTimes(1);
const [switcher] = coreSetup.capabilities.registerSwitcher.mock.calls[0];

const capabilities = {
navLinks: {},
management: {},
catalogue: {},
fileUpload: {
show: true,
},
} as Capabilities;

const request = httpServerMock.createKibanaRequest();

await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(`
Object {
"catalogue": Object {},
"fileUpload": Object {
"show": true,
},
"management": Object {},
"navLinks": Object {},
}
`);
});

it('registers a capabilities switcher that returns unaltered capabilities when default capabilities are requested', async () => {
const coreSetup = coreMock.createSetup();
const security = securityMock.createStart();
security.authz.mode.useRbacForRequest.mockReturnValue(true);
coreSetup.getStartServices.mockResolvedValue([
(undefined as unknown) as CoreStart,
{ security },
undefined,
]);
setupCapabilities(coreSetup);

expect(coreSetup.capabilities.registerSwitcher).toHaveBeenCalledTimes(1);
const [switcher] = coreSetup.capabilities.registerSwitcher.mock.calls[0];

const capabilities = {
navLinks: {},
management: {},
catalogue: {},
fileUpload: {
show: true,
},
} as Capabilities;

const request = httpServerMock.createKibanaRequest();

await expect(switcher(request, capabilities, true)).resolves.toMatchInlineSnapshot(`
Object {
"catalogue": Object {},
"fileUpload": Object {
"show": true,
},
"management": Object {},
"navLinks": Object {},
}
`);

expect(security.authz.mode.useRbacForRequest).not.toHaveBeenCalled();
expect(security.authz.checkPrivilegesDynamicallyWithRequest).not.toHaveBeenCalled();
});

it('registers a capabilities switcher that disables capabilities for underprivileged users', async () => {
const coreSetup = coreMock.createSetup();
const security = securityMock.createStart();
security.authz.mode.useRbacForRequest.mockReturnValue(true);

const mockCheckPrivileges = jest.fn().mockResolvedValue({ hasAllRequested: false });
security.authz.checkPrivilegesDynamicallyWithRequest.mockReturnValue(mockCheckPrivileges);
coreSetup.getStartServices.mockResolvedValue([
(undefined as unknown) as CoreStart,
{ security },
undefined,
]);
setupCapabilities(coreSetup);

expect(coreSetup.capabilities.registerSwitcher).toHaveBeenCalledTimes(1);
const [switcher] = coreSetup.capabilities.registerSwitcher.mock.calls[0];

const capabilities = {
navLinks: {},
management: {},
catalogue: {},
fileUpload: {
show: true,
},
} as Capabilities;

const request = httpServerMock.createKibanaRequest();

await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(`
Object {
"fileUpload": Object {
"show": false,
},
}
`);

expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledTimes(1);
expect(security.authz.checkPrivilegesDynamicallyWithRequest).toHaveBeenCalledTimes(1);
legrego marked this conversation as resolved.
Show resolved Hide resolved
});

it('registers a capabilities switcher that enables capabilities for privileged users', async () => {
const coreSetup = coreMock.createSetup();
const security = securityMock.createStart();
security.authz.mode.useRbacForRequest.mockReturnValue(true);

const mockCheckPrivileges = jest.fn().mockResolvedValue({ hasAllRequested: true });
security.authz.checkPrivilegesDynamicallyWithRequest.mockReturnValue(mockCheckPrivileges);
coreSetup.getStartServices.mockResolvedValue([
(undefined as unknown) as CoreStart,
{ security },
undefined,
]);
setupCapabilities(coreSetup);

expect(coreSetup.capabilities.registerSwitcher).toHaveBeenCalledTimes(1);
const [switcher] = coreSetup.capabilities.registerSwitcher.mock.calls[0];

const capabilities = {
navLinks: {},
management: {},
catalogue: {},
fileUpload: {
show: true,
},
} as Capabilities;

const request = httpServerMock.createKibanaRequest();

await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(`
Object {
"catalogue": Object {},
"fileUpload": Object {
"show": true,
},
"management": Object {},
"navLinks": Object {},
}
`);

expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledTimes(1);
expect(security.authz.checkPrivilegesDynamicallyWithRequest).toHaveBeenCalledTimes(1);
});

it('registers a capabilities switcher that skips privilege check for anonymous routes', async () => {
const coreSetup = coreMock.createSetup();
const security = securityMock.createStart();
security.authz.mode.useRbacForRequest.mockReturnValue(true);
coreSetup.getStartServices.mockResolvedValue([
(undefined as unknown) as CoreStart,
{ security },
undefined,
]);
setupCapabilities(coreSetup);

expect(coreSetup.capabilities.registerSwitcher).toHaveBeenCalledTimes(1);
const [switcher] = coreSetup.capabilities.registerSwitcher.mock.calls[0];

const capabilities = {
navLinks: {},
management: {},
catalogue: {},
fileUpload: {
show: true,
},
} as Capabilities;

const request = httpServerMock.createKibanaRequest({ routeAuthRequired: false });

await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(`
Object {
"catalogue": Object {},
"fileUpload": Object {
"show": true,
},
"management": Object {},
"navLinks": Object {},
}
`);

expect(security.authz.mode.useRbacForRequest).not.toHaveBeenCalled();
expect(security.authz.checkPrivilegesDynamicallyWithRequest).not.toHaveBeenCalled();
});

it('registers a capabilities switcher that skips privilege check for requests not using rbac', async () => {
const coreSetup = coreMock.createSetup();
const security = securityMock.createStart();
security.authz.mode.useRbacForRequest.mockReturnValue(false);
coreSetup.getStartServices.mockResolvedValue([
(undefined as unknown) as CoreStart,
{ security },
undefined,
]);
setupCapabilities(coreSetup);

expect(coreSetup.capabilities.registerSwitcher).toHaveBeenCalledTimes(1);
const [switcher] = coreSetup.capabilities.registerSwitcher.mock.calls[0];

const capabilities = {
navLinks: {},
management: {},
catalogue: {},
fileUpload: {
show: true,
},
} as Capabilities;

const request = httpServerMock.createKibanaRequest();

await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(`
Object {
"catalogue": Object {},
"fileUpload": Object {
"show": true,
},
"management": Object {},
"navLinks": Object {},
}
`);

expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledTimes(1);
expect(security.authz.checkPrivilegesDynamicallyWithRequest).not.toHaveBeenCalled();
});
});
48 changes: 48 additions & 0 deletions x-pack/plugins/file_upload/server/capabilities.ts
@@ -0,0 +1,48 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { CoreSetup } from 'kibana/server';
import { checkFileUploadPrivileges } from './check_privileges';
import { StartDeps } from './types';

export const setupCapabilities = (
core: Pick<CoreSetup<StartDeps, unknown>, 'capabilities' | 'getStartServices'>
legrego marked this conversation as resolved.
Show resolved Hide resolved
) => {
core.capabilities.registerProvider(() => {
return {
fileUpload: {
show: true,
},
};
});

core.capabilities.registerSwitcher(async (request, capabilities, useDefaultCapabilities) => {
const isAnonymousRequest = !request.route.options.authRequired;
Copy link
Member

Choose a reason for hiding this comment

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

question: is there any reason why we cannot use request.auth.isAuthenticated (or request.route.options.authRequired !== true && !request.auth.isAuthenticated) here instead? If authRequired === 'optional' and request is not authenticated, it seems we can treat it as a legitimate anonymous request as well.

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 wish I had a better memory -- there used to be a reason we couldn't use request.auth.isAuthenticated with the other capability switchers, but I can't remember the details. That said, it looks like the security switcher is using this very check, so maybe it's safe to do so again.

I'll experiment with this and report back!

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 modeled this switcher after the security plugin's switcher, and it appears to be working correctly!

if (useDefaultCapabilities || isAnonymousRequest) {
return capabilities;
}
const [, { security }] = await core.getStartServices();

// Check the bare minimum set of privileges required to get some utility out of this feature
const { hasImportPermission } = await checkFileUploadPrivileges({
authorization: security?.authz,
request,
checkCreateIndexPattern: true,
checkHasManagePipeline: false,
});
Comment on lines +30 to +35
Copy link
Member Author

Choose a reason for hiding this comment

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

We have the flexibility to change these privilege checks to whatever we need...I just picked something to start the conversation. Should we also test that the user can create pipelines?


if (!hasImportPermission) {
return {
fileUpload: {
show: false,
},
};
}

return capabilities;
});
};
51 changes: 51 additions & 0 deletions x-pack/plugins/file_upload/server/check_privileges.ts
@@ -0,0 +1,51 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { KibanaRequest } from 'kibana/server';
import { AuthorizationServiceSetup, CheckPrivilegesPayload } from '../../security/server';

interface Deps {
request: KibanaRequest;
authorization?: Pick<
AuthorizationServiceSetup,
'mode' | 'actions' | 'checkPrivilegesDynamicallyWithRequest'
>;
checkHasManagePipeline: boolean;
checkCreateIndexPattern: boolean;
indexName?: string;
}

export const checkFileUploadPrivileges = async ({
request,
authorization,
checkHasManagePipeline,
checkCreateIndexPattern,
indexName,
}: Deps) => {
const requiresAuthz = authorization?.mode.useRbacForRequest(request) ?? false;

if (!authorization || !requiresAuthz) {
return { hasImportPermission: true };
}

const checkPrivilegesPayload: CheckPrivilegesPayload = {
elasticsearch: {
cluster: checkHasManagePipeline ? ['manage_pipeline'] : [],
index: indexName ? { [indexName]: ['create', 'create_index'] } : {},
},
};
if (checkCreateIndexPattern) {
checkPrivilegesPayload.kibana = [
authorization.actions.savedObject.get('index-pattern', 'create'),
];
}

const checkPrivileges = authorization.checkPrivilegesDynamicallyWithRequest(request);
const checkPrivilegesResp = await checkPrivileges(checkPrivilegesPayload);

return { hasImportPermission: checkPrivilegesResp.hasAllRequested };
};
3 changes: 3 additions & 0 deletions x-pack/plugins/file_upload/server/plugin.ts
Expand Up @@ -13,6 +13,7 @@ import { initFileUploadTelemetry } from './telemetry';
import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/server';
import { UI_SETTING_MAX_FILE_SIZE, MAX_FILE_SIZE } from '../common';
import { StartDeps } from './types';
import { setupCapabilities } from './capabilities';

interface SetupDeps {
usageCollection: UsageCollectionSetup;
Expand All @@ -28,6 +29,8 @@ export class FileUploadPlugin implements Plugin {
async setup(coreSetup: CoreSetup<StartDeps, unknown>, plugins: SetupDeps) {
fileUploadRoutes(coreSetup, this._logger);

setupCapabilities(coreSetup);

coreSetup.uiSettings.register({
[UI_SETTING_MAX_FILE_SIZE]: {
name: i18n.translate('xpack.fileUpload.maxFileSizeUiSetting.name', {
Expand Down