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 "Incorrect username or password" message #272

Merged
merged 1 commit into from
Feb 23, 2021
Merged

Conversation

psvenk
Copy link
Member

@psvenk psvenk commented Feb 23, 2021

Closes #226. Instead of simply sending users back to the login screen when a login fails, alert them to this fact in text above the login form.

Instead of simply sending users back to the login screen when a login
fails, alert them to this fact in text above the login form.
@psvenk psvenk added this to the 2.7.0 milestone Feb 23, 2021
Copy link
Collaborator

@jadebuckwalter jadebuckwalter left a comment

Choose a reason for hiding this comment

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

@psvenk Looks good, although it's a little jarring that it goes to the homepage for a split second before redirecting to the login page. Is there any way to fix that?

@psvenk
Copy link
Member Author

psvenk commented Feb 23, 2021

@jadebuckwalter Making login validation work without needing to go to the home page would entail communicating with the backend from the login page, so that the backend can talk to Aspen to validate the login. We would furthermore need to do another handshake between the frontend and the backend so that the frontend redirects to the home page or to the "incorrect password" login page depending on the result of this request, and we would need to account for possible race conditions in sending data to the frontend while it is still loading the home page (upon a successful login) and in properly disposing of sessions. This would not only bring a great deal of complexity to the login page (which currently is quite light and only has a little bit of JS) but also make the backend, including scrape.ts, much more complex.

Your thoughts?

@jadebuckwalter
Copy link
Collaborator

@psvenk Ok, makes sense. I wasn't sure how complicated it would be, and based on what you said, it seems like the hassle would outweigh the benefit.

@psvenk psvenk merged commit 6a87129 into master Feb 23, 2021
@psvenk psvenk deleted the login-incorrect branch February 23, 2021 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Give a visual indication of incorrect login info
2 participants