feat: update auth navigation flows and guest navbar behavior#176
Conversation
kckagancan
left a comment
There was a problem hiding this comment.
web/src/components/auth/AuthRouteGate.tsx:
1. ready state can preserve a stale authorization result across transitions
ready is only ever set to true and is never reset when mode, pathname, or auth state changes. If the same AuthRouteGate instance remains mounted across client-side transitions, this can briefly render protected or guest-only content before the redirect effect runs. I think the gate should avoid relying on a previously computed ready=true value from another route/state.
2. The gate treats any stored token as authenticated without validating it
Access is currently granted purely based on getAccessToken() returning a non-empty string. An expired or otherwise invalid token will still pass this gate and allow rendering until a later API call fails. That makes the route protection unreliable in practice. I think this should be backed by validated auth state (for example session context or a verified /me state) or enforced at the middleware/server level.
web/src/app/(auth)/page.tsx:
Removing the welcome page is not a good idea I think. It would also be inconsistend with android side since we have welcome page there as well. No need to do these extra unnecessary works I think for now.
web/src/components/feature/auth/LoginForm.tsx:
1. Login success is coupled too tightly to fetchMyProfile()
Right now the login flow only persists the access token after fetchMyProfile() succeeds, except for the 404 complete-profile case. That means a user can successfully authenticate, receive a valid access token, and still be shown a login failure if /profiles/me temporarily fails with a network error or a 5xx response.
This makes the login flow brittle and can incorrectly block already authenticated users. I think the authenticated session should not depend on profile fetch succeeding. The token should be persisted once login succeeds, and profile fetch should only decide the post-login route (/home vs /complete-profile) or show a recoverable error state.
2. returnTo blocking only checks exact route strings
safeReturnTo blocks "/", "/login", "/signup", "/forgot-password", and "/verify-email" only as exact strings. Values like "/login?foo=1" or "/signup?step=2" are not blocked and will still be treated as safe targets.
That can send an already authenticated user back into guest-only auth routes and create redirect confusion. I think the route check should normalize returnTo to its pathname before comparing against the blocked auth routes.
web/src/components/layout/TopNavbar.tsx:
1. Switch Account does not actually switch accounts
The Switch Account action only does router.push("/login") but does not clear the current access token first. If the login page is guest-only, an already authenticated user will immediately be redirected away from /login, so this action will not work as expected.
I think this should either clear the current session before navigating to the login page, or use a dedicated account-switch flow that bypasses the guest-only redirect logic.
2. Navbar auth state is only refreshed on pathname changes
isAuthenticated is derived from getAccessToken() inside an effect that depends only on pathname. That means auth state changes that happen without a route change (token cleared elsewhere, token updated, cross-tab logout, session expiry handling, etc.) will leave the navbar in a stale state until navigation happens.
I think the navbar should read from a shared auth state/store/context instead of keeping its own route-triggered copy of authentication state.
Thanks for the detailed review. Addressed all points in the latest update. AuthRouteGate stale ready state
Token presence vs token validity
Welcome page removal
Login coupled to fetchMyProfile
returnTo exact-string blocking
Switch Account behavior
Navbar auth freshness
|
kckagancan
left a comment
There was a problem hiding this comment.
Looks fine now. Great work.
gulcetahtasiz
left a comment
There was a problem hiding this comment.
Everything looks fine, thanks for the effort. Merging
This PR refines web auth/navigation behavior by separating guest vs authenticated flows, improving the login verification experience, and simplifying legal-page navigation.
What Changed
Added centralized auth route gating
Updated guest access behavior
Improved login UX
Refined navbar behavior
Legal pages switched to navbar-based navigation
Root route behavior updated
Auth showcase visual update
Note: For the MVP scope, the planned end-to-end web user flow is now considered complete (guest navigation, auth entry points, protected routing behavior, legal page navigation, and core login/verification UX). Further changes can continue as post-MVP refinements.
Closes #173