From d3d22846e7c02dc2c814e81dbd973b290c9abd0a Mon Sep 17 00:00:00 2001 From: Myx Date: Tue, 14 Jul 2026 01:54:01 +0200 Subject: [PATCH] fix: Bug - User login status showing as both logged in and logged out Stop treating transient auth_required as home-session expiry, keep auth scope consistent across login redirect and server-rail joins, and leave /login when the in-memory user is still authenticated. Co-authored-by: Cursor --- agents-docs/features/authentication.md | 3 +- .../signal-server-authorize.service.spec.ts | 120 ++++++++++++++++++ .../signal-server-authorize.service.ts | 11 ++ .../logic/auth-navigation.rules.spec.ts | 23 ++++ .../domain/logic/auth-navigation.rules.ts | 33 +++++ .../domain/logic/auth-session.rules.spec.ts | 21 ++- .../domain/logic/auth-session.rules.ts | 13 ++ .../signal-server-auth-failure.rules.spec.ts | 99 +++++++++++++++ .../logic/signal-server-auth-failure.rules.ts | 67 ++++++++++ .../feature/login/login.component.ts | 44 ++++--- .../servers-rail/servers-rail.component.ts | 21 ++- .../realtime/realtime-session.service.ts | 6 +- toju-app/src/app/store/users/users.actions.ts | 3 +- toju-app/src/app/store/users/users.effects.ts | 95 ++++++++------ 14 files changed, 491 insertions(+), 68 deletions(-) create mode 100644 toju-app/src/app/domains/authentication/application/services/signal-server-authorize.service.spec.ts create mode 100644 toju-app/src/app/domains/authentication/domain/logic/signal-server-auth-failure.rules.spec.ts create mode 100644 toju-app/src/app/domains/authentication/domain/logic/signal-server-auth-failure.rules.ts diff --git a/agents-docs/features/authentication.md b/agents-docs/features/authentication.md index 35f49dd..589bd3b 100644 --- a/agents-docs/features/authentication.md +++ b/agents-docs/features/authentication.md @@ -195,7 +195,7 @@ A per-install **provision secret** enables silent account creation on newly adde | Foreign login/register | `authorizeSignalServer` | Upserts credential for that URL only; home session unchanged | | Auto-provision | `SignalServerProvisionerService` | Registers or logs in on foreign server using provision secret; on username collision tries suffixed username (`alice-`) and prefixes the display name with `# #` so same-name accounts stay distinguishable | | Create/join on foreign server | `RoomsEffects.createRoom$`, invite/join flows | `ensureCredentialForServerUrl` provisions (or reuses) the per-server session token first; REST/WebSocket calls use the **actor user id** for that signal URL, not the home registration id | -| Foreign auth failure | `signalServerAuthFailed` | Clears that URL's credential and re-provisions when home token is still valid; global logout only when home server rejects auth | +| Foreign auth failure | `signalServerAuthFailed` | `auth_required` (message raced ahead of identify) re-identifies or is ignored while a valid local credential exists; `auth_error` (token rejected) clears that URL's credential and re-provisions on foreign servers or expires the home session | Unreachable or offline signal servers must **not** open `/login?mode=authorize`. `ensureEndpointVersionCompatibility()` treats only `online` endpoints as connectable, and `ensureCredentialForServerUrl()` skips authorize navigation when health checks report the server offline (or provisioning fails over the network). @@ -223,4 +223,5 @@ Startup routing for signed-out visitors is decided by `resolveUnauthenticatedSta | Date | Change | |------|--------| +| 2026-07-14 | Distinguish `auth_required` vs `auth_error` on `signalServerAuthFailed`; stop false home-session expiry; leave `/login` when in-memory user still authenticated | | 2026-07-05 | Expanded protected-route inventory; clarified signing-key registration scope; cross-links | diff --git a/toju-app/src/app/domains/authentication/application/services/signal-server-authorize.service.spec.ts b/toju-app/src/app/domains/authentication/application/services/signal-server-authorize.service.spec.ts new file mode 100644 index 0000000..20b2c11 --- /dev/null +++ b/toju-app/src/app/domains/authentication/application/services/signal-server-authorize.service.spec.ts @@ -0,0 +1,120 @@ +import '@angular/compiler'; +import { Injector, runInInjectionContext } from '@angular/core'; +import { + beforeEach, + describe, + expect, + it, + vi +} from 'vitest'; +import { Router } from '@angular/router'; +import { Store } from '@ngrx/store'; +import { of } from 'rxjs'; + +import { ServerDirectoryFacade } from '../../../server-directory'; +import { SignalServerAuthService } from './signal-server-auth.service'; +import { SignalServerAuthorizeService } from './signal-server-authorize.service'; + +const HOME_URL = 'https://signal.toju.app'; +const FOREIGN_URL = 'https://signal-sweden.toju.app'; +const homeUser = { + id: 'user-1', + username: 'alice', + displayName: 'Alice', + homeSignalServerUrl: HOME_URL +}; + +describe('SignalServerAuthorizeService', () => { + let router: { navigate: ReturnType; url: string }; + let store: { select: ReturnType }; + let serverDirectory: { + ensureServerEndpoint: ReturnType; + findServerByUrl: ReturnType; + servers: ReturnType; + testServer: ReturnType; + }; + let signalServerAuth: { + ensureProvisioned: ReturnType; + hasValidCredential: ReturnType; + migrateHomeCredential: ReturnType; + }; + let service: SignalServerAuthorizeService; + + beforeEach(() => { + router = { navigate: vi.fn(() => Promise.resolve(true)), url: '/room/room-1' }; + store = { select: vi.fn(() => of(homeUser)) }; + serverDirectory = { + ensureServerEndpoint: vi.fn(() => ({ id: 'endpoint-1' })), + findServerByUrl: vi.fn(() => ({ id: 'endpoint-1', status: 'online' })), + servers: vi.fn(() => []), + testServer: vi.fn(() => Promise.resolve()) + }; + + signalServerAuth = { + ensureProvisioned: vi.fn(() => Promise.resolve({ kind: 'skipped', reason: 'no-provision-secret' })), + hasValidCredential: vi.fn(() => false), + migrateHomeCredential: vi.fn() + }; + + const injector = Injector.create({ + providers: [ + SignalServerAuthorizeService, + { provide: Router, useValue: router }, + { provide: Store, useValue: store }, + { provide: ServerDirectoryFacade, useValue: serverDirectory }, + { provide: SignalServerAuthService, useValue: signalServerAuth } + ] + }); + + service = runInInjectionContext(injector, () => injector.get(SignalServerAuthorizeService)); + }); + + it('returns true immediately when a valid credential already exists', async () => { + signalServerAuth.hasValidCredential.mockReturnValue(true); + + await expect(service.ensureCredentialForServerUrl(FOREIGN_URL)).resolves.toBe(true); + expect(signalServerAuth.ensureProvisioned).not.toHaveBeenCalled(); + }); + + it('never provisions or opens the authorize login page for the home signal server', async () => { + await expect(service.ensureCredentialForServerUrl(HOME_URL)).resolves.toBe(false); + + expect(signalServerAuth.ensureProvisioned).not.toHaveBeenCalled(); + expect(router.navigate).not.toHaveBeenCalled(); + }); + + it('restores the home credential from the legacy token store instead of provisioning', async () => { + signalServerAuth.hasValidCredential + .mockReturnValueOnce(false) + .mockReturnValue(true); + + await expect(service.ensureCredentialForServerUrl(HOME_URL)).resolves.toBe(true); + + expect(signalServerAuth.migrateHomeCredential).toHaveBeenCalledWith(homeUser); + expect(signalServerAuth.ensureProvisioned).not.toHaveBeenCalled(); + expect(router.navigate).not.toHaveBeenCalled(); + }); + + it('treats scheme variants of the home url as the home server', async () => { + await expect(service.ensureCredentialForServerUrl('wss://signal.toju.app/')).resolves.toBe(false); + + expect(signalServerAuth.ensureProvisioned).not.toHaveBeenCalled(); + expect(router.navigate).not.toHaveBeenCalled(); + }); + + it('still provisions foreign servers and navigates to authorize when the secret is missing', async () => { + await expect(service.ensureCredentialForServerUrl(FOREIGN_URL)).resolves.toBe(false); + + expect(signalServerAuth.ensureProvisioned).toHaveBeenCalledWith(FOREIGN_URL, homeUser); + expect(router.navigate).toHaveBeenCalledWith(['/login'], expect.objectContaining({ + queryParams: expect.objectContaining({ mode: 'authorize' }) + })); + }); + + it('returns true when foreign provisioning succeeds', async () => { + signalServerAuth.ensureProvisioned.mockResolvedValue({ kind: 'provisioned', result: {} }); + + await expect(service.ensureCredentialForServerUrl(FOREIGN_URL)).resolves.toBe(true); + expect(router.navigate).not.toHaveBeenCalled(); + }); +}); diff --git a/toju-app/src/app/domains/authentication/application/services/signal-server-authorize.service.ts b/toju-app/src/app/domains/authentication/application/services/signal-server-authorize.service.ts index 6375ae1..ced596a 100644 --- a/toju-app/src/app/domains/authentication/application/services/signal-server-authorize.service.ts +++ b/toju-app/src/app/domains/authentication/application/services/signal-server-authorize.service.ts @@ -7,6 +7,7 @@ import { ServerDirectoryFacade } from '../../../server-directory'; import { AUTH_MODE_AUTHORIZE, buildLoginReturnQueryParams } from '../../domain/logic/auth-navigation.rules'; import { isEndpointOnlineForConnection } from '../../../server-directory/domain/logic/server-endpoint-connectivity.rules'; import { shouldNavigateToAuthorizeSignalServer } from '../../domain/logic/signal-server-authorize.rules'; +import { isSameSignalServerUrl } from '../../domain/logic/signal-server-auth-failure.rules'; import { SignalServerAuthService } from './signal-server-auth.service'; @Injectable({ providedIn: 'root' }) @@ -27,6 +28,16 @@ export class SignalServerAuthorizeService { return false; } + if (isSameSignalServerUrl(serverUrl, currentUser.homeSignalServerUrl)) { + // The home account is never auto-provisioned or re-authorized as a + // foreign account. Resurrect the credential from the legacy token store + // (session-restore case); a genuinely missing home session is handled by + // the session-expiry flow, not the authorize login page. + this.signalServerAuth.migrateHomeCredential(currentUser); + + return this.signalServerAuth.hasValidCredential(serverUrl); + } + let result; try { 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 a3b83bf..b0c12e6 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, + resolveSessionExpiredNavigation, resolveUnauthenticatedStartupRedirect, waitForAuthenticationOutcome } from './auth-navigation.rules'; @@ -89,6 +90,28 @@ describe('resolveUnauthenticatedStartupRedirect', () => { }); }); +describe('resolveSessionExpiredNavigation', () => { + const currentUser = { id: 'user-1' }; + + it('keeps an authenticated user on protected routes', () => { + expect(resolveSessionExpiredNavigation(currentUser, '/room/abc')).toEqual({ kind: 'stay' }); + }); + + it('leaves the login page when the in-memory user is still authenticated', () => { + expect(resolveSessionExpiredNavigation(currentUser, '/login?returnUrl=%2Fservers')).toEqual({ + kind: 'leave-auth-route', + returnUrl: '/servers' + }); + }); + + it('sends fully signed-out users to login with a safe returnUrl', () => { + expect(resolveSessionExpiredNavigation(null, '/room/abc')).toEqual({ + kind: 'navigate-login', + queryParams: { returnUrl: '/room/abc' } + }); + }); +}); + 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 77d1ed0..a0c0800 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 @@ -134,6 +134,39 @@ export function isAuthorizeAuthMode(mode: string | null | undefined): boolean { return mode?.trim() === AUTH_MODE_AUTHORIZE; } +export type SessionExpiredNavigation = + | { kind: 'stay' } + | { kind: 'navigate-login'; queryParams: Record } + | { kind: 'leave-auth-route'; returnUrl: string }; + +/** + * Decide how to react when a persisted session is rejected as expired. + * A live in-memory user must never be left on the login page while the rail + * still treats them as authenticated. + */ +export function resolveSessionExpiredNavigation( + currentUser: Pick | null | undefined, + currentUrl: string +): SessionExpiredNavigation { + if (currentUser) { + const path = getRoutePathFromUrl(currentUrl); + + if (isAuthRoutePath(path)) { + return { + kind: 'leave-auth-route', + returnUrl: resolveSafeReturnUrl(extractReturnUrlParam(currentUrl)) + }; + } + + return { kind: 'stay' }; + } + + return { + kind: 'navigate-login', + queryParams: buildLoginReturnQueryParams(currentUrl) + }; +} + export function waitForAuthenticationOutcome( actions$: Observable<{ type: string; user?: User; error?: string }> ): Observable { diff --git a/toju-app/src/app/domains/authentication/domain/logic/auth-session.rules.spec.ts b/toju-app/src/app/domains/authentication/domain/logic/auth-session.rules.spec.ts index 3d28dc4..6a60015 100644 --- a/toju-app/src/app/domains/authentication/domain/logic/auth-session.rules.spec.ts +++ b/toju-app/src/app/domains/authentication/domain/logic/auth-session.rules.spec.ts @@ -7,7 +7,8 @@ import type { User } from '../../../../shared-kernel'; import { SESSION_EXPIRED_ERROR_CODE, collectSessionTokenLookupUrls, - hasValidPersistedSession + hasValidPersistedSession, + resolveAuthenticatedLocalUserId } from './auth-session.rules'; describe('auth-session.rules', () => { @@ -46,4 +47,22 @@ describe('auth-session.rules', () => { it('exports a stable session-expired error code', () => { expect(SESSION_EXPIRED_ERROR_CODE).toBe('SESSION_EXPIRED'); }); + + describe('resolveAuthenticatedLocalUserId', () => { + it('prefers the persisted user id', () => { + expect(resolveAuthenticatedLocalUserId('stored-id', { id: 'live-id' })).toBe('stored-id'); + }); + + it('falls back to the live in-memory user when persisted scope was cleared', () => { + // A transient clearStoredCurrentUserId() must never make a logged-in + // user look signed out (half-logged-in login page with the rail visible). + expect(resolveAuthenticatedLocalUserId(null, { id: 'live-id' })).toBe('live-id'); + expect(resolveAuthenticatedLocalUserId('', { id: 'live-id' })).toBe('live-id'); + }); + + it('returns null when neither source knows a user', () => { + expect(resolveAuthenticatedLocalUserId(null, null)).toBeNull(); + expect(resolveAuthenticatedLocalUserId(null, undefined)).toBeNull(); + }); + }); }); diff --git a/toju-app/src/app/domains/authentication/domain/logic/auth-session.rules.ts b/toju-app/src/app/domains/authentication/domain/logic/auth-session.rules.ts index b8c86e1..2a9a8c2 100644 --- a/toju-app/src/app/domains/authentication/domain/logic/auth-session.rules.ts +++ b/toju-app/src/app/domains/authentication/domain/logic/auth-session.rules.ts @@ -30,6 +30,19 @@ export function hasValidSessionTokenForUrls( return urls.some((url) => !!getToken(url)); } +/** + * Resolve the acting local user id from the persisted storage scope with a + * fallback to the live in-memory user. A transient `clearStoredCurrentUserId()` + * (e.g. during signal-server auth churn) must never make a logged-in user look + * signed out. + */ +export function resolveAuthenticatedLocalUserId( + storedUserId: string | null | undefined, + currentUser: Pick | null | undefined +): string | null { + return storedUserId || currentUser?.id || null; +} + export function hasValidPersistedSession( user: Pick, activeServerUrl: string | null | undefined, diff --git a/toju-app/src/app/domains/authentication/domain/logic/signal-server-auth-failure.rules.spec.ts b/toju-app/src/app/domains/authentication/domain/logic/signal-server-auth-failure.rules.spec.ts new file mode 100644 index 0000000..3301df5 --- /dev/null +++ b/toju-app/src/app/domains/authentication/domain/logic/signal-server-auth-failure.rules.spec.ts @@ -0,0 +1,99 @@ +import { + describe, + expect, + it +} from 'vitest'; +import { isSameSignalServerUrl, resolveSignalServerAuthFailure } from './signal-server-auth-failure.rules'; + +describe('isSameSignalServerUrl', () => { + it('matches identical http urls ignoring trailing slashes', () => { + expect(isSameSignalServerUrl('https://signal.toju.app', 'https://signal.toju.app/')).toBe(true); + }); + + it('matches ws/wss urls against their http/https equivalents', () => { + expect(isSameSignalServerUrl('wss://signal.toju.app', 'https://signal.toju.app')).toBe(true); + expect(isSameSignalServerUrl('ws://localhost:3001', 'http://localhost:3001')).toBe(true); + }); + + it('does not match different hosts or missing urls', () => { + expect(isSameSignalServerUrl('https://signal.toju.app', 'https://signal-sweden.toju.app')).toBe(false); + expect(isSameSignalServerUrl('https://signal.toju.app', undefined)).toBe(false); + expect(isSameSignalServerUrl('', 'https://signal.toju.app')).toBe(false); + }); + + it('treats host case-insensitively', () => { + expect(isSameSignalServerUrl('https://Signal.Toju.App', 'https://signal.toju.app')).toBe(true); + }); +}); + +describe('resolveSignalServerAuthFailure', () => { + it('re-identifies on any auth failure while a valid credential and retry budget exist', () => { + for (const reason of ['auth_required', 'auth_error'] as const) { + expect(resolveSignalServerAuthFailure({ + reason, + hasValidCredential: true, + retryAllowed: true, + isHomeServer: true + })).toBe('reidentify'); + } + }); + + it('never tears down a session for auth_required while the local credential is still valid', () => { + // auth_required only means a message raced ahead of identify - the server + // never evaluated the token, so exhausted retries must not log the user out. + expect(resolveSignalServerAuthFailure({ + reason: 'auth_required', + hasValidCredential: true, + retryAllowed: false, + isHomeServer: true + })).toBe('ignore'); + + expect(resolveSignalServerAuthFailure({ + reason: 'auth_required', + hasValidCredential: true, + retryAllowed: false, + isHomeServer: false + })).toBe('ignore'); + }); + + it('expires the home session when the home server rejects an identify token', () => { + expect(resolveSignalServerAuthFailure({ + reason: 'auth_error', + hasValidCredential: true, + retryAllowed: false, + isHomeServer: true + })).toBe('expire-home-session'); + + expect(resolveSignalServerAuthFailure({ + reason: 'auth_error', + hasValidCredential: false, + retryAllowed: false, + isHomeServer: true + })).toBe('expire-home-session'); + }); + + it('re-provisions foreign servers when their credential is rejected or missing', () => { + expect(resolveSignalServerAuthFailure({ + reason: 'auth_error', + hasValidCredential: false, + retryAllowed: false, + isHomeServer: false + })).toBe('provision-foreign'); + + expect(resolveSignalServerAuthFailure({ + reason: 'auth_required', + hasValidCredential: false, + retryAllowed: false, + isHomeServer: false + })).toBe('provision-foreign'); + }); + + it('expires the home session when auth_required arrives with no resolvable home credential', () => { + expect(resolveSignalServerAuthFailure({ + reason: 'auth_required', + hasValidCredential: false, + retryAllowed: false, + isHomeServer: true + })).toBe('expire-home-session'); + }); +}); diff --git a/toju-app/src/app/domains/authentication/domain/logic/signal-server-auth-failure.rules.ts b/toju-app/src/app/domains/authentication/domain/logic/signal-server-auth-failure.rules.ts new file mode 100644 index 0000000..8a7d4f3 --- /dev/null +++ b/toju-app/src/app/domains/authentication/domain/logic/signal-server-auth-failure.rules.ts @@ -0,0 +1,67 @@ +export type SignalServerAuthFailureReason = 'auth_required' | 'auth_error'; + +export type SignalServerAuthFailureResolution = + | 'reidentify' + | 'ignore' + | 'expire-home-session' + | 'provision-foreign'; + +function normalizeSignalServerUrlForComparison(serverUrl: string): string { + const withHttpScheme = serverUrl + .trim() + .replace(/\/+$/, '') + .replace(/^ws/i, 'http'); + + try { + const parsed = new URL(withHttpScheme); + const portSuffix = parsed.port ? `:${parsed.port}` : ''; + const path = parsed.pathname.replace(/\/+$/, ''); + + return `${parsed.protocol.toLowerCase()}//${parsed.hostname.toLowerCase()}${portSuffix}${path}`; + } catch { + return withHttpScheme.toLowerCase(); + } +} + +/** + * Compare two signal-server URLs regardless of ws/http scheme flavor, + * trailing slashes, or host casing. Used to decide whether an auth event + * concerns the user's home signal server. + */ +export function isSameSignalServerUrl( + leftUrl: string | null | undefined, + rightUrl: string | null | undefined +): boolean { + if (!leftUrl?.trim() || !rightUrl?.trim()) { + return false; + } + + return normalizeSignalServerUrlForComparison(leftUrl) === normalizeSignalServerUrlForComparison(rightUrl); +} + +/** + * Decide how to react to a signal-server auth failure message. + * + * `auth_required` is sent when any non-identify message arrives before the + * connection authenticated - it says nothing about token validity, so while a + * locally valid credential exists it may only trigger a (bounded) re-identify, + * never a session teardown. `auth_error` is the server rejecting the identify + * token itself: after the transient-retry budget, the credential is genuinely + * unusable, which means session expiry (home) or re-provisioning (foreign). + */ +export function resolveSignalServerAuthFailure(params: { + reason: SignalServerAuthFailureReason; + hasValidCredential: boolean; + retryAllowed: boolean; + isHomeServer: boolean; +}): SignalServerAuthFailureResolution { + if (params.hasValidCredential && params.retryAllowed) { + return 'reidentify'; + } + + if (params.reason === 'auth_required' && params.hasValidCredential) { + return 'ignore'; + } + + return params.isHomeServer ? 'expire-home-session' : 'provision-foreign'; +} diff --git a/toju-app/src/app/domains/authentication/feature/login/login.component.ts b/toju-app/src/app/domains/authentication/feature/login/login.component.ts index 9785d7a..c2de04c 100644 --- a/toju-app/src/app/domains/authentication/feature/login/login.component.ts +++ b/toju-app/src/app/domains/authentication/feature/login/login.component.ts @@ -2,6 +2,7 @@ import { Component, computed, + effect, inject, OnInit, signal @@ -13,11 +14,7 @@ import { Actions } from '@ngrx/effects'; import { Store } from '@ngrx/store'; import { NgIcon, provideIcons } from '@ng-icons/core'; import { lucideLogIn } from '@ng-icons/lucide'; -import { - filter, - firstValueFrom, - take -} from 'rxjs'; +import { firstValueFrom } from 'rxjs'; import { AuthenticationService } from '../../application/services/authentication.service'; import { ServerDirectoryFacade } from '../../../server-directory'; @@ -71,8 +68,27 @@ export class LoginComponent implements OnInit { private auth = inject(AuthenticationService); private actions$ = inject(Actions); private store = inject(Store); - private route = inject(ActivatedRoute); - private router = inject(Router); + private readonly route = inject(ActivatedRoute); + private readonly router = inject(Router); + private readonly currentUser = this.store.selectSignal(selectCurrentUser); + + constructor() { + effect(() => { + if (this.isAuthorizeMode()) { + return; + } + + const user = this.currentUser(); + + if (!user) { + return; + } + + const returnUrl = resolveSafeReturnUrl(this.route.snapshot.queryParamMap.get('returnUrl')); + + void this.router.navigateByUrl(returnUrl); + }); + } /** TrackBy function for server list rendering. */ trackById(_index: number, item: { id: string }) { return item.id; } @@ -86,20 +102,6 @@ export class LoginComponent implements OnInit { if (requestedServerId) { this.serverId = requestedServerId; } - - if (this.isAuthorizeMode()) { - return; - } - - this.store.select(selectCurrentUser).pipe( - filter(Boolean), - take(1) - ) - .subscribe(() => { - const returnUrl = resolveSafeReturnUrl(this.route.snapshot.queryParamMap.get('returnUrl')); - - void this.router.navigateByUrl(returnUrl); - }); } /** Validate and submit the login form, then navigate to search on success. */ diff --git a/toju-app/src/app/features/servers/servers-rail/servers-rail.component.ts b/toju-app/src/app/features/servers/servers-rail/servers-rail.component.ts index 2c040f9..e70cc66 100644 --- a/toju-app/src/app/features/servers/servers-rail/servers-rail.component.ts +++ b/toju-app/src/app/features/servers/servers-rail/servers-rail.component.ts @@ -28,6 +28,8 @@ import { import { Room, User } from '../../../shared-kernel'; import { buildLoginReturnQueryParams } from '../../../domains/authentication/domain/logic/auth-navigation.rules'; +import { resolveAuthenticatedLocalUserId } from '../../../domains/authentication/domain/logic/auth-session.rules'; +import { setStoredCurrentUserId } from '../../../core/storage/current-user-storage'; import { UserBarComponent } from '../../../domains/authentication/feature/user-bar/user-bar.component'; import { VoiceSessionFacade } from '../../../domains/voice-session'; import { selectSavedRooms, selectCurrentRoom } from '../../../store/rooms/rooms.selectors'; @@ -274,7 +276,11 @@ export class ServersRailComponent { joinSavedRoom(room: Room): void { const targetRoom = this.savedRooms().find((savedRoom) => savedRoom.id === room.id) ?? room; - const currentUserId = localStorage.getItem('metoyou_currentUserId'); + const currentUser = this.currentUser(); + const currentUserId = resolveAuthenticatedLocalUserId( + localStorage.getItem('metoyou_currentUserId'), + currentUser + ); if (!currentUserId) { this.router.navigate(['/login'], { @@ -284,6 +290,10 @@ export class ServersRailComponent { return; } + if (currentUser) { + setStoredCurrentUserId(currentUser.id); + } + if (this.isRoomMarkedBanned(targetRoom)) { this.bannedServerName.set(targetRoom.name); this.showBannedDialog.set(true); @@ -585,12 +595,19 @@ export class ServersRailComponent { } private requestJoinInBackground(room: Room, password?: string) { - const currentUserId = localStorage.getItem('metoyou_currentUserId'); const currentUser = this.currentUser(); + const currentUserId = resolveAuthenticatedLocalUserId( + localStorage.getItem('metoyou_currentUserId'), + currentUser + ); if (!currentUserId) return EMPTY; + if (currentUser) { + setStoredCurrentUserId(currentUser.id); + } + this.joinPasswordError.set(null); return from(this.resolveRoomJoinTarget(room)).pipe( diff --git a/toju-app/src/app/infrastructure/realtime/realtime-session.service.ts b/toju-app/src/app/infrastructure/realtime/realtime-session.service.ts index 65bf2d1..234d753 100644 --- a/toju-app/src/app/infrastructure/realtime/realtime-session.service.ts +++ b/toju-app/src/app/infrastructure/realtime/realtime-session.service.ts @@ -312,7 +312,11 @@ export class WebRTCService implements OnDestroy { private handleSignalingMessage(message: IncomingSignalingMessage, signalUrl: string): void { if (message.type === 'auth_required' || message.type === 'auth_error') { - this.store.dispatch(UsersActions.signalServerAuthFailed({ signalUrl })); + this.store.dispatch(UsersActions.signalServerAuthFailed({ + signalUrl, + reason: message.type + })); + return; } diff --git a/toju-app/src/app/store/users/users.actions.ts b/toju-app/src/app/store/users/users.actions.ts index 8aa5204..116d7f3 100644 --- a/toju-app/src/app/store/users/users.actions.ts +++ b/toju-app/src/app/store/users/users.actions.ts @@ -16,6 +16,7 @@ import { GameActivity } from '../../shared-kernel'; import type { LoginResponse } from '../../domains/authentication/domain/models/authentication.model'; +import type { SignalServerAuthFailureReason } from '../../domains/authentication/domain/logic/signal-server-auth-failure.rules'; export const UsersActions = createActionGroup({ source: 'Users', @@ -27,7 +28,7 @@ export const UsersActions = createActionGroup({ provisioned?: boolean; }>(), 'Revoke Signal Server Credential': props<{ serverUrl: string }>(), - 'Signal Server Auth Failed': props<{ signalUrl: string }>(), + 'Signal Server Auth Failed': props<{ signalUrl: string; reason: SignalServerAuthFailureReason }>(), 'Load Current User': emptyProps(), 'Load Current User Success': props<{ user: User }>(), 'Load Current User Failure': props<{ error: string }>(), diff --git a/toju-app/src/app/store/users/users.effects.ts b/toju-app/src/app/store/users/users.effects.ts index 7a98398..6bf8a42 100644 --- a/toju-app/src/app/store/users/users.effects.ts +++ b/toju-app/src/app/store/users/users.effects.ts @@ -55,7 +55,8 @@ import { AppI18nService } from '../../core/i18n'; import { AuthTokenStoreService } from '../../domains/authentication/application/services/auth-token-store.service'; import { SignalServerAuthService } from '../../domains/authentication/application/services/signal-server-auth.service'; import { hasValidPersistedSession, SESSION_EXPIRED_ERROR_CODE } from '../../domains/authentication/domain/logic/auth-session.rules'; -import { buildLoginReturnQueryParams } from '../../domains/authentication/domain/logic/auth-navigation.rules'; +import { isSameSignalServerUrl, resolveSignalServerAuthFailure } from '../../domains/authentication/domain/logic/signal-server-auth-failure.rules'; +import { resolveSessionExpiredNavigation } from '../../domains/authentication/domain/logic/auth-navigation.rules'; type IncomingModerationExtraAction = | ReturnType @@ -126,43 +127,43 @@ export class UsersEffects { { dispatch: false } ); - /** Re-provisions or logs out depending on which signal server rejected auth. */ + /** Re-identifies, re-provisions, or logs out depending on why and where auth failed. */ signalServerAuthFailed$ = createEffect(() => this.actions$.pipe( ofType(UsersActions.signalServerAuthFailed), withLatestFrom(this.store.select(selectCurrentUser)), - switchMap(([{ signalUrl }, currentUser]) => { + switchMap(([{ signalUrl, reason }, currentUser]) => { const normalizedSignalUrl = signalUrl.replace(/^ws/i, 'http').replace(/\/+$/, ''); - const homeSignalServerUrl = currentUser?.homeSignalServerUrl?.replace(/\/+$/, ''); + const hasValidCredential = !!currentUser + && this.signalServerAuth.hasValidCredential(normalizedSignalUrl); + const resolution = resolveSignalServerAuthFailure({ + reason, + hasValidCredential, + // Only consume the retry budget when a re-identify is actually possible. + retryAllowed: hasValidCredential && this.shouldRetrySignalIdentify(normalizedSignalUrl), + isHomeServer: isSameSignalServerUrl(signalUrl, currentUser?.homeSignalServerUrl) + }); - // A rejection while we still hold a valid credential is almost always - // transient (a message raced ahead of identify on a (re)connect). Re-identify - // with the existing credential instead of tearing down the session or - // provisioning a duplicate account. Bounded so a genuinely invalid token - // (server-side revocation) still falls through to expiry/provisioning. - if ( - currentUser - && this.signalServerAuth.hasValidCredential(normalizedSignalUrl) - && this.shouldRetrySignalIdentify(normalizedSignalUrl) - ) { - this.webrtc.identify( - currentUser.oderId || currentUser.id, - this.resolveDisplayName(currentUser), - signalUrl, - { - description: currentUser.description, - profileUpdatedAt: currentUser.profileUpdatedAt, - homeSignalServerUrl: currentUser.homeSignalServerUrl - } - ); + if (resolution === 'reidentify' && currentUser) { + // Almost always transient (a message raced ahead of identify on a + // (re)connect). Re-identify with the existing credential instead of + // tearing down the session or provisioning a duplicate account. + this.identifyOnSignalUrl(currentUser, signalUrl); return EMPTY; } + if (resolution === 'ignore') { + // auth_required never means the token is invalid - the server only + // saw a message before identify completed. With the retry budget + // spent, wait for the in-flight identify instead of logging out. + return EMPTY; + } + this.signalServerAuthRetries.delete(normalizedSignalUrl); this.signalServerAuth.clearCredential(normalizedSignalUrl); - if (homeSignalServerUrl && normalizedSignalUrl === homeSignalServerUrl) { + if (resolution === 'expire-home-session') { clearStoredCurrentUserId(); return of(UsersActions.loadCurrentUserFailure({ error: SESSION_EXPIRED_ERROR_CODE })); @@ -170,21 +171,8 @@ export class UsersEffects { return from(this.signalServerAuth.ensureProvisioned(normalizedSignalUrl, currentUser)).pipe( mergeMap((result) => { - if (result.kind === 'provisioned' || result.kind === 'existing') { - if (currentUser) { - this.webrtc.identify( - currentUser.oderId || currentUser.id, - this.resolveDisplayName(currentUser), - signalUrl, - { - description: currentUser.description, - profileUpdatedAt: currentUser.profileUpdatedAt, - homeSignalServerUrl: currentUser.homeSignalServerUrl - } - ); - } - - return EMPTY; + if ((result.kind === 'provisioned' || result.kind === 'existing') && currentUser) { + this.identifyOnSignalUrl(currentUser, signalUrl); } return EMPTY; @@ -195,6 +183,19 @@ export class UsersEffects { ) ); + private identifyOnSignalUrl(currentUser: User, signalUrl: string): void { + this.webrtc.identify( + currentUser.oderId || currentUser.id, + this.resolveDisplayName(currentUser), + signalUrl, + { + description: currentUser.description, + profileUpdatedAt: currentUser.profileUpdatedAt, + homeSignalServerUrl: currentUser.homeSignalServerUrl + } + ); + } + /** Provisions missing credentials for active signal servers after home login loads. */ provisionActiveSignalServers$ = createEffect( () => @@ -229,6 +230,8 @@ export class UsersEffects { const sanitizedUser = this.clearStartupVoiceConnection(user); + this.signalServerAuth.migrateHomeCredential(sanitizedUser); + if (!this.hasPersistedSessionToken(sanitizedUser)) { clearStoredCurrentUserId(); @@ -626,14 +629,24 @@ export class UsersEffects { filter(({ error }) => error === SESSION_EXPIRED_ERROR_CODE), withLatestFrom(this.store.select(selectCurrentUser)), tap(([, currentUser]) => { + const navigation = resolveSessionExpiredNavigation(currentUser, this.router.url); + if (currentUser) { setStoredCurrentUserId(currentUser.id); + } + + if (navigation.kind === 'stay') { + return; + } + + if (navigation.kind === 'leave-auth-route') { + void this.router.navigateByUrl(navigation.returnUrl); return; } clearStoredCurrentUserId(); void this.router.navigate(['/login'], { - queryParams: buildLoginReturnQueryParams(this.router.url) + queryParams: navigation.queryParams }); }) ),