From cb59af6b6cdb01fbdf3a8211ea642ceae68807db Mon Sep 17 00:00:00 2001 From: Myx Date: Thu, 11 Jun 2026 22:08:53 +0200 Subject: [PATCH] fix: Bug - Login screen shows up on unreachable signal servers Skip authorize login navigation when a signal server endpoint is offline or unreachable; gate connection and credential flows on online status only. Co-authored-by: Cursor --- agents-docs/features/authentication.md | 2 + ...ffline-signal-server-no-login-loop.spec.ts | 88 +++++++++++++++++++ .../signal-server-authorize.service.ts | 34 +++++-- .../signal-server-authorize.rules.spec.ts | 44 ++++++++++ .../logic/signal-server-authorize.rules.ts | 18 ++++ .../services/server-directory.service.ts | 5 +- ...server-endpoint-connectivity.rules.spec.ts | 17 ++++ .../server-endpoint-connectivity.rules.ts | 8 ++ 8 files changed, 207 insertions(+), 9 deletions(-) create mode 100644 e2e/tests/auth/offline-signal-server-no-login-loop.spec.ts create mode 100644 toju-app/src/app/domains/authentication/domain/logic/signal-server-authorize.rules.spec.ts create mode 100644 toju-app/src/app/domains/authentication/domain/logic/signal-server-authorize.rules.ts create mode 100644 toju-app/src/app/domains/server-directory/domain/logic/server-endpoint-connectivity.rules.spec.ts create mode 100644 toju-app/src/app/domains/server-directory/domain/logic/server-endpoint-connectivity.rules.ts diff --git a/agents-docs/features/authentication.md b/agents-docs/features/authentication.md index e564d15..838a539 100644 --- a/agents-docs/features/authentication.md +++ b/agents-docs/features/authentication.md @@ -115,6 +115,8 @@ A per-install **provision secret** enables silent account creation on newly adde | 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 | +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). + Authorize UI: `/login?mode=authorize&serverId=…&returnUrl=…` (also supported on `/register`). Settings → Network shows per-endpoint `Authorized` / `Needs sign-in` badges. 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. diff --git a/e2e/tests/auth/offline-signal-server-no-login-loop.spec.ts b/e2e/tests/auth/offline-signal-server-no-login-loop.spec.ts new file mode 100644 index 0000000..7846c89 --- /dev/null +++ b/e2e/tests/auth/offline-signal-server-no-login-loop.spec.ts @@ -0,0 +1,88 @@ +import { expect } from '@playwright/test'; +import { test } from '../../fixtures/multi-client'; +import { openSettingsFromMenu } from '../../helpers/app-menu'; +import { expectDashboardReady } from '../../helpers/dashboard'; +import { installTestServerEndpoints } from '../../helpers/seed-test-endpoint'; +import { startTestServer } from '../../helpers/test-server'; +import { readSignalServerCredentialFromPage, SIGNAL_SERVER_CREDENTIALS_STORAGE_KEY } from '../../helpers/auth-api'; +import { RegisterPage } from '../../pages/register.page'; + +const PRIMARY_ENDPOINT_ID = 'e2e-offline-login-primary'; +const USER_PASSWORD = 'TestPass123!'; + +test.describe('Offline signal server navigation', () => { + test('does not redirect to authorize login after a foreign server goes offline', async ({ createClient }) => { + const primaryServer = await startTestServer(); + const secondaryServer = await startTestServer(); + const suffix = `offline_login_${Date.now()}`; + const username = `user_${suffix}`; + + try { + const client = await createClient(); + + await installTestServerEndpoints(client.context, [ + { + id: PRIMARY_ENDPOINT_ID, + name: 'E2E Primary Signal', + url: primaryServer.url, + isActive: true, + status: 'online' + } + ]); + + await test.step('Register and provision a secondary signal server', async () => { + const register = new RegisterPage(client.page); + + await register.goto(); + await register.register(username, 'Offline Login User', USER_PASSWORD); + await expectDashboardReady(client.page); + + await openSettingsFromMenu(client.page); + await client.page.getByRole('button', { name: 'Network' }).click(); + await client.page.getByPlaceholder('Server name').fill('E2E Secondary Signal'); + await client.page.getByPlaceholder('Server URL (e.g., http://localhost:3001)').fill(secondaryServer.url); + await client.page.getByTestId('add-signal-server-button').click(); + + await expect(client.page.getByText(secondaryServer.url)).toBeVisible({ timeout: 15_000 }); + await expect.poll(async () => + await readSignalServerCredentialFromPage(client.page, secondaryServer.url), + { timeout: 30_000 } + ).not.toBeNull(); + + await client.page.keyboard.press('Escape'); + }); + + await test.step('Offline secondary endpoints do not trigger authorize login', async () => { + await secondaryServer.stop(); + + await client.page.evaluate(({ storageKey, url }) => { + const normalizedUrl = url.trim().replace(/\/+$/, ''); + const credentialStore = JSON.parse(localStorage.getItem(storageKey) || '{}') as Record; + const nextCredentialStore = Object.fromEntries( + Object.entries(credentialStore).filter(([key]) => key !== normalizedUrl) + ); + + localStorage.setItem(storageKey, JSON.stringify(nextCredentialStore)); + + const endpoints = JSON.parse(localStorage.getItem('metoyou_server_endpoints') || '[]') as { + url: string; + status: string; + }[]; + + localStorage.setItem('metoyou_server_endpoints', JSON.stringify(endpoints.map((endpoint) => + endpoint.url.trim().replace(/\/+$/, '') === normalizedUrl + ? { ...endpoint, status: 'offline' } + : endpoint + ))); + }, { storageKey: SIGNAL_SERVER_CREDENTIALS_STORAGE_KEY, url: secondaryServer.url }); + + await client.page.goto('/dashboard', { waitUntil: 'commit', timeout: 10_000 }); + await expect(client.page).not.toHaveURL(/\/login/); + await expect(client.page.url()).not.toMatch(/mode=authorize/); + }); + } finally { + await primaryServer.stop(); + await secondaryServer.stop(); + } + }); +}); 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 b49691c..6375ae1 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 @@ -5,6 +5,8 @@ import { firstValueFrom } from 'rxjs'; import { selectCurrentUser } from '../../../../store/users/users.selectors'; 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 { SignalServerAuthService } from './signal-server-auth.service'; @Injectable({ providedIn: 'root' }) @@ -25,25 +27,43 @@ export class SignalServerAuthorizeService { return false; } - const result = await this.signalServerAuth.ensureProvisioned(serverUrl, currentUser); + let result; + + try { + result = await this.signalServerAuth.ensureProvisioned(serverUrl, currentUser); + } catch { + return false; + } if (result.kind === 'existing' || result.kind === 'provisioned') { return true; } - if (result.kind === 'collision') { - await this.navigateToAuthorize(serverUrl, this.router.url); - return false; - } + const endpointStatus = await this.resolveEndpointStatusForAuthorize(serverUrl); - if (result.kind === 'skipped' && result.reason === 'no-provision-secret') { + if (shouldNavigateToAuthorizeSignalServer(endpointStatus, result)) { await this.navigateToAuthorize(serverUrl, this.router.url); - return false; } return false; } + private async resolveEndpointStatusForAuthorize(serverUrl: string) { + const endpoint = this.serverDirectory.findServerByUrl(serverUrl); + + if (!endpoint) { + return null; + } + + if (isEndpointOnlineForConnection(endpoint.status) || endpoint.status === 'offline' || endpoint.status === 'incompatible') { + return endpoint.status; + } + + await this.serverDirectory.testServer(endpoint.id); + + return this.serverDirectory.servers().find((candidate) => candidate.id === endpoint.id)?.status ?? endpoint.status; + } + async navigateToAuthorize(serverUrl: string, returnUrl: string): Promise { const endpoint = this.serverDirectory.ensureServerEndpoint({ name: this.buildEndpointName(serverUrl), diff --git a/toju-app/src/app/domains/authentication/domain/logic/signal-server-authorize.rules.spec.ts b/toju-app/src/app/domains/authentication/domain/logic/signal-server-authorize.rules.spec.ts new file mode 100644 index 0000000..6c65a48 --- /dev/null +++ b/toju-app/src/app/domains/authentication/domain/logic/signal-server-authorize.rules.spec.ts @@ -0,0 +1,44 @@ +import { + describe, + expect, + it +} from 'vitest'; +import { shouldNavigateToAuthorizeSignalServer } from './signal-server-authorize.rules'; + +describe('signal-server-authorize rules', () => { + it('does not navigate to authorize when the signal server is offline', () => { + expect(shouldNavigateToAuthorizeSignalServer('offline', { + kind: 'skipped', + reason: 'no-provision-secret' + })).toBe(false); + + expect(shouldNavigateToAuthorizeSignalServer('offline', { + kind: 'collision', + error: new Error('collision') as never + })).toBe(false); + }); + + it('navigates to authorize on online servers that need manual sign-in', () => { + expect(shouldNavigateToAuthorizeSignalServer('online', { + kind: 'skipped', + reason: 'no-provision-secret' + })).toBe(true); + + expect(shouldNavigateToAuthorizeSignalServer('online', { + kind: 'collision', + error: new Error('collision') as never + })).toBe(true); + }); + + it('does not navigate for unknown endpoint status or non-authorize provision outcomes', () => { + expect(shouldNavigateToAuthorizeSignalServer('unknown', { + kind: 'skipped', + reason: 'no-provision-secret' + })).toBe(false); + + expect(shouldNavigateToAuthorizeSignalServer('online', { + kind: 'skipped', + reason: 'no-home-user' + })).toBe(false); + }); +}); diff --git a/toju-app/src/app/domains/authentication/domain/logic/signal-server-authorize.rules.ts b/toju-app/src/app/domains/authentication/domain/logic/signal-server-authorize.rules.ts new file mode 100644 index 0000000..b9728af --- /dev/null +++ b/toju-app/src/app/domains/authentication/domain/logic/signal-server-authorize.rules.ts @@ -0,0 +1,18 @@ +import type { EnsureProvisionedResult } from '../../application/services/signal-server-auth.service'; +import type { ServerEndpointStatus } from '../../../server-directory/domain/models/server-directory.model'; +import { isEndpointOnlineForConnection } from '../../../server-directory/domain/logic/server-endpoint-connectivity.rules'; + +export function shouldNavigateToAuthorizeSignalServer( + endpointStatus: ServerEndpointStatus | undefined | null, + provisionResult: EnsureProvisionedResult +): boolean { + if (!isEndpointOnlineForConnection(endpointStatus)) { + return false; + } + + if (provisionResult.kind === 'collision') { + return true; + } + + return provisionResult.kind === 'skipped' && provisionResult.reason === 'no-provision-secret'; +} diff --git a/toju-app/src/app/domains/server-directory/application/services/server-directory.service.ts b/toju-app/src/app/domains/server-directory/application/services/server-directory.service.ts index fb65796..1fd5913 100644 --- a/toju-app/src/app/domains/server-directory/application/services/server-directory.service.ts +++ b/toju-app/src/app/domains/server-directory/application/services/server-directory.service.ts @@ -26,6 +26,7 @@ import { type RoomSignalSource, type RoomSignalSourceInput } from '../../domain/logic/room-signal-source.logic'; +import { isEndpointOnlineForConnection } from '../../domain/logic/server-endpoint-connectivity.rules'; import { ServerEndpointCompatibilityService } from '../../infrastructure/services/server-endpoint-compatibility.service'; import { ServerEndpointHealthService } from '../../infrastructure/services/server-endpoint-health.service'; import { ServerEndpointStateService } from './server-endpoint-state.service'; @@ -110,7 +111,7 @@ export class ServerDirectoryService { const clientVersion = await this.endpointCompatibility.getClientVersion(); - if (!clientVersion) { + if (!clientVersion && isEndpointOnlineForConnection(endpoint.status)) { return true; } @@ -118,7 +119,7 @@ export class ServerDirectoryService { const refreshedEndpoint = this.servers().find((candidate) => candidate.id === endpoint.id); - return !!refreshedEndpoint && refreshedEndpoint.status !== 'incompatible'; + return isEndpointOnlineForConnection(refreshedEndpoint?.status); } resolveRoomEndpoint( diff --git a/toju-app/src/app/domains/server-directory/domain/logic/server-endpoint-connectivity.rules.spec.ts b/toju-app/src/app/domains/server-directory/domain/logic/server-endpoint-connectivity.rules.spec.ts new file mode 100644 index 0000000..b16a3e5 --- /dev/null +++ b/toju-app/src/app/domains/server-directory/domain/logic/server-endpoint-connectivity.rules.spec.ts @@ -0,0 +1,17 @@ +import { + describe, + expect, + it +} from 'vitest'; +import { isEndpointOnlineForConnection } from './server-endpoint-connectivity.rules'; + +describe('server-endpoint-connectivity rules', () => { + it('treats only online endpoints as reachable for connection', () => { + expect(isEndpointOnlineForConnection('online')).toBe(true); + expect(isEndpointOnlineForConnection('offline')).toBe(false); + expect(isEndpointOnlineForConnection('checking')).toBe(false); + expect(isEndpointOnlineForConnection('unknown')).toBe(false); + expect(isEndpointOnlineForConnection('incompatible')).toBe(false); + expect(isEndpointOnlineForConnection(undefined)).toBe(false); + }); +}); diff --git a/toju-app/src/app/domains/server-directory/domain/logic/server-endpoint-connectivity.rules.ts b/toju-app/src/app/domains/server-directory/domain/logic/server-endpoint-connectivity.rules.ts new file mode 100644 index 0000000..b010bdc --- /dev/null +++ b/toju-app/src/app/domains/server-directory/domain/logic/server-endpoint-connectivity.rules.ts @@ -0,0 +1,8 @@ +import type { ServerEndpointStatus } from '../models/server-directory.model'; + +/** True only when health checks report the endpoint is reachable. */ +export function isEndpointOnlineForConnection( + status: ServerEndpointStatus | undefined | null +): boolean { + return status === 'online'; +}