From b9c526a01329770d4b185e7e44d8efd81df9a907 Mon Sep 17 00:00:00 2001 From: Sid Date: Thu, 5 Sep 2024 17:50:18 +0200 Subject: [PATCH] [Serverless] Add role name validation to prevent white spaces in serverless custom roles (#191894) Closes https://github.com/elastic/kibana/issues/191049 ### Summary Enforces strict role name validation in serverless offering. In addition to enforcing no whitespace, we will restrict role names to only contain alphanumeric characters, dots, hyphens, and underscores. Additionally, validation will restrict the leading character to only alphanumeric characters. ### Notes - Added a new regular expression for checking the name to only have letters, numbers and trailing dots, underscores or hyphens. - Added a separate check in the role name validation to check if Kibana is running in serverless mode, and if the name matches the new serverless regular expression. - To prevent any breaking changes, this validation only takes place on Save (as it does in stateful) and not on blur (which is a separate validation to run against any existing role names) Examples - _rolename: invalid - role name: invalid - role<>name: invalid - role_name: valid - 123role_name-123: valid ### Screenshots image --------- Co-authored-by: Elastic Machine --- x-pack/plugins/security/common/constants.ts | 9 +++ .../roles/edit_role/edit_role_page.tsx | 3 +- .../elasticsearch_privileges.test.tsx.snap | 2 + .../index_privileges.test.tsx.snap | 1 + .../remote_cluster_privileges.test.tsx.snap | 1 + .../es/elasticsearch_privileges.test.tsx | 7 +- .../kibana_privileges_region.test.tsx.snap | 1 + .../roles/edit_role/validate_role.test.ts | 79 +++++++++++++++++++ .../roles/edit_role/validate_role.ts | 19 ++++- .../common/platform_security/roles.ts | 2 +- 10 files changed, 119 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/security/common/constants.ts b/x-pack/plugins/security/common/constants.ts index 8958e392c3a61..60644c5c6d21a 100644 --- a/x-pack/plugins/security/common/constants.ts +++ b/x-pack/plugins/security/common/constants.ts @@ -65,6 +65,15 @@ export const SESSION_ERROR_REASON_HEADER = 'kbn-session-error-reason'; export const NAME_REGEX = /^(?! )[a-zA-Z0-9 !"#$%&'()*+,\-./\\:;<=>?@\[\]^_`{|}~]*[a-zA-Z0-9!"#$%&'()*+,\-./\\:;<=>?@\[\]^_`{|}~]$/; +/** + * Matches valid usernames and role names for serverless offering. + * + * - Must contain only alphanumeric characters, and non-leading dots, hyphens, or underscores. + * - Must not contain white spaces characters. + * - Must not have a leading dot, hyphen, or underscore. + */ +export const SERVERLESS_NAME_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; + /** * Maximum length of usernames and role names. */ diff --git a/x-pack/plugins/security/public/management/roles/edit_role/edit_role_page.tsx b/x-pack/plugins/security/public/management/roles/edit_role/edit_role_page.tsx index e867bf0c418cc..63229be3c8683 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/edit_role_page.tsx +++ b/x-pack/plugins/security/public/management/roles/edit_role/edit_role_page.tsx @@ -205,6 +205,7 @@ function useRole( roleName?: string ) { const [role, setRole] = useState(null); + useEffect(() => { const rolePromise = roleName ? rolesAPIClient.getRole(roleName) @@ -350,7 +351,7 @@ export const EditRolePage: FunctionComponent = ({ // We should keep the same mutable instance of Validator for every re-render since we'll // eventually enable validation after the first time user tries to save a role. - const { current: validator } = useRef(new RoleValidator({ shouldValidate: false })); + const { current: validator } = useRef(new RoleValidator({ shouldValidate: false, buildFlavor })); const [formError, setFormError] = useState(null); const [creatingRoleAlreadyExists, setCreatingRoleAlreadyExists] = useState(false); const [previousName, setPreviousName] = useState(''); diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap index 38f88b6763b63..ce6c7d3543aa4 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap +++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap @@ -142,6 +142,7 @@ exports[`it renders correctly in serverless mode 1`] = ` } validator={ RoleValidator { + "buildFlavor": "serverless", "shouldValidate": undefined, } } @@ -337,6 +338,7 @@ exports[`it renders without crashing 1`] = ` } validator={ RoleValidator { + "buildFlavor": undefined, "shouldValidate": undefined, } } diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/index_privileges.test.tsx.snap b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/index_privileges.test.tsx.snap index 705af534bc71d..abb716908eccf 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/index_privileges.test.tsx.snap +++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/index_privileges.test.tsx.snap @@ -46,6 +46,7 @@ exports[`it renders without crashing 1`] = ` } validator={ RoleValidator { + "buildFlavor": undefined, "shouldValidate": undefined, } } diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/remote_cluster_privileges.test.tsx.snap b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/remote_cluster_privileges.test.tsx.snap index 7cc4e67ece4fc..a0fe6456df036 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/remote_cluster_privileges.test.tsx.snap +++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/remote_cluster_privileges.test.tsx.snap @@ -46,6 +46,7 @@ exports[`it renders without crashing 1`] = ` } validator={ RoleValidator { + "buildFlavor": undefined, "shouldValidate": undefined, } } diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/elasticsearch_privileges.test.tsx b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/elasticsearch_privileges.test.tsx index ef914c58522c0..674314a5a5e29 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/elasticsearch_privileges.test.tsx +++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/elasticsearch_privileges.test.tsx @@ -103,7 +103,12 @@ test('it renders correctly in serverless mode', () => { expect( shallowWithIntl( // Enabled remote indices privilege to make sure remote indices is still not rendered due to build flavor - + ) ).toMatchSnapshot(); }); diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/__snapshots__/kibana_privileges_region.test.tsx.snap b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/__snapshots__/kibana_privileges_region.test.tsx.snap index c9b82c53570b4..92cbd0ec7962e 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/__snapshots__/kibana_privileges_region.test.tsx.snap +++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/__snapshots__/kibana_privileges_region.test.tsx.snap @@ -69,6 +69,7 @@ exports[` renders without crashing 1`] = ` } validator={ RoleValidator { + "buildFlavor": undefined, "shouldValidate": undefined, } } diff --git a/x-pack/plugins/security/public/management/roles/edit_role/validate_role.test.ts b/x-pack/plugins/security/public/management/roles/edit_role/validate_role.test.ts index f1acb67fef92f..628a9662d050c 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/validate_role.test.ts +++ b/x-pack/plugins/security/public/management/roles/edit_role/validate_role.test.ts @@ -98,6 +98,85 @@ describe('validateRoleName', () => { }); }); +describe('validateRoleName for serverless', () => { + beforeEach(() => { + validator = new RoleValidator({ shouldValidate: true, buildFlavor: 'serverless' }); + }); + test('should not allow whitespace', () => { + const role = { + name: 'role name', + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: [], + }; + expect(validator.validateRoleName(role)).toEqual({ + isInvalid: true, + error: `Name must contain only alphanumeric characters, and non-leading dots, hyphens, or underscores.`, + }); + }); + test('should not allow leading symbols', () => { + const role = { + name: '.rolename', + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: [], + }; + expect(validator.validateRoleName(role)).toEqual({ + isInvalid: true, + error: `Name must contain only alphanumeric characters, and non-leading dots, hyphens, or underscores.`, + }); + }); + test('should allow underscores contained', () => { + const role = { + name: 'role_name', + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: [], + }; + expect(validator.validateRoleName(role)).toEqual({ + isInvalid: false, + }); + }); + test('should allow valid names', () => { + const role = { + name: 'rolename_', + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: [], + }; + expect(validator.validateRoleName(role)).toEqual({ + isInvalid: false, + }); + }); + test('should not allow any special characters except for underscore, dots and hyphens', () => { + const role = { + name: 'role+name', + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: [], + }; + expect(validator.validateRoleName(role)).toEqual({ + isInvalid: true, + error: `Name must contain only alphanumeric characters, and non-leading dots, hyphens, or underscores.`, + }); + }); +}); + describe('validateIndexPrivileges', () => { beforeEach(() => { validator = new RoleValidator({ shouldValidate: true }); diff --git a/x-pack/plugins/security/public/management/roles/edit_role/validate_role.ts b/x-pack/plugins/security/public/management/roles/edit_role/validate_role.ts index a425578ed98e5..de1e4bbd81c7d 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/validate_role.ts +++ b/x-pack/plugins/security/public/management/roles/edit_role/validate_role.ts @@ -5,6 +5,7 @@ * 2.0. */ +import type { BuildFlavor } from '@kbn/config'; import { i18n } from '@kbn/i18n'; import type { @@ -13,10 +14,11 @@ import type { RoleRemoteClusterPrivilege, RoleRemoteIndexPrivilege, } from '../../../../common'; -import { MAX_NAME_LENGTH, NAME_REGEX } from '../../../../common/constants'; +import { MAX_NAME_LENGTH, NAME_REGEX, SERVERLESS_NAME_REGEX } from '../../../../common/constants'; interface RoleValidatorOptions { shouldValidate?: boolean; + buildFlavor?: BuildFlavor; } export interface RoleValidationResult { @@ -26,9 +28,11 @@ export interface RoleValidationResult { export class RoleValidator { private shouldValidate?: boolean; + private buildFlavor?: BuildFlavor; constructor(options: RoleValidatorOptions = {}) { this.shouldValidate = options.shouldValidate; + this.buildFlavor = options.buildFlavor; } public enableValidation() { @@ -72,7 +76,17 @@ export class RoleValidator { ) ); } - if (!role.name.match(NAME_REGEX)) { + if (this.buildFlavor === 'serverless' && !role.name.match(SERVERLESS_NAME_REGEX)) { + return invalid( + i18n.translate( + 'xpack.security.management.editRole.validateRole.serverlessNameAllowedCharactersWarningMessage', + { + defaultMessage: + 'Name must contain only alphanumeric characters, and non-leading dots, hyphens, or underscores.', + } + ) + ); + } else if (this.buildFlavor !== 'serverless' && !role.name.match(NAME_REGEX)) { return invalid( i18n.translate( 'xpack.security.management.editRole.validateRole.nameAllowedCharactersWarningMessage', @@ -83,6 +97,7 @@ export class RoleValidator { ) ); } + return valid(); } public validateRemoteClusterPrivileges(role: Role): RoleValidationResult { diff --git a/x-pack/test_serverless/functional/test_suites/common/platform_security/roles.ts b/x-pack/test_serverless/functional/test_suites/common/platform_security/roles.ts index 31dbc2a1aaa8c..8f70e48b7191d 100644 --- a/x-pack/test_serverless/functional/test_suites/common/platform_security/roles.ts +++ b/x-pack/test_serverless/functional/test_suites/common/platform_security/roles.ts @@ -56,7 +56,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { }); it('should create and only display custom roles', async () => { - const customRole = 'serverless-custom-role'; + const customRole = 'serverlesscustomrole'; await pageObjects.security.addRole(customRole, { elasticsearch: { indices: [