Skip to content

Commit

Permalink
[Serverless] Add role name validation to prevent white spaces in serv…
Browse files Browse the repository at this point in the history
…erless custom roles (elastic#191894)

Closes elastic#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
<img width="557" alt="image"
src="https://github.com/user-attachments/assets/dba96205-25e5-46e6-aa81-55b8f0952270">

---------

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
SiddharthMantri and elasticmachine authored Sep 5, 2024
1 parent 3c4e502 commit b9c526a
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 5 deletions.
9 changes: 9 additions & 0 deletions x-pack/plugins/security/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ function useRole(
roleName?: string
) {
const [role, setRole] = useState<Role | null>(null);

useEffect(() => {
const rolePromise = roleName
? rolesAPIClient.getRole(roleName)
Expand Down Expand Up @@ -350,7 +351,7 @@ export const EditRolePage: FunctionComponent<Props> = ({

// 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<RoleValidationResult | null>(null);
const [creatingRoleAlreadyExists, setCreatingRoleAlreadyExists] = useState<boolean>(false);
const [previousName, setPreviousName] = useState<string>('');
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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
<ElasticsearchPrivileges {...getProps()} buildFlavor={'serverless'} canUseRemoteIndices />
<ElasticsearchPrivileges
{...getProps()}
validator={new RoleValidator({ buildFlavor: 'serverless' })}
buildFlavor={'serverless'}
canUseRemoteIndices
/>
)
).toMatchSnapshot();
});

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import type { BuildFlavor } from '@kbn/config';
import { i18n } from '@kbn/i18n';

import type {
Expand All @@ -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 {
Expand All @@ -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() {
Expand Down Expand Up @@ -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',
Expand All @@ -83,6 +97,7 @@ export class RoleValidator {
)
);
}

return valid();
}
public validateRemoteClusterPrivileges(role: Role): RoleValidationResult {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down

0 comments on commit b9c526a

Please sign in to comment.