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

[BUG] Users can be created the violate the policy defined below the text field #1234

Open
peternied opened this issue Dec 1, 2022 · 4 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers triaged

Comments

@peternied
Copy link
Member

What is the bug?
A clear and concise description of the bug.

How can one reproduce the bug?
Steps to reproduce the behavior:

  1. Go to the user creation page http://localhost:5601/app/security-dashboards-plugin#/users/create
  2. See the text The user name must contain from 2 to 50 characters. Valid characters are A-Z, a-z, 0-9, (_)underscore, (-) hyphen and unicode characters.
  3. Enter a username into the text box that includes special characters
  4. Provide a password
  5. Click Create

What is the expected behavior?
If the username violates the outlined parameters it should not be created through the UI

What is your host/environment?

  • Version 2.2

Do you have any screenshots?
image

@peternied peternied added bug Something isn't working untriaged labels Dec 1, 2022
@peternied
Copy link
Member Author

Note; when fixing this bug, limited the allowed characters to the regex [A-Za-z0-9_-]+ seems far too restrictive, seems like UTF-8 characters should be supported. I think we need to determine what this scheme should be like while making broaders changes.

@peternied
Copy link
Member Author

Found while investigating opensearch-project/security#2277

@DarshitChanpura
Copy link
Member

[Triage] UI/Backend should enforce the same restrictions. Follow-up with a PR/issue that addresses the following acceptance criteria:

  1. How to handle the error returned from API
  2. A hint on the text-box that suggests one or more rules are being violated.

@shanilpa Can you please verify and update the acceptance criteria?

@DarshitChanpura DarshitChanpura added good first issue Good for newcomers triaged and removed untriaged labels Dec 5, 2022
@shanilpa
Copy link

shanilpa commented Dec 6, 2022

Thanks for filing this. This solution should work on both the OS Dashboards UI as well as the API. If rules are violated the user should not be allowed to perform the action through either of the experiences.

Acceptance Criteria:

  1. A user should be warned that they are violating one or more rules when inputting information in the credentials section. This should be true for the creation and editing of users. (Refer to Image 1 for UI errors)
  2. If a user tries to create or edit a user after ignoring the warning messages on form fields they should not be able to complete the action. In this case a banner on the top should be presented with appropriate error messaging. (Refer to Image 2 for UI errors)
  3. If a user tries to create or edit a user through the API and violates one or more rules they should be presented with an error message and not allowed to create the user.
  4. (optional) As @peternied mentioned this is also a good opportunity to re-visit and make changes to the scheme we use for these validation rules. If the scheme is changed the error messages and hint text should be updated to reflect these changes.

Image 1:
If a user violates on more rules these are the error messages that should be presented.
image

Image 2:
User ignores warning messages and hits Create CTA should be presented with this banner
image

UI implementation guidelines
Use the OUI/EUI Form or FormRow components and pass the appropriate validation props to achieve the desired UX as shown in the images above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants