-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
refactor: authentication on public routes #6765
Conversation
5a077e8
to
70f84c9
Compare
60febd8
to
c748d54
Compare
2b53107
to
6647a57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
web/src/lib/components/user-settings-page/user-profile-settings.svelte
Outdated
Show resolved
Hide resolved
web/src/lib/utils/auth.ts
Outdated
|
||
if (!user) { | ||
redirect(302, AppRoute.AUTH_LOGIN); | ||
if (options.public && !user) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you care if the user is defined or not if the route is public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems like you want:
- if public return we're done with all the checks
- if not user, redirect to login, this is an auth required route
- if admin route and not an admin, redirect to /photos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
5a9af37
to
b6b1c52
Compare
This PR aims to refactor the authentication on public routes to avoid 400 requests on shared links