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

Improve the handling of unauthenticated users #517

Merged
merged 4 commits into from
Oct 5, 2023

Conversation

JVT038
Copy link
Collaborator

@JVT038 JVT038 commented Sep 30, 2023

This PR automatically redirect user to the login page if they're not authenticated with a warning that they have to login before viewing the page.

Besides that, the user will be redirected back to the page they attempted to view if they log in.

and redirect the user back if they have logged in
@JVT038 JVT038 requested a review from leepeuker as a code owner September 30, 2023 16:52
@JVT038 JVT038 linked an issue Sep 30, 2023 that may be closed by this pull request
@JVT038
Copy link
Collaborator Author

JVT038 commented Oct 1, 2023

BTW, currently the redirection target URL encoded and stored in the GET parameters.
So it's like localhost/login?redirect={target url}.

In theory, anyone could just manually edit the target URL in the get parameter and be redirected to whatever they want. Is this dangerous, or should we check if the target url exists and stuff?

@leepeuker
Copy link
Owner

BTW, currently the redirection target URL encoded and stored in the GET parameters. So it's like localhost/login?redirect={target url}.

In theory, anyone could just manually edit the target URL in the get parameter and be redirected to whatever they want. Is this dangerous, or should we check if the target url exists and stuff?

we should make sure to only allow relative urls, I think than we are fine

@leepeuker leepeuker merged commit 797306d into main Oct 5, 2023
@leepeuker leepeuker deleted the improve-unauthenticated-users-response branch October 5, 2023 19:09
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.

Properly handle unauthenticated users
2 participants