fix: Bug - No login screen mobile phone on startup
Signed-out mobile visitors landing on / or /dashboard were intentionally kept on a logged-out /dashboard, so they were never greeted with a login screen on startup. Replace the imperative startup-redirect logic in App.ngOnInit with a platform-agnostic pure rule resolveUnauthenticatedStartupRedirect: non-public routes redirect to /login (with a safe returnUrl), public routes (/login, /register, /invite/...) are left alone. Mobile is no longer special-cased. - Unit: auth-navigation.rules.spec.ts - E2E: e2e/tests/mobile/mobile-login-on-startup.spec.ts (mobile viewport set before navigation; /dashboard and / both land on /login) Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -25,6 +25,13 @@ Durable rules for AI agents working on this project. Read this file at session s
|
||||
|
||||
## Lessons
|
||||
|
||||
### Don't strand signed-out mobile users on a logged-out dashboard [auth] [mobile] [routing]
|
||||
|
||||
- **Trigger:** `App.ngOnInit` special-cased mobile — signed-out visitors landing on `/` or `/dashboard` were kept on `/dashboard` (the "login form has no mobile chrome" rationale), so mobile users got a logged-out dashboard and never saw a login screen on startup.
|
||||
- **Rule:** decide startup routing for signed-out users with the platform-agnostic pure rule `resolveUnauthenticatedStartupRedirect(currentUrl)` (`auth-navigation.rules.ts`) — non-public routes → `/login` (with safe `returnUrl`), public routes (`/login`, `/register`, `/invite/...`) → stay; do not branch on `isMobile()` here.
|
||||
- **Why:** the mobile exception directly contradicted the product expectation ("greet signed-out users with the login screen"); the login form already links to register, so there is no dead-end to avoid.
|
||||
- **Example:** unit `auth-navigation.rules.spec.ts` (`resolveUnauthenticatedStartupRedirect('/dashboard') === { path:'/login', queryParams:{} }`); e2e `e2e/tests/mobile/mobile-login-on-startup.spec.ts` sets a 390×844 viewport **before** navigating (so `ViewportService.isMobile` is true at bootstrap) and asserts `/dashboard` and `/` both land on `/login`.
|
||||
|
||||
### "Shared from your device" must gate on local bytes, not uploader user id [attachments] [multi-device]
|
||||
|
||||
- **Trigger:** a second device of the same user showed "Shared from your device" and hid the download affordance for a file uploaded from another device — `isUploader(attachment)` returned `uploaderPeerId === currentUserId`, but `uploaderPeerId` is the **user** id (set to `currentUser.id` in `publishAttachments`), so it is true on every device of the uploader, including ones that only synced metadata.
|
||||
|
||||
@@ -118,6 +118,8 @@ Authorize UI: `/login?mode=authorize&serverId=…&returnUrl=…` (also supported
|
||||
|
||||
Persisted local user state (`metoyou_currentUserId` + IndexedDB/SQLite profile) is **not** sufficient to use chat or presence. On startup, `loadCurrentUser$` requires a non-expired session token for the user's home signaling server (or any stored token as a fallback). Missing or rejected **home** tokens dispatch `SESSION_EXPIRED` and redirect to `/login`. Foreign-server `auth_required` / `auth_error` responses clear only that server's credential and attempt re-provision.
|
||||
|
||||
Startup routing for signed-out visitors is decided by `resolveUnauthenticatedStartupRedirect(currentUrl)` (`auth-navigation.rules.ts`), called from `App.ngOnInit`: any non-public route is redirected to `/login` (carrying a safe `returnUrl`), while public routes (`/login`, `/register`, `/invite/...`) are left alone. This is **platform-agnostic** — mobile is intentionally not special-cased, so a signed-out mobile user is greeted with the login screen on startup rather than a logged-out `/dashboard`.
|
||||
|
||||
## Security considerations
|
||||
|
||||
- Rate limits: login/register (100 / 15 min), server join (30 / min).
|
||||
|
||||
39
e2e/tests/mobile/mobile-login-on-startup.spec.ts
Normal file
39
e2e/tests/mobile/mobile-login-on-startup.spec.ts
Normal file
@@ -0,0 +1,39 @@
|
||||
import { test, expect } from '../../fixtures/multi-client';
|
||||
|
||||
/**
|
||||
* Regression coverage for: "No login screen mobile phone on startup".
|
||||
*
|
||||
* Signed-out mobile users used to be left on a logged-out /dashboard (the
|
||||
* startup redirect special-cased mobile + root/dashboard and kept them there),
|
||||
* so they were never greeted with the login screen. The fix removes that mobile
|
||||
* exception: signed-out visitors are sent to /login on every platform.
|
||||
*
|
||||
* The mobile viewport must be set BEFORE navigation so ViewportService reports
|
||||
* `isMobile === true` at app bootstrap, which is exactly when the redirect ran.
|
||||
*/
|
||||
|
||||
const MOBILE_VIEWPORT = { width: 390, height: 844 };
|
||||
|
||||
test.describe('Mobile login screen on startup', () => {
|
||||
test.describe.configure({ timeout: 120_000 });
|
||||
|
||||
test('greets a signed-out mobile visitor on /dashboard with the login screen', async ({ createClient }) => {
|
||||
const { page } = await createClient();
|
||||
|
||||
await page.setViewportSize(MOBILE_VIEWPORT);
|
||||
await page.goto('/dashboard', { waitUntil: 'domcontentloaded' });
|
||||
|
||||
await expect(page).toHaveURL(/\/login/, { timeout: 15_000 });
|
||||
await expect(page.locator('#login-username')).toBeVisible({ timeout: 15_000 });
|
||||
});
|
||||
|
||||
test('greets a signed-out mobile visitor on the app root with the login screen', async ({ createClient }) => {
|
||||
const { page } = await createClient();
|
||||
|
||||
await page.setViewportSize(MOBILE_VIEWPORT);
|
||||
await page.goto('/', { waitUntil: 'domcontentloaded' });
|
||||
|
||||
await expect(page).toHaveURL(/\/login/, { timeout: 15_000 });
|
||||
await expect(page.locator('#login-username')).toBeVisible({ timeout: 15_000 });
|
||||
});
|
||||
});
|
||||
@@ -58,7 +58,7 @@ import { RoomsActions } from './store/rooms/rooms.actions';
|
||||
import { selectCurrentRoom } from './store/rooms/rooms.selectors';
|
||||
import { ROOM_URL_PATTERN } from './core/constants';
|
||||
import { clearStoredCurrentUserId, getStoredCurrentUserId } from './core/storage/current-user-storage';
|
||||
import { buildLoginReturnQueryParams } from './domains/authentication/domain/logic/auth-navigation.rules';
|
||||
import { resolveUnauthenticatedStartupRedirect } from './domains/authentication/domain/logic/auth-navigation.rules';
|
||||
import { runWhenIdle } from './shared/rxjs';
|
||||
import {
|
||||
ThemeNodeDirective,
|
||||
@@ -308,22 +308,17 @@ export class App implements OnInit, OnDestroy {
|
||||
const currentUrl = this.getCurrentRouteUrl();
|
||||
|
||||
if (!currentUserId) {
|
||||
if (!this.isPublicRoute(currentUrl)) {
|
||||
// On mobile, new/unauthenticated visitors landing on the app root or
|
||||
// /dashboard should stay on /dashboard (which already exposes a login
|
||||
// CTA). The login form has no mobile chrome / back button, so dropping
|
||||
// new users straight onto it leaves them with no way to navigate away.
|
||||
const currentPath = this.getRoutePath(currentUrl);
|
||||
const isSearchLanding = currentPath === '/' || currentPath === '/dashboard';
|
||||
// Signed-out visitors are greeted with the login screen on every platform.
|
||||
// Mobile is intentionally not special-cased here: previously mobile users
|
||||
// landing on the root or /dashboard were left on a logged-out dashboard,
|
||||
// which read as "no login screen on startup".
|
||||
const startupRedirect = resolveUnauthenticatedStartupRedirect(currentUrl);
|
||||
|
||||
if (this.isMobile() && isSearchLanding) {
|
||||
this.router.navigate(['/dashboard'], { replaceUrl: true }).catch(() => {});
|
||||
} else {
|
||||
this.router.navigate(['/login'], {
|
||||
queryParams: buildLoginReturnQueryParams(currentUrl)
|
||||
if (startupRedirect) {
|
||||
this.router.navigate([startupRedirect.path], {
|
||||
queryParams: startupRedirect.queryParams
|
||||
}).catch(() => {});
|
||||
}
|
||||
}
|
||||
} else {
|
||||
this.store.dispatch(UsersActions.loadCurrentUser());
|
||||
this.store.dispatch(RoomsActions.loadRooms());
|
||||
@@ -483,14 +478,6 @@ export class App implements OnInit, OnDestroy {
|
||||
});
|
||||
}
|
||||
|
||||
private isPublicRoute(url: string): boolean {
|
||||
const path = this.getRoutePath(url);
|
||||
|
||||
return path === '/login' ||
|
||||
path === '/register' ||
|
||||
path.startsWith('/invite/');
|
||||
}
|
||||
|
||||
private getCurrentRouteUrl(): string {
|
||||
if (typeof window === 'undefined') {
|
||||
return this.router.url;
|
||||
|
||||
@@ -9,6 +9,7 @@ import { UsersActions } from '../../../../store/users/users.actions';
|
||||
import {
|
||||
buildLoginReturnQueryParams,
|
||||
resolveSafeReturnUrl,
|
||||
resolveUnauthenticatedStartupRedirect,
|
||||
waitForAuthenticationOutcome
|
||||
} from './auth-navigation.rules';
|
||||
|
||||
@@ -55,6 +56,39 @@ describe('buildLoginReturnQueryParams', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveUnauthenticatedStartupRedirect', () => {
|
||||
it('sends signed-out visitors on the dashboard/root to login (no mobile exception)', () => {
|
||||
expect(resolveUnauthenticatedStartupRedirect('/dashboard')).toEqual({
|
||||
path: '/login',
|
||||
queryParams: {}
|
||||
});
|
||||
|
||||
expect(resolveUnauthenticatedStartupRedirect('/')).toEqual({
|
||||
path: '/login',
|
||||
queryParams: { returnUrl: '/' }
|
||||
});
|
||||
});
|
||||
|
||||
it('preserves a safe returnUrl when redirecting from a protected route', () => {
|
||||
expect(resolveUnauthenticatedStartupRedirect('/servers')).toEqual({
|
||||
path: '/login',
|
||||
queryParams: { returnUrl: '/servers' }
|
||||
});
|
||||
|
||||
expect(resolveUnauthenticatedStartupRedirect('/room/abc')).toEqual({
|
||||
path: '/login',
|
||||
queryParams: { returnUrl: '/room/abc' }
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves public auth/invite routes untouched', () => {
|
||||
expect(resolveUnauthenticatedStartupRedirect('/login')).toBeNull();
|
||||
expect(resolveUnauthenticatedStartupRedirect('/login?returnUrl=%2Fservers')).toBeNull();
|
||||
expect(resolveUnauthenticatedStartupRedirect('/register')).toBeNull();
|
||||
expect(resolveUnauthenticatedStartupRedirect('/invite/abc123')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('waitForAuthenticationOutcome', () => {
|
||||
it('resolves when authentication storage preparation succeeds', async () => {
|
||||
const user = {
|
||||
|
||||
@@ -18,10 +18,41 @@ export type AuthenticationOutcome =
|
||||
| { kind: 'success'; user: User }
|
||||
| { kind: 'failure'; error: string };
|
||||
|
||||
export interface UnauthenticatedStartupRedirect {
|
||||
path: string;
|
||||
queryParams: Record<string, string>;
|
||||
}
|
||||
|
||||
export function isAuthRoutePath(path: string): boolean {
|
||||
return AUTH_ROUTE_PATHS.has(path);
|
||||
}
|
||||
|
||||
/** Routes a signed-out visitor may stay on without being bounced to login. */
|
||||
export function isPublicStartupUrl(url: string): boolean {
|
||||
const path = getRoutePathFromUrl(url);
|
||||
|
||||
return isAuthRoutePath(path) || path.startsWith('/invite/');
|
||||
}
|
||||
|
||||
/**
|
||||
* Decides where an unauthenticated visitor should land at startup. Signed-out
|
||||
* users are always sent to `/login` (regardless of viewport) so mobile users are
|
||||
* greeted with the login screen instead of a logged-out dashboard. Public routes
|
||||
* (`/login`, `/register`, `/invite/...`) are left untouched (return `null`).
|
||||
*/
|
||||
export function resolveUnauthenticatedStartupRedirect(
|
||||
currentUrl: string
|
||||
): UnauthenticatedStartupRedirect | null {
|
||||
if (isPublicStartupUrl(currentUrl)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return {
|
||||
path: '/login',
|
||||
queryParams: buildLoginReturnQueryParams(currentUrl)
|
||||
};
|
||||
}
|
||||
|
||||
export function getRoutePathFromUrl(url: string): string {
|
||||
if (!url) {
|
||||
return '/';
|
||||
|
||||
Reference in New Issue
Block a user