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

Change the handling of login errors. #2729

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

BlackDex
Copy link
Collaborator

@BlackDex BlackDex commented Sep 6, 2022

Previously FlashMessage was used to provide an error message during login.
This PR changes that flow to not use redirect for this, but renders the HTML and responds using the correct status code where needed. This should solve some issues which were reported in the past.

Thanks to @RealOrangeOne, for initiating this with a PR.

Fixes #2448
Fixes #2712
Closes #2715

Previously FlashMessage was used to provide an error message during login.
This PR changes that flow to not use redirect for this, but renders the HTML and responds using the correct status code where needed. This should solve some issues which were reported in the past.

Thanks to @RealOrangeOne, for initiating this with a PR.

Fixes dani-garcia#2448
Fixes dani-garcia#2712
Closes dani-garcia#2715

Co-authored-by: Jake Howard <git@theorangeone.net>
Copy link
Contributor

@RealOrangeOne RealOrangeOne left a comment

Choose a reason for hiding this comment

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

Looks good to me, much nicer!

Notice you've removed boostrap - is that intentional? CI isn't failing, so presume it just wasn't ever referenced?

@BlackDex
Copy link
Collaborator Author

BlackDex commented Sep 8, 2022

@RealOrangeOne i didn't removed it, It's just updated, but since the diff is very large Github doesn't show it by default

@dani-garcia dani-garcia merged commit 50c5eb9 into dani-garcia:main Sep 8, 2022
@BlackDex BlackDex deleted the vw-admin-updates branch September 9, 2022 08:42
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.

Admin API rejects invalid tokens with 303 instead of 401 status code
3 participants