diff --git a/agents-docs/LESSONS.md b/agents-docs/LESSONS.md index db5b996..0dc2f3b 100644 --- a/agents-docs/LESSONS.md +++ b/agents-docs/LESSONS.md @@ -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. diff --git a/agents-docs/features/authentication.md b/agents-docs/features/authentication.md index 3bb3021..c47964e 100644 --- a/agents-docs/features/authentication.md +++ b/agents-docs/features/authentication.md @@ -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). diff --git a/e2e/tests/mobile/mobile-login-on-startup.spec.ts b/e2e/tests/mobile/mobile-login-on-startup.spec.ts new file mode 100644 index 0000000..2c53f0c --- /dev/null +++ b/e2e/tests/mobile/mobile-login-on-startup.spec.ts @@ -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 }); + }); +}); diff --git a/toju-app/src/app/app.ts b/toju-app/src/app/app.ts index 408a5a4..471b4bb 100644 --- a/toju-app/src/app/app.ts +++ b/toju-app/src/app/app.ts @@ -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,21 +308,16 @@ 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) - }).catch(() => {}); - } + if (startupRedirect) { + this.router.navigate([startupRedirect.path], { + queryParams: startupRedirect.queryParams + }).catch(() => {}); } } else { this.store.dispatch(UsersActions.loadCurrentUser()); @@ -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; diff --git a/toju-app/src/app/domains/authentication/domain/logic/auth-navigation.rules.spec.ts b/toju-app/src/app/domains/authentication/domain/logic/auth-navigation.rules.spec.ts index 41bcfb9..a3b83bf 100644 --- a/toju-app/src/app/domains/authentication/domain/logic/auth-navigation.rules.spec.ts +++ b/toju-app/src/app/domains/authentication/domain/logic/auth-navigation.rules.spec.ts @@ -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 = { diff --git a/toju-app/src/app/domains/authentication/domain/logic/auth-navigation.rules.ts b/toju-app/src/app/domains/authentication/domain/logic/auth-navigation.rules.ts index 873be22..77d1ed0 100644 --- a/toju-app/src/app/domains/authentication/domain/logic/auth-navigation.rules.ts +++ b/toju-app/src/app/domains/authentication/domain/logic/auth-navigation.rules.ts @@ -18,10 +18,41 @@ export type AuthenticationOutcome = | { kind: 'success'; user: User } | { kind: 'failure'; error: string }; +export interface UnauthenticatedStartupRedirect { + path: string; + queryParams: Record; +} + 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 '/';