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

Fix #5938 error when logging in with empty credentials. #5990

Merged
merged 2 commits into from
May 4, 2020
Merged

Conversation

sjrd218
Copy link
Contributor

@sjrd218 sjrd218 commented Apr 21, 2020

Disallows submitting the login form with an empty username.
Login toggles a spinner to replace the login button so multiple form submissions cannot be attempted.

@mergify mergify bot added the 3.2.x label Apr 21, 2020
@sjrd218 sjrd218 added this to the 3.2.7 milestone Apr 21, 2020
@sjrd218 sjrd218 self-assigned this Apr 21, 2020
Copy link
Contributor

@ahormazabal ahormazabal left a comment

Choose a reason for hiding this comment

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

Summarizing, i'm not sure if we should manage this at JS level on the login form. I would feel more comfortable letting the springsec subsystem to deal with this case.

Copy link
Contributor

@ahormazabal ahormazabal left a comment

Choose a reason for hiding this comment

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

Playing with it, the spinner looks quite cool white waiting for login.

I found a small bug. If i supply a wrong password (rendering the "Invalid username and password.") and then supply an empty username, i get two error messages:
Screen Shot 2020-04-21 at 16 04 51

I also tested the password auth using curl and could not replicate the original problem, so that part should be ok too.

@sjrd218
Copy link
Contributor Author

sjrd218 commented Apr 21, 2020

Playing with it, the spinner looks quite cool white waiting for login.

I found a small bug. If i supply a wrong password (rendering the "Invalid username and password.") and then supply an empty username, i get two error messages:
Screen Shot 2020-04-21 at 16 04 51

I also tested the password auth using curl and could not replicate the original problem, so that part should be ok too.

I'm not sure I agree that the screenshot shows a bug. The user is attempting to submit the form with an empty username so they get a message telling them that. The other message is the normal static message a user receives when they have not entered correct credentials. That one doesn't disappear until the user logs in successfully. The new message is merely additive.

@ahormazabal
Copy link
Contributor

Fair enough.

Disallows submitting the login form with an empty username.
Login toggles a spinner to replace the login button so multiple form submissions cannot be attempted.
Check authentication.name first in AuditEventsService.
@sjrd218 sjrd218 merged commit 0fee627 into master May 4, 2020
@sjrd218 sjrd218 deleted the issue/5938 branch May 4, 2020 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants