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 <cursoragent@cursor.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
88
e2e/tests/auth/offline-signal-server-no-login-loop.spec.ts
Normal file
88
e2e/tests/auth/offline-signal-server-no-login-loop.spec.ts
Normal file
@@ -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<string, unknown>;
|
||||
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();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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<void> {
|
||||
const endpoint = this.serverDirectory.ensureServerEndpoint({
|
||||
name: this.buildEndpointName(serverUrl),
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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';
|
||||
}
|
||||
@@ -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(
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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';
|
||||
}
|
||||
Reference in New Issue
Block a user