Pigmice2733 / peregrine-frontend

The future of FRC scouting
https://peregrinefrc.com/
The Unlicense
23 stars 4 forks source link

add frontend validation for alphanumeric usernames and passwords #1537

Open varCepheid opened 9 months ago

varCepheid commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

@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 commented 8 months ago

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?

varCepheid commented 8 months ago

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.

varCepheid commented 4 months ago

I didn't fix the problem; I just removed the code that was causing it to fail. The new test doesn't currently check anything. I can't figure out what is unique to an invalid username that the test is able to find, given that the waitFor method won't work in the way that I want it to.