Limiting maximum number of Spaces (#23673) (#23774)

* Limiting the number of spaces

* Adding docs

* Adding forgotten fixture

* Fixing tslint error

* Adjusting docs

* Changing test descriptions from Boom.badRequest to bad request

* Updating error snapshots
This commit is contained in:
Brandon Kobel 2018-10-03 12:43:29 -07:00 committed by GitHub
parent 8a442ac4a7
commit d543e3f713
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 293 additions and 43 deletions

View file

@ -14,3 +14,4 @@ include::dev-settings.asciidoc[]
include::graph-settings.asciidoc[]
include::ml-settings.asciidoc[]
include::reporting-settings.asciidoc[]
include::spaces-settings.asciidoc[]

View file

@ -0,0 +1,22 @@
[role="xpack"]
[[spaces-settings-kb]]
=== Spaces settings in {kib}
++++
<titleabbrev>Spaces settings</titleabbrev>
++++
By default, Spaces is enabled in Kibana, and you can secure Spaces using
roles when Security is enabled.
[float]
[[spaces-settings]]
==== Spaces settings
`xpack.spaces.enabled`::
Set to `true` (default) to enable Spaces in {kib}.
`xpack.spaces.maxSpaces`::
The maximum amount of Spaces that can be used with this instance of Kibana. Some operations
in Kibana return all spaces using a single `_search` from Elasticsearch, so this must be
set lower than the `index.max_result_window` in Elasticsearch.
Defaults to `1000`.

View file

@ -36,6 +36,7 @@ export const spaces = (kibana: any) =>
config(Joi: any) {
return Joi.object({
enabled: Joi.boolean().default(true),
maxSpaces: Joi.number().default(1000),
}).default();
},
@ -123,6 +124,7 @@ export const spaces = (kibana: any) =>
spacesAuditLogger,
authorization,
callWithRequestRepository,
server.config(),
internalRepository,
request
);

View file

@ -1,15 +1,21 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`#create authorization is null throws bad request when we are at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`;
exports[`#create authorization.mode.useRbacForRequest returns false throws bad request when we're at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`;
exports[`#create useRbacForRequest is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to create spaces"`;
exports[`#delete authorization is null throws Boom.badRequest when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
exports[`#create useRbacForRequest is true throws bad request when we are at the maximum number of spaces 1`] = `"Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting"`;
exports[`#delete authorization.mode.useRbacForRequest returns false throws Boom.badRequest when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
exports[`#delete authorization is null throws bad request when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
exports[`#delete authorization.mode.useRbacForRequest returns true throws Boom.badRequest if the user is authorized but the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
exports[`#delete authorization.mode.useRbacForRequest returns false throws bad request when the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
exports[`#delete authorization.mode.useRbacForRequest returns true throws Boom.forbidden if the user isn't authorized 1`] = `"Unauthorized to delete spaces"`;
exports[`#delete authorization.mode.useRbacForRequest returns true throws bad request if the user is authorized but the space is reserved 1`] = `"This Space cannot be deleted because it is reserved."`;
exports[`#get useRbacForRequest is true throws Boom.forbidden if the user isn't authorized at space 1`] = `"Unauthorized to get foo-space space"`;
exports[`#getAll useRbacForRequest is true throws Boom.forbidden when user isn't authorized for any spaces 1`] = `"Forbidden"`;

View file

@ -41,6 +41,18 @@ const createMockAuthorization = () => {
};
};
const createMockConfig = (settings: { [key: string]: any } = {}) => {
const mockConfig = {
get: jest.fn(),
};
mockConfig.get.mockImplementation(key => {
return settings[key];
});
return mockConfig;
};
describe('#getAll', () => {
const savedObjects = [
{
@ -87,11 +99,16 @@ describe('#getAll', () => {
saved_objects: savedObjects,
});
const request = Symbol();
const maxSpaces = 1234;
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': 1234,
});
const client = new SpacesClient(
mockAuditLogger as any,
authorization,
mockCallWithRequestRepository,
mockConfig,
null,
request
);
@ -101,7 +118,7 @@ describe('#getAll', () => {
expect(mockCallWithRequestRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 1000,
perPage: maxSpaces,
sortField: 'name.keyword',
});
expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0);
@ -119,12 +136,17 @@ describe('#getAll', () => {
saved_objects: savedObjects,
}),
};
const maxSpaces = 1234;
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': 1234,
});
const request = Symbol();
const client = new SpacesClient(
mockAuditLogger as any,
mockAuthorization,
mockCallWithRequestRepository,
mockConfig,
null,
request
);
@ -134,7 +156,7 @@ describe('#getAll', () => {
expect(mockCallWithRequestRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 1000,
perPage: maxSpaces,
sortField: 'name.keyword',
});
expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request);
@ -160,6 +182,10 @@ describe('#getAll', () => {
},
},
});
const maxSpaces = 1234;
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': 1234,
});
const mockInternalRepository = {
find: jest.fn().mockReturnValue({
saved_objects: savedObjects,
@ -171,6 +197,7 @@ describe('#getAll', () => {
mockAuditLogger as any,
mockAuthorization,
null,
mockConfig,
mockInternalRepository,
request
);
@ -179,7 +206,7 @@ describe('#getAll', () => {
expect(mockInternalRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 1000,
perPage: maxSpaces,
sortField: 'name.keyword',
});
expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request);
@ -213,12 +240,17 @@ describe('#getAll', () => {
saved_objects: savedObjects,
}),
};
const maxSpaces = 1234;
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': 1234,
});
const request = Symbol();
const client = new SpacesClient(
mockAuditLogger as any,
mockAuthorization,
null,
mockConfig,
mockInternalRepository,
request
);
@ -228,7 +260,7 @@ describe('#getAll', () => {
expect(mockInternalRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 1000,
perPage: maxSpaces,
sortField: 'name.keyword',
});
expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request);
@ -252,7 +284,14 @@ describe('#canEnumerateSpaces', () => {
const authorization = null;
const request = Symbol();
const client = new SpacesClient(mockAuditLogger as any, authorization, null, null, request);
const client = new SpacesClient(
mockAuditLogger as any,
authorization,
null,
null,
null,
request
);
const canEnumerateSpaces = await client.canEnumerateSpaces();
expect(canEnumerateSpaces).toEqual(true);
@ -273,6 +312,7 @@ describe('#canEnumerateSpaces', () => {
mockAuthorization,
null,
null,
null,
request
);
const canEnumerateSpaces = await client.canEnumerateSpaces();
@ -300,6 +340,7 @@ describe('#canEnumerateSpaces', () => {
mockAuthorization,
null,
null,
null,
request
);
@ -331,6 +372,7 @@ describe('#canEnumerateSpaces', () => {
mockAuthorization,
null,
null,
null,
request
);
@ -379,6 +421,7 @@ describe('#get', () => {
authorization,
mockCallWithRequestRepository,
null,
null,
request
);
const id = savedObject.id;
@ -406,6 +449,7 @@ describe('#get', () => {
mockAuthorization,
mockCallWithRequestRepository,
null,
null,
request
);
const id = savedObject.id;
@ -436,6 +480,7 @@ describe('#get', () => {
mockAuthorization,
null,
null,
null,
request
);
const id = 'foo-space';
@ -468,6 +513,7 @@ describe('#get', () => {
mockAuditLogger as any,
mockAuthorization,
null,
null,
mockInternalRepository,
request
);
@ -521,18 +567,26 @@ describe('#create', () => {
};
describe(`authorization is null`, () => {
test(`creates space using callWithRequestRepository`, async () => {
test(`creates space using callWithRequestRepository when we're under the max`, async () => {
const maxSpaces = 5;
const mockAuditLogger = createMockAuditLogger();
const authorization = null;
const mockCallWithRequestRepository = {
create: jest.fn().mockReturnValue(savedObject),
find: jest.fn().mockReturnValue({
total: maxSpaces - 1,
}),
};
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': maxSpaces,
});
const request = Symbol();
const client = new SpacesClient(
mockAuditLogger as any,
authorization,
mockCallWithRequestRepository,
mockConfig,
null,
request
);
@ -540,28 +594,77 @@ describe('#create', () => {
const actualSpace = await client.create(spaceToCreate);
expect(actualSpace).toEqual(expectedReturnedSpace);
expect(mockCallWithRequestRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 0,
});
expect(mockCallWithRequestRepository.create).toHaveBeenCalledWith('space', attributes, {
id,
});
expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0);
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0);
});
test(`throws bad request when we are at the maximum number of spaces`, async () => {
const maxSpaces = 5;
const mockAuditLogger = createMockAuditLogger();
const authorization = null;
const mockCallWithRequestRepository = {
create: jest.fn().mockReturnValue(savedObject),
find: jest.fn().mockReturnValue({
total: maxSpaces,
}),
};
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': maxSpaces,
});
const request = Symbol();
const client = new SpacesClient(
mockAuditLogger as any,
authorization,
mockCallWithRequestRepository,
mockConfig,
null,
request
);
await expect(client.create(spaceToCreate)).rejects.toThrowErrorMatchingSnapshot();
expect(mockCallWithRequestRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 0,
});
expect(mockCallWithRequestRepository.create).not.toHaveBeenCalled();
expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0);
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0);
});
});
describe(`authorization.mode.useRbacForRequest returns false`, () => {
test(`creates space using callWithRequestRepository`, async () => {
test(`creates space using callWithRequestRepository when we're under the max`, async () => {
const maxSpaces = 5;
const mockAuditLogger = createMockAuditLogger();
const { mockAuthorization } = createMockAuthorization();
mockAuthorization.mode.useRbacForRequest.mockReturnValue(false);
const mockCallWithRequestRepository = {
create: jest.fn().mockReturnValue(savedObject),
find: jest.fn().mockReturnValue({
total: maxSpaces - 1,
}),
};
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': maxSpaces,
});
const request = Symbol();
const client = new SpacesClient(
mockAuditLogger as any,
mockAuthorization,
mockCallWithRequestRepository,
mockConfig,
null,
request
);
@ -570,12 +673,55 @@ describe('#create', () => {
expect(actualSpace).toEqual(expectedReturnedSpace);
expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request);
expect(mockCallWithRequestRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 0,
});
expect(mockCallWithRequestRepository.create).toHaveBeenCalledWith('space', attributes, {
id,
});
expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0);
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0);
});
test(`throws bad request when we're at the maximum number of spaces`, async () => {
const maxSpaces = 5;
const mockAuditLogger = createMockAuditLogger();
const { mockAuthorization } = createMockAuthorization();
mockAuthorization.mode.useRbacForRequest.mockReturnValue(false);
const mockCallWithRequestRepository = {
create: jest.fn().mockReturnValue(savedObject),
find: jest.fn().mockReturnValue({
total: maxSpaces,
}),
};
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': maxSpaces,
});
const request = Symbol();
const client = new SpacesClient(
mockAuditLogger as any,
mockAuthorization,
mockCallWithRequestRepository,
mockConfig,
null,
request
);
await expect(client.create(spaceToCreate)).rejects.toThrowErrorMatchingSnapshot();
expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request);
expect(mockCallWithRequestRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 0,
});
expect(mockCallWithRequestRepository.create).not.toHaveBeenCalled();
expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0);
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0);
});
});
describe('useRbacForRequest is true', () => {
@ -595,6 +741,7 @@ describe('#create', () => {
mockAuthorization,
null,
null,
null,
request
);
@ -609,8 +756,9 @@ describe('#create', () => {
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0);
});
test(`creates space using internalRepository if the user is authorized`, async () => {
test(`creates space using internalRepository if the user is authorized and we're under the max`, async () => {
const username = Symbol();
const maxSpaces = 5;
const mockAuditLogger = createMockAuditLogger();
const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization();
mockAuthorization.mode.useRbacForRequest.mockReturnValue(true);
@ -620,13 +768,20 @@ describe('#create', () => {
});
const mockInternalRepository = {
create: jest.fn().mockReturnValue(savedObject),
find: jest.fn().mockReturnValue({
total: maxSpaces - 1,
}),
};
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': maxSpaces,
});
const request = Symbol();
const client = new SpacesClient(
mockAuditLogger as any,
mockAuthorization,
null,
mockConfig,
mockInternalRepository,
request
);
@ -634,6 +789,61 @@ describe('#create', () => {
const actualSpace = await client.create(spaceToCreate);
expect(actualSpace).toEqual(expectedReturnedSpace);
expect(mockInternalRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 0,
});
expect(mockInternalRepository.create).toHaveBeenCalledWith('space', attributes, {
id,
});
expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request);
expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request);
expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith(
mockAuthorization.actions.manageSpaces
);
expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0);
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledWith(username, 'create');
});
test(`throws bad request when we are at the maximum number of spaces`, async () => {
const username = Symbol();
const maxSpaces = 5;
const mockAuditLogger = createMockAuditLogger();
const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization();
mockAuthorization.mode.useRbacForRequest.mockReturnValue(true);
mockCheckPrivilegesGlobally.mockReturnValue({
username,
hasAllRequested: true,
});
const mockInternalRepository = {
create: jest.fn().mockReturnValue(savedObject),
find: jest.fn().mockReturnValue({
total: maxSpaces,
}),
};
const mockConfig = createMockConfig({
'xpack.spaces.maxSpaces': maxSpaces,
});
const request = Symbol();
const client = new SpacesClient(
mockAuditLogger as any,
mockAuthorization,
null,
mockConfig,
mockInternalRepository,
request
);
await expect(client.create(spaceToCreate)).rejects.toThrowErrorMatchingSnapshot();
expect(mockInternalRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 0,
});
expect(mockInternalRepository.create).not.toHaveBeenCalled();
expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request);
expect(mockAuthorization.checkPrivilegesWithRequest).toHaveBeenCalledWith(request);
expect(mockCheckPrivilegesGlobally).toHaveBeenCalledWith(
@ -693,6 +903,7 @@ describe('#update', () => {
authorization,
mockCallWithRequestRepository,
null,
null,
request
);
const id = savedObject.id;
@ -721,6 +932,7 @@ describe('#update', () => {
mockAuthorization,
mockCallWithRequestRepository,
null,
null,
request
);
const id = savedObject.id;
@ -752,6 +964,7 @@ describe('#update', () => {
mockAuthorization,
null,
null,
null,
request
);
const id = savedObject.id;
@ -785,6 +998,7 @@ describe('#update', () => {
mockAuditLogger as any,
mockAuthorization,
null,
null,
mockInternalRepository,
request
);
@ -828,7 +1042,7 @@ describe('#delete', () => {
};
describe(`authorization is null`, () => {
test(`throws Boom.badRequest when the space is reserved`, async () => {
test(`throws bad request when the space is reserved`, async () => {
const mockAuditLogger = createMockAuditLogger();
const authorization = null;
const mockCallWithRequestRepository = {
@ -841,6 +1055,7 @@ describe('#delete', () => {
authorization,
mockCallWithRequestRepository,
null,
null,
request
);
@ -865,6 +1080,7 @@ describe('#delete', () => {
authorization,
mockCallWithRequestRepository,
null,
null,
request
);
@ -878,7 +1094,7 @@ describe('#delete', () => {
});
describe(`authorization.mode.useRbacForRequest returns false`, () => {
test(`throws Boom.badRequest when the space is reserved`, async () => {
test(`throws bad request when the space is reserved`, async () => {
const mockAuditLogger = createMockAuditLogger();
const { mockAuthorization } = createMockAuthorization();
mockAuthorization.mode.useRbacForRequest.mockReturnValue(false);
@ -892,6 +1108,7 @@ describe('#delete', () => {
mockAuthorization,
mockCallWithRequestRepository,
null,
null,
request
);
@ -918,6 +1135,7 @@ describe('#delete', () => {
mockAuthorization,
mockCallWithRequestRepository,
null,
null,
request
);
@ -947,6 +1165,7 @@ describe('#delete', () => {
mockAuthorization,
null,
null,
null,
request
);
@ -961,7 +1180,7 @@ describe('#delete', () => {
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0);
});
test(`throws Boom.badRequest if the user is authorized but the space is reserved`, async () => {
test(`throws bad request if the user is authorized but the space is reserved`, async () => {
const username = Symbol();
const mockAuditLogger = createMockAuditLogger();
const { mockAuthorization, mockCheckPrivilegesGlobally } = createMockAuthorization();
@ -978,6 +1197,7 @@ describe('#delete', () => {
mockAuditLogger as any,
mockAuthorization,
null,
null,
mockInternalRepository,
request
);
@ -1012,6 +1232,7 @@ describe('#delete', () => {
mockAuditLogger as any,
mockAuthorization,
null,
null,
mockInternalRepository,
request
);

View file

@ -10,25 +10,14 @@ import { Space } from '../../common/model/space';
import { SpacesAuditLogger } from './audit_logger';
export class SpacesClient {
private readonly auditLogger: SpacesAuditLogger;
private readonly authorization: any;
private readonly callWithRequestSavedObjectRepository: any;
private readonly internalSavedObjectRepository: any;
private readonly request: any;
constructor(
auditLogger: SpacesAuditLogger,
authorization: any,
callWithRequestSavedObjectRepository: any,
internalSavedObjectRepository: any,
request: any
) {
this.auditLogger = auditLogger;
this.authorization = authorization;
this.callWithRequestSavedObjectRepository = callWithRequestSavedObjectRepository;
this.internalSavedObjectRepository = internalSavedObjectRepository;
this.request = request;
}
private readonly auditLogger: SpacesAuditLogger,
private readonly authorization: any,
private readonly callWithRequestSavedObjectRepository: any,
private readonly config: any,
private readonly internalSavedObjectRepository: any,
private readonly request: any
) {}
public async canEnumerateSpaces(): Promise<boolean> {
if (this.useRbac()) {
@ -48,7 +37,7 @@ export class SpacesClient {
const { saved_objects } = await this.internalSavedObjectRepository.find({
type: 'space',
page: 1,
perPage: 1000,
perPage: this.config.get('xpack.spaces.maxSpaces'),
sortField: 'name.keyword',
});
@ -76,7 +65,7 @@ export class SpacesClient {
const { saved_objects } = await this.callWithRequestSavedObjectRepository.find({
type: 'space',
page: 1,
perPage: 1000,
perPage: this.config.get('xpack.spaces.maxSpaces'),
sortField: 'name.keyword',
});
@ -113,6 +102,17 @@ export class SpacesClient {
? this.internalSavedObjectRepository
: this.callWithRequestSavedObjectRepository;
const { total } = await repository.find({
type: 'space',
page: 1,
perPage: 0,
});
if (total >= this.config.get('xpack.spaces.maxSpaces')) {
throw Boom.badRequest(
'Unable to create Space, this exceeds the maximum number of spaces set by the xpack.spaces.maxSpaces setting'
);
}
const attributes = omit(space, ['id', '_reserved']);
const id = space.id;
const createdSavedObject = await repository.create('space', attributes, { id });

View file

@ -39,6 +39,7 @@ export const defaultPreCheckLicenseImpl = (request: any, reply: any) => reply();
const baseConfig: TestConfig = {
'server.basePath': '',
'xpack.spaces.maxSpaces': 1000,
};
export function createTestHandler(initApiFn: (server: any, preCheckLicenseImpl: any) => void) {
@ -77,15 +78,11 @@ export function createTestHandler(initApiFn: (server: any, preCheckLicenseImpl:
await setupFn(server);
server.decorate(
'server',
'config',
jest.fn(() => {
return {
get: (key: string) => config[key],
};
})
);
const mockConfig = {
get: (key: string) => config[key],
};
server.decorate('server', 'config', jest.fn(() => mockConfig));
initApiFn(server, pre);
@ -139,6 +136,7 @@ export function createTestHandler(initApiFn: (server: any, preCheckLicenseImpl:
null as any,
null,
mockSavedObjectsRepository,
mockConfig,
mockSavedObjectsRepository,
req
);