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

add frontend validation for alphanumeric usernames and passwords #1537

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

varCepheid
Copy link
Collaborator

closes #1528
This PR creates an alert on the login and signup pages that will be thrown if the user submits the form with non-alphanum characters in the username or password, except for underscores.

@varCepheid varCepheid marked this pull request as draft February 9, 2024 01:56
@varCepheid varCepheid added bug Addresses something that isn't working. question Further information is requested. labels Feb 9, 2024
@varCepheid
Copy link
Collaborator Author

@calebeby I don't know exactly the purpose of the test check or what it's catching on here. I figured the problem might have come from me modifying index.ts in /components/authenticated but not modifying index.test.ts, but I have no idea if that would cause a problem. Also, do you mind if underscores are allowed? Will the backend prevent it?

@calebeby
Copy link
Member

Also, do you mind if underscores are allowed? Will the backend prevent it?

Alphanumeric is only a-z, A-Z, and 0-9. So yeah, the backend will not allow underscores in usernames. I tested it too to double check:

image

I do not mind if you change it to allow underscores, but you'd need to update the backend code.

@calebeby
Copy link
Member

@varCepheid Not sure your background with automated software testing, let me know if you want a more detailed description, otherwise I'll start with this:

authenticated/index.test.ts is an automated test that makes sure the authenticated component works correctly. To do this, it renders the authenticated component, types in the login form, and clicks the log in button. The test then makes sure that the authenticated component has made a fetch request (to try to talk to the backend) with the correct information in it. Our frontend automated tests run in isolation, not connected to a real backend, so we just check that it sent fetch requests without stuff actually getting put in a database somewhere. That's what mockFetch is for, it intercepts the fetch requests before they actually try to go to the backend.

In this case, the test I wrote types out a username that has a space in it. I assume your code changes in this PR prevent the form from submitting if there is a space. So the automated test noticed that the fetch request is no longer going through when "log in" is clicked.

You can fix the test by changing the username that it types in to be alphanumeric.

Even better, you can add a test case for your changes in this PR that check that if you try to submit the form with non-alphanumeric characters in username, it doesn't submit the form and instead displays a helpful error message!

@varCepheid
Copy link
Collaborator Author

I'm not super familiar with automated testing. The code is pretty human-readable, which helps, but despite making what seemed like an appropriate change, the test check is still failing. I added the second test to confirm that the error alert is working, but the test check told me that it couldn't find an element with the alert's text, even though that's definitely the text that the alert contains. Maybe I'm understanding something wrong?

- allowed passwords to have special characters (the backend handles them fine)
- made progress on new test for new alert
@varCepheid
Copy link
Collaborator Author

I played around a bunch with the tests and couldn't quite get them to work. The documentation for waitFor says that it will repeatedly call the function until it stops throwing an error, but the test check is failing when the inner function throws an error. I can't figure out why waitFor isn't catching it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses something that isn't working. question Further information is requested.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

error messages on login
2 participants