github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 17:28:26 -04:00
Code Issues Projects Releases Packages Wiki Activity

Restrict ability to disable features within a Space (#160416)

Browse source 
Resolves https://github.com/elastic/kibana/issues/159392

## Summary

This PR hides the feature visibility section on the space management
screen and disables adding `disabledFeatures` when creating or updating
spaces using the REST API or spaces client on serverless.

## Screenshot

![Spaces-Elastic
(2)](14d4900b-989d-420c-bddf-5ff70d305934)

## Testing

1. Start Kibana in serverless mode: `yarn start --serverless`
2. Edit default space and observe that the feature visibility section is
not rendered
3. Quit Kibana and restart using classic mode: `yarn start`
4. Edit default space and observe that the feature visibility section is
rendered correctly
5. Other considerations:
- Disabling feature visibility in the classic offering should throw an
error (`xpack.spaces.allowFeatureVisibility: false`)
- Enabling feature visibility on serverless should throw an error
(`xpack.spaces.allowFeatureVisibility: true`)
This commit is contained in:
Thom Heymann 2023-07-03 21:02:31 +01:00 • committed by GitHub
parent 04f8885727
commit 6b02be4488
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 296 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

3
config/serverless.yml
View file

@ -53,8 +53,9 @@ server.versioned.versionResolution: newest
# do not enforce client version check
server.versioned.strictClientVersionCheck: false
# Enforce single "default" space
# Enforce single "default" space and disable feature visibility controls
xpack.spaces.maxSpaces: 1
xpack.spaces.allowFeatureVisibility: false
# Temporarily allow unauthenticated access to task manager utilization & status/stats APIs for autoscaling
status.allowAnonymous: true

1
test/plugin_functional/test_suites/core_plugins/rendering.ts
View file

@ -258,6 +258,7 @@ export default function ({ getService }: PluginFunctionalProviderContext) {
'xpack.security.showNavLinks (boolean)',
'xpack.security.ui (any)',
'xpack.spaces.maxSpaces (number)',
'xpack.spaces.allowFeatureVisibility (any)',
'xpack.securitySolution.enableExperimental (array)',
'xpack.securitySolution.prebuiltRulesPackageVersion (string)',
'xpack.snapshot_restore.slm_ui.enabled (boolean)',

1
x-pack/plugins/spaces/public/config.ts
View file

@ -7,4 +7,5 @@
export interface ConfigType {
maxSpaces: number;
allowFeatureVisibility: boolean;
}

65
x-pack/plugins/spaces/public/management/edit_space/manage_space_page.test.tsx
View file

@ -20,6 +20,7 @@ import { mountWithIntl } from '@kbn/test-jest-helpers';
import type { SpacesManager } from '../../spaces_manager';
import { spacesManagerMock } from '../../spaces_manager/mocks';
import { ConfirmAlterActiveSpaceModal } from './confirm_alter_active_space_modal';
import { EnabledFeatures } from './enabled_features';
import { ManageSpacePage } from './manage_space_page';
// To be resolved by EUI team.
@ -74,6 +75,7 @@ describe('ManageSpacePage', () => {
catalogue: {},
spaces: { manage: true },
}}
allowFeatureVisibility
/>
);
@ -103,6 +105,64 @@ describe('ManageSpacePage', () => {
});
});
it('shows feature visibility controls when allowed', async () => {
const spacesManager = spacesManagerMock.create();
spacesManager.createSpace = jest.fn(spacesManager.createSpace);
spacesManager.getActiveSpace = jest.fn().mockResolvedValue(space);
const wrapper = mountWithIntl(
<ManageSpacePage
spacesManager={spacesManager as unknown as SpacesManager}
getFeatures={featuresStart.getFeatures}
notifications={notificationServiceMock.createStartContract()}
history={history}
capabilities={{
navLinks: {},
management: {},
catalogue: {},
spaces: { manage: true },
}}
allowFeatureVisibility
/>
);
await waitFor(() => {
wrapper.update();
expect(wrapper.find('input[name="name"]')).toHaveLength(1);
});
expect(wrapper.find(EnabledFeatures)).toHaveLength(1);
});
it('hides feature visibility controls when not allowed', async () => {
const spacesManager = spacesManagerMock.create();
spacesManager.createSpace = jest.fn(spacesManager.createSpace);
spacesManager.getActiveSpace = jest.fn().mockResolvedValue(space);
const wrapper = mountWithIntl(
<ManageSpacePage
spacesManager={spacesManager as unknown as SpacesManager}
getFeatures={featuresStart.getFeatures}
notifications={notificationServiceMock.createStartContract()}
history={history}
capabilities={{
navLinks: {},
management: {},
catalogue: {},
spaces: { manage: true },
}}
allowFeatureVisibility={false}
/>
);
await waitFor(() => {
wrapper.update();
expect(wrapper.find('input[name="name"]')).toHaveLength(1);
});
expect(wrapper.find(EnabledFeatures)).toHaveLength(0);
});
it('allows a space to be updated', async () => {
const spaceToUpdate = {
id: 'existing-space',
@ -135,6 +195,7 @@ describe('ManageSpacePage', () => {
catalogue: {},
spaces: { manage: true },
}}
allowFeatureVisibility
/>
);
@ -202,6 +263,7 @@ describe('ManageSpacePage', () => {
catalogue: {},
spaces: { manage: true },
}}
allowFeatureVisibility
/>
);
@ -250,6 +312,7 @@ describe('ManageSpacePage', () => {
catalogue: {},
spaces: { manage: true },
}}
allowFeatureVisibility
/>
);
@ -286,6 +349,7 @@ describe('ManageSpacePage', () => {
catalogue: {},
spaces: { manage: true },
}}
allowFeatureVisibility
/>
);
@ -346,6 +410,7 @@ describe('ManageSpacePage', () => {
catalogue: {},
spaces: { manage: true },
}}
allowFeatureVisibility
/>
);

18
x-pack/plugins/spaces/public/management/edit_space/manage_space_page.tsx
View file

@ -54,6 +54,7 @@ interface Props {
onLoadSpace?: (space: Space) => void;
capabilities: Capabilities;
history: ScopedHistory;
allowFeatureVisibility: boolean;
}
interface State {
@ -161,13 +162,16 @@ export class ManageSpacePage extends Component<Props, State> {
validator={this.validator}
/>
<EuiSpacer />
<EnabledFeatures
space={this.state.space}
features={this.state.features}
onChange={this.onSpaceChange}
/>
{this.props.allowFeatureVisibility && (
<>
<EuiSpacer />
<EnabledFeatures
space={this.state.space}
features={this.state.features}
onChange={this.onSpaceChange}
/>
</>
)}
<EuiSpacer />

1
x-pack/plugins/spaces/public/management/management_service.test.ts
View file

@ -18,6 +18,7 @@ import { ManagementService } from './management_service';
describe('ManagementService', () => {
const config: ConfigType = {
maxSpaces: 1000,
allowFeatureVisibility: true,
};
describe('#setup', () => {

5
x-pack/plugins/spaces/public/management/spaces_management_app.test.tsx
View file

@ -28,6 +28,7 @@ import { spacesManagementApp } from './spaces_management_app';
const config: ConfigType = {
maxSpaces: 1000,
allowFeatureVisibility: true,
};
async function mountApp(basePath: string, pathname: string, spaceId?: string) {
@ -119,7 +120,7 @@ describe('spacesManagementApp', () => {
<div
css="You have tried to stringify object returned from \`css\` function. It isn't supposed to be used directly (e.g. as value of the \`className\` prop), but rather handed to emotion so it can handle it (e.g. as value of \`css\` prop)."
>
Spaces Edit Page: {"capabilities":{"catalogue":{},"management":{},"navLinks":{}},"notifications":{"toasts":{}},"spacesManager":{"onActiveSpaceChange$":{}},"history":{"action":"PUSH","length":1,"location":{"pathname":"/create","search":"","hash":""}}}
Spaces Edit Page: {"capabilities":{"catalogue":{},"management":{},"navLinks":{}},"notifications":{"toasts":{}},"spacesManager":{"onActiveSpaceChange$":{}},"history":{"action":"PUSH","length":1,"location":{"pathname":"/create","search":"","hash":""}},"allowFeatureVisibility":true}
</div>
</div>
`);
@ -151,7 +152,7 @@ describe('spacesManagementApp', () => {
<div
css="You have tried to stringify object returned from \`css\` function. It isn't supposed to be used directly (e.g. as value of the \`className\` prop), but rather handed to emotion so it can handle it (e.g. as value of \`css\` prop)."
>
Spaces Edit Page: {"capabilities":{"catalogue":{},"management":{},"navLinks":{}},"notifications":{"toasts":{}},"spacesManager":{"onActiveSpaceChange$":{}},"spaceId":"some-space","history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/some-space","search":"","hash":""}}}
Spaces Edit Page: {"capabilities":{"catalogue":{},"management":{},"navLinks":{}},"notifications":{"toasts":{}},"spacesManager":{"onActiveSpaceChange$":{}},"spaceId":"some-space","history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/some-space","search":"","hash":""}},"allowFeatureVisibility":true}
</div>
</div>
`);

2
x-pack/plugins/spaces/public/management/spaces_management_app.tsx
View file

@ -83,6 +83,7 @@ export const spacesManagementApp = Object.freeze({
notifications={notifications}
spacesManager={spacesManager}
history={history}
allowFeatureVisibility={config.allowFeatureVisibility}
/>
);
};
@ -108,6 +109,7 @@ export const spacesManagementApp = Object.freeze({
spaceId={spaceId}
onLoadSpace={onLoadSpace}
history={history}
allowFeatureVisibility={config.allowFeatureVisibility}
/>
);
};

23
x-pack/plugins/spaces/server/config.test.ts
View file

@ -20,6 +20,7 @@ describe('config schema', () => {
it('generates proper defaults', () => {
expect(ConfigSchema.validate({})).toMatchInlineSnapshot(`
Object {
"allowFeatureVisibility": true,
"enabled": true,
"maxSpaces": 1000,
}
@ -27,6 +28,7 @@ describe('config schema', () => {
expect(ConfigSchema.validate({}, { dev: false })).toMatchInlineSnapshot(`
Object {
"allowFeatureVisibility": true,
"enabled": true,
"maxSpaces": 1000,
}
@ -34,6 +36,7 @@ describe('config schema', () => {
expect(ConfigSchema.validate({}, { dev: true })).toMatchInlineSnapshot(`
Object {
"allowFeatureVisibility": true,
"enabled": true,
"maxSpaces": 1000,
}
@ -53,4 +56,24 @@ describe('config schema', () => {
it('should not throw error if spaces is disabled in development mode', () => {
expect(() => ConfigSchema.validate({ enabled: false }, { dev: true })).not.toThrow();
});
it('should throw error if allowFeatureVisibility is disabled in classic offering', () => {
expect(() => ConfigSchema.validate({ allowFeatureVisibility: false }, {})).toThrow();
});
it('should not throw error if allowFeatureVisibility is disabled in serverless offering', () => {
expect(() =>
ConfigSchema.validate({ allowFeatureVisibility: false }, { serverless: true })
).not.toThrow();
});
it('should not throw error if allowFeatureVisibility is enabled in classic offering', () => {
expect(() => ConfigSchema.validate({ allowFeatureVisibility: true }, {})).not.toThrow();
});
it('should throw error if allowFeatureVisibility is enabled in serverless offering', () => {
expect(() =>
ConfigSchema.validate({ allowFeatureVisibility: true }, { serverless: true })
).toThrow();
});
});

16
x-pack/plugins/spaces/server/config.ts
View file

@ -26,6 +26,22 @@ export const ConfigSchema = schema.object({
})
),
maxSpaces: schema.number({ defaultValue: 1000 }),
allowFeatureVisibility: schema.conditional(
schema.contextRef('serverless'),
true,
schema.literal(false),
schema.boolean({
validate: (rawValue) => {
// This setting should not be configurable on-prem to avoid bugs when e.g. existing spaces
// have feature visibility customized but admins would be unable to change them back if the
// UI/APIs are disabled.
if (rawValue === false) {
return 'Feature visibility can only be disabled on serverless';
}
},
defaultValue: true,
})
),
});
export function createConfig$(context: PluginInitializerContext) {

1
x-pack/plugins/spaces/server/index.ts
View file

@ -33,6 +33,7 @@ export const config: PluginConfigDescriptor = {
schema: ConfigSchema,
exposeToBrowser: {
maxSpaces: true,
allowFeatureVisibility: true,
},
};

2
x-pack/plugins/spaces/server/routes/api/external/post.test.ts vendored
View file

@ -130,6 +130,7 @@ describe('Spaces Public API', () => {
id: 'a-space',
name: 'my updated space',
description: 'with a description',
disabledFeatures: [],
};
const { routeHandler } = await setup();
@ -152,6 +153,7 @@ describe('Spaces Public API', () => {
id: 'my-space-id',
name: 'my new space',
description: 'with a description',
disabledFeatures: [],
};
const { routeValidation, routeHandler, savedObjectsRepositoryMock } = await setup();

1
x-pack/plugins/spaces/server/routes/api/external/put.test.ts vendored
View file

@ -175,6 +175,7 @@ describe('PUT /api/spaces/space', () => {
id: 'a-new-space',
name: 'my new space',
description: 'with a description',
disabledFeatures: [],
};
const { routeHandler } = await setup();

139
x-pack/plugins/spaces/server/spaces_client/spaces_client.test.ts
View file

@ -17,8 +17,10 @@ const createMockDebugLogger = () => {
return jest.fn();
};
const createMockConfig = (mockConfig: ConfigType = { enabled: true, maxSpaces: 1000 }) => {
return ConfigSchema.validate(mockConfig);
const createMockConfig = (
mockConfig: ConfigType = { enabled: true, maxSpaces: 1000, allowFeatureVisibility: true }
) => {
return ConfigSchema.validate(mockConfig, { serverless: !mockConfig.allowFeatureVisibility });
};
describe('#getAll', () => {
@ -97,7 +99,7 @@ describe('#getAll', () => {
mockCallWithRequestRepository.find.mockResolvedValue({
saved_objects: savedObjects,
} as any);
const mockConfig = createMockConfig({ enabled: true, maxSpaces: 1234 });
const mockConfig = createMockConfig();
const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
const actualSpaces = await client.getAll();
@ -207,7 +209,7 @@ describe('#create', () => {
total: maxSpaces - 1,
} as any);
const mockConfig = createMockConfig({ enabled: true, maxSpaces });
const mockConfig = createMockConfig({ enabled: true, maxSpaces, allowFeatureVisibility: true });
const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
@ -233,7 +235,7 @@ describe('#create', () => {
total: maxSpaces,
} as any);
const mockConfig = createMockConfig({ enabled: true, maxSpaces });
const mockConfig = createMockConfig({ enabled: true, maxSpaces, allowFeatureVisibility: true });
const client = new SpacesClient(mockDebugLogger, mockConfig, mockCallWithRequestRepository, []);
@ -248,6 +250,79 @@ describe('#create', () => {
});
expect(mockCallWithRequestRepository.create).not.toHaveBeenCalled();
});
describe('when config.allowFeatureVisibility is disabled', () => {
test(`creates space without disabledFeatures`, async () => {
const maxSpaces = 5;
const mockDebugLogger = createMockDebugLogger();
const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
mockCallWithRequestRepository.create.mockResolvedValue(savedObject);
mockCallWithRequestRepository.find.mockResolvedValue({
total: maxSpaces - 1,
} as any);
const mockConfig = createMockConfig({
enabled: true,
maxSpaces,
allowFeatureVisibility: false,
});
const client = new SpacesClient(
mockDebugLogger,
mockConfig,
mockCallWithRequestRepository,
[]
);
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,
});
});
test(`throws bad request when creating space with disabledFeatures`, async () => {
const maxSpaces = 5;
const mockDebugLogger = createMockDebugLogger();
const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
mockCallWithRequestRepository.create.mockResolvedValue(savedObject);
mockCallWithRequestRepository.find.mockResolvedValue({
total: maxSpaces - 1,
} as any);
const mockConfig = createMockConfig({
enabled: true,
maxSpaces,
allowFeatureVisibility: false,
});
const client = new SpacesClient(
mockDebugLogger,
mockConfig,
mockCallWithRequestRepository,
[]
);
expect(
client.create({ ...spaceToCreate, disabledFeatures: ['some-feature'] })
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Unable to create Space, the disabledFeatures array must be empty when xpack.spaces.allowFeatureVisibility setting is disabled"`
);
expect(mockCallWithRequestRepository.find).toHaveBeenCalledWith({
type: 'space',
page: 1,
perPage: 0,
});
expect(mockCallWithRequestRepository.create).not.toHaveBeenCalled();
});
});
});
describe('#update', () => {
@ -298,6 +373,60 @@ describe('#update', () => {
expect(mockCallWithRequestRepository.update).toHaveBeenCalledWith('space', id, attributes);
expect(mockCallWithRequestRepository.get).toHaveBeenCalledWith('space', id);
});
describe('when config.allowFeatureVisibility is disabled', () => {
test(`updates space without disabledFeatures`, async () => {
const mockDebugLogger = createMockDebugLogger();
const mockConfig = createMockConfig({
enabled: true,
maxSpaces: 1000,
allowFeatureVisibility: false,
});
const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
mockCallWithRequestRepository.get.mockResolvedValue(savedObject);
const client = new SpacesClient(
mockDebugLogger,
mockConfig,
mockCallWithRequestRepository,
[]
);
const id = savedObject.id;
const actualSpace = await client.update(id, spaceToUpdate);
expect(actualSpace).toEqual(expectedReturnedSpace);
expect(mockCallWithRequestRepository.update).toHaveBeenCalledWith('space', id, attributes);
expect(mockCallWithRequestRepository.get).toHaveBeenCalledWith('space', id);
});
test(`throws bad request when updating space with disabledFeatures`, async () => {
const mockDebugLogger = createMockDebugLogger();
const mockConfig = createMockConfig({
enabled: true,
maxSpaces: 1000,
allowFeatureVisibility: false,
});
const mockCallWithRequestRepository = savedObjectsRepositoryMock.create();
mockCallWithRequestRepository.get.mockResolvedValue(savedObject);
const client = new SpacesClient(
mockDebugLogger,
mockConfig,
mockCallWithRequestRepository,
[]
);
const id = savedObject.id;
expect(
client.update(id, { ...spaceToUpdate, disabledFeatures: ['some-feature'] })
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Unable to update Space, the disabledFeatures array must be empty when xpack.spaces.allowFeatureVisibility setting is disabled"`
);
expect(mockCallWithRequestRepository.update).not.toHaveBeenCalled();
expect(mockCallWithRequestRepository.get).not.toHaveBeenCalled();
});
});
});
describe('#delete', () => {

12
x-pack/plugins/spaces/server/spaces_client/spaces_client.ts
View file

@ -124,6 +124,12 @@ export class SpacesClient implements ISpacesClient {
);
}
if (space.disabledFeatures.length > 0 && !this.config.allowFeatureVisibility) {
throw Boom.badRequest(
'Unable to create Space, the disabledFeatures array must be empty when xpack.spaces.allowFeatureVisibility setting is disabled'
);
}
this.debugLogger(`SpacesClient.create(), using RBAC. Attempting to create space`);
const id = space.id;
@ -136,6 +142,12 @@ export class SpacesClient implements ISpacesClient {
}
public async update(id: string, space: v1.Space) {
if (space.disabledFeatures.length > 0 && !this.config.allowFeatureVisibility) {
throw Boom.badRequest(
'Unable to update Space, the disabledFeatures array must be empty when xpack.spaces.allowFeatureVisibility setting is disabled'
);
}
const attributes = this.generateSpaceAttributes(space);
await this.repository.update('space', id, attributes);
const updatedSavedObject = await this.repository.get('space', id);

22
x-pack/test_serverless/api_integration/test_suites/common/spaces.ts
View file

@ -15,7 +15,7 @@ export default function ({ getService }: FtrProviderContext) {
describe('spaces', function () {
it('rejects request to create a space', async () => {
const { body, status } = await supertest
.post(`/api/spaces/space`)
.post('/api/spaces/space')
.set(svlCommonApi.getCommonRequestHeader())
.send({
id: 'custom',
@ -32,5 +32,25 @@ export default function ({ getService }: FtrProviderContext) {
});
expect(status).toBe(400);
});
it('rejects request to update a space with disabledFeatures', async () => {
const { body, status } = await supertest
.put('/api/spaces/space/default')
.set(svlCommonApi.getCommonRequestHeader())
.send({
id: 'custom',
name: 'Custom',
disabledFeatures: ['some-feature'],
});
// in a non-serverless environment this would succeed with a 200
expect(body).toEqual({
statusCode: 400,
error: 'Bad Request',
message:
'Unable to update Space, the disabledFeatures array must be empty when xpack.spaces.allowFeatureVisibility setting is disabled',
});
expect(status).toBe(400);
});
});
}