fix(offline,server,docs): apply PR #116 review findings to offline-sync
Carries the still-applicable findings from the PR #116 review into PR #120, since #120 includes the mobile-bootstrap commit. CORS hardening (removing the unconditional localhost/capacitor origins) is intentionally left out of scope. Service worker routing (latent bug fix + testability): - vite.config.ts: anchor Workbox path matching to a segment boundary (^/<seg>(/|$)) instead of startsWith, so siblings like /apidocs, /collaborators, /socket.iox are no longer mis-routed as API/realtime and forced NetworkOnly; align navigateFallbackDenylist with the same anchors. - new apps/client/src/pwa/sw-strategy.ts holds the canonical predicates (isApiPath, isCollabOrSocketPath) + unit tests; the vite.config regexes mirror it inline (Workbox generateSW serializes urlPattern fns standalone, so they cannot import the module). Server CORS (R1 extraction + coverage): - extract buildCorsAllowlist / isOriginAllowed into cors.util.ts with unit tests (evil-origin rejected, WebView/no-Origin allowed); main.ts rewired to use them with byte-for-byte identical behavior. Privacy — clear offline cache on logout: - new clear-offline-cache.ts purges the persisted query cache (idb-keyval gitmost-rq-cache), the Yjs page.* IndexedDB databases, and the service-worker api-get-cache; wired into handleLogout (best-effort, before the redirect) so a previous user's private data does not linger locally. Conventions & docs: - prettier fixes on main.ts and login.dto.ts. - CHANGELOG: document offline reading, returnToken opt-in, optional Swagger, new env vars, logout cache-clear, and the CORS open->allowlist breaking change. - docs/mobile-app-plan.md: correct the now-false §2.4 claims and update the §12 checklist (native cap add ios left unchecked — generated locally, gitignored). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
committed by
claude code agent 227
parent
aa6a09633a
commit
c2e857bbbd
@@ -1,4 +1,10 @@
|
||||
import { IsBoolean, IsEmail, IsNotEmpty, IsOptional, IsString } from 'class-validator';
|
||||
import {
|
||||
IsBoolean,
|
||||
IsEmail,
|
||||
IsNotEmpty,
|
||||
IsOptional,
|
||||
IsString,
|
||||
} from 'class-validator';
|
||||
|
||||
export class LoginDto {
|
||||
@IsNotEmpty()
|
||||
|
||||
87
apps/server/src/integrations/environment/cors.util.spec.ts
Normal file
87
apps/server/src/integrations/environment/cors.util.spec.ts
Normal file
@@ -0,0 +1,87 @@
|
||||
import { buildCorsAllowlist, isOriginAllowed } from './cors.util';
|
||||
|
||||
const WEBVIEW_ORIGINS = [
|
||||
'capacitor://localhost',
|
||||
'ionic://localhost',
|
||||
'http://localhost',
|
||||
'https://localhost',
|
||||
];
|
||||
|
||||
describe('isOriginAllowed', () => {
|
||||
const allowlist = buildCorsAllowlist({
|
||||
appUrl: 'https://app.example',
|
||||
configuredOrigins: ['https://other.example'],
|
||||
});
|
||||
|
||||
it('allows requests with no Origin header', () => {
|
||||
expect(isOriginAllowed(undefined, allowlist)).toBe(true);
|
||||
expect(isOriginAllowed('', allowlist)).toBe(true);
|
||||
});
|
||||
|
||||
it('allows an exact allowlisted origin', () => {
|
||||
expect(isOriginAllowed('https://app.example', allowlist)).toBe(true);
|
||||
expect(isOriginAllowed('https://other.example', allowlist)).toBe(true);
|
||||
});
|
||||
|
||||
it('allows each native WebView origin', () => {
|
||||
for (const origin of WEBVIEW_ORIGINS) {
|
||||
expect(isOriginAllowed(origin, allowlist)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects a foreign credentialed origin', () => {
|
||||
// With credentials:true a foreign credentialed origin must be rejected.
|
||||
expect(isOriginAllowed('https://evil.example', allowlist)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects a trailing-slash mismatch', () => {
|
||||
expect(isOriginAllowed('https://app.example/', allowlist)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects a host-case mismatch', () => {
|
||||
expect(isOriginAllowed('https://APP.example', allowlist)).toBe(false);
|
||||
});
|
||||
|
||||
it('allows no-Origin but rejects cross-origin with an empty allowlist', () => {
|
||||
const empty: ReadonlySet<string> = new Set<string>();
|
||||
expect(isOriginAllowed(undefined, empty)).toBe(true);
|
||||
expect(isOriginAllowed('https://app.example', empty)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildCorsAllowlist', () => {
|
||||
it('contains the app URL, each configured origin, and all WebView origins', () => {
|
||||
const allowlist = buildCorsAllowlist({
|
||||
appUrl: 'https://app.example',
|
||||
configuredOrigins: ['https://a.example', 'https://b.example'],
|
||||
});
|
||||
|
||||
expect(allowlist.has('https://app.example')).toBe(true);
|
||||
expect(allowlist.has('https://a.example')).toBe(true);
|
||||
expect(allowlist.has('https://b.example')).toBe(true);
|
||||
for (const origin of WEBVIEW_ORIGINS) {
|
||||
expect(allowlist.has(origin)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('deduplicates when a configured origin coincides with the app URL', () => {
|
||||
const allowlist = buildCorsAllowlist({
|
||||
appUrl: 'https://app.example',
|
||||
configuredOrigins: ['https://app.example'],
|
||||
});
|
||||
|
||||
// app URL + 4 WebView origins, the duplicate configured origin collapses.
|
||||
expect(allowlist.size).toBe(1 + WEBVIEW_ORIGINS.length);
|
||||
});
|
||||
|
||||
it('always includes the four WebView origins even with no configured origins', () => {
|
||||
const allowlist = buildCorsAllowlist({
|
||||
appUrl: 'https://app.example',
|
||||
configuredOrigins: [],
|
||||
});
|
||||
|
||||
for (const origin of WEBVIEW_ORIGINS) {
|
||||
expect(allowlist.has(origin)).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
39
apps/server/src/integrations/environment/cors.util.ts
Normal file
39
apps/server/src/integrations/environment/cors.util.ts
Normal file
@@ -0,0 +1,39 @@
|
||||
// CORS trust boundary helpers. `buildCorsAllowlist` produces the exact set of
|
||||
// origins the API trusts, and `isOriginAllowed` is the predicate the enableCors
|
||||
// origin callback uses to accept/reject each request. With credentials:true a
|
||||
// foreign credentialed origin must never be allowed, so anything not in the
|
||||
// allowlist (apart from no-Origin requests) is rejected.
|
||||
|
||||
// Native WebView origins used by the Capacitor/Ionic mobile shell. Always
|
||||
// trusted so the native client can call the API. CORS hardening of these is
|
||||
// intentionally out of scope.
|
||||
const NATIVE_WEBVIEW_ORIGINS = [
|
||||
'capacitor://localhost',
|
||||
'ionic://localhost',
|
||||
'http://localhost',
|
||||
'https://localhost',
|
||||
] as const;
|
||||
|
||||
// Build the CORS allowlist: the app URL, all configured cross-origin clients,
|
||||
// and the native WebView origins. Dedup is automatic via Set.
|
||||
export function buildCorsAllowlist(input: {
|
||||
appUrl: string;
|
||||
configuredOrigins: readonly string[];
|
||||
}): Set<string> {
|
||||
return new Set<string>([
|
||||
input.appUrl,
|
||||
...input.configuredOrigins,
|
||||
...NATIVE_WEBVIEW_ORIGINS,
|
||||
]);
|
||||
}
|
||||
|
||||
// Decide whether a request's Origin is allowed. A missing Origin header (curl,
|
||||
// server-to-server, some native WebViews) is allowed; otherwise the origin must
|
||||
// be present in the allowlist.
|
||||
export function isOriginAllowed(
|
||||
origin: string | undefined,
|
||||
allowlist: ReadonlySet<string>,
|
||||
): boolean {
|
||||
if (!origin) return true;
|
||||
return allowlist.has(origin);
|
||||
}
|
||||
@@ -15,6 +15,10 @@ import { InternalLogFilter } from './common/logger/internal-log-filter';
|
||||
import { EnvironmentService } from './integrations/environment/environment.service';
|
||||
import { resolveFrameHeader } from './common/helpers';
|
||||
import { resolveTrustProxy } from './integrations/environment/trust-proxy.util';
|
||||
import {
|
||||
buildCorsAllowlist,
|
||||
isOriginAllowed,
|
||||
} from './integrations/environment/cors.util';
|
||||
import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger';
|
||||
|
||||
async function bootstrap() {
|
||||
@@ -154,25 +158,19 @@ async function bootstrap() {
|
||||
// The web client is same-origin in production; an explicit allowlist lets
|
||||
// native/mobile WebView origins (Capacitor) and any configured cross-origin
|
||||
// clients call the API, while everything else is rejected.
|
||||
const corsAllowedOrigins = new Set<string>([
|
||||
environmentService.getAppUrl(),
|
||||
...environmentService.getCorsAllowedOrigins(),
|
||||
// Capacitor / Ionic WebView origins used by the native shell.
|
||||
'capacitor://localhost',
|
||||
'ionic://localhost',
|
||||
'http://localhost',
|
||||
'https://localhost',
|
||||
]);
|
||||
const corsAllowedOrigins = buildCorsAllowlist({
|
||||
appUrl: environmentService.getAppUrl(),
|
||||
configuredOrigins: environmentService.getCorsAllowedOrigins(),
|
||||
});
|
||||
|
||||
app.enableCors({
|
||||
// Allow requests with no Origin header (curl, server-to-server, some native
|
||||
// WebView requests) and any origin in the allowlist; reject the rest.
|
||||
origin: (origin: string | undefined, callback: (err: Error | null, allow?: boolean) => void) => {
|
||||
if (!origin || corsAllowedOrigins.has(origin)) {
|
||||
callback(null, true);
|
||||
return;
|
||||
}
|
||||
callback(null, false);
|
||||
origin: (
|
||||
origin: string | undefined,
|
||||
callback: (err: Error | null, allow?: boolean) => void,
|
||||
) => {
|
||||
callback(null, isOriginAllowed(origin, corsAllowedOrigins));
|
||||
},
|
||||
credentials: true,
|
||||
methods: ['GET', 'HEAD', 'PUT', 'PATCH', 'POST', 'DELETE', 'OPTIONS'],
|
||||
|
||||
Reference in New Issue
Block a user