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

Make user and role name constraint consistent with max document ID. #86728

Conversation

slobodanadamovic
Copy link
Contributor

@slobodanadamovic slobodanadamovic commented May 12, 2022

This change tries to make user and role name constraints consistent with
what is currently allowed to store in the native realm (512).
Because the document IDs are prefixed by either user- or role-,
the actual possible max length is a funky looking 507 chars.
If we choose (in the future) to allow more than 507 chars we should
either consider increasing the max allowed size for document ID or
consider hashing names longer than 507 in order to fit into document ID.

Note: File realm validation is left as it was and allows max 1024 chars.
I'm not sure if this is something we should reflect in the documentation.

Closes #66020

This change tries to make user and role name constraints consistent with
what is currently allowed to store in the native realm (512).
Because the document IDs are prefixed by either `user-` or `role-`,
the actual possible max length is a funky looking 507 chars.
If we choose (in the future) to allow more than 507 chars we should
either consider increasing the max allowed size for document ID or
consider hashing names longer than 507 in order to fit into document ID.

Closes elastic#66020
@slobodanadamovic slobodanadamovic self-assigned this May 12, 2022
@slobodanadamovic slobodanadamovic added the Team:Security Meta label for security team label May 12, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@slobodanadamovic slobodanadamovic added :Security/Security Security issues without another label >bug labels May 12, 2022
@elasticsearchmachine
Copy link
Collaborator

Hi @slobodanadamovic, I've created a changelog YAML for you.

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

@slobodanadamovic slobodanadamovic marked this pull request as draft May 16, 2022 14:33
@slobodanadamovic slobodanadamovic marked this pull request as ready for review May 18, 2022 08:37
@slobodanadamovic
Copy link
Contributor Author

slobodanadamovic commented May 18, 2022

Some of the tests are failing due to #86877.

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for being thorough.

@slobodanadamovic slobodanadamovic merged commit 096d7fe into elastic:master May 19, 2022
@slobodanadamovic slobodanadamovic deleted the fix-user-and-role-name-validation branch May 19, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Security/Security Security issues without another label Team:Security Meta label for security team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Max length constaint for role/user name should be more consistent
4 participants