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 <cursoragent@cursor.com>
2026-07-14 01:54:01 +02:00
parent 59dfd2de85
commit d3d22846e7
14 changed files with 491 additions and 68 deletions
@@ -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 = {
@@ -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<string, string> }
+  | { 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<User, 'id'> | 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<AuthenticationOutcome> {
@@ -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();
+    });
+  });
 });
@@ -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<User, 'id'> | null | undefined
+): string | null {
+  return storedUserId || currentUser?.id || null;
+}
+
 export function hasValidPersistedSession(
  user: Pick<User, 'homeSignalServerUrl'>,
  activeServerUrl: string | null | undefined,
@@ -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');
+  });
+});
@@ -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';
+}