diff --git a/.github/workflows/ios-tests.yml b/.github/workflows/ios-tests.yml index 696a66585..ef51fdf3b 100644 --- a/.github/workflows/ios-tests.yml +++ b/.github/workflows/ios-tests.yml @@ -51,8 +51,12 @@ jobs: -quiet \ build + # --retries matches the iPad job. It covers macOS runner flake - cold + # simulator boots have repeatedly exceeded the launch budget - not + # product assertions. A test that fails every attempt still fails the + # job; retries never convert a real regression into a pass. - name: Detox iPhone tests - run: yarn detox test --configuration ios.sim.release --cleanup --record-logs all + run: yarn detox test --configuration ios.sim.release --cleanup --record-logs all --retries 2 - name: Upload artifacts if: failure() diff --git a/docs/NATIVE-NOTIFICATION-INTENTS.md b/docs/NATIVE-NOTIFICATION-INTENTS.md new file mode 100644 index 000000000..1dcf5a9ec --- /dev/null +++ b/docs/NATIVE-NOTIFICATION-INTENTS.md @@ -0,0 +1,64 @@ +# Native notification intents + +A notification tap resolves to an **intent**, computed from the notification +payload before any URL is built. Four kinds exist: + +| Kind | Meaning | +| --- | --- | +| `native` | an existing native screen (Topic, MemberProfile, Collection, Search, Bookmarks, Settings, Ask) | +| `badge` | `granted_badge`, presented by the native BadgeEarned screen | +| `staff_external` | staff-only `/admin`, the single documented external handoff | +| `unavailable` | one explicit bounded state for everything else | + +## Why the payload, not the URL + +`DiscourseUtils.endpointForSiteNotification` is lossy. A `granted_badge` +becomes `/badges/:id/basic?username=:u`, and `badge_name` is discarded. Every +earlier attempt to route badges started from that URL, which is why they all +ended at a web page: by then the information needed to build a native screen +was already gone. `notificationIntent` reads the payload first. + +## Type matrix + +**Native.** 1-11, 13, 14, 15, 17, 18, 20, 24, 25, 27, 28, 34, 36, 801, 802 open +Topic. 800 opens MemberProfile. 21 opens Topic when it carries one, and +otherwise resolves to `/u/:me/activity/approval-given`, which the profile +pattern already matches - a valid native destination, left as it was. + +**Badge.** 12 opens BadgeEarned. + +**Staff external.** 37 and 38 open `/admin` externally, for staff only. +`classifyFirstPartyMemberRoute` returns `privileged_external` only when +`isStaff` is true; a member gets `unavailable`. This is an explicit, documented +exception and does not widen external navigation for anyone else. + +**Unavailable.** 16, 19, 22, 23, 26, 29, 30, 31, 32, unknown and absent types. +No silent no-op, no WebView, no external browser, no second authentication. + +## BadgeEarned + +Renders `badge_name` only, from the payload, with a Close action. It makes **no +network request**, so it cannot stall or fail. + +`badge_title` is a boolean in Discourse - whether the badge may be worn as a +title - not descriptive text, and is never rendered. The payload carries no +badge description; showing one would require a fetch, which V1 does not do. + +## What was removed, and why + +A first-party WebView fallback and then an OTP-based authenticated WebView +session were both tried and abandoned. The OTP contract itself was eventually +correct - the request succeeded and the confirmation form rendered - but +Finish Login failed with "Missing, invalid or expired token", and more +importantly the architecture was wrong: a member already authenticated in the +app should never perform a second web authentication to read a notification. + +Removed with it: `js/webViewSession.js`, the `first_party_web` disposition and +its path allowlist, the WebView destination-bootstrap wiring, and the +`web_session` diagnostic stages. The strict WebView navigation guard, which had +been relaxed to admit the bootstrap, is restored. + +One instrumentation lesson is worth keeping: the abandoned diagnostics recorded +`destination_resume: succeeded` for a flow that had actually failed, because +the stage measured navigation mechanics rather than whether a session existed. +A success signal must observe the thing it claims to prove. diff --git a/docs/NATIVE-OTA-OPERATIONS.md b/docs/NATIVE-OTA-OPERATIONS.md index fbd335ad5..efb36890a 100644 --- a/docs/NATIVE-OTA-OPERATIONS.md +++ b/docs/NATIVE-OTA-OPERATIONS.md @@ -61,6 +61,34 @@ channel, signer key ID, rollout percentage, and the verification result. The update must contain no credentials or private member payloads. Never print the private key, auth tokens, or notification payloads in release evidence. +## OTA provenance tags (required) + +Every OTA artifact published or promoted to production must receive an +immutable annotated git tag **at promotion time**, before any merge can rewrite +or drop the shipped commit. Squash and rebase merges both rewrite commits, so a +shipped SHA recorded in EAS routinely becomes unreachable from the trunk +minutes after it ships. Four production artifacts were lost this way before the +requirement existed. + +Naming: `ota--` — the first 8 characters of the update +group UUID and of the full commit SHA. + +The tag message must record: + +- OTA group ID +- iOS and Android update IDs +- runtime version +- full git SHA +- rollback pointer (the group being superseded) + +Tag before merging the pull request. A tag created afterwards can only be +justified by authoritative EAS records; never create one from recollection or +from inferred content equivalence, because a rewritten commit with identical +content is a different artifact for audit purposes. + +Verify with `git ls-remote --tags origin | grep ota-`, and cross-check a group's +shipped SHA with `eas update:view --json`. + ## Recovery and kill switch - Pause a rollout or revert it to the control update for an immediate rollout diff --git a/e2e/jest.config.js b/e2e/jest.config.js index da4e7aa29..edb677036 100644 --- a/e2e/jest.config.js +++ b/e2e/jest.config.js @@ -3,7 +3,12 @@ module.exports = { rootDir: '..', roots: ['/e2e'], testMatch: ['/e2e/**/*.test.js'], - testTimeout: 120000, + // Governs hooks as well as tests. The logged-out suite's beforeEach performs + // a full device.launchApp({ delete: true }) reinstall, and on a cold macOS + // runner that has exceeded 120s outright - one observed sibling test passed + // at 119027ms, a second under the old budget. 180s accommodates the + // documented slowness without touching any element matcher. + testTimeout: 180000, maxWorkers: 1, globalSetup: 'detox/runners/jest/globalSetup', globalTeardown: 'detox/runners/jest/globalTeardown', diff --git a/e2e/loggedOutLaunch.js b/e2e/loggedOutLaunch.js index a3e0ab286..cf539ada2 100644 --- a/e2e/loggedOutLaunch.js +++ b/e2e/loggedOutLaunch.js @@ -1,5 +1,9 @@ import { by, device, element, waitFor } from 'detox'; +// The element-level waits below are deliberately unchanged. The flake this +// helper is associated with was never the matcher: it was the outer Jest +// hook/test budget in e2e/jest.config.js, which a cold-boot reinstall could +// exceed before these waits had a chance to run. Keep the assertions strict. export async function waitForLoggedOutWelcome() { const welcome = element(by.id('logged-out-welcome-scroll')); diff --git a/js/Discourse.js b/js/Discourse.js index 7e1c8fb3f..301e1da9e 100644 --- a/js/Discourse.js +++ b/js/Discourse.js @@ -101,7 +101,13 @@ import { import NativeTopicScreen from './product/NativeTopicScreen'; import NativeCollectionScreen from './product/NativeCollectionScreen'; import NativeProfileScreen from './product/NativeProfileScreen'; +import BadgeEarnedScreen from './product/BadgeEarnedScreen'; import { classifyFirstPartyMemberRoute } from './nativeMemberRouting'; +import { notificationIntent } from './notificationIntent'; +import { + NOTIFICATION_UNAVAILABLE, + destinationPresentation, +} from './notificationDestination'; import { consumePendingShareIntent } from './shareIntentCoordinator'; import { loadOnboardingState, @@ -391,6 +397,7 @@ class Discourse extends React.Component { authenticated: Boolean(site?.authToken), navigationReady, openUrl: this.openUrl.bind(this), + openNotification: this.openNotification.bind(this), }); if (!routed && this._pushRoute.path) { securityEvent('push.route.deferred'); @@ -558,6 +565,7 @@ class Discourse extends React.Component { navigationReady: this._navigationReady, nativeModule: DiscourseKeyboardShortcuts, openUrl: this.openUrl.bind(this), + openNotification: this.openNotification.bind(this), }).finally(() => { this._shareIntentConsumption = null; }); @@ -876,6 +884,10 @@ class Discourse extends React.Component { if (now - this._lastForegroundRefreshAt < 30000) return false; this._lastForegroundRefreshAt = now; const generation = ++this._foregroundRefreshGeneration; + // A server-side rename must reach the app without a logout or reinstall. + // This refreshes the active site only and reuses the guard above, so it + // cannot reintroduce the retired multi-site refresh loop. + await this._siteManager.refreshActiveIdentity().catch(() => false); await this._siteManager.refreshNotificationState(reason).catch(() => []); if (generation !== this._foregroundRefreshGeneration) return false; this.setState(current => ({ @@ -966,20 +978,93 @@ class Discourse extends React.Component { authenticated: Boolean(site), isStaff: Boolean(site?.isStaff), }); - if (route.disposition === 'native') { + const presentation = destinationPresentation(route); + if (presentation.kind === 'native') { this._siteManager.setActiveSite(site); - if (route.screen === 'Ask') { - this._navigation.navigate('HomeWrapper', { screen: 'Ask' }); - } else { - this._navigation.navigate(route.screen, route.params); - } + this._navigateNative(presentation.screen, presentation.params); + return; + } + if (presentation.kind === 'external') { + Linking.openURL(presentation.url).catch(() => {}); return; } - if (route.disposition === 'privileged_external') { - Linking.openURL(route.url).catch(() => {}); + // Denied: off-origin, unauthenticated, a non-staff admin path, or an + // unrecognised destination. Nothing opens and nothing loads. + securityEvent('navigation.rejected'); + } + + _navigateNative(screen, params) { + if (screen === 'Ask') { + this._navigation.navigate('HomeWrapper', { screen: 'Ask' }); + return; + } + this._navigation.navigate(screen, params); + } + + // Notification taps resolve from the payload rather than from a URL, so a + // granted_badge keeps its badge_name and can open a native screen. Anything + // without a native destination ends in one explicit bounded state: no + // WebView, no second login, no external browser, and never a silent no-op. + openNotification(site, notification) { + const intent = notificationIntent(site, notification, { + authenticated: Boolean(site?.authToken), + isStaff: Boolean(site?.isStaff), + }); + switch (intent.kind) { + case 'native': + this._siteManager.setActiveSite(site); + this._navigateNative(intent.screen, intent.params); + return; + case 'badge': + this._siteManager.setActiveSite(site); + this._navigation.navigate('BadgeEarned', { name: intent.badge.name }); + return; + case 'staff_external': + // Staff-only admin handoff. notificationIntent returns this kind only + // for a staff member on a canonical /admin path. + Linking.openURL(intent.url).catch(() => {}); + return; + default: + securityEvent('notification.unavailable'); + Alert.alert( + NOTIFICATION_UNAVAILABLE.title, + NOTIFICATION_UNAVAILABLE.message, + [{ text: NOTIFICATION_UNAVAILABLE.close, style: 'cancel' }], + ); } } + // A member must never be trapped behind an identity they did not choose in + // this attempt. Retire every client-side identity carrier, then start a + // normal authorization. This does not depend on the browser honouring an + // ephemeral session, and it never revokes the server-side credential of an + // account the member may still want. + async useDifferentAccount() { + if (this.state.connecting) return; + Alert.alert( + 'Use a different account?', + 'Adjuster Network will forget the saved sign-in on this device and ask for credentials again. Your account is not deleted.', + [ + { text: 'Cancel', style: 'cancel' }, + { + text: 'Continue', + onPress: async () => { + this.setState({ connecting: true }); + try { + await this._siteManager.resetAuthorizationIdentity(); + securityEvent('auth.identity.reset'); + } catch { + securityEvent('auth.identity.reset_failed'); + } finally { + this.setState({ connecting: false }); + } + await this.connectCanonical(); + }, + }, + ], + ); + } + async connectCanonical() { if (!adjusterNetwork.canonicalOrigin) { Alert.alert( @@ -1071,6 +1156,7 @@ class Discourse extends React.Component { // TODO: pass only relevant props to each screen component const screenProps = { openUrl: this.openUrl.bind(this), + openNotification: this.openNotification.bind(this), _handleOpenUrl: this._handleOpenUrl, seenNotificationMap: this._seenNotificationMap, setSeenNotificationMap: map => { @@ -1160,6 +1246,7 @@ class Discourse extends React.Component { this.connectCanonical()} + onUseDifferentAccount={() => this.useDifferentAccount()} /> {this.state.privacyShield && this._blurView(theme.name)} @@ -1536,6 +1623,14 @@ class Discourse extends React.Component { /> )} + + {props => ( + + )} + {props => ( diff --git a/js/__tests__/authEphemeralSession.test.js b/js/__tests__/authEphemeralSession.test.js new file mode 100644 index 000000000..f8306038d --- /dev/null +++ b/js/__tests__/authEphemeralSession.test.js @@ -0,0 +1,385 @@ +import fs from 'fs'; +import path from 'path'; + +jest.mock('react-native-safari-web-auth', () => ({ requestAuth: jest.fn() })); +jest.mock('@react-native-community/push-notification-ios', () => ({})); +jest.mock('react-native-key-pair', () => ({ generate: jest.fn() })); +jest.mock('react-native-device-info', () => ({ + getDeviceName: jest.fn(() => Promise.resolve('Adjuster Network - Test')), +})); +jest.mock('@react-native-cookies/cookies', () => ({ + clearAll: jest.fn(() => Promise.resolve()), +})); +jest.mock('@react-native-async-storage/async-storage', () => { + const store = new Map(); + return { + getItem: jest.fn(key => Promise.resolve(store.get(key) ?? null)), + setItem: jest.fn((key, value) => { + store.set(key, value); + return Promise.resolve(); + }), + removeItem: jest.fn(key => { + store.delete(key); + return Promise.resolve(); + }), + }; +}); +jest.mock('../secureCredentialStore', () => ({ + credentialStore: { + storeSiteToken: jest.fn(() => Promise.resolve()), + removeSiteToken: jest.fn(() => Promise.resolve()), + removeRSAKeys: jest.fn(() => Promise.resolve()), + readRSAKeys: jest.fn(() => Promise.resolve(null)), + }, +})); + +import SafariWebAuth from 'react-native-safari-web-auth'; +import SiteManager from '../site_manager'; +import { credentialStore } from '../secureCredentialStore'; +import { EPHEMERAL_AUTH_SESSION, requestIOSAuth } from '../iosAuthSession'; +import { isSafeAuthCallback } from '../adjusterNetworkSecurity'; +import { AUTH_REDIRECT } from '../authorizationConsent'; +import { + AUTHORIZATION_PROFILE_ID, + REQUIRED_AUTHORIZATION_SCOPES, +} from '../authorizationProfile'; +import { + adjusterNetwork, + canonicalOriginForChannel, + trustedPushEnvironment, + trustedUpdateChannel, +} from '../adjusterNetworkConfig'; + +const readSource = (...segments) => + fs.readFileSync(path.join(__dirname, '..', ...segments), 'utf8'); + +// A SiteManager built without its constructor: the authorization binding logic +// under test must not depend on storage load, key generation, or device state. +function authManager(activeSite) { + const manager = Object.create(SiteManager.prototype); + manager.sites = []; + manager.activeSite = activeSite || null; + manager.customScheme = 'adjusternetwork'; + manager.urlScheme = AUTH_REDIRECT; + manager._nonce = null; + manager._nonceSite = null; + manager.save = jest.fn(); + manager._onChange = jest.fn(); + return manager; +} + +const CLIENT_ID = 'synthetic-client-id'; + +function authorizationProfile(clientId = CLIENT_ID) { + return { + profile_id: AUTHORIZATION_PROFILE_ID, + client_id: clientId, + exact_match: true, + granted_scopes: [...REQUIRED_AUTHORIZATION_SCOPES], + required_scopes: [...REQUIRED_AUTHORIZATION_SCOPES], + }; +} + +function memberSite(url, clientId = CLIENT_ID) { + return { + url, + clientId, + authToken: null, + credentialRetired: false, + logoff: jest.fn(), + refresh: jest.fn(() => Promise.resolve()), + refreshIdentity: jest.fn(() => Promise.resolve(false)), + // A fresh authorization is only accepted after the server confirms the + // profile is bound to this client ID. + jsonApi: jest.fn(() => Promise.resolve(authorizationProfile(clientId))), + }; +} + +beforeEach(() => jest.clearAllMocks()); + +describe('native authorization starts in a fresh browser-auth context', () => { + test('every authorization launches an ephemeral session', async () => { + SafariWebAuth.requestAuth.mockResolvedValueOnce( + `${AUTH_REDIRECT}?payload=opaque`, + ); + + await requestIOSAuth( + 'https://adjusternetwork.org/user-api-key/new', + 'adjusternetwork', + ); + + expect(EPHEMERAL_AUTH_SESSION).toBe(true); + expect(SafariWebAuth.requestAuth).toHaveBeenCalledWith( + 'https://adjusternetwork.org/user-api-key/new', + 'adjusternetwork', + true, + ); + }); + + test('the SiteManager call site cannot opt out of the ephemeral session', async () => { + const site = memberSite('https://adjusternetwork.org'); + const manager = authManager(site); + SafariWebAuth.requestAuth.mockResolvedValueOnce(AUTH_REDIRECT); + + await manager.requestAuth('https://adjusternetwork.org/user-api-key/new'); + + expect(SafariWebAuth.requestAuth.mock.calls[0][2]).toBe(true); + }); + + test('no source path can request a persistent shared-Safari session', () => { + const sessionSource = readSource('iosAuthSession.js'); + const managerSource = readSource('site_manager.js'); + + // The flag is a module constant, never a caller-supplied argument. + expect(sessionSource).toContain( + 'export const EPHEMERAL_AUTH_SESSION = true', + ); + expect(sessionSource).toMatch( + /requestIOSAuth\(url, callbackScheme\)[\s\S]*EPHEMERAL_AUTH_SESSION/, + ); + expect(sessionSource).not.toMatch(/ephemeral\s*=\s*false/); + expect(managerSource).toContain('requestIOSAuth(url, this.customScheme)'); + expect(managerSource).not.toMatch(/requestIOSAuth\([^)]*false/); + }); + + test('the shipped native bridge honours the ephemeral flag on the same interface', () => { + const nativeSource = fs.readFileSync( + path.join( + __dirname, + '..', + '..', + 'vendor', + 'react-native-safari-web-auth', + 'ios', + 'SafariWebAuth.mm', + ), + 'utf8', + ); + + // This JS/native interface is unchanged by the stale-identity fix, which is + // what keeps the fix inside the existing runtime contract and OTA-eligible. + expect(nativeSource).toContain('ephemeral:(BOOL)ephemeral'); + expect(nativeSource).toContain( + 'session.prefersEphemeralWebBrowserSession = ephemeral;', + ); + }); +}); + +describe('a prior account cannot bind a subsequent authorization', () => { + test('a replayed prior-account payload is rejected and binds nothing', async () => { + const accountA = memberSite('https://adjusternetwork.org'); + const manager = authManager(accountA); + manager._nonceSite = accountA; + manager._nonce = 'nonce-a'; + manager.decryptHelper = jest.fn(() => + JSON.stringify({ nonce: 'nonce-a', key: 'qa-test-key' }), + ); + + await expect(manager.handleAuthPayload('payload-a')).resolves.toBe(true); + expect(accountA.authToken).toBe('qa-test-key'); + + // The pending attempt is one-shot. Replaying the same prior-account + // callback after it is consumed must not rebind anything. + const later = memberSite('https://adjusternetwork.org'); + const secondAttempt = authManager(later); + secondAttempt.decryptHelper = jest.fn(() => + JSON.stringify({ nonce: 'nonce-a', key: 'qa-test-key' }), + ); + + await expect(secondAttempt.handleAuthPayload('payload-a')).resolves.toBe( + false, + ); + expect(later.authToken).toBeNull(); + }); + + test('a callback whose nonce does not match this attempt is rejected', async () => { + const accountB = memberSite('https://adjusternetwork.org'); + const manager = authManager(accountB); + manager._nonceSite = accountB; + manager._nonce = 'nonce-b'; + manager.decryptHelper = jest.fn(() => + JSON.stringify({ nonce: 'nonce-a', key: 'qa-test-key' }), + ); + + await expect(manager.handleAuthPayload('stale')).resolves.toBe(false); + expect(accountB.authToken).toBeNull(); + expect(credentialStore.storeSiteToken).not.toHaveBeenCalled(); + }); + + test('a rejected payload fails the authorization instead of silently continuing', async () => { + const site = memberSite('https://adjusternetwork.org'); + const manager = authManager(site); + manager._nonceSite = site; + manager._nonce = 'nonce-b'; + manager.decryptHelper = jest.fn(() => + JSON.stringify({ nonce: 'nonce-a', key: 'qa-test-key' }), + ); + SafariWebAuth.requestAuth.mockResolvedValueOnce( + `${AUTH_REDIRECT}?payload=stale`, + ); + + await expect( + manager.requestAuth('https://adjusternetwork.org/user-api-key/new'), + ).rejects.toThrow('auth_payload_rejected'); + expect(site.authToken).toBeNull(); + }); +}); + +describe('signing in as account B binds the User API key to B', () => { + test('the key is stored against the site of the pending attempt only', async () => { + const accountA = memberSite('https://adjusternetwork.org'); + accountA.authToken = 'account-a-key'; + const accountB = memberSite('https://adjusternetwork.org'); + const manager = authManager(accountB); + manager.sites = [accountA, accountB]; + manager._nonceSite = accountB; + manager._nonce = 'nonce-b'; + manager.decryptHelper = jest.fn(() => + JSON.stringify({ + nonce: 'nonce-b', + key: 'account-b-key', + push: false, + api: 2, + }), + ); + + await expect(manager.handleAuthPayload('payload-b')).resolves.toBe(true); + + expect(accountB.authToken).toBe('account-b-key'); + expect(accountA.authToken).toBe('account-a-key'); + expect(credentialStore.storeSiteToken).toHaveBeenCalledWith( + 'https://adjusternetwork.org', + 'account-b-key', + ); + }); +}); + +describe('the authorization callback stays on the governed redirect', () => { + test('only the Adjuster Network redirect is accepted', () => { + expect(AUTH_REDIRECT).toBe( + 'adjusternetwork://adjusternetwork.org/auth_redirect', + ); + expect(isSafeAuthCallback(AUTH_REDIRECT)).toBe(true); + expect(isSafeAuthCallback(`${AUTH_REDIRECT}?payload=opaque`)).toBe(true); + + for (const hostile of [ + 'evil://adjusternetwork.org/auth_redirect?payload=x', + 'adjusternetwork://adjusternetwork.org/auth_redirect.evil?payload=x', + 'adjusternetwork://evil.example.com/auth_redirect?payload=x', + // The pre-hardening unqualified redirect is no longer accepted. + 'adjusternetwork://auth_redirect?payload=x', + 'https://adjusternetwork.org/auth_redirect?payload=x', + 'about:blank', + ]) { + expect(isSafeAuthCallback(hostile)).toBe(false); + } + }); + + test('an unapproved callback from the ephemeral session is refused', async () => { + SafariWebAuth.requestAuth.mockResolvedValueOnce( + 'evil://auth_redirect?payload=x', + ); + await expect( + requestIOSAuth('https://adjusternetwork.org/auth', 'adjusternetwork'), + ).rejects.toThrow('auth_callback_invalid'); + }); + + test('the authorization request keeps the governed redirect contract', () => { + const managerSource = readSource('site_manager.js'); + expect(managerSource).toContain('urlScheme = AUTH_REDIRECT'); + expect(managerSource).toContain('auth_redirect: this.urlScheme'); + }); +}); + +describe('environment resolution is unchanged and fail-closed', () => { + test('only the two governed channels resolve an origin', () => { + expect(adjusterNetwork.canonicalOrigin).toBe('https://adjusternetwork.org'); + expect(canonicalOriginForChannel('production')).toBe( + 'https://adjusternetwork.org', + ); + expect(canonicalOriginForChannel('staging')).toBe( + 'https://staging.adjusternetwork.org', + ); + for (const untrusted of [null, undefined, '', 'preview', 'PRODUCTION']) { + expect(trustedUpdateChannel(untrusted)).toBeNull(); + expect(canonicalOriginForChannel(untrusted)).toBeNull(); + } + }); + + test('push environment resolution stays iOS-only and fail-closed', () => { + expect(trustedPushEnvironment('ios', 'production')).toBe('production'); + expect(trustedPushEnvironment('ios', 'staging')).toBe('staging'); + expect(trustedPushEnvironment('ios', 'preview')).toBeNull(); + expect(trustedPushEnvironment('ios', undefined)).toBeNull(); + expect(trustedPushEnvironment('android', 'production')).toBeNull(); + }); + + test('authorization refuses a site outside the resolved canonical origin', async () => { + const manager = authManager(null); + await expect( + manager.generateAuthURL({ url: 'https://evil.example.com' }), + ).rejects.toThrow('auth_origin_not_allowed'); + await expect(manager.generateAuthURL(null)).rejects.toThrow( + 'auth_origin_not_allowed', + ); + }); +}); + +describe('an account switch retires every client-side identity carrier', () => { + test('cookies, Keychain token, RSA material, profile and client ID are cleared', async () => { + const AsyncStorage = require('@react-native-async-storage/async-storage'); + const CookieManager = require('@react-native-cookies/cookies'); + const site = memberSite('https://adjusternetwork.org'); + site.authToken = 'qa-test-key'; + const manager = authManager(site); + manager.sites = [site]; + manager.clientId = CLIENT_ID; + manager.rsaKeys = { public: 'pub', private: 'priv' }; + manager._nonce = 'nonce'; + manager._nonceSite = site; + + await manager.resetAuthorizationIdentity(); + + // The browser cookie jar and every stored credential carrier are gone. + expect(CookieManager.clearAll).toHaveBeenCalledWith(true); + expect(credentialStore.removeSiteToken).toHaveBeenCalledWith( + 'https://adjusternetwork.org', + ); + expect(credentialStore.removeRSAKeys).toHaveBeenCalled(); + expect(AsyncStorage.removeItem).toHaveBeenCalledWith('@ClientId'); + expect(AsyncStorage.removeItem).toHaveBeenCalledWith('@Discourse.rsaKeys'); + + // In-memory authorization state cannot survive the switch either. + expect(manager.rsaKeys).toBeNull(); + expect(manager.clientId).toBeNull(); + expect(manager._nonce).toBeNull(); + expect(manager._nonceSite).toBeNull(); + expect(site.logoff).toHaveBeenCalled(); + }); + + test('a failing carrier never aborts the switch', async () => { + const CookieManager = require('@react-native-cookies/cookies'); + CookieManager.clearAll.mockRejectedValueOnce(new Error('cookie failure')); + credentialStore.removeRSAKeys.mockRejectedValueOnce(new Error('keychain')); + const site = memberSite('https://adjusternetwork.org'); + const manager = authManager(site); + manager.sites = [site]; + manager.clientId = CLIENT_ID; + + await expect(manager.resetAuthorizationIdentity()).resolves.toBeUndefined(); + expect(manager.clientId).toBeNull(); + }); + + test('the next authorization after a switch still launches ephemerally', async () => { + const site = memberSite('https://adjusternetwork.org'); + const manager = authManager(site); + manager.sites = [site]; + manager.clientId = CLIENT_ID; + await manager.resetAuthorizationIdentity(); + + SafariWebAuth.requestAuth.mockResolvedValueOnce(AUTH_REDIRECT); + await manager.requestAuth('https://adjusternetwork.org/user-api-key/new'); + + expect(SafariWebAuth.requestAuth.mock.calls[0][2]).toBe(true); + }); +}); diff --git a/js/__tests__/authFailure.test.js b/js/__tests__/authFailure.test.js index 1abb8ad4d..0f7c8b232 100644 --- a/js/__tests__/authFailure.test.js +++ b/js/__tests__/authFailure.test.js @@ -33,4 +33,95 @@ describe('authentication failure categories', () => { /token|payload|nonce|url/i, ); }); + + // Regression: the staging certification lane surfaced OSStatus -34018 + // ("Client has neither application-identifier nor keychain-access-groups + // entitlements") as "Unable to connect", which pointed operators at + // connectivity instead of the real signing/entitlement defect. + describe('security configuration failures are never network failures', () => { + const securityFailures = [ + 'The operation couldn’t be completed. (OSStatus error -34018.)', + 'Client has neither application-identifier nor keychain-access-groups entitlements', + 'errSecMissingEntitlement', + 'SecItemCopyMatching failed', + 'secure_storage_unavailable', + ]; + + test.each(securityFailures)('classifies %s as keychain', message => { + expect(classifyAuthFailure(new Error(message))).toBe( + AUTH_FAILURE.KEYCHAIN, + ); + }); + + test.each(securityFailures)('never shows network copy for %s', message => { + const alert = authFailureAlert(classifyAuthFailure(new Error(message))); + expect(alert.title).not.toBe('Unable to connect'); + expect(alert).toEqual({ + title: 'Secure sign-in unavailable', + message: 'Secure sign-in could not be prepared. Please try again.', + }); + }); + + test('classifies a numeric-code keychain rejection by its message', () => { + const error = Object.assign(new Error('OSStatus error -34018'), { + code: '-34018', + }); + expect(classifyAuthFailure(error)).toBe(AUTH_FAILURE.KEYCHAIN); + }); + }); + + describe('unknown and programming faults do not borrow network copy', () => { + const unknownFailures = [ + new TypeError('undefined is not a function'), + new TypeError("Cannot read property 'requestAuth' of undefined"), + new Error('something entirely unexpected'), + new Error(''), + undefined, + null, + ]; + + test.each(unknownFailures.map((e, i) => [i, e]))( + 'classifies unknown fault %i as UNKNOWN', + (_index, error) => { + expect(classifyAuthFailure(error)).toBe(AUTH_FAILURE.UNKNOWN); + }, + ); + + test('unknown copy is bounded and not connectivity-flavoured', () => { + const alert = authFailureAlert(AUTH_FAILURE.UNKNOWN); + expect(alert.title).not.toBe('Unable to connect'); + expect(alert.message).not.toMatch(/connect|network|offline|internet/i); + expect(alert.title).toBe('Sign-in could not be completed'); + }); + }); + + describe('genuine transport failures keep existing behaviour', () => { + test.each([ + 'Network request failed', + 'The request timed out', + 'The Internet connection appears to be offline.', + 'Could not connect to the server', + 'getaddrinfo ENOTFOUND staging.adjusternetwork.org', + ])('classifies %s as network', message => { + expect(classifyAuthFailure(new Error(message))).toBe( + AUTH_FAILURE.NETWORK, + ); + }); + + test('network copy is unchanged from the shipped build', () => { + expect(authFailureAlert(AUTH_FAILURE.NETWORK)).toEqual({ + title: 'Unable to connect', + message: 'Please try again in a moment.', + }); + }); + }); + + test('every category maps to bounded non-empty copy', () => { + Object.values(AUTH_FAILURE).forEach(category => { + const alert = authFailureAlert(category); + expect(alert.title.length).toBeGreaterThan(0); + expect(alert.message.length).toBeGreaterThan(0); + expect(/^[a-z0-9_.-]{1,64}$/.test(category)).toBe(true); + }); + }); }); diff --git a/js/__tests__/authResponsePolicy.test.js b/js/__tests__/authResponsePolicy.test.js index c4fa91964..242717fd9 100644 --- a/js/__tests__/authResponsePolicy.test.js +++ b/js/__tests__/authResponsePolicy.test.js @@ -1,8 +1,17 @@ -import { classifyAuthResponse } from '../authResponsePolicy'; +import { + classifyAuthResponse, + INVALID_USER_API_CREDENTIAL, +} from '../authResponsePolicy'; describe('auth response policy', () => { test('revokes a session only when authentication is invalid', () => { - expect(classifyAuthResponse(401)).toBe('revoked'); + expect( + classifyAuthResponse(401, { + error_type: INVALID_USER_API_CREDENTIAL.errorType, + reason: INVALID_USER_API_CREDENTIAL.reason, + }), + ).toBe('revoked'); + expect(classifyAuthResponse(401, null)).toBe('other'); }); test('preserves narrowly scoped sessions when an endpoint is forbidden', () => { diff --git a/js/__tests__/authorizationUpgrade.test.js b/js/__tests__/authorizationUpgrade.test.js index 3f8d67df0..42942d2ac 100644 --- a/js/__tests__/authorizationUpgrade.test.js +++ b/js/__tests__/authorizationUpgrade.test.js @@ -59,6 +59,7 @@ test('attests exact scopes before replacing the stored credential', async () => required_scopes: [...REQUIRED_AUTHORIZATION_SCOPES], }), refresh: jest.fn().mockResolvedValue(), + refreshIdentity: jest.fn().mockResolvedValue(false), }; const manager = managerWith(site); await expect(manager.handleAuthPayload('encrypted')).resolves.toBe(true); diff --git a/js/__tests__/communityData.test.js b/js/__tests__/communityData.test.js index 449d9320d..fcf29432e 100644 --- a/js/__tests__/communityData.test.js +++ b/js/__tests__/communityData.test.js @@ -115,6 +115,57 @@ describe('community startup recovery', () => { expect(site.jsonApi).toHaveBeenCalledTimes(4); }); + test('authoritative author and avatar changes replace a stale shared topic snapshot', async () => { + const stale = { + id: 97, + last_poster_username: 'alex', + posters: [{ username: 'alex', avatar_template: '/alex/{size}.png' }], + }; + const editorial = { + id: 97, + last_poster_username: 'an_editorial', + posters: [ + { + username: 'an_editorial', + avatar_template: '/an_editorial/{size}/174_2.png', + description: 'Original Poster, Most Recent Poster', + }, + ], + }; + let topic = stale; + const site = { + jsonApi: jest.fn(path => + Promise.resolve( + path === '/latest.json' + ? { topic_list: { topics: [topic] } } + : { categories: [] }, + ), + ), + }; + + await expect(loadCommunity(site)).resolves.toMatchObject({ + topics: [stale], + }); + topic = editorial; + await expect(loadCommunity(site)).resolves.toMatchObject({ + topics: [editorial], + }); + expect(cachedCommunity(site)).toMatchObject({ topics: [editorial] }); + + const relaunchedSite = { + jsonApi: jest.fn(path => + Promise.resolve( + path === '/latest.json' + ? { topic_list: { topics: [editorial] } } + : { categories: [] }, + ), + ), + }; + await expect(loadCommunity(relaunchedSite)).resolves.toMatchObject({ + topics: [editorial], + }); + }); + test('retains the last successful snapshot when a later refresh fails', async () => { const site = { jsonApi: jest.fn(path => @@ -159,7 +210,9 @@ describe('community startup recovery', () => { expect(source).toContain( "unavailableWithoutSnapshot ? '—' : data.topics.length", ); - expect(source).toContain("unavailableWithoutSnapshot ? '—' : unanswered"); + expect(source).toContain( + "unavailableWithoutSnapshot ? '—' : activeConversations", + ); expect(source).toContain( "unavailableWithoutSnapshot ? '—' : data.categories.length", ); diff --git a/js/__tests__/credentialRetirement.test.js b/js/__tests__/credentialRetirement.test.js new file mode 100644 index 000000000..08daab7e8 --- /dev/null +++ b/js/__tests__/credentialRetirement.test.js @@ -0,0 +1,130 @@ +/* @flow */ +'use strict'; + +import { + classifyAuthResponse, + INVALID_USER_API_CREDENTIAL, +} from '../authResponsePolicy'; +import Site from '../site'; + +jest.mock('../secureCredentialStore', () => ({ + credentialStore: { + removeSiteToken: jest.fn().mockResolvedValue(undefined), + storeSiteToken: jest.fn().mockResolvedValue(undefined), + readSiteToken: jest.fn().mockResolvedValue(null), + }, +})); + +const makeSite = () => + new Site({ + url: 'https://adjusternetwork.org', + title: 'Adjuster Network', + apiVersion: 4, + authToken: 'live-token', + }); + +describe('authoritative credential retirement', () => { + test('only the canonical server credential tuple is classified as revoked', () => { + const canonical = { + error_type: INVALID_USER_API_CREDENTIAL.errorType, + reason: INVALID_USER_API_CREDENTIAL.reason, + }; + expect(classifyAuthResponse(401, canonical)).toBe('revoked'); + expect(classifyAuthResponse(401, null)).not.toBe('revoked'); + expect( + classifyAuthResponse(401, { error_type: canonical.error_type }), + ).not.toBe('revoked'); + expect(classifyAuthResponse(403, canonical)).toBe('forbidden'); + [403, 429, 404, 500, 502, 503, 200, 204].forEach(status => { + expect(classifyAuthResponse(status, canonical)).not.toBe('revoked'); + }); + }); + + test('retireCredential clears the token and notifies the manager once', () => { + const site = makeSite(); + const seen = []; + site.onCredentialRetired = (retired, reason) => + seen.push([retired.url, reason]); + + site.retireCredential('revoked'); + expect(site.authToken).toBeNull(); + expect(site.credentialRetired).toBe(true); + expect(seen).toEqual([['https://adjusternetwork.org', 'revoked']]); + + // Idempotent: a second authoritative failure must not re-notify. + site.retireCredential('revoked'); + expect(seen).toHaveLength(1); + }); + + test('logoff alone does not mark the credential retired', () => { + const site = makeSite(); + const seen = []; + site.onCredentialRetired = () => seen.push('notified'); + site.logoff(); + expect(site.authToken).toBeNull(); + expect(site.credentialRetired).toBeFalsy(); + expect(seen).toHaveLength(0); + }); + + describe('non-authoritative failures never retire the credential', () => { + test.each([ + ['ordinary authorization 403', 403], + ['rate limit 429', 429], + ['not found 404', 404], + ['server failure 500', 500], + ['bad gateway 502', 502], + ['unavailable 503', 503], + ])('%s preserves the session', (_label, status) => { + const site = makeSite(); + let notified = false; + site.onCredentialRetired = () => { + notified = true; + }; + if (classifyAuthResponse(status, null) === 'revoked') { + site.retireCredential('revoked'); + } + expect(site.authToken).toBe('live-token'); + expect(site.credentialRetired).toBeFalsy(); + expect(notified).toBe(false); + }); + + test('offline/network rejection preserves the session', () => { + const site = makeSite(); + let notified = false; + site.onCredentialRetired = () => { + notified = true; + }; + // A transport rejection never reaches a status classification at all. + const error = new Error('Network request failed'); + expect(error.status).toBeUndefined(); + expect(site.authToken).toBe('live-token'); + expect(notified).toBe(false); + }); + }); + + test('a retired site reports as not connected so the root signs out', () => { + const site = makeSite(); + site.onCredentialRetired = () => {}; + const connected = sites => sites.filter(s => s.authToken).length; + expect(connected([site])).toBe(1); + site.retireCredential('revoked'); + // This is exactly the predicate the root navigator uses to choose between + // the authenticated shell and the signed-out welcome screen. + expect(connected([site])).toBe(0); + expect([site].find(s => s.authToken)).toBeUndefined(); + }); + + test('a fresh verified authorization clears the retirement latch', () => { + const site = makeSite(); + site.onCredentialRetired = () => {}; + site.retireCredential('revoked'); + expect(site.credentialRetired).toBe(true); + + // Mirrors handleAuthPayload's post-verification assignment. + site.authToken = 'new-token'; + site.credentialRetired = false; + site.credentialRetiredReason = null; + expect(site.authToken).toBe('new-token'); + expect(site.credentialRetired).toBe(false); + }); +}); diff --git a/js/__tests__/floorAttentionClassification.test.js b/js/__tests__/floorAttentionClassification.test.js new file mode 100644 index 000000000..06c55c1b8 --- /dev/null +++ b/js/__tests__/floorAttentionClassification.test.js @@ -0,0 +1,31 @@ +import { floorAttentionState } from '../product/floorAttention'; + +describe('Floor attention classification', () => { + test('does not present an official zero-reply announcement as needing a reply', () => { + expect( + floorAttentionState({ + posts_count: 1, + an_network_activity_class: 'owner_editorial', + }), + ).toEqual({ label: 'OFFICIAL', icon: 'bullhorn', needsReply: false }); + }); + + test('preserves attention for a genuinely unanswered member discussion', () => { + expect( + floorAttentionState({ + posts_count: 1, + an_network_activity_class: 'member_activity', + }), + ).toEqual({ label: 'NEEDS A REPLY', icon: 'question', needsReply: true }); + }); + + test('fails closed for unknown and replied topic classes', () => { + expect(floorAttentionState({ posts_count: 1 }).needsReply).toBe(false); + expect( + floorAttentionState({ + posts_count: 2, + an_network_activity_class: 'member_activity', + }), + ).toEqual({ label: 'ACTIVE', icon: 'comments', needsReply: false }); + }); +}); diff --git a/js/__tests__/floorMicroPolish.test.js b/js/__tests__/floorMicroPolish.test.js new file mode 100644 index 000000000..c1dc95144 --- /dev/null +++ b/js/__tests__/floorMicroPolish.test.js @@ -0,0 +1,24 @@ +import { memberDisplayName } from '../product/floorPresentation'; + +describe('Floor launch micro-polish', () => { + test('prefers a known display name and uses a bounded username fallback', () => { + expect(memberDisplayName('Alex Rivera', 'alex_rivera')).toBe('Alex Rivera'); + expect(memberDisplayName('', 'alex_rivera')).toBe('Alex Rivera'); + }); + + test('never greets an unknown person as Member', () => { + expect(memberDisplayName('', '')).toBeNull(); + expect(memberDisplayName('Member', 'member')).toBeNull(); + }); + + test('counts replied conversations instead of calling every seed topic unanswered', () => { + const source = require('fs').readFileSync( + require.resolve('../product/ProductScreens'), + 'utf8', + ); + expect(source).toContain('label="Conversations"'); + expect(source).toContain('detail="With replies"'); + expect(source).toContain('topic => (topic.posts_count || 1) > 1'); + expect(source).not.toContain('label="Unanswered"'); + }); +}); diff --git a/js/__tests__/limiterBucketReview.test.js b/js/__tests__/limiterBucketReview.test.js new file mode 100644 index 000000000..e219558ac --- /dev/null +++ b/js/__tests__/limiterBucketReview.test.js @@ -0,0 +1,417 @@ +import Site from '../site'; +import fetch from '../../lib/fetch'; +import { + RATE_LIMIT_COOLDOWN_MAX_MS, + RATE_LIMIT_MAX_MS, + apiRateLimitCoordinator, +} from '../apiRateLimit'; +import { limiterBucket, requestOrchestrator } from '../requestOrchestrator'; + +jest.mock('../../lib/fetch', () => jest.fn()); + +const ORIGIN = 'https://adjusternetwork.org'; +const bucketFor = (errorCode, path = '/latest.json', clientId = 'client-A') => + limiterBucket({ origin: ORIGIN, clientId, path, errorCode }); + +const limited = (retryAfter, code) => ({ + status: 429, + headers: { + get: name => + name === 'Retry-After' + ? retryAfter + : name === 'Discourse-Rate-Limit-Error-Code' + ? code + : null, + }, +}); + +beforeEach(() => { + fetch.mockReset(); + apiRateLimitCoordinator.reset(); + requestOrchestrator.reset(); +}); + +describe('Q1: which bucket a user-api 429 enters', () => { + test('user_api_key_limiter_60_secs keys on origin + User API client id', () => { + expect(bucketFor('user_api_key_limiter_60_secs')).toBe( + `${ORIGIN}:user-api:client-A`, + ); + // The daily limiter shares that bucket, matching the server keying. + expect(bucketFor('user_api_key_limiter_1_day')).toBe( + `${ORIGIN}:user-api:client-A`, + ); + // A different key is a different bucket: one member cannot stall another. + expect( + bucketFor('user_api_key_limiter_60_secs', '/latest.json', 'client-B'), + ).toBe(`${ORIGIN}:user-api:client-B`); + }); + + test('the user-api bucket is path independent', () => { + for (const path of [ + '/native/v1/profile', + '/u/tomrodriguez.json', + '/chat/api/me/channels.json', + '/site.json', + '/latest.json', + ]) { + expect(bucketFor('user_api_key_limiter_60_secs', path)).toBe( + `${ORIGIN}:user-api:client-A`, + ); + } + }); +}); + +describe('Q2: which later paths a user-api cooldown blocks', () => { + test.each([ + '/native/v1/profile', + '/u/tomrodriguez.json', + '/chat/api/me/channels.json', + '/site.json', + '/latest.json', + ])('%s waits on the shared user-api cooldown', async path => { + jest.useFakeTimers(); + // Seed a live cooldown on the user-api bucket for this client. + requestOrchestrator.beginCooldown( + `${ORIGIN}:user-api:client-A`, + limited('30', 'user_api_key_limiter_60_secs'), + 0, + ); + fetch.mockResolvedValue({ + status: 200, + json: () => Promise.resolve({ ok: 1 }), + }); + const site = new Site({ + url: ORIGIN, + authToken: 'k', + clientId: 'client-A', + }); + + const pending = site.jsonApi(path); + await jest.advanceTimersByTimeAsync(0); + // Blocked: no request issued inside the window. + expect(fetch).not.toHaveBeenCalled(); + + await jest.advanceTimersByTimeAsync(31000); + await expect(pending).resolves.toEqual({ ok: 1 }); + expect(fetch).toHaveBeenCalledTimes(1); + jest.useRealTimers(); + }); +}); + +describe('the IP-bucket pre-request wait was removed from this package', () => { + const fs = require('fs'); + const path = require('path'); + const source = fs.readFileSync(path.join(__dirname, '..', 'site.js'), 'utf8'); + + test('jsonApi waits only on the user-api and endpoint-class buckets', () => { + expect(source).toContain( + 'await requestOrchestrator.waitForBucket(globalUserBucket)', + ); + expect(source).toContain( + 'await requestOrchestrator.waitForBucket(fallbackBucket)', + ); + // Scope kept minimal to the proven User API limiter defect. + expect(source).not.toContain('ipBucket'); + expect(source).not.toContain("errorCode: 'ip_60_secs_limit'"); + }); + + test('the pre-existing IP bucket machinery is preserved', () => { + expect( + limiterBucket({ + origin: ORIGIN, + clientId: 'c', + path: '/x', + errorCode: 'ip_60_secs_limit', + }), + ).toBe(`${ORIGIN}:ip`); + }); +}); + +describe('Q3/Q4: IP bucket is literal and separate from user-api', () => { + test('the IP codes map to their own bucket, not a general label', () => { + expect(bucketFor('ip_10_secs_limit')).toBe(`${ORIGIN}:ip`); + expect(bucketFor('ip_60_secs_limit')).toBe(`${ORIGIN}:ip`); + // It is client-id independent, which is what an IP limiter means. + expect(bucketFor('ip_60_secs_limit', '/latest.json', 'client-B')).toBe( + `${ORIGIN}:ip`, + ); + }); + + test('user-api and IP cooldowns are separate and do not leak into each other', async () => { + jest.useFakeTimers(); + requestOrchestrator.beginCooldown( + `${ORIGIN}:ip`, + limited('30', 'ip_60_secs_limit'), + 0, + ); + // The user-api bucket is untouched by an IP cooldown. + expect( + requestOrchestrator.cooldowns.has(`${ORIGIN}:user-api:client-A`), + ).toBe(false); + expect(requestOrchestrator.cooldowns.has(`${ORIGIN}:ip`)).toBe(true); + jest.useRealTimers(); + }); + + test('native and endpoint-class buckets are also distinct', () => { + expect(bucketFor('an_admission_required')).toBe( + `${ORIGIN}:native:an_admission_required`, + ); + expect(bucketFor(null, '/native/v1/profile')).toBe( + `${ORIGIN}:class:profile`, + ); + expect(bucketFor(null, '/chat/api/me/channels.json')).toBe( + `${ORIGIN}:class:/chat/api`, + ); + }); +}); + +describe('Q5: correctness of the wait', () => { + test('the wait expires and the bucket is cleared', async () => { + let clock = 0; + const orchestrator = new requestOrchestrator.constructor({ + now: () => clock, + sleep: ms => { + clock += ms; + return Promise.resolve(); + }, + }); + orchestrator.beginCooldown( + 'b', + limited('30', 'user_api_key_limiter_60_secs'), + 0, + ); + expect(orchestrator.cooldowns.get('b')).toBe(30000); + await orchestrator.waitForBucket('b'); + expect(orchestrator.cooldowns.has('b')).toBe(false); + }); + + test('a different client id is not blocked: unrelated traffic proceeds', async () => { + jest.useFakeTimers(); + requestOrchestrator.beginCooldown( + `${ORIGIN}:user-api:client-A`, + limited('30', 'user_api_key_limiter_60_secs'), + 0, + ); + fetch.mockResolvedValue({ + status: 200, + json: () => Promise.resolve({ ok: 2 }), + }); + // Signed out: clientId absent, so the bucket is :user-api:unknown. + const other = new Site({ url: ORIGIN }); + const pending = other.jsonApi('/site.json'); + await jest.advanceTimersByTimeAsync(0); + expect(fetch).toHaveBeenCalledTimes(1); + await expect(pending).resolves.toEqual({ ok: 2 }); + jest.useRealTimers(); + }); + + test('auth, logout and session recovery cannot deadlock', async () => { + let clock = 0; + const orchestrator = new requestOrchestrator.constructor({ + now: () => clock, + sleep: ms => { + clock += ms; + return Promise.resolve(); + }, + }); + // A hostile Retry-After is capped at the cooldown ceiling, and no single + // request waits for it: the waiter fails fast instead of hanging. + orchestrator.beginCooldown( + 'b', + limited('999999', 'user_api_key_limiter_60_secs'), + 0, + ); + expect(orchestrator.cooldowns.get('b')).toBe(RATE_LIMIT_COOLDOWN_MAX_MS); + await expect(orchestrator.waitForBucket('b')).rejects.toMatchObject({ + message: 'api_rate_limited', + status: 429, + }); + // Nothing slept, so no caller can be held. + expect(clock).toBe(0); + // The cooldown is preserved for later requests rather than cleared. + expect(orchestrator.cooldowns.get('b')).toBe(RATE_LIMIT_COOLDOWN_MAX_MS); + }); + + test('a rate-limited mutation is never replayed automatically', async () => { + jest.useFakeTimers(); + fetch.mockResolvedValue(limited('30', 'user_api_key_limiter_60_secs')); + const site = new Site({ + url: ORIGIN, + authToken: 'k', + clientId: 'client-A', + }); + const rejection = expect( + site.jsonApi('/native/v1/profile', 'PATCH', { a: 1 }), + ).rejects.toMatchObject({ + status: 429, + }); + await jest.advanceTimersByTimeAsync(0); + await rejection; + // One attempt only: writes are not retried after a limiter response. + expect(fetch).toHaveBeenCalledTimes(1); + jest.useRealTimers(); + }); +}); + +describe('Q6: cooldown lifetime vs per-request ceiling', () => { + const makeOrchestrator = () => { + const state = { clock: 0 }; + const orchestrator = new requestOrchestrator.constructor({ + now: () => state.clock, + sleep: ms => { + state.clock += ms; + return Promise.resolve(); + }, + }); + return { orchestrator, state }; + }; + + test('the two ceilings are distinct', () => { + expect(RATE_LIMIT_MAX_MS).toBe(60000); + expect(RATE_LIMIT_COOLDOWN_MAX_MS).toBe(180000); + }); + + test.each([ + ['30', 30000, 'waits'], + ['60', 60000, 'waits'], + ['136', 136000, 'fails fast'], + ['300', RATE_LIMIT_COOLDOWN_MAX_MS, 'fails fast'], + ])( + 'Retry-After %s records a %s ms cooldown and then %s', + async (header, expected) => { + const { orchestrator, state } = makeOrchestrator(); + const delay = orchestrator.beginCooldown( + 'b', + limited(header, 'user_api_key_limiter_60_secs'), + 0, + ); + expect(delay).toBe(expected); + expect(orchestrator.cooldowns.get('b')).toBe(expected); + + if (expected > RATE_LIMIT_MAX_MS) { + await expect(orchestrator.waitForBucket('b')).rejects.toMatchObject({ + status: 429, + }); + expect(state.clock).toBe(0); + } else { + await orchestrator.waitForBucket('b'); + expect(state.clock).toBeGreaterThanOrEqual(expected); + expect(orchestrator.cooldowns.has('b')).toBe(false); + } + }, + ); + + test('no request is sent inside an active 136s cooldown', async () => { + jest.useFakeTimers(); + requestOrchestrator.beginCooldown( + `${ORIGIN}:user-api:client-A`, + limited('136', 'user_api_key_limiter_60_secs'), + 0, + ); + fetch.mockResolvedValue({ + status: 200, + json: () => Promise.resolve({ ok: 1 }), + }); + const site = new Site({ + url: ORIGIN, + authToken: 'k', + clientId: 'client-A', + }); + await expect(site.jsonApi('/latest.json')).rejects.toMatchObject({ + message: 'api_rate_limited', + status: 429, + }); + // Fail fast: nothing reached the network inside the window. + expect(fetch).not.toHaveBeenCalled(); + jest.useRealTimers(); + }); + + test('later requests keep observing the cooldown until it truly expires', async () => { + const { orchestrator } = makeOrchestrator(); + orchestrator.beginCooldown( + 'b', + limited('136', 'user_api_key_limiter_60_secs'), + 0, + ); + await expect(orchestrator.waitForBucket('b')).rejects.toMatchObject({ + status: 429, + }); + // Still active after the per-request ceiling would have elapsed. + orchestrator.now = () => 61000; + await expect(orchestrator.waitForBucket('b')).rejects.toMatchObject({ + status: 429, + }); + // Inside the final minute it becomes a normal bounded wait again. + orchestrator.now = () => 100000; + await orchestrator.waitForBucket('b'); + }); + + test('normal requests resume after the cooldown expires', async () => { + jest.useFakeTimers(); + requestOrchestrator.beginCooldown( + `${ORIGIN}:user-api:client-A`, + limited('30', 'user_api_key_limiter_60_secs'), + 0, + ); + fetch.mockResolvedValue({ + status: 200, + json: () => Promise.resolve({ ok: 3 }), + }); + const site = new Site({ + url: ORIGIN, + authToken: 'k', + clientId: 'client-A', + }); + const pending = site.jsonApi('/latest.json'); + await jest.advanceTimersByTimeAsync(0); + expect(fetch).not.toHaveBeenCalled(); + await jest.advanceTimersByTimeAsync(31000); + await expect(pending).resolves.toEqual({ ok: 3 }); + expect(fetch).toHaveBeenCalledTimes(1); + jest.useRealTimers(); + }); + + test('a repeated 429 extends the cooldown without fan-out', async () => { + const { orchestrator } = makeOrchestrator(); + orchestrator.beginCooldown( + 'b', + limited('30', 'user_api_key_limiter_60_secs'), + 0, + ); + expect(orchestrator.cooldowns.get('b')).toBe(30000); + // A longer directive extends it. + orchestrator.beginCooldown( + 'b', + limited('136', 'user_api_key_limiter_60_secs'), + 1, + ); + expect(orchestrator.cooldowns.get('b')).toBe(136000); + // A shorter directive never shortens it. + orchestrator.beginCooldown( + 'b', + limited('5', 'user_api_key_limiter_60_secs'), + 2, + ); + expect(orchestrator.cooldowns.get('b')).toBe(136000); + // One shared window, not one per caller. + expect(orchestrator.cooldowns.size).toBe(1); + }); + + test('GET retry count stays bounded when the window is short', async () => { + jest.useFakeTimers(); + fetch.mockResolvedValue(limited('2', 'user_api_key_limiter_60_secs')); + const site = new Site({ + url: ORIGIN, + authToken: 'k', + clientId: 'client-A', + }); + const rejection = expect( + site.jsonApi('/latest.json'), + ).rejects.toMatchObject({ status: 429 }); + await jest.advanceTimersByTimeAsync(0); + await jest.advanceTimersByTimeAsync(60000); + await rejection; + expect(fetch).toHaveBeenCalledTimes(3); + jest.useRealTimers(); + }); +}); diff --git a/js/__tests__/memberIdentityRefresh.test.js b/js/__tests__/memberIdentityRefresh.test.js new file mode 100644 index 000000000..339f9e26f --- /dev/null +++ b/js/__tests__/memberIdentityRefresh.test.js @@ -0,0 +1,339 @@ +jest.mock('react-native-safari-web-auth', () => ({ requestAuth: jest.fn() })); +jest.mock('@react-native-community/push-notification-ios', () => ({})); +jest.mock('react-native-key-pair', () => ({ generate: jest.fn() })); +jest.mock('react-native-device-info', () => ({ + getDeviceName: jest.fn(() => Promise.resolve('Adjuster Network - Test')), +})); +jest.mock('@react-native-cookies/cookies', () => ({ + clearAll: jest.fn(() => Promise.resolve()), +})); +jest.mock('@react-native-async-storage/async-storage', () => { + const store = new Map(); + return { + __store: store, + getItem: jest.fn(key => Promise.resolve(store.get(key) ?? null)), + setItem: jest.fn((key, value) => { + store.set(key, value); + return Promise.resolve(); + }), + removeItem: jest.fn(key => { + store.delete(key); + return Promise.resolve(); + }), + }; +}); +jest.mock('../secureCredentialStore', () => ({ + credentialStore: { + storeSiteToken: jest.fn(() => Promise.resolve()), + removeSiteToken: jest.fn(() => Promise.resolve()), + removeRSAKeys: jest.fn(() => Promise.resolve()), + readRSAKeys: jest.fn(() => Promise.resolve(null)), + }, +})); + +import AsyncStorage from '@react-native-async-storage/async-storage'; +import Site from '../site'; +import SiteManager from '../site_manager'; +import { memberDisplayName } from '../product/floorPresentation'; + +const ORIGIN = 'https://adjusternetwork.org'; + +// A Site driven by a scripted /session/current.json, with no transport. +function identitySite(responses, initial = {}) { + const site = Object.create(Site.prototype); + site.url = ORIGIN; + site.authToken = 'token'; + site.username = initial.username ?? null; + site.name = initial.name ?? null; + site.jsonApi = jest.fn(path => { + expect(path).toBe('/session/current.json'); + const next = responses.shift(); + if (next instanceof Error) return Promise.reject(next); + return Promise.resolve(next); + }); + return site; +} + +function manager(site) { + const instance = Object.create(SiteManager.prototype); + instance.sites = site ? [site] : []; + instance.activeSite = site || null; + instance._identityRefresh = null; + instance.save = jest.fn(); + instance._onChange = jest.fn(); + return instance; +} + +const session = (username, name) => ({ current_user: { username, name } }); + +beforeEach(() => jest.clearAllMocks()); + +describe('member identity is refreshed from the current session', () => { + test('authorizing as A stores both the username and the display name', async () => { + const site = identitySite([session('finale2e', 'Finale E2E')]); + await expect(site.refreshIdentity()).resolves.toBe(true); + expect(site.username).toBe('finale2e'); + expect(site.name).toBe('Finale E2E'); + }); + + test('a server-side rename to B is picked up without reauthorization', async () => { + const site = identitySite([session('tomrodriguez', 'Tom Rodriguez')], { + username: 'finale2e', + name: 'Finale E2E', + }); + const instance = manager(site); + + await expect(instance.refreshActiveIdentity()).resolves.toBe(true); + + expect(site.username).toBe('tomrodriguez'); + expect(site.name).toBe('Tom Rodriguez'); + // The refreshed identity is persisted, so a relaunch cannot resurrect A. + expect(instance.save).toHaveBeenCalled(); + expect(instance._onChange).toHaveBeenCalled(); + }); + + test('the Floor greeting renders the display name, not the handle', () => { + expect(memberDisplayName('Tom Rodriguez', 'tomrodriguez')).toBe( + 'Tom Rodriguez', + ); + // Before the fix there was no stored name, so the handle was title-cased. + expect(memberDisplayName(null, 'tomrodriguez')).toBe('Tomrodriguez'); + expect(memberDisplayName(null, 'finale2e')).toBe('Finale2e'); + expect(memberDisplayName(null, null)).toBeNull(); + }); + + test('an unchanged identity does not rewrite storage', async () => { + const site = identitySite([session('tomrodriguez', 'Tom Rodriguez')], { + username: 'tomrodriguez', + name: 'Tom Rodriguez', + }); + const instance = manager(site); + + await expect(instance.refreshActiveIdentity()).resolves.toBe(false); + expect(instance.save).not.toHaveBeenCalled(); + }); +}); + +describe('identity refresh fails closed', () => { + test('a transport error preserves the last known identity', async () => { + const site = identitySite([new Error('offline')], { + username: 'tomrodriguez', + name: 'Tom Rodriguez', + }); + const instance = manager(site); + + await expect(instance.refreshActiveIdentity()).resolves.toBe(false); + expect(site.username).toBe('tomrodriguez'); + expect(site.name).toBe('Tom Rodriguez'); + expect(instance.save).not.toHaveBeenCalled(); + }); + + test('a malformed or empty payload never blanks the identity', async () => { + for (const payload of [ + null, + {}, + { current_user: null }, + { current_user: {} }, + { current_user: { username: ' ' } }, + ]) { + const site = identitySite([payload], { + username: 'tomrodriguez', + name: 'Tom Rodriguez', + }); + await expect(site.refreshIdentity()).resolves.toBe(false); + expect(site.username).toBe('tomrodriguez'); + expect(site.name).toBe('Tom Rodriguez'); + } + }); + + test('a signed-out site is never asked for an identity', async () => { + const site = identitySite([session('a', 'A')]); + site.authToken = null; + await expect(site.refreshIdentity()).resolves.toBe(false); + expect(site.jsonApi).not.toHaveBeenCalled(); + }); + + test('a missing display name falls back to the handle without corrupting state', async () => { + const site = identitySite([session('tomrodriguez', '')]); + await expect(site.refreshIdentity()).resolves.toBe(true); + expect(site.name).toBeNull(); + expect(memberDisplayName(site.name, site.username)).toBe('Tomrodriguez'); + }); +}); + +describe('refresh is bounded and cannot loop', () => { + test('concurrent foreground triggers share one in-flight request', async () => { + const site = identitySite([session('tomrodriguez', 'Tom Rodriguez')]); + const instance = manager(site); + + const results = await Promise.all([ + instance.refreshActiveIdentity(), + instance.refreshActiveIdentity(), + instance.refreshActiveIdentity(), + ]); + + expect(site.jsonApi).toHaveBeenCalledTimes(1); + expect(results).toEqual([true, true, true]); + // The in-flight slot is released so a later foreground can refresh again. + expect(instance._identityRefresh).toBeNull(); + }); + + test('the retired multi-site refresh loop is not revived', async () => { + const site = identitySite([session('tomrodriguez', 'Tom Rodriguez')]); + const instance = manager(site); + instance.refreshSites = jest.fn(); + instance._throttledRefreshSites = jest.fn(); + + await instance.refreshActiveIdentity(); + + expect(instance.refreshSites).not.toHaveBeenCalled(); + expect(instance._throttledRefreshSites).not.toHaveBeenCalled(); + expect(site.jsonApi).toHaveBeenCalledTimes(1); + }); + + test('no authenticated site means no request at all', async () => { + const instance = manager(null); + await expect(instance.refreshActiveIdentity()).resolves.toBe(false); + expect(instance.save).not.toHaveBeenCalled(); + }); +}); + +describe('persistence and backward compatibility', () => { + test('name is serialized so a relaunch keeps the refreshed identity', () => { + expect(Site.FIELDS).toContain('name'); + expect(Site.FIELDS).toContain('username'); + + const site = new Site({ + url: ORIGIN, + username: 'tomrodriguez', + name: 'Tom Rodriguez', + }); + const persisted = JSON.parse(JSON.stringify(site)); + expect(persisted.name).toBe('Tom Rodriguez'); + expect(persisted.username).toBe('tomrodriguez'); + // Rehydration is what a cold launch performs. + const rehydrated = new Site(persisted); + expect(memberDisplayName(rehydrated.name, rehydrated.username)).toBe( + 'Tom Rodriguez', + ); + }); + + test('an existing record with no name upgrades without migration', async () => { + // A record written by a build that never stored a display name. + const legacy = new Site({ url: ORIGIN, username: 'finale2e' }); + expect(legacy.name).toBeUndefined(); + expect(memberDisplayName(legacy.name, legacy.username)).toBe('Finale2e'); + + legacy.authToken = 'token'; + legacy.jsonApi = jest.fn(() => + Promise.resolve(session('tomrodriguez', 'Tom Rodriguez')), + ); + + await expect(legacy.refreshIdentity()).resolves.toBe(true); + expect(memberDisplayName(legacy.name, legacy.username)).toBe( + 'Tom Rodriguez', + ); + }); + + test('logging off clears the whole identity, not just the handle', () => { + const site = new Site({ url: ORIGIN }); + site.authToken = 'token'; + site.username = 'tomrodriguez'; + site.name = 'Tom Rodriguez'; + + site.logoff(); + + expect(site.username).toBeNull(); + expect(site.name).toBeNull(); + expect(site.authToken).toBeNull(); + }); +}); + +describe('username-dependent surfaces follow the refreshed identity', () => { + test('routes and self-checks all read the refreshed username', async () => { + const site = identitySite([session('tomrodriguez', 'Tom Rodriguez')], { + username: 'finale2e', + name: null, + }); + await site.refreshIdentity(); + + // Own-profile and bookmarks routes are built from site.username. + expect(`/u/${encodeURIComponent(site.username)}.json`).toBe( + '/u/tomrodriguez.json', + ); + expect( + `/u/${encodeURIComponent(site.username)}/activity/bookmarks.json`, + ).toBe('/u/tomrodriguez/activity/bookmarks.json'); + + // Self-detection used by can-edit, Lounge self-delete and moderation. + expect('tomrodriguez' === site.username).toBe(true); + expect('finale2e' === site.username).toBe(false); + }); +}); + +describe('a new authorization never inherits the previous identity', () => { + test('the prior identity is cleared before the new credential is used', async () => { + const previous = new Site({ + url: ORIGIN, + username: 'finale2e', + name: 'Finale E2E', + }); + previous.authToken = 'old-token'; + + // handleAuthPayload clears identity before binding; model that boundary. + previous.username = null; + previous.name = null; + previous.authToken = 'new-token'; + previous.jsonApi = jest.fn(() => + Promise.resolve(session('cert_probe_01', 'Cert Probe 01')), + ); + + await expect(previous.refreshIdentity()).resolves.toBe(true); + expect(previous.username).toBe('cert_probe_01'); + expect(previous.name).toBe('Cert Probe 01'); + }); + + test('handleAuthPayload clears identity and refreshes it', () => { + const fs = require('fs'); + const path = require('path'); + const source = fs.readFileSync( + path.join(__dirname, '..', 'site_manager.js'), + 'utf8', + ); + expect(source).toContain('nonceSite.username = null'); + expect(source).toContain('nonceSite.name = null'); + expect(source).toContain('await nonceSite.refreshIdentity()'); + }); + + test('the foreground lifecycle refreshes identity before notifications', () => { + const fs = require('fs'); + const path = require('path'); + const source = fs.readFileSync( + path.join(__dirname, '..', 'Discourse.js'), + 'utf8', + ); + expect(source).toContain('await this._siteManager.refreshActiveIdentity()'); + // The retired multi-site loop stays retired on the authenticated path. + const foreground = source.slice( + source.indexOf('async _refreshAuthenticatedResources'), + source.indexOf('async _refresh()'), + ); + expect(foreground).not.toContain('refreshSites()'); + }); +}); + +test('AsyncStorage is the identity store of record', async () => { + const site = new Site({ url: ORIGIN, username: 'tomrodriguez' }); + site.name = 'Tom Rodriguez'; + const instance = manager(site); + instance.save = SiteManager.prototype.save.bind(instance); + + instance.save(); + + const raw = await AsyncStorage.getItem('@Discourse.sites'); + const parsed = JSON.parse(raw); + expect(parsed[0].username).toBe('tomrodriguez'); + expect(parsed[0].name).toBe('Tom Rodriguez'); + // The credential is never written to AsyncStorage. + expect(raw).not.toContain('authToken":"'); +}); diff --git a/js/__tests__/memberImageSource.test.js b/js/__tests__/memberImageSource.test.js new file mode 100644 index 000000000..2c22e827c --- /dev/null +++ b/js/__tests__/memberImageSource.test.js @@ -0,0 +1,255 @@ +import { + authenticatedOriginHeaders, + isMemberPhotoUrl, + memberImageSource, +} from '../product/memberImageSource'; + +const CANONICAL = 'https://adjusternetwork.org'; +const site = { url: CANONICAL, authToken: 'user-api-key', clientId: 'client' }; + +describe('authenticated member image requests', () => { + test('attaches the User API credential for the governed origin', () => { + const source = memberImageSource( + site, + `${CANONICAL}/renaissance/member-photo/tomrodriguez/120/7`, + ); + + expect(source).toEqual({ + uri: `${CANONICAL}/renaissance/member-photo/tomrodriguez/120/7`, + headers: { + 'User-Api-Key': 'user-api-key', + 'User-Api-Client-Id': 'client', + }, + }); + }); + + test('never sends the credential to any other host', () => { + for (const uri of [ + 'https://evil.example.com/renaissance/member-photo/tomrodriguez/120/7', + 'https://adjusternetwork.org.evil.example.com/a.png', + 'https://cdn.adjusternetwork.org/a.png', + 'https://staging.adjusternetwork.org/renaissance/member-photo/x/120/7', + 'https://www.gravatar.com/avatar/abc.png', + ]) { + expect(memberImageSource(site, uri)).toEqual({ uri }); + } + }); + + test('never sends the credential over plain HTTP', () => { + const uri = 'http://adjusternetwork.org/renaissance/member-photo/x/120/7'; + expect(memberImageSource(site, uri)).toEqual({ uri }); + }); + + test('a signed-out viewer sends no credential', () => { + const uri = `${CANONICAL}/renaissance/member-photo/tomrodriguez/120/7`; + expect(memberImageSource({ url: CANONICAL }, uri)).toEqual({ uri }); + expect(memberImageSource(null, uri)).toEqual({ uri }); + expect(memberImageSource({ ...site, authToken: null }, uri)).toEqual({ + uri, + }); + }); + + test('no credential ever reaches the URL, query string or cache key', () => { + const source = memberImageSource( + site, + `${CANONICAL}/renaissance/member-photo/tomrodriguez/120/7`, + ); + expect(source.uri).not.toContain('user-api-key'); + expect(source.uri).not.toContain('User-Api-Key'); + expect(source.uri).toBe( + `${CANONICAL}/renaissance/member-photo/tomrodriguez/120/7`, + ); + expect(source.uri.includes('?')).toBe(false); + }); + + test('a missing URL yields no source so the letter avatar renders', () => { + expect(memberImageSource(site, null)).toBeNull(); + expect(memberImageSource(site, '')).toBeNull(); + expect(memberImageSource(site, undefined)).toBeNull(); + }); + + test('a missing client id still sends a well-formed header pair', () => { + const source = memberImageSource( + { url: CANONICAL, authToken: 'key' }, + `${CANONICAL}/renaissance/member-photo/x/120/7`, + ); + expect(source.headers['User-Api-Client-Id']).toBe(''); + expect(source.headers['User-Api-Key']).toBe('key'); + }); +}); + +describe('the shared Avatar is the only member-photo loader', () => { + const fs = require('fs'); + const path = require('path'); + const read = file => + fs.readFileSync(path.join(__dirname, '..', file), 'utf8'); + + test('Avatar routes its image through the authenticated source', () => { + const source = read('product/ProductComponents.js'); + expect(source).toContain('memberImageSource(site, resolvedUri)'); + // The unauthenticated form must not survive anywhere in the component. + expect(source).not.toContain('source={{ uri: resolvedUri }}'); + expect(source).toContain('export const MemberAvatar = Avatar'); + }); + + test('an account switch clears every cached avatar record', () => { + const source = read('site_manager.js'); + const reset = source.slice( + source.indexOf('async resetAuthorizationIdentity'), + source.indexOf('setActiveSite(site)'), + ); + expect(reset).toContain('clearAvatarAuthorities()'); + // Logout already clears the per-site records. + expect(source).toContain('clearAvatarAuthorityForSite(removableSite)'); + }); +}); + +describe('adversarial origins never receive the User API credential', () => { + // Post cooked HTML is member-authored, so a hostile absolute URL can reach + // the media loader directly. Every one of these must come back bare. + const HOSTILE = [ + 'https://evil.example.com/uploads/a.png', + 'https://adjusternetwork.org.evil.example.com/uploads/a.png', + 'https://evil.example.com/?next=https://adjusternetwork.org/uploads/a.png', + 'https://evil.example.com#https://adjusternetwork.org/uploads/a.png', + 'https://cdn.adjusternetwork.org/uploads/a.png', + 'https://staging.adjusternetwork.org/uploads/a.png', + 'https://adjusternetwork.org.example.com/uploads/a.png', + 'https://adjusternetwork.orgevil.com/uploads/a.png', + 'http://adjusternetwork.org/uploads/a.png', + 'http://evil.example.com/uploads/a.png', + 'https://user:pass@evil.example.com/uploads/a.png', + 'https://adjusternetwork.org:8443/uploads/a.png', + 'ftp://adjusternetwork.org/uploads/a.png', + 'file:///etc/passwd', + 'data:image/png;base64,AAAA', + 'javascript:alert(1)', + '//evil.example.com/uploads/a.png', + ]; + + test.each(HOSTILE)('no credential for %s', uri => { + expect(authenticatedOriginHeaders(site, uri)).toBeUndefined(); + expect(memberImageSource(site, uri)).toEqual({ uri }); + }); + + test('the trusted origin still authenticates media on the shared guard', () => { + for (const uri of [ + `${CANONICAL}/renaissance/member-photo/tomrodriguez/120/7`, + `${CANONICAL}/secure-uploads/original/1X/abc.png`, + `${CANONICAL}/uploads/default/original/1X/abc.png`, + ]) { + expect(authenticatedOriginHeaders(site, uri)).toEqual({ + 'User-Api-Key': 'user-api-key', + 'User-Api-Client-Id': 'client', + }); + } + }); + + test('a signed-out or missing viewer never authenticates', () => { + const uri = `${CANONICAL}/secure-uploads/original/1X/abc.png`; + expect(authenticatedOriginHeaders(null, uri)).toBeUndefined(); + expect(authenticatedOriginHeaders({ url: CANONICAL }, uri)).toBeUndefined(); + expect(authenticatedOriginHeaders(site, null)).toBeUndefined(); + expect(authenticatedOriginHeaders(site, '')).toBeUndefined(); + }); +}); + +describe('secure media reuses the same origin guard', () => { + const fs = require('fs'); + const path = require('path'); + const source = fs.readFileSync( + path.join(__dirname, '..', 'product', 'DiscourseMedia.js'), + 'utf8', + ); + + test('media images and the attachment viewer are both guarded', () => { + expect(source).toContain( + 'const headers = authenticatedOriginHeaders(site, state.url)', + ); + expect(source).toContain( + 'const headers = authenticatedOriginHeaders(site, state.authorizedUrl)', + ); + }); + + test('no unguarded credential construction remains anywhere', () => { + // The only place these header names may appear is the guarded helper. + expect(source).not.toContain("'User-Api-Key': site.authToken"); + const helper = fs.readFileSync( + path.join(__dirname, '..', 'product', 'memberImageSource.js'), + 'utf8', + ); + expect(helper).toContain('isCanonicalUrl(uri)'); + const components = fs.readFileSync( + path.join(__dirname, '..', 'product', 'ProductComponents.js'), + 'utf8', + ); + expect(components).not.toContain("'User-Api-Key'"); + }); +}); + +describe('only the private member-photo route authenticates an avatar', () => { + // Regression guard. Authenticating ordinary Discourse avatars turned every + // rendered avatar into a counted user-API request. Those requests are issued + // by the native image pipeline, outside the app's request orchestrator, so + // they exhausted the member's rate limit and starved /latest.json - which is + // what Discussions needs to load. + test('ordinary Discourse avatars are loaded without any credential', () => { + for (const path of [ + '/user_avatar/adjusternetwork.org/tomrodriguez/120/1234_2.png', + '/letter_avatar_proxy/v4/letter/t/abc/120.png', + '/uploads/default/original/1X/abc.png', + '/images/avatar.png', + ]) { + const uri = `${CANONICAL}${path}`; + expect(memberImageSource(site, uri)).toEqual({ uri }); + expect(isMemberPhotoUrl(uri)).toBe(false); + } + }); + + test('the governed member-photo route still authenticates', () => { + const uri = `${CANONICAL}/renaissance/member-photo/tomrodriguez/120/7`; + expect(isMemberPhotoUrl(uri)).toBe(true); + expect(memberImageSource(site, uri)).toEqual({ + uri, + headers: { + 'User-Api-Key': 'user-api-key', + 'User-Api-Client-Id': 'client', + }, + }); + }); + + test('a member-photo path on an untrusted origin is still refused', () => { + for (const uri of [ + 'https://evil.example.com/renaissance/member-photo/tomrodriguez/120/7', + 'http://adjusternetwork.org/renaissance/member-photo/tomrodriguez/120/7', + 'https://adjusternetwork.org.evil.example.com/renaissance/member-photo/x/1/1', + ]) { + expect(memberImageSource(site, uri)).toEqual({ uri }); + } + }); + + test('a look-alike path prefix does not qualify', () => { + for (const path of [ + '/renaissance/member-photos/x/120/7', + '/renaissance/member-photo', + '/x/renaissance/member-photo/x/120/7', + '/renaissance/member-photox/x/120/7', + ]) { + expect(isMemberPhotoUrl(`${CANONICAL}${path}`)).toBe(false); + } + }); + + test('secure media keeps its own guard and is unaffected', () => { + // Media already authenticated before this work, so its request count is + // unchanged; only the origin restriction was added. + expect( + authenticatedOriginHeaders( + site, + `${CANONICAL}/secure-uploads/original/1X/abc.png`, + ), + ).toEqual({ + 'User-Api-Key': 'user-api-key', + 'User-Api-Client-Id': 'client', + }); + }); +}); diff --git a/js/__tests__/memberRouteBoundaries.test.js b/js/__tests__/memberRouteBoundaries.test.js new file mode 100644 index 000000000..65ee2123e --- /dev/null +++ b/js/__tests__/memberRouteBoundaries.test.js @@ -0,0 +1,100 @@ +import DiscourseUtils from '../DiscourseUtils'; +import { classifyFirstPartyMemberRoute } from '../nativeMemberRouting'; + +// Salvaged from the notification-routing experiment's suite. These assert the +// URL-level classifier boundaries, which are unchanged by the move to native +// notification intents and are still the last line of defence for deep links. +const ORIGIN = 'https://adjusternetwork.org'; +const site = { url: ORIGIN, username: 'tomrodriguez', isStaff: false }; +const member = { authenticated: true, isStaff: false }; +const staff = { authenticated: true, isStaff: true }; + +const routeFor = (notification, opts = member) => + classifyFirstPartyMemberRoute( + DiscourseUtils.endpointForSiteNotification(site, notification), + opts, + ); + +describe('member route classifier boundaries', () => { + test('off-origin destinations remain rejected', () => { + for (const url of [ + 'https://evil.example.com/badges/7/basic', + 'https://adjusternetwork.org.evil.example.com/g/staff', + 'https://staging.adjusternetwork.org/chat/channel/2/lounge', + 'http://adjusternetwork.org/badges/7/basic', + ]) { + expect(classifyFirstPartyMemberRoute(url, member)).toEqual({ + disposition: 'rejected', + }); + } + }); + + test('non-staff /admin remains rejected and staff behaviour is unchanged', () => { + expect(routeFor({ notification_type: 37, data: {} })).toEqual({ + disposition: 'rejected', + }); + expect(routeFor({ notification_type: 37, data: {} }, staff)).toEqual({ + disposition: 'privileged_external', + url: `${ORIGIN}/admin`, + }); + }); + + test('unknown or empty notification endpoints remain rejected', () => { + expect(routeFor({ notification_type: 999, data: {} })).toEqual({ + disposition: 'rejected', + }); + expect(classifyFirstPartyMemberRoute(ORIGIN, member)).toEqual({ + disposition: 'rejected', + }); + expect(classifyFirstPartyMemberRoute(`${ORIGIN}/`, member)).toEqual({ + disposition: 'rejected', + }); + }); + + test('unauthenticated callers never route anywhere', () => { + expect( + classifyFirstPartyMemberRoute(`${ORIGIN}/badges/7/basic`, { + authenticated: false, + }), + ).toEqual({ disposition: 'rejected' }); + }); + + test('canonical pages without a native screen are rejected, not opened', () => { + // The first-party-web allowlist is gone: an internal page with no native + // screen must not fall through to a WebView. + for (const path of [ + '/badges/7/basic', + '/g/staff', + '/chat/channel/2/lounge', + '/u/tomrodriguez/messages/group/staff', + '/latest', + '/site.json', + '/badges', + ]) { + expect( + classifyFirstPartyMemberRoute(`${ORIGIN}${path}`, member).disposition, + ).toBe('rejected'); + } + }); + + test('openUrl handles every remaining presentation explicitly', () => { + const fs = require('fs'); + const path = require('path'); + const source = fs.readFileSync( + path.join(__dirname, '..', 'Discourse.js'), + 'utf8', + ); + const openUrl = source.slice( + source.indexOf(' openUrl(url) {'), + source.indexOf(' _navigateNative('), + ); + expect(openUrl).toContain('destinationPresentation(route)'); + for (const kind of ['native', 'external']) { + expect(openUrl).toContain(`presentation.kind === '${kind}'`); + } + // The web kind no longer exists, and deep links never open a WebView. + expect(openUrl).not.toContain("presentation.kind === 'web'"); + expect(openUrl).not.toContain("navigate('WebView'"); + expect(openUrl).toContain("securityEvent('navigation.rejected')"); + }); +}); diff --git a/js/__tests__/memberUtilitySurfaces.test.js b/js/__tests__/memberUtilitySurfaces.test.js index ee0577660..0ff3f7740 100644 --- a/js/__tests__/memberUtilitySurfaces.test.js +++ b/js/__tests__/memberUtilitySurfaces.test.js @@ -3,6 +3,11 @@ import fs from 'fs'; import path from 'path'; +import { + discussionSearchEligible, + memberSearchResults, + searchResults, +} from '../product/memberUtilities'; const read = file => fs.readFileSync(path.join(__dirname, '..', file), 'utf8'); @@ -53,6 +58,23 @@ describe('native member utility surfaces', () => { expect(source).not.toContain('WebView'); }); + test.each([ + ['qa', false], + ['QA', false], + ['reviewer', true], + ['admin', true], + ['roof', true], + ])('discussion search eligibility for %s is %s', (query, expected) => { + expect(discussionSearchEligible(query)).toBe(expected); + }); + + test('a two-character member query can return the normal empty state', () => { + expect(searchResults({ topics: [], posts: [], users: [] })).toEqual([]); + expect( + memberSearchResults({ schema: 'an.member-search.v1', results: [] }), + ).toEqual([]); + }); + test('member search uses only contract-returned professional metadata', () => { const helper = read('product/memberUtilities.js'); expect(helper).toContain("payload?.schema !== 'an.member-search.v1'"); diff --git a/js/__tests__/nativeMediaAttachments.test.js b/js/__tests__/nativeMediaAttachments.test.js index 9ad484235..4996a933f 100644 --- a/js/__tests__/nativeMediaAttachments.test.js +++ b/js/__tests__/nativeMediaAttachments.test.js @@ -249,7 +249,7 @@ describe('native Discourse media attachments', () => { test('opens the real PDF route in an authenticated in-app viewer', async () => { const site = { - url: 'https://staging.adjusternetwork.org', + url: 'https://adjusternetwork.org', authToken: 'test-user-api-key', clientId: 'test-client', }; @@ -287,6 +287,42 @@ describe('native Discourse media attachments', () => { }); }); + test('never sends the credential to an off-origin attachment', async () => { + // The OTA channel is production in this suite, so any other host - the + // staging origin included - must not receive the User API credential. + const site = { + url: 'https://adjusternetwork.org', + authToken: 'test-user-api-key', + clientId: 'test-client', + }; + const url = + 'https://staging.adjusternetwork.org/secure-uploads/original/1X/synthetic.pdf'; + const refreshMedia = jest.fn().mockResolvedValue(url); + let renderer; + act(() => { + renderer = TestRenderer.create( + , + ); + }); + + await act(async () => { + renderer.root + .findByProps({ accessibilityLabel: 'Open attachment field-notes.pdf' }) + .props.onPress(); + await Promise.resolve(); + }); + + const viewer = renderer.root.findByProps({ + accessibilityLabel: 'Attachment field-notes.pdf', + }); + expect(viewer.props.source).toEqual({ uri: url, headers: undefined }); + }); + test('supports canonical and legacy Discourse attachment representations', () => { const site = { url: 'https://staging.adjusternetwork.org' }; expect( diff --git a/js/__tests__/notificationIntent.test.js b/js/__tests__/notificationIntent.test.js new file mode 100644 index 000000000..e3c78152a --- /dev/null +++ b/js/__tests__/notificationIntent.test.js @@ -0,0 +1,364 @@ +jest.mock('@react-native-vector-icons/fontawesome5', () => 'FontAwesome5'); + +import React from 'react'; +import renderer from 'react-test-renderer'; +import DiscourseUtils from '../DiscourseUtils'; +import { classifyFirstPartyMemberRoute } from '../nativeMemberRouting'; +import { + NOTIFICATION_UNAVAILABLE, + destinationPresentation, +} from '../notificationDestination'; +import { notificationIntent } from '../notificationIntent'; +import BadgeEarnedScreen from '../product/BadgeEarnedScreen'; + +const ORIGIN = 'https://adjusternetwork.org'; +const site = { + url: ORIGIN, + username: 'tomrodriguez', + authToken: 'user-api-key', + isStaff: false, +}; +const staffSite = { ...site, isStaff: true }; +const member = { authenticated: true, isStaff: false }; +const staff = { authenticated: true, isStaff: true }; + +const intent = (notification, opts = member) => + notificationIntent(site, notification, opts); + +// The real Discourse granted_badge payload. badge_title is a boolean - whether +// the badge may be worn as a title - not descriptive text, which is why V1 +// renders badge_name only (app/services/badge_granter.rb). +const grantedBadge = { + notification_type: 12, + topic_id: null, + post_number: null, + data: { + badge_id: 7, + badge_name: 'Autobiographer', + badge_slug: 'autobiographer', + badge_title: false, + username: 'tomrodriguez', + }, +}; + +const topicNotification = type => ({ + notification_type: type, + slug: 'a-discussion', + topic_id: 41, + post_number: 3, + data: { topic_title: 'A discussion' }, +}); + +describe('B: granted_badge resolves to a native badge intent', () => { + test('badge_name survives directly from the payload', () => { + expect(intent(grantedBadge)).toEqual({ + kind: 'badge', + badge: { name: 'Autobiographer' }, + }); + }); + + test('the URL form would have lost badge_name, which is why the payload wins', () => { + // Proof of the architectural reason for the intent layer: the endpoint + // carries the id and username only. + const url = DiscourseUtils.endpointForSiteNotification(site, grantedBadge); + expect(url).toContain('/badges/7/'); + expect(url).not.toContain('Autobiographer'); + }); + + test('badge_title is never treated as descriptive text', () => { + const titled = { + ...grantedBadge, + data: { ...grantedBadge.data, badge_title: true }, + }; + expect(intent(titled)).toEqual({ + kind: 'badge', + badge: { name: 'Autobiographer' }, + }); + }); + + test('a badge payload without a usable name falls to the bounded state', () => { + for (const badge_name of [ + undefined, + null, + '', + ' ', + 42, + {}, + 'x'.repeat(121), + ]) { + expect( + intent({ ...grantedBadge, data: { ...grantedBadge.data, badge_name } }), + ).toEqual({ kind: 'unavailable' }); + } + expect(intent({ notification_type: 12 })).toEqual({ kind: 'unavailable' }); + }); +}); + +describe('BadgeEarned screen', () => { + const render = (name, navigation) => { + let tree; + renderer.act(() => { + tree = renderer.create( + , + ); + }); + return tree; + }; + + test('renders the badge name and the earned framing', () => { + const json = JSON.stringify(render('Autobiographer').toJSON()); + expect(json).toContain('Autobiographer'); + expect(json).toContain('Badge earned'); + }); + + test('performs no network request', async () => { + const fetchSpy = jest + .spyOn(global, 'fetch') + .mockRejectedValue(new Error('no network expected')); + render('Autobiographer'); + await Promise.resolve(); + expect(fetchSpy).not.toHaveBeenCalled(); + fetchSpy.mockRestore(); + }); + + test('closes through goBack from both the header and the action', () => { + const goBack = jest.fn(); + const tree = render('Autobiographer', { goBack }); + const pressables = tree.root.findAll( + node => + typeof node.props.onPress === 'function' && + (node.props.accessibilityLabel === 'Back' || + node.props.accessibilityLabel === 'Close'), + ); + expect(pressables.length).toBeGreaterThanOrEqual(2); + for (const node of pressables) { + renderer.act(() => node.props.onPress()); + } + expect(goBack).toHaveBeenCalledTimes(pressables.length); + }); +}); + +describe('A: existing native routing is unchanged', () => { + test.each([ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 13, 14, 15, 17, 18, 20, 24, 25, 27, 28, + 34, 36, 801, 802, + ])('type %i still opens the Topic screen', type => { + expect(intent(topicNotification(type))).toEqual({ + kind: 'native', + screen: 'Topic', + params: { topicId: 41, url: `${ORIGIN}/t/a-discussion/41/3` }, + }); + }); + + test('following notifications still open MemberProfile', () => { + expect( + intent({ notification_type: 800, data: { display_username: 'ana' } }), + ).toMatchObject({ kind: 'native', screen: 'MemberProfile' }); + }); + + test('approval_given without a topic opens the profile activity it names', () => { + // Pre-existing behaviour: the endpoint is /u/:me/activity/approval-given, + // which the profile pattern already matches. A valid native destination + // exists, so this is class A and is deliberately left alone. + expect(intent({ notification_type: 21, data: {} })).toEqual({ + kind: 'native', + screen: 'MemberProfile', + params: { username: 'tomrodriguez' }, + }); + }); + + test('code review approval opens the topic when it has one', () => { + expect( + intent({ + notification_type: 21, + fancy_title: 'Approved', + slug: 'a-discussion', + topic_id: 41, + post_number: 3, + data: {}, + }), + ).toMatchObject({ kind: 'native', screen: 'Topic' }); + }); +}); + +describe('C: unsupported member types reach one bounded state', () => { + test.each([ + [ + 'group_message_summary', + { + notification_type: 16, + data: { username: 'tomrodriguez', group_name: 'staff' }, + }, + ], + [ + 'liked_consolidated', + { notification_type: 19, data: { username: 'ana' } }, + ], + [ + 'membership_request_accepted', + { notification_type: 22, data: { group_name: 'adjusters' } }, + ], + ['membership_request_consolidated', { notification_type: 23, data: {} }], + ['votes_released', { notification_type: 26, data: {} }], + [ + 'chat_mention', + { + notification_type: 29, + data: { + chat_channel_id: 2, + chat_channel_title: 'lounge', + chat_message_id: 9, + }, + }, + ], + [ + 'chat_message', + { + notification_type: 30, + data: { chat_channel_id: 2, chat_channel_title: 'lounge' }, + }, + ], + [ + 'chat_invitation', + { + notification_type: 31, + data: { chat_channel_id: 2, chat_channel_title: 'lounge' }, + }, + ], + [ + 'chat_group_mention', + { + notification_type: 32, + data: { + chat_channel_id: 2, + chat_channel_title: 'lounge', + chat_message_id: 9, + }, + }, + ], + ['unknown type', { notification_type: 9999, data: {} }], + ['absent type', { data: {} }], + ])('%s is unavailable, never a silent no-op', (_label, notification) => { + expect(intent(notification)).toEqual({ kind: 'unavailable' }); + }); + + test('the copy promises no loading, no login and no browser', () => { + expect(NOTIFICATION_UNAVAILABLE.close).toBe('Close'); + expect(NOTIFICATION_UNAVAILABLE.message).toMatch(/marked as read/i); + expect(NOTIFICATION_UNAVAILABLE.message).not.toMatch( + /log ?in|sign ?in|browser|Safari|try again/i, + ); + }); +}); + +describe('security boundaries', () => { + test('staff /admin remains the one external handoff', () => { + for (const type of [37, 38]) { + expect( + notificationIntent( + staffSite, + { notification_type: type, data: {} }, + staff, + ), + ).toEqual({ kind: 'staff_external', url: `${ORIGIN}/admin` }); + } + }); + + test('a member never receives the staff external handoff', () => { + for (const type of [37, 38]) { + expect(intent({ notification_type: type, data: {} })).toEqual({ + kind: 'unavailable', + }); + } + }); + + test('an unauthenticated caller resolves nothing', () => { + expect( + notificationIntent(site, grantedBadge, { authenticated: false }), + ).toEqual({ kind: 'unavailable' }); + expect( + notificationIntent(site, topicNotification(2), { authenticated: false }), + ).toEqual({ kind: 'unavailable' }); + }); + + test('off-origin and non-canonical destinations stay unavailable', () => { + const evil = { + url: 'https://evil.example.com', + username: 'x', + authToken: 't', + }; + expect(notificationIntent(evil, topicNotification(2), member)).toEqual({ + kind: 'unavailable', + }); + }); + + test('destinationPresentation no longer emits a web disposition', () => { + expect( + destinationPresentation({ disposition: 'first_party_web', url: ORIGIN }), + ).toEqual({ + kind: 'denied', + }); + expect( + destinationPresentation( + classifyFirstPartyMemberRoute(`${ORIGIN}/badges/7/basic`, member), + ), + ).toEqual({ kind: 'denied' }); + }); +}); + +describe('the experiment is gone and the guard is back', () => { + const fs = require('fs'); + const path = require('path'); + const read = f => fs.readFileSync(path.join(__dirname, '..', f), 'utf8'); + + test('webViewSession and the first-party-web allowlist no longer exist', () => { + expect(fs.existsSync(path.join(__dirname, '..', 'webViewSession.js'))).toBe( + false, + ); + expect(read('nativeMemberRouting.js')).not.toContain('first_party_web'); + expect(read('nativeMemberRouting.js')).not.toContain( + 'FIRST_PARTY_WEB_PATHS', + ); + expect(read('Discourse.js')).not.toContain('webViewSession'); + expect(read('Discourse.js')).not.toContain('_openFirstPartyWeb'); + }); + + test('no notification intent can bootstrap a WebView', () => { + const source = read('Discourse.js'); + const handler = source.slice( + source.indexOf(' openNotification(site, notification) {'), + source.indexOf(' // A member must never be trapped'), + ); + expect(handler).not.toContain('WebView'); + expect(handler).not.toContain('otp'); + expect(handler).toContain("navigate('BadgeEarned'"); + expect(handler).toContain('NOTIFICATION_UNAVAILABLE.title'); + }); + + test('the strict WebView navigation guard is restored', () => { + const source = read('screens/WebViewScreenComponents/WebViewComponent.js'); + expect(source).toContain( + '// Canonical pages without an explicit native route must not', + ); + expect(source).not.toContain('pendingDestination'); + expect(source).not.toContain('isOtpBootstrapUrl'); + expect(read('screens/WebViewScreen.js')).not.toContain('destination='); + }); + + test('read-marking still precedes intent resolution', () => { + const source = read('screens/NotificationsScreen.js'); + const block = source.slice( + source.indexOf('_openNotificationForSite('), + source.indexOf('_listIndex(row)'), + ); + expect(block.indexOf('markNotificationRead')).toBeLessThan( + block.indexOf('openNotification('), + ); + // The tap site hands on the notification, not a lossy URL. + expect(block).not.toContain('endpointForSiteNotification('); + expect(block).not.toContain('DiscourseUtils'); + }); +}); diff --git a/js/__tests__/rateLimitResilience.test.js b/js/__tests__/rateLimitResilience.test.js new file mode 100644 index 000000000..2d84c7936 --- /dev/null +++ b/js/__tests__/rateLimitResilience.test.js @@ -0,0 +1,217 @@ +import { + RATE_LIMIT_MAX_MS, + RATE_LIMIT_MIN_MS, + rateLimitDelayMs, + retryAfterDelayMs, +} from '../apiRateLimit'; +import { RequestOrchestrator } from '../requestOrchestrator'; +import { + AVATAR_RECOVERY_MAX_ATTEMPTS, + avatarRecoveryDelayMs, + avatarShowsImage, + shouldAttemptAvatarRecovery, +} from '../product/avatarRecovery'; + +const responseWith = retryAfter => ({ + status: 429, + headers: { get: name => (name === 'Retry-After' ? retryAfter : null) }, +}); + +describe('P1: Retry-After is honored by the shared cooldown', () => { + test('regression: passing the response set a zero-length cooldown', () => { + // retryAfterDelayMs takes the header VALUE. Handing it the response + // returned null, and now() + null === now(), so the cooldown expired + // immediately and every request sailed through an active limiter window. + expect(retryAfterDelayMs(responseWith('30'))).toBeNull(); + expect(Date.now() + null).toBe(Date.now() + 0); + }); + + test('rateLimitDelayMs reads Retry-After off the response', () => { + expect(rateLimitDelayMs(responseWith('30'), 0)).toBe(30000); + expect(rateLimitDelayMs(responseWith('5'), 0)).toBe(5000); + }); + + test('a 429 now produces a real cooldown that blocks new requests', async () => { + let clock = 1000; + const slept = []; + const orchestrator = new RequestOrchestrator({ + now: () => clock, + sleep: ms => { + slept.push(ms); + clock += ms; + return Promise.resolve(); + }, + }); + + const delay = orchestrator.beginCooldown('b', responseWith('30'), 0); + expect(delay).toBe(30000); + expect(orchestrator.cooldowns.get('b')).toBe(1000 + 30000); + + await orchestrator.waitForBucket('b'); + // The waiter actually slept for the directed window. + expect(slept.reduce((a, b) => a + b, 0)).toBeGreaterThanOrEqual(30000); + }); + + test('an absent Retry-After falls back to a bounded backoff, never zero', () => { + let clock = 0; + const orchestrator = new RequestOrchestrator({ + now: () => clock, + sleep: () => Promise.resolve(), + }); + const first = orchestrator.beginCooldown('b', responseWith(null), 0); + expect(first).toBeGreaterThanOrEqual(RATE_LIMIT_MIN_MS); + expect(orchestrator.cooldowns.get('b')).toBeGreaterThan(clock); + }); + + test('cooldowns stay bounded so auth and logout cannot deadlock', () => { + expect(rateLimitDelayMs(responseWith('99999'), 0)).toBe(RATE_LIMIT_MAX_MS); + expect(rateLimitDelayMs(responseWith('-5'), 0)).toBeLessThanOrEqual( + RATE_LIMIT_MAX_MS, + ); + }); + + test('a longer directed window never shortens an existing cooldown', () => { + let clock = 0; + const orchestrator = new RequestOrchestrator({ + now: () => clock, + sleep: () => Promise.resolve(), + }); + orchestrator.beginCooldown('b', responseWith('40'), 0); + orchestrator.beginCooldown('b', responseWith('5'), 1); + expect(orchestrator.cooldowns.get('b')).toBe(40000); + }); + + test('no amplification: concurrent waiters observe one window', async () => { + let clock = 0; + let sleeps = 0; + const orchestrator = new RequestOrchestrator({ + now: () => clock, + sleep: ms => { + sleeps += 1; + clock += ms; + return Promise.resolve(); + }, + }); + orchestrator.beginCooldown('b', responseWith('10'), 0); + await Promise.all([ + orchestrator.waitForBucket('b'), + orchestrator.waitForBucket('b'), + orchestrator.waitForBucket('b'), + ]); + // Once the window has elapsed the bucket is cleared, not re-slept forever. + expect(orchestrator.cooldowns.has('b')).toBe(false); + expect(sleeps).toBeLessThanOrEqual(3); + }); +}); + +describe('P1: new requests are gated on the shared user-api cooldown', () => { + const fs = require('fs'); + const path = require('path'); + const source = fs.readFileSync(path.join(__dirname, '..', 'site.js'), 'utf8'); + + test('site.jsonApi waits on the user-api and endpoint buckets only', () => { + expect(source).toContain( + 'await requestOrchestrator.waitForBucket(globalUserBucket)', + ); + expect(source).toContain( + 'await requestOrchestrator.waitForBucket(fallbackBucket)', + ); + // The IP-bucket pre-request wait was reverted: scope stays minimal to the + // proven User API limiter defect. + expect(source).not.toContain('ipBucket'); + }); +}); + +describe('P2: avatar recovers from a transient failure, boundedly', () => { + test('the initial is not terminal while attempts remain', () => { + const uri = + 'https://adjusternetwork.org/renaissance/member-photo/t/72/67.png'; + expect( + avatarShowsImage({ resolvedUri: uri, failedUri: null, attempt: 0 }), + ).toBe(true); + // A failure with attempts remaining still resolves to the image. + expect( + avatarShowsImage({ resolvedUri: uri, failedUri: uri, attempt: 0 }), + ).toBe(true); + expect( + avatarShowsImage({ resolvedUri: uri, failedUri: uri, attempt: 1 }), + ).toBe(true); + // Exhausted: the initial now stands. + expect( + avatarShowsImage({ + resolvedUri: uri, + failedUri: uri, + attempt: AVATAR_RECOVERY_MAX_ATTEMPTS, + }), + ).toBe(false); + expect( + avatarShowsImage({ resolvedUri: null, failedUri: null, attempt: 0 }), + ).toBe(false); + }); + + test('recovery is bounded and never infinite', () => { + expect(avatarRecoveryDelayMs(0)).toBe(1500); + expect(avatarRecoveryDelayMs(1)).toBe(6000); + expect(avatarRecoveryDelayMs(AVATAR_RECOVERY_MAX_ATTEMPTS)).toBeNull(); + expect(avatarRecoveryDelayMs(99)).toBeNull(); + expect(avatarRecoveryDelayMs(-1)).toBeNull(); + expect(shouldAttemptAvatarRecovery(AVATAR_RECOVERY_MAX_ATTEMPTS)).toBe( + false, + ); + }); + + test('delays back off rather than hammering the limiter', () => { + const delays = [avatarRecoveryDelayMs(0), avatarRecoveryDelayMs(1)]; + expect(delays[1]).toBeGreaterThan(delays[0]); + expect(delays[0]).toBeGreaterThanOrEqual(1000); + }); +}); + +describe('P2: Avatar wiring', () => { + const fs = require('fs'); + const path = require('path'); + const source = fs.readFileSync( + path.join(__dirname, '..', 'product', 'ProductComponents.js'), + 'utf8', + ); + + test('the permanent latch is gone and attempts reset per URI', () => { + expect(source).toContain('setRecoveryAttempt(current => current + 1)'); + expect(source).toContain('key={`${resolvedUri}#${recoveryAttempt}`}'); + // Changing URI clears both the failure and the attempt count. + expect(source).toMatch(/setFailedUri\(null\);\s*setRecoveryAttempt\(0\);/); + // The pending timer is cleared on unmount so no work escapes the instance. + expect(source).toContain('clearTimeout(recoveryTimer.current)'); + }); + + test('the private member-photo credential boundary is untouched', () => { + expect(source).toContain('source={memberImageSource(site, resolvedUri)}'); + const helper = fs.readFileSync( + path.join(__dirname, '..', 'product', 'memberImageSource.js'), + 'utf8', + ); + expect(helper).toContain( + 'MEMBER_PHOTO_PATH = /^\\/renaissance\\/member-photo\\//', + ); + expect(helper).toContain('isCanonicalUrl(uri)'); + }); +}); + +describe('P4: duplicate profile reads are coalesced, chat is not', () => { + const fs = require('fs'); + const path = require('path'); + const source = fs.readFileSync(path.join(__dirname, '..', 'site.js'), 'utf8'); + + test('member profile reads get a short TTL without serving stale', () => { + expect(source).toContain( + 'const ttlMs = isNativeRead ? 30000 : isMemberRead ? 15000 : 0;', + ); + expect(source).toContain('allowStale: isNativeRead,'); + }); + + test('only /u/:username.json GETs qualify, never chat or mutations', () => { + const re = /\/\^\\\/u\\\/\[\^\/\]\+\\\.json\//; + expect(re.test(source)).toBe(true); + expect(source).not.toMatch(/isMemberRead[\s\S]{0,80}chat/); + }); +}); diff --git a/js/__tests__/sitePrivacy.test.js b/js/__tests__/sitePrivacy.test.js index c2f2da807..2c6595a44 100644 --- a/js/__tests__/sitePrivacy.test.js +++ b/js/__tests__/sitePrivacy.test.js @@ -80,11 +80,9 @@ describe('site privacy serialization', () => { authToken: 'synthetic-key', }); const pending = site.jsonApi('/latest.json'); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); + await jest.advanceTimersByTimeAsync(0); expect(fetch).toHaveBeenCalledTimes(1); - await jest.runOnlyPendingTimersAsync(); + await jest.advanceTimersByTimeAsync(20000); await expect(pending).resolves.toEqual({ ok: true }); expect(fetch).toHaveBeenCalledTimes(2); jest.useRealTimers(); @@ -113,10 +111,9 @@ describe('site privacy serialization', () => { }); const floor = site.jsonApi('/latest.json'); const notifications = site.jsonApi('/notifications.json'); - await Promise.resolve(); - await Promise.resolve(); + await jest.advanceTimersByTimeAsync(0); expect(fetch).toHaveBeenCalledTimes(2); - await jest.runOnlyPendingTimersAsync(); + await jest.advanceTimersByTimeAsync(20000); await expect(Promise.all([floor, notifications])).resolves.toEqual([ { topics: true }, { notifications: true }, @@ -140,9 +137,8 @@ describe('site privacy serialization', () => { message: 'api_rate_limited', status: 429, }); - await Promise.resolve(); - await jest.advanceTimersByTimeAsync(2000); - await jest.advanceTimersByTimeAsync(5000); + await jest.advanceTimersByTimeAsync(0); + await jest.advanceTimersByTimeAsync(20000); await rejection; expect(fetch).toHaveBeenCalledTimes(3); apiRateLimitCoordinator.reset(); diff --git a/js/__tests__/stagingDiagnostics.test.js b/js/__tests__/stagingDiagnostics.test.js new file mode 100644 index 000000000..b391f8c39 --- /dev/null +++ b/js/__tests__/stagingDiagnostics.test.js @@ -0,0 +1,94 @@ +jest.mock('../secureCredentialStore', () => ({ + credentialStore: { readSiteToken: jest.fn() }, +})); + +import { credentialStore } from '../secureCredentialStore'; +import { + browserSessionProbeUrl, + collectStagingDiagnostics, + stagingDiagnosticsEnabled, +} from '../stagingDiagnostics'; + +beforeEach(() => jest.clearAllMocks()); + +describe('staging certification diagnostics', () => { + test('render only on the staging channel and fail closed elsewhere', () => { + expect(stagingDiagnosticsEnabled('staging')).toBe(true); + for (const channel of [ + 'production', + 'preview', + null, + undefined, + '', + 'STAGING', + ]) { + expect(stagingDiagnosticsEnabled(channel)).toBe(false); + } + }); + + test('the browser probe targets the canonical session endpoint only', () => { + expect(browserSessionProbeUrl('https://adjusternetwork.org')).toBe( + 'https://adjusternetwork.org/session/current.json', + ); + expect(browserSessionProbeUrl(null)).toBeNull(); + }); + + test('reports the active update identity for OTA activation proof', async () => { + credentialStore.readSiteToken.mockResolvedValueOnce(null); + const diagnostics = await collectStagingDiagnostics({ + isEnabled: true, + updateId: 'update-id', + runtimeVersion: 'an-ios-android-1.0.0-native-2', + channel: 'staging', + isEmbeddedLaunch: false, + isEmergencyLaunch: false, + manifest: { extra: { ota: { gitSha: 'abc123' } } }, + }); + + expect(diagnostics).toMatchObject({ + gitSha: 'abc123', + updateId: 'update-id', + channel: 'staging', + source: 'remote', + runtimeVersion: 'an-ios-android-1.0.0-native-2', + retainedCredential: 'absent', + }); + }); + + test('distinguishes an embedded launch from an activated remote update', async () => { + credentialStore.readSiteToken.mockResolvedValueOnce(null); + const diagnostics = await collectStagingDiagnostics({ + isEnabled: true, + updateId: null, + channel: 'staging', + isEmbeddedLaunch: true, + manifest: {}, + }); + + expect(diagnostics.source).toBe('embedded'); + expect(diagnostics.gitSha).toBeNull(); + }); + + test('reports a credential that survived reinstall without exposing it', async () => { + credentialStore.readSiteToken.mockResolvedValueOnce('secret-user-api-key'); + const diagnostics = await collectStagingDiagnostics({ + isEnabled: true, + channel: 'staging', + manifest: {}, + }); + + expect(diagnostics.retainedCredential).toBe('present'); + expect(JSON.stringify(diagnostics)).not.toContain('secret-user-api-key'); + }); + + test('a Keychain read failure never blocks the diagnostics surface', async () => { + credentialStore.readSiteToken.mockRejectedValueOnce(new Error('locked')); + const diagnostics = await collectStagingDiagnostics({ + isEnabled: true, + channel: 'staging', + manifest: {}, + }); + + expect(diagnostics.retainedCredential).toBe('unreadable'); + }); +}); diff --git a/js/apiRateLimit.js b/js/apiRateLimit.js index 83d4d25ad..f5c820b20 100644 --- a/js/apiRateLimit.js +++ b/js/apiRateLimit.js @@ -3,21 +3,38 @@ export const RATE_LIMIT_FALLBACK_MS = Object.freeze([2000, 5000]); export const RATE_LIMIT_MIN_MS = 1000; +// Two independent ceilings. RATE_LIMIT_MAX_MS bounds how long a single request +// may block, so auth, logout and session recovery can never hang. +// RATE_LIMIT_COOLDOWN_MAX_MS bounds the recorded cooldown lifetime, which the +// server directs through Retry-After. Production has returned values up to +// 136s; clamping the cooldown to the per-request ceiling made GET chains +// re-enter a known-active window at ~60s and ~120s. export const RATE_LIMIT_MAX_MS = 60000; +export const RATE_LIMIT_COOLDOWN_MAX_MS = 180000; export const RATE_LIMIT_MAX_RETRIES = 2; -const boundedDelay = value => - Math.min(RATE_LIMIT_MAX_MS, Math.max(RATE_LIMIT_MIN_MS, value)); +const bounded = (value, max) => + Math.min(max, Math.max(RATE_LIMIT_MIN_MS, value)); +const boundedDelay = value => bounded(value, RATE_LIMIT_MAX_MS); +const boundedCooldown = value => bounded(value, RATE_LIMIT_COOLDOWN_MAX_MS); -export function retryAfterDelayMs(value, now = Date.now()) { +function parseRetryAfterMs(value, now) { if (typeof value !== 'string' || !value.trim()) return null; const seconds = Number(value.trim()); - if (Number.isFinite(seconds) && seconds >= 0) { - return boundedDelay(seconds * 1000); - } + if (Number.isFinite(seconds) && seconds >= 0) return seconds * 1000; const timestamp = Date.parse(value); if (!Number.isFinite(timestamp)) return null; - return boundedDelay(Math.max(0, timestamp - now)); + return Math.max(0, timestamp - now); +} + +export function retryAfterDelayMs(value, now = Date.now()) { + const raw = parseRetryAfterMs(value, now); + return raw === null ? null : boundedDelay(raw); +} + +export function retryAfterCooldownMs(value, now = Date.now()) { + const raw = parseRetryAfterMs(value, now); + return raw === null ? null : boundedCooldown(raw); } export function rateLimitDelayMs(response, retryIndex, now = Date.now()) { @@ -28,6 +45,18 @@ export function rateLimitDelayMs(response, retryIndex, now = Date.now()) { return directed ?? RATE_LIMIT_FALLBACK_MS[retryIndex] ?? RATE_LIMIT_MAX_MS; } +// The cooldown lifetime honors the directed value up to the cooldown ceiling. +export function rateLimitCooldownMs(response, retryIndex, now = Date.now()) { + const directed = retryAfterCooldownMs( + response?.headers?.get?.('Retry-After'), + now, + ); + return ( + directed ?? + boundedCooldown(RATE_LIMIT_FALLBACK_MS[retryIndex] ?? RATE_LIMIT_MAX_MS) + ); +} + export class ApiRateLimitCoordinator { constructor({ now = () => Date.now(), sleep } = {}) { this.now = now; diff --git a/js/authFailure.js b/js/authFailure.js index fddfd65e1..c269f6ee7 100644 --- a/js/authFailure.js +++ b/js/authFailure.js @@ -8,37 +8,86 @@ export const AUTH_FAILURE = Object.freeze({ PRESENTATION: 'presentation_failure', USER_CANCEL: 'user_cancel', CALLBACK: 'callback_failure', + UNKNOWN: 'unknown_failure', }); +// Keychain access can fail for reasons that never mention "keychain": iOS +// reports a missing entitlement as OSStatus -34018, and react-native-keychain +// surfaces it as a bare "OSStatus error" string. Those are security +// configuration faults, not connectivity faults, and must never be presented +// to a member as a network problem. +const SECURITY_PATTERNS = [ + 'keychain', + 'credential', + 'rsa_key', + 'secure_storage', + 'entitlement', + 'osstatus', + 'errsec', + '-34018', + 'secitem', +]; + +// Only a genuine transport fault may use the connectivity copy. Anything that +// is not recognised here stays UNKNOWN rather than borrowing that message. +const NETWORK_PATTERNS = [ + 'network request failed', + 'network error', + 'timeout', + 'timed out', + 'offline', + 'connection', + 'unable to connect', + 'could not connect', + 'host', + 'dns', + 'econn', + 'enotfound', + 'internet', +]; + +function haystack(error) { + // `code` alone is not enough: a native rejection can carry a numeric code + // while the diagnostic detail lives only in the message. + return [error?.code, error?.message, error?.name] + .filter(value => value !== undefined && value !== null) + .map(value => String(value)) + .join(' ') + .toLowerCase(); +} + export function classifyAuthFailure(error) { - const code = String(error?.code || error?.message || '').toLowerCase(); - if (code.includes('auth_user_cancelled')) return AUTH_FAILURE.USER_CANCEL; + const text = haystack(error); + + if (text.includes('auth_user_cancelled')) return AUTH_FAILURE.USER_CANCEL; if ( - code.includes('auth_presentation') || - code.includes('auth_start_failed') || - code.includes('auth_session_failed') + text.includes('auth_presentation') || + text.includes('auth_start_failed') || + text.includes('auth_session_failed') ) { return AUTH_FAILURE.PRESENTATION; } - if ( - code.includes('keychain') || - code.includes('credential') || - code.includes('rsa_key') || - code.includes('secure_storage') - ) { + if (SECURITY_PATTERNS.some(pattern => text.includes(pattern))) { return AUTH_FAILURE.KEYCHAIN; } - if (code.includes('auth_invalid_url') || code.includes('auth_origin')) { + if (text.includes('auth_invalid_url') || text.includes('auth_origin')) { return AUTH_FAILURE.AUTH_URL; } if ( - code.includes('auth_callback') || - code.includes('auth_payload') || - code.includes('auth_nonce') + text.includes('auth_callback') || + text.includes('auth_payload') || + text.includes('auth_nonce') ) { return AUTH_FAILURE.CALLBACK; } - return AUTH_FAILURE.NETWORK; + if (NETWORK_PATTERNS.some(pattern => text.includes(pattern))) { + return AUTH_FAILURE.NETWORK; + } + + // A programming fault (TypeError, undefined native module) reaching this + // point previously rendered as "Unable to connect", which sent members and + // operators to look at connectivity instead of the real defect. + return AUTH_FAILURE.UNKNOWN; } export function authFailureAlert(category) { @@ -65,10 +114,15 @@ export function authFailureAlert(category) { message: 'The secure sign-in response could not be verified. Please try again.', }; - default: + case AUTH_FAILURE.NETWORK: return { title: 'Unable to connect', message: 'Please try again in a moment.', }; + default: + return { + title: 'Sign-in could not be completed', + message: 'Sign-in could not be completed. Please try again.', + }; } } diff --git a/js/authResponsePolicy.js b/js/authResponsePolicy.js index def51aa0c..3869ab71b 100644 --- a/js/authResponsePolicy.js +++ b/js/authResponsePolicy.js @@ -1,8 +1,19 @@ /* @flow */ 'use strict'; -export const classifyAuthResponse = (status: number) => { - if (status === 401) return 'revoked'; +export const INVALID_USER_API_CREDENTIAL = Object.freeze({ + errorType: 'invalid_user_api_credential', + reason: 'invalid_or_revoked_or_expired', +}); + +export const classifyAuthResponse = (status: number, payload: ?Object) => { + if ( + status === 401 && + payload?.error_type === INVALID_USER_API_CREDENTIAL.errorType && + payload?.reason === INVALID_USER_API_CREDENTIAL.reason + ) { + return 'revoked'; + } if (status === 403) return 'forbidden'; return 'other'; }; diff --git a/js/iosAuthSession.js b/js/iosAuthSession.js index 6ad04b8f9..d5f6581cb 100644 --- a/js/iosAuthSession.js +++ b/js/iosAuthSession.js @@ -4,11 +4,19 @@ import SafariWebAuth from 'react-native-safari-web-auth'; import { isSafeAuthCallback } from './adjusterNetworkSecurity'; -export async function requestIOSAuth(url, callbackScheme, ephemeral = false) { +// Adjuster Network authorization must never inherit an identity the member did +// not choose in this attempt. A non-ephemeral ASWebAuthenticationSession shares +// the system Safari data store, so a previously signed-in account survives app +// deletion and reinstall and silently binds the next User API Key. Every +// authorization therefore starts in a fresh, ephemeral browser-auth context. +// This is not caller-configurable: no call site may opt out. +export const EPHEMERAL_AUTH_SESSION = true; + +export async function requestIOSAuth(url, callbackScheme) { const callback = await SafariWebAuth.requestAuth( url, callbackScheme, - ephemeral, + EPHEMERAL_AUTH_SESSION, ); if (!callback || !isSafeAuthCallback(callback)) { throw new Error('auth_callback_invalid'); diff --git a/js/notificationDestination.js b/js/notificationDestination.js new file mode 100644 index 000000000..8cec813c2 --- /dev/null +++ b/js/notificationDestination.js @@ -0,0 +1,28 @@ +/* @flow */ +'use strict'; + +// Presentation decision for a classified member destination. Kept pure and +// separate from Discourse.js so every branch is directly testable. +// +// There is no in-between disposition here. A destination either has a native +// screen or it does not; one that does not ends in an explicit bounded state. +// Opening a canonical page in a WebView to work around a missing native screen +// was tried and abandoned - see docs/NATIVE-NOTIFICATION-INTENTS.md. +export const NOTIFICATION_UNAVAILABLE = Object.freeze({ + title: 'Not available in the app yet', + message: + 'Adjuster Network cannot open this notification in the app yet. It has been marked as read, and nothing else is affected.', + close: 'Close', +}); + +export function destinationPresentation(route) { + switch (route?.disposition) { + case 'native': + return { kind: 'native', screen: route.screen, params: route.params }; + case 'privileged_external': + return { kind: 'external', url: route.url }; + default: + // Off-origin, unauthenticated, non-staff /admin, malformed or unknown. + return { kind: 'denied' }; + } +} diff --git a/js/notificationIntent.js b/js/notificationIntent.js new file mode 100644 index 000000000..794cd886d --- /dev/null +++ b/js/notificationIntent.js @@ -0,0 +1,53 @@ +/* @flow */ +'use strict'; + +import DiscourseUtils from './DiscourseUtils'; +import { classifyFirstPartyMemberRoute } from './nativeMemberRouting'; + +export const GRANTED_BADGE = 12; + +// Longest badge name we will render. Discourse badge names are short; this only +// bounds a hostile or corrupted payload, it is not a product limit. +const MAX_BADGE_NAME = 120; + +// Intent resolution starts from the notification payload, not from the URL +// DiscourseUtils builds. That conversion is lossy: a granted_badge becomes +// /badges/:id/basic?username=:u and badge_name is discarded. Reading the +// payload first is what makes a native badge destination possible at all. +export function notificationIntent( + site, + notification, + { authenticated = false, isStaff = false } = {}, +) { + if (!authenticated || !notification) return { kind: 'unavailable' }; + + if (notification.notification_type === GRANTED_BADGE) { + const name = badgeName(notification); + // A badge notification with no usable name has nothing to present + // natively, so it takes the same bounded state as any other gap. + return name ? { kind: 'badge', badge: { name } } : { kind: 'unavailable' }; + } + + const url = DiscourseUtils.endpointForSiteNotification(site, notification); + const route = classifyFirstPartyMemberRoute(url, { authenticated, isStaff }); + switch (route.disposition) { + case 'native': + return { kind: 'native', screen: route.screen, params: route.params }; + case 'privileged_external': + // Staff-only /admin. Admin has no native surface and the boundary is + // enforced in classifyFirstPartyMemberRoute, which returns this + // disposition only when isStaff is true. This is the single documented + // external handoff; it does not widen external navigation for members. + return { kind: 'staff_external', url: route.url }; + default: + return { kind: 'unavailable' }; + } +} + +function badgeName(notification) { + const value = notification?.data?.badge_name; + if (typeof value !== 'string') return null; + const trimmed = value.trim(); + if (!trimmed || trimmed.length > MAX_BADGE_NAME) return null; + return trimmed; +} diff --git a/js/product/BadgeEarnedScreen.js b/js/product/BadgeEarnedScreen.js new file mode 100644 index 000000000..93ab132f8 --- /dev/null +++ b/js/product/BadgeEarnedScreen.js @@ -0,0 +1,52 @@ +/* @flow */ +'use strict'; + +import React from 'react'; +import { StyleSheet, Text, View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; +import { + Action, + Card, + V2BrandHeader, + useProductTheme, +} from './ProductComponents'; +import { spacing } from './DesignSystem'; + +// Renders entirely from the notification payload. It makes no network request: +// everything shown here arrived with the notification, so the screen cannot +// fail, stall, or need a session. Badge artwork and description are +// deliberately absent - both would require a fetch. +const BadgeEarnedScreen = ({ navigation, route }) => { + const colors = useProductTheme(); + const name = route?.params?.name; + const close = () => navigation.goBack(); + + return ( + + + + + {name} + + + + + + + ); +}; + +export default BadgeEarnedScreen; + +const styles = StyleSheet.create({ + safe: { flex: 1 }, + body: { padding: spacing.md }, + name: { + fontSize: 22, + lineHeight: 28, + fontWeight: '750', + textAlign: 'center', + paddingVertical: spacing.md, + }, + action: { marginTop: spacing.lg, alignItems: 'center' }, +}); diff --git a/js/product/DiscourseMedia.js b/js/product/DiscourseMedia.js index b78ad4dab..98da1d64d 100644 --- a/js/product/DiscourseMedia.js +++ b/js/product/DiscourseMedia.js @@ -16,6 +16,7 @@ import { decode } from 'html-entities'; import { SafeAreaView } from 'react-native-safe-area-context'; import { WebView } from 'react-native-webview'; import { radius, spacing, type } from './DesignSystem'; +import { authenticatedOriginHeaders } from './memberImageSource'; import { useProductTheme } from './ProductComponents'; const SIGNED_ACCESS_REFRESH_MS = 240000; @@ -153,12 +154,12 @@ function SecureMediaImage({ refreshing: false, error: null, }); - const headers = site?.authToken - ? { - 'User-Api-Key': site.authToken, - 'User-Api-Client-Id': site.clientId || '', - } - : undefined; + // Media URLs come from post cooked HTML, which passes absolute external + // URLs through unchanged. The credential must therefore be bound to the + // trusted HTTPS origin, exactly as member photos are. Canonical-origin + // secure uploads are unaffected; a pre-signed off-origin object never + // needed the header. + const headers = authenticatedOriginHeaders(site, state.url); useEffect(() => { resolvedAt.current = Date.now(); @@ -263,12 +264,12 @@ function SecureMediaFile({ item, site, resourceKey, refreshMedia }) { error: null, authorizedUrl: null, }); - const headers = site?.authToken - ? { - 'User-Api-Key': site.authToken, - 'User-Api-Client-Id': site.clientId || '', - } - : undefined; + // Media URLs come from post cooked HTML, which passes absolute external + // URLs through unchanged. The credential must therefore be bound to the + // trusted HTTPS origin, exactly as member photos are. Canonical-origin + // secure uploads are unaffected; a pre-signed off-origin object never + // needed the header. + const headers = authenticatedOriginHeaders(site, state.authorizedUrl); const open = useCallback(async () => { if (state.opening) return; setState(current => ({ ...current, opening: true, error: null })); diff --git a/js/product/NativeMemberUtilityScreens.js b/js/product/NativeMemberUtilityScreens.js index 742967792..b2120d50c 100644 --- a/js/product/NativeMemberUtilityScreens.js +++ b/js/product/NativeMemberUtilityScreens.js @@ -32,6 +32,7 @@ import { } from '../notificationStatus'; import { bookmarkDeletePath, + discussionSearchEligible, memberSearchResults, searchResults, supportedNotificationPreferences, @@ -597,7 +598,9 @@ export function NativeSearchScreen({ navigation, screenProps }) { memberError: null, }); const [contentResponse, memberResponse] = await Promise.allSettled([ - site.jsonApi(`/search.json?q=${encodeURIComponent(term)}`), + discussionSearchEligible(term) + ? site.jsonApi(`/search.json?q=${encodeURIComponent(term)}`) + : Promise.resolve({ topics: [], posts: [], users: [] }), site.jsonApi( `/native/v1/member-search?q=${encodeURIComponent(term)}&limit=10`, ), diff --git a/js/product/ProductComponents.js b/js/product/ProductComponents.js index 08628c801..b468aab48 100644 --- a/js/product/ProductComponents.js +++ b/js/product/ProductComponents.js @@ -1,7 +1,7 @@ /* @flow */ 'use strict'; -import React, { useContext, useEffect, useState } from 'react'; +import React, { useContext, useEffect, useRef, useState } from 'react'; import { ActivityIndicator, Image, @@ -14,6 +14,8 @@ import FontAwesome5 from '@react-native-vector-icons/fontawesome5'; import { ThemeContext } from '../ThemeContext'; import { productTheme, radius, spacing, type } from './DesignSystem'; import { useAvatarAuthorityRecord } from './avatarAuthority'; +import { memberImageSource } from './memberImageSource'; +import { avatarRecoveryDelayMs } from './avatarRecovery'; export const useProductTheme = () => productTheme(useContext(ThemeContext).name); @@ -394,18 +396,48 @@ export const Avatar = ({ uri || avatarUri(site, authority ? authority.template : avatarTemplate, size); const [failedUri, setFailedUri] = useState(null); - useEffect(() => setFailedUri(null), [resolvedUri]); + // Bounded recovery from a transient image failure. Counted per URI so a + // changed photo always starts fresh, and capped so a genuinely broken image + // settles on the initial instead of retrying forever. + const [recoveryAttempt, setRecoveryAttempt] = useState(0); + const recoveryTimer = useRef(null); + useEffect(() => { + setFailedUri(null); + setRecoveryAttempt(0); + }, [resolvedUri]); + useEffect( + () => () => { + if (recoveryTimer.current) clearTimeout(recoveryTimer.current); + }, + [], + ); const style = [ { width: size, height: size, borderRadius: size / 2 }, suppliedStyle, ]; + const scheduleRecovery = () => { + const delay = avatarRecoveryDelayMs(recoveryAttempt); + if (delay === null) return; + if (recoveryTimer.current) clearTimeout(recoveryTimer.current); + recoveryTimer.current = setTimeout(() => { + recoveryTimer.current = null; + // Clearing failedUri re-mounts the same URI for one more attempt. The + // attempt counter is what terminates this, not the URI changing. + setFailedUri(null); + setRecoveryAttempt(current => current + 1); + }, delay); + }; + if (resolvedUri && failedUri !== resolvedUri) { return ( setFailedUri(resolvedUri)} - source={{ uri: resolvedUri }} + onError={() => { + setFailedUri(resolvedUri); + scheduleRecovery(); + }} + source={memberImageSource(site, resolvedUri)} style={style} /> ); diff --git a/js/product/ProductScreens.js b/js/product/ProductScreens.js index 9b38e7b7b..5bc57617b 100644 --- a/js/product/ProductScreens.js +++ b/js/product/ProductScreens.js @@ -16,6 +16,13 @@ import { import { SafeAreaView } from 'react-native-safe-area-context'; import FontAwesome5 from '@react-native-vector-icons/fontawesome5'; import { useAssets } from 'expo-asset'; +import { Alert } from 'react-native'; +import { + browserSessionProbeUrl, + collectStagingDiagnostics, + stagingDiagnosticsEnabled, +} from '../stagingDiagnostics'; +import { requestIOSAuth } from '../iosAuthSession'; import { Action, Avatar, @@ -34,6 +41,7 @@ import { loadCommunity, topicPath, } from './ProductData'; +import { floorAttentionState } from './floorAttention'; import { elevation, floorV2, radius, spacing, type } from './DesignSystem'; import { adjusterNetwork } from '../adjusterNetworkConfig'; export { default as LoungeScreen } from './NativeLoungeScreen'; @@ -44,6 +52,7 @@ import { optionLabel, stateLabel } from './adjusterCardPresentation'; import AttachmentComposer, { useAttachmentQueue } from './AttachmentComposer'; import { reconcileAskSubmission, submitAskQuestion } from './AskSubmission'; import NotificationEducation from './NotificationEducation'; +import { memberDisplayName } from './floorPresentation'; import { captureAvatarAuthorityVersion, reconcileAvatarAuthority, @@ -117,12 +126,43 @@ const FloorHeader = ({ navigation, screenProps }) => { ); }; -export function WelcomeScreen({ onConnect, busy }) { +export function WelcomeScreen({ onConnect, onUseDifferentAccount, busy }) { const colors = useProductTheme(); const { width, fontScale } = useWindowDimensions(); const [brandAssets] = useAssets([ require('../../img/adjuster-network-logo.png'), ]); + const diagnosticsVisible = stagingDiagnosticsEnabled(); + const [diagnostics, setDiagnostics] = useState(null); + useEffect(() => { + if (!diagnosticsVisible) return; + let active = true; + collectStagingDiagnostics() + .then(result => { + if (active) setDiagnostics(result); + }) + .catch(() => {}); + return () => { + active = false; + }; + }, [diagnosticsVisible]); + const probeBrowserSession = useCallback(async () => { + const url = browserSessionProbeUrl(); + if (!url) return; + try { + // The probe never completes an authorization: it opens the canonical + // origin in the same browser-auth context the sign-in flow uses so the + // rendered session JSON can be read, then it is dismissed by hand. + await requestIOSAuth(url, 'adjusternetwork'); + } catch (error) { + if (error?.code !== 'auth_user_cancelled') { + Alert.alert( + 'Browser session probe', + 'The probe sheet closed. Read the rendered JSON before dismissing it.', + ); + } + } + }, []); return ( + {onUseDifferentAccount ? ( + + + Use a different account + + + ) : null} + {diagnosticsVisible && ( + + + Staging certification diagnostics + + + {diagnostics + ? [ + `gitSha: ${diagnostics.gitSha || 'unset'}`, + `updateId: ${diagnostics.updateId || 'none'}`, + `channel: ${diagnostics.channel || 'none'}`, + `source: ${diagnostics.source}`, + `runtime: ${diagnostics.runtimeVersion || 'none'}`, + `retainedCredential: ${diagnostics.retainedCredential}`, + ].join('\n') + : 'Reading…'} + + + + Probe browser session + + + + )} - String(username || 'member') - .split(/[_-]+/) - .filter(Boolean) - .map(part => - part.length <= 2 - ? part.toUpperCase() - : `${part.slice(0, 1).toUpperCase()}${part.slice(1)}`, - ) - .join(' '); - const topicActivityDate = topic => topic.last_posted_at ? new Date(topic.last_posted_at).toLocaleDateString(undefined, { @@ -375,6 +457,7 @@ const FloorActivityRow = ({ topic, site, navigation, openUrl, category }) => { const FloorAttentionCard = ({ topic, site, category, openUrl, cardWidth }) => { const colors = useProductTheme(); const replies = Math.max(0, (topic.posts_count || 1) - 1); + const attentionState = floorAttentionState(topic); const username = topic.last_poster_username || 'Network member'; const categoryColor = /^#?[0-9a-f]{6}$/i.test(category?.color || '') ? `#${String(category.color).replace('#', '')}` @@ -383,7 +466,7 @@ const FloorAttentionCard = ({ topic, site, category, openUrl, cardWidth }) => { openUrl(`${site.url}${topicPath(topic)}`)} style={({ pressed }) => [ @@ -414,24 +497,31 @@ const FloorAttentionCard = ({ topic, site, category, openUrl, cardWidth }) => { style={[ styles.floorAttentionState, { - backgroundColor: - replies === 0 ? colors.brandAccentSoft : colors.accentSoft, + backgroundColor: attentionState.needsReply + ? colors.brandAccentSoft + : colors.accentSoft, }, ]} > - {replies === 0 ? 'NEEDS A REPLY' : 'ACTIVE'} + {attentionState.label} @@ -487,12 +577,12 @@ export function FloorScreen({ navigation, screenProps }) { screenProps.memberContentVersion, ); const rateLimited = classifyCommunityLoadError(data.error) === 'rate_limited'; - const memberName = memberDisplayName(site?.username); + const memberName = memberDisplayName(site?.name, site?.username); const greeting = new Date().getHours() < 12 ? 'Good morning' : 'Welcome back'; const attentionTopics = data.topics.slice(0, 5); const attentionCardWidth = Math.min(Math.max(width - 64, 280), 340); - const unanswered = data.topics.filter( - topic => (topic.posts_count || 1) <= 1, + const activeConversations = data.topics.filter( + topic => (topic.posts_count || 1) > 1, ).length; const unavailableWithoutSnapshot = Boolean(data.error) && @@ -506,7 +596,7 @@ export function FloorScreen({ navigation, screenProps }) { maxFontSizeMultiplier={1.5} style={[styles.floorGreetingTitle, { color: colors.text }]} > - {greeting}, {memberName} + {memberName ? `${greeting}, ${memberName}` : greeting} navigation.navigate('Discussions')} tone="red" accessibleLayout={fontScale >= 1.6} diff --git a/js/product/avatarRecovery.js b/js/product/avatarRecovery.js new file mode 100644 index 000000000..f142a5c30 --- /dev/null +++ b/js/product/avatarRecovery.js @@ -0,0 +1,30 @@ +/* @flow */ +'use strict'; + +// A transient image failure must not permanently show the letter initial for +// the lifetime of a mounted Avatar. Stack screens remount on every navigation +// and the private member-photo route sends Cache-Control: private, no-store, +// so each mount issues a fresh authenticated request; a single 429 inside a +// limiter window used to latch the initial until the component unmounted. +// +// Recovery is bounded, not infinite: a small number of delayed attempts, then +// the initial stands. Attempts are per URI, so a changed photo starts fresh. +export const AVATAR_RECOVERY_MAX_ATTEMPTS = 2; +export const AVATAR_RECOVERY_DELAYS_MS = Object.freeze([1500, 6000]); + +export function avatarRecoveryDelayMs(attempt) { + if (!Number.isFinite(attempt) || attempt < 0) return null; + if (attempt >= AVATAR_RECOVERY_MAX_ATTEMPTS) return null; + return AVATAR_RECOVERY_DELAYS_MS[attempt] ?? null; +} + +export function shouldAttemptAvatarRecovery(attempt) { + return avatarRecoveryDelayMs(attempt) !== null; +} + +// The initial is shown only once recovery is exhausted for this exact URI. +export function avatarShowsImage({ resolvedUri, failedUri, attempt }) { + if (!resolvedUri) return false; + if (failedUri !== resolvedUri) return true; + return shouldAttemptAvatarRecovery(attempt); +} diff --git a/js/product/floorAttention.js b/js/product/floorAttention.js new file mode 100644 index 000000000..6f135c169 --- /dev/null +++ b/js/product/floorAttention.js @@ -0,0 +1,13 @@ +/* @flow */ +'use strict'; + +export const floorAttentionState = topic => { + if (topic?.an_network_activity_class === 'owner_editorial') { + return { label: 'OFFICIAL', icon: 'bullhorn', needsReply: false }; + } + const replies = Math.max(0, (topic?.posts_count || 1) - 1); + if (topic?.an_network_activity_class === 'member_activity' && replies === 0) { + return { label: 'NEEDS A REPLY', icon: 'question', needsReply: true }; + } + return { label: 'ACTIVE', icon: 'comments', needsReply: false }; +}; diff --git a/js/product/floorPresentation.js b/js/product/floorPresentation.js new file mode 100644 index 000000000..d93ee0202 --- /dev/null +++ b/js/product/floorPresentation.js @@ -0,0 +1,22 @@ +/* @flow */ +'use strict'; + +export const memberDisplayName = (name, username) => { + const suppliedName = String(name || '').trim(); + if (suppliedName && suppliedName.toLowerCase() !== 'member') { + return suppliedName; + } + const suppliedUsername = String(username || '').trim(); + if (!suppliedUsername || suppliedUsername.toLowerCase() === 'member') { + return null; + } + return suppliedUsername + .split(/[_-]+/) + .filter(Boolean) + .map(part => + part.length <= 2 + ? part.toUpperCase() + : `${part.slice(0, 1).toUpperCase()}${part.slice(1)}`, + ) + .join(' '); +}; diff --git a/js/product/memberImageSource.js b/js/product/memberImageSource.js new file mode 100644 index 000000000..b75b1b85e --- /dev/null +++ b/js/product/memberImageSource.js @@ -0,0 +1,53 @@ +/* @flow */ +'use strict'; + +import { isCanonicalUrl, parseHttpsUrl } from '../adjusterNetworkSecurity'; + +// Only the governed private member-photo route requires the User API +// credential. Ordinary Discourse avatars are served from /user_avatar/ and are +// readable without one; authenticating them turns every rendered avatar into a +// counted user-API request, which exhausts the member's rate limit and starves +// the real API calls behind it. +const MEMBER_PHOTO_PATH = /^\/renaissance\/member-photo\//; + +// Private member photos are served by the governed origin and require the same +// User API credential the JSON API already sends. React Native's image loader +// runs its own native pipeline and attaches none of the app's credentials, so +// an authenticated image request must carry them explicitly. +// +// The credential is attached only when the resolved URL is on the trusted +// Adjuster Network origin. An avatar_template may carry an absolute URL to an +// external host - letter avatars, gravatar-style services, a CDN - and those +// must never receive a User API key. isCanonicalUrl also rejects plain HTTP, +// so the credential cannot leave over an unencrypted connection. +// +// Nothing is placed in the URL, query string, or any log: the credential +// travels only as a request header. +export function authenticatedOriginHeaders(site, uri) { + if (!uri || !site?.authToken || !isCanonicalUrl(uri)) { + return undefined; + } + return { + 'User-Api-Key': site.authToken, + 'User-Api-Client-Id': site.clientId || '', + }; +} + +export function isMemberPhotoUrl(uri) { + const url = parseHttpsUrl(String(uri || '')); + return !!url && MEMBER_PHOTO_PATH.test(url.pathname); +} + +export function memberImageSource(site, uri) { + if (!uri) { + return null; + } + // Avatars render many-per-screen and are loaded by the native image + // pipeline, outside the app's request orchestrator and its rate-limit + // cooldowns. Only the private member-photo route may carry the credential. + if (!isMemberPhotoUrl(uri)) { + return { uri }; + } + const headers = authenticatedOriginHeaders(site, uri); + return headers ? { uri, headers } : { uri }; +} diff --git a/js/product/memberUtilities.js b/js/product/memberUtilities.js index ca6593ea6..e51830f66 100644 --- a/js/product/memberUtilities.js +++ b/js/product/memberUtilities.js @@ -1,6 +1,12 @@ /* @flow */ 'use strict'; +export const DISCUSSION_SEARCH_MIN_LENGTH = 3; + +export const discussionSearchEligible = query => + typeof query === 'string' && + query.trim().length >= DISCUSSION_SEARCH_MIN_LENGTH; + export const searchResults = payload => { const topics = Array.isArray(payload?.topics) ? payload.topics : []; const posts = Array.isArray(payload?.posts) ? payload.posts : []; diff --git a/js/requestLedgerDiagnostics.js b/js/requestLedgerDiagnostics.js index 14bae4d39..43bf77ad9 100644 --- a/js/requestLedgerDiagnostics.js +++ b/js/requestLedgerDiagnostics.js @@ -13,6 +13,7 @@ const ALLOWED_EVENTS = new Set([ 'cache_hit', 'cooldown_begin', 'cooldown_wait', + 'cooldown_reject', ]); let write = Promise.resolve(); diff --git a/js/requestOrchestrator.js b/js/requestOrchestrator.js index 19c47a03f..150beef16 100644 --- a/js/requestOrchestrator.js +++ b/js/requestOrchestrator.js @@ -1,7 +1,7 @@ /* @flow */ 'use strict'; -import { retryAfterDelayMs } from './apiRateLimit'; +import { RATE_LIMIT_MAX_MS, rateLimitCooldownMs } from './apiRateLimit'; import { recordRequestLedger } from './requestLedgerDiagnostics'; const MAX_CONCURRENCY = 3; @@ -80,6 +80,23 @@ export class RequestOrchestrator { async waitForBucket(bucket) { const until = this.cooldowns.get(bucket) || 0; const remaining = until - this.now(); + // No single request may block longer than the per-request ceiling. When the + // shared cooldown still has more than that left, fail fast with the same + // bounded rate-limit error instead of sending a request into an active + // window. The cooldown itself is untouched, so later requests keep waiting + // on it until it genuinely expires. + if (remaining > RATE_LIMIT_MAX_MS) { + this.record({ + event: 'cooldown_reject', + bucket, + status: 429, + durationClass: 'long', + }); + const error = new Error('api_rate_limited'); + error.status = 429; + error.retryAfterMs = remaining; + throw error; + } if (remaining > 0) { this.record({ event: 'cooldown_wait', @@ -93,7 +110,15 @@ export class RequestOrchestrator { } beginCooldown(bucket, response, retryIndex) { - const delay = retryAfterDelayMs(response, retryIndex); + // retryAfterDelayMs takes the Retry-After header value; passing the whole + // response made it return null, so every cooldown was set to now() and + // expired instantly. That is why the client kept issuing requests inside + // an active limiter window, and why every recorded cooldown_begin carried + // durationClass "short". rateLimitCooldownMs reads the header off the + // response and falls back to a bounded backoff when it is absent, honoring + // the directed value up to the cooldown ceiling rather than the + // per-request ceiling. + const delay = rateLimitCooldownMs(response, retryIndex, this.now()); this.cooldowns.set( bucket, Math.max(this.cooldowns.get(bucket) || 0, this.now() + delay), diff --git a/js/screens/NotificationsScreen.js b/js/screens/NotificationsScreen.js index b11913c0c..bb420d632 100644 --- a/js/screens/NotificationsScreen.js +++ b/js/screens/NotificationsScreen.js @@ -14,7 +14,6 @@ import { ImmutableVirtualizedList } from 'react-native-immutable-list-view'; import FontAwesome5 from '@react-native-vector-icons/fontawesome5'; import Components from './NotificationsScreenComponents'; import Common from './CommonComponents'; -import DiscourseUtils from '../DiscourseUtils'; import { ThemeContext } from '../ThemeContext'; import i18n from 'i18n-js'; import { BottomTabBarHeightContext } from '@react-navigation/bottom-tabs'; @@ -246,9 +245,10 @@ class NotificationsScreen extends React.Component { _openNotificationForSite(notification, site) { this._siteManager.markNotificationRead(site, notification).catch(() => {}); - let url = DiscourseUtils.endpointForSiteNotification(site, notification); + // The whole notification is handed on, not a URL built from it: intent + // resolution needs payload fields that endpointForSiteNotification drops. this._siteManager.setActiveSite(site); - this.props.screenProps.openUrl(url); + this.props.screenProps.openNotification(site, notification); } _listIndex(row) { diff --git a/js/site.js b/js/site.js index 396707bd2..80ceba9ef 100644 --- a/js/site.js +++ b/js/site.js @@ -31,6 +31,7 @@ class Site { 'lastVisitedPath', 'lastVisitedPathAt', 'loginRequired', + 'name', 'queueCount', 'title', 'totalNew', @@ -142,12 +143,21 @@ class Site { jsonApi(path, method, data) { const normalizedMethod = method || 'GET'; const key = this.apiRequestKey(path, normalizedMethod); - const ttlMs = - normalizedMethod === 'GET' && path.startsWith('/native/v1/') ? 30000 : 0; + // Member profile reads are refetched on every navigation because stack + // screens remount, and each one consumes User API budget. A short TTL + // coalesces those repeats without changing what a screen displays. + // Chat message loads are deliberately excluded: they are real-time and + // caching them would show stale conversation. + const isNativeRead = + normalizedMethod === 'GET' && path.startsWith('/native/v1/'); + const isMemberRead = + normalizedMethod === 'GET' && /^\/u\/[^/]+\.json/.test(path); + const ttlMs = isNativeRead ? 30000 : isMemberRead ? 15000 : 0; return requestOrchestrator.request({ key, ttlMs, - allowStale: normalizedMethod === 'GET' && path.startsWith('/native/v1/'), + // Member reads never serve stale; they only deduplicate inside the TTL. + allowStale: isNativeRead, priority: normalizedMethod === 'GET' ? 'visible' : 'bootstrap', task: () => this._jsonApi(path, normalizedMethod, data), }); @@ -233,20 +243,28 @@ class Site { error.retryAfterMs = retryAfterMs; error.rateLimitCode = errorCode || null; throw error; - } else if (classifyAuthResponse(r1.status) === 'revoked') { - this.logoff(); - credentialStore.removeSiteToken(this.url).catch(() => {}); - const error = new Error('auth_revoked'); - error.status = r1.status; - throw error; - } else if (classifyAuthResponse(r1.status) === 'forbidden') { - // A valid, narrowly scoped user API key can be forbidden from an - // endpoint without being revoked. Preserve the session and let the - // caller render an unavailable state. - const error = new Error('auth_forbidden'); - error.status = r1.status; + } else { + let payload = null; try { - const payload = await r1.json(); + payload = await r1.json(); + } catch { + // A non-JSON error body cannot carry the canonical credential signal. + } + const authClassification = classifyAuthResponse(r1.status, payload); + if (authClassification === 'revoked') { + this.retireCredential('revoked'); + credentialStore.removeSiteToken(this.url).catch(() => {}); + const error = new Error('auth_revoked'); + error.status = r1.status; + error.code = payload?.error_type || null; + error.reason = payload?.reason || null; + throw error; + } else if (authClassification === 'forbidden') { + // A valid, narrowly scoped user API key can be forbidden from an + // endpoint without being revoked. Preserve the session and let the + // caller render an unavailable state. + const error = new Error('auth_forbidden'); + error.status = r1.status; error.code = typeof payload?.error === 'string' ? payload.error : null; error.reason = @@ -258,22 +276,15 @@ class Site { error.userMessages = Array.isArray(payload?.errors) ? payload.errors.filter(message => typeof message === 'string') : []; - } catch { - error.userMessages = []; - } - throw error; - } else { - const error = new Error('api_request_failed'); - error.status = r1.status; - try { - const payload = await r1.json(); + throw error; + } else { + const error = new Error('api_request_failed'); + error.status = r1.status; error.userMessages = Array.isArray(payload?.errors) ? payload.errors.filter(message => typeof message === 'string') : []; - } catch { - error.userMessages = []; + throw error; } - throw error; } } finally { if (this._currentFetch === activeFetch) this._currentFetch = undefined; @@ -299,9 +310,15 @@ class Site { if (response.status >= 200 && response.status < 300) { return response.json(); } - const classification = classifyAuthResponse(response.status); + let payload = null; + try { + payload = await response.json(); + } catch { + // A non-JSON error body cannot carry the canonical credential signal. + } + const classification = classifyAuthResponse(response.status, payload); if (classification === 'revoked') { - this.logoff(); + this.retireCredential('revoked'); credentialStore.removeSiteToken(this.url).catch(() => {}); } const error = new Error( @@ -312,14 +329,13 @@ class Site { : 'api_request_failed', ); error.status = response.status; - try { - const payload = await response.json(); - error.userMessages = Array.isArray(payload?.errors) - ? payload.errors.filter(message => typeof message === 'string') - : []; - } catch { - error.userMessages = []; - } + error.code = + typeof payload?.error_type === 'string' ? payload.error_type : null; + error.reason = + typeof payload?.reason === 'string' ? payload.reason : null; + error.userMessages = Array.isArray(payload?.errors) + ? payload.errors.filter(message => typeof message === 'string') + : []; throw error; }) .finally(() => { @@ -333,9 +349,52 @@ class Site { logoff() { this.authToken = null; this.username = null; + this.name = null; this.isStaff = null; } + // The signed-in member identity is captured once at authorization and then + // never refetched by the notification lifecycle, so a server-side rename + // leaves every username-dependent surface pointing at the old handle. Read + // the current session instead: /session/current.json is the narrowest + // authenticated endpoint carrying both fields and is already covered by the + // granted session_info scope, so no server contract changes. + // + // Fail closed. A malformed or empty payload preserves the last known good + // identity rather than blanking it; transport errors propagate to the + // caller, which treats them the same way. + async refreshIdentity() { + if (!this.authToken) { + return false; + } + + const payload = await this.jsonApi('/session/current.json'); + const current = payload?.current_user; + const username = String(current?.username || '').trim(); + if (!username) { + return false; + } + const name = String(current?.name || '').trim() || null; + const changed = this.username !== username || this.name !== name; + this.username = username; + this.name = name; + return changed; + } + + // Only the canonical server 401 + machine-readable credential tuple means + // the stored User API credential no longer exists server-side. Clearing it + // in memory is not enough: without telling the + // manager, the persisted record keeps the dead token and the signed-in + // navigator never re-evaluates, stranding the member on an authenticated + // screen whose only action retries the same doomed request. + retireCredential(reason) { + if (this.credentialRetired) return; + this.credentialRetired = true; + this.credentialRetiredReason = reason || 'revoked'; + this.logoff(); + this.onCredentialRetired?.(this, this.credentialRetiredReason); + } + ensureLatestApi() { if (this.apiVersion < 2) { this.logoff(); diff --git a/js/site_manager.js b/js/site_manager.js index 951326086..6d661a2a8 100644 --- a/js/site_manager.js +++ b/js/site_manager.js @@ -26,15 +26,28 @@ import { recordNotificationDiagnostic, supportedNotification, } from './notificationState'; -import { clearAvatarAuthorityForSite } from './product/avatarAuthority'; import { + clearAvatarAuthorities, + clearAvatarAuthorityForSite, +} from './product/avatarAuthority'; +import { + clearAuthorizationProfile, markAuthorizationProfileCurrent, REQUIRED_AUTHORIZATION_SCOPES, validateAuthorizationProfile, } from './authorizationProfile'; +import { requestOrchestrator } from './requestOrchestrator'; const { DiscourseKeyboardShortcuts } = NativeModules; const REFRESH_THROTTLE_MS = 5000; +// Authenticated GETs the orchestrator may still be caching for a site whose +// credential has just been retired. They must not survive into a fresh +// authorization for the same client id and path. +const RETIRED_CREDENTIAL_CACHE_PATHS = Object.freeze([ + '/native/v1/profile', + '/native/v1/onboarding', + '/native/v1/authorization-profile', +]); class SiteManager { lastRefresh = null; @@ -78,7 +91,7 @@ class SiteManager { } site.createdAt = Date.now(); - this.sites.push(site); + this.sites.push(this._adoptSite(site)); this.save(); this._onChange(); this.updateNativeMenu(); @@ -108,6 +121,73 @@ class SiteManager { this.updateNativeMenu(); } + // Every client-side carrier of a previous member identity, retired together. + // Deleting the app clears AsyncStorage but not the Keychain, and it never + // clears the browser cookie jar, so "delete and reinstall" does not by + // itself produce a clean identity. An account switch must therefore retire + // the browser cookies, the stored User API Key, the RSA material that + // identifies the app in the authorization request, the recorded + // authorization profile, and the client ID before a new authorization + // begins. Server-side credentials are revoked separately by remove(). + async resetAuthorizationIdentity() { + await CookieManager.clearAll(true).catch(() => {}); + const origins = new Set(this.sites.map(site => site.url)); + if (adjusterNetwork.canonicalOrigin) { + origins.add(adjusterNetwork.canonicalOrigin); + } + for (const origin of origins) { + await credentialStore.removeSiteToken(origin).catch(() => {}); + } + const clientId = this.clientId || (await this.getClientId()); + await clearAuthorizationProfile(clientId).catch(() => {}); + await credentialStore.removeRSAKeys().catch(() => {}); + await AsyncStorage.removeItem('@Discourse.rsaKeys').catch(() => {}); + await AsyncStorage.removeItem('@ClientId').catch(() => {}); + this.rsaKeys = null; + this.clientId = null; + this._nonce = null; + this._nonceSite = null; + // Member photos are authenticated per viewer. Drop every cached avatar + // record so a switched account cannot render bytes resolved under the + // previous member's admission. + clearAvatarAuthorities(); + this.sites.forEach(site => site.logoff()); + this.save(); + this._onChange(); + } + + // Refresh the signed-in member identity for the active site only. The broad + // legacy refreshSites() loop stays retired: it fans out across every site and + // was deliberately removed from the authenticated lifecycle to stop it + // extending rate limits. A single in-flight request is shared so sibling + // foreground triggers cannot stack, and the record is persisted only when the + // identity actually changed. A failed refresh preserves the last known + // identity rather than corrupting it. + refreshActiveIdentity() { + if (this._identityRefresh) return this._identityRefresh; + const site = + this.activeSite || this.sites.find(candidate => candidate.authToken); + if (!site?.authToken) return Promise.resolve(false); + + const request = site + .refreshIdentity() + .then(changed => { + if (changed) { + this.save(); + this._onChange(); + } + return changed; + }) + .catch(() => false) + .finally(() => { + if (this._identityRefresh === request) { + this._identityRefresh = null; + } + }); + this._identityRefresh = request; + return request; + } + setActiveSite(site) { return new Promise(resolve => { if (typeof site === 'string' || site instanceof String) { @@ -231,7 +311,7 @@ class SiteManager { const clientId = await this.getClientId(); this.sites = await Promise.all( records.map(async obj => { - const site = new Site(obj); + const site = this._adoptSite(new Site(obj)); // Repair sites saved by older builds that retained the manager // client ID but did not serialize it with site metadata. site.clientId = site.clientId || clientId; @@ -450,16 +530,27 @@ class SiteManager { const previousToken = nonceSite.authToken; const previousHasPush = nonceSite.hasPush; const previousApiVersion = nonceSite.apiVersion; + const retiredBefore = nonceSite.credentialRetired === true; const restorePreviousAuthorization = async () => { - nonceSite.authToken = previousToken; + // A credential retired by an authoritative 401 must stay retired. Only + // restore the prior token when it was still live when this attempt + // started and nothing retired it while the attempt was in flight. + const stillRetired = + retiredBefore || nonceSite.credentialRetired === true; nonceSite.hasPush = previousHasPush; nonceSite.apiVersion = previousApiVersion; - if (previousToken) { + if (previousToken && !stillRetired) { + nonceSite.authToken = previousToken; await credentialStore.storeSiteToken(nonceSite.url, previousToken); } else { + nonceSite.authToken = null; await credentialStore.removeSiteToken(nonceSite.url); } }; + // A fresh authorization may be for a different member. Drop the previous + // identity before the new credential is used so nothing can inherit it. + nonceSite.username = null; + nonceSite.name = null; nonceSite.authToken = decrypted.key; nonceSite.hasPush = decrypted.push; nonceSite.apiVersion = decrypted.api; @@ -485,6 +576,10 @@ class SiteManager { await restorePreviousAuthorization(); return false; } + // A verified fresh authorization supersedes any earlier retirement. + nonceSite.credentialRetired = false; + nonceSite.credentialRetiredReason = null; + await nonceSite.refreshIdentity().catch(() => false); this.save(); // cause we want to stop rendering connect @@ -565,7 +660,7 @@ class SiteManager { } async requestAuth(url) { - const authRequest = await requestIOSAuth(url, this.customScheme, false); + const authRequest = await requestIOSAuth(url, this.customScheme); const urlParams = this.parseURLparameters(authRequest); let acceptedPayload = false; @@ -771,6 +866,32 @@ class SiteManager { this._subscribers.forEach(sub => sub({ event: 'change' })); } + // Only an authoritative 401 reaches this path. Ordinary 403 authorization + // limits, onboarding/policy gating, 429 cooldowns, offline failures and 5xx + // errors all preserve the session by design and must never retire a + // credential. + _adoptSite(site) { + site.onCredentialRetired = retiredSite => + this._handleCredentialRetired(retiredSite); + return site; + } + + async _handleCredentialRetired(site) { + if (!site) return; + // Persist first so a relaunch cannot rehydrate the dead token, then drop + // every authenticated artifact tied to it, then let the root navigator + // re-evaluate and fall back to the signed-out welcome screen. + this.save(); + await credentialStore.removeSiteToken(site.url).catch(() => {}); + await clearAuthorizationProfile(site.clientId).catch(() => {}); + clearAvatarAuthorityForSite(site); + requestOrchestrator.invalidate( + RETIRED_CREDENTIAL_CACHE_PATHS.map(path => site.apiRequestKey(path)), + ); + this.updateUnreadBadge(); + this._onChange(); + } + storeLastPath(navState) { let shouldSave = false; diff --git a/js/stagingDiagnostics.js b/js/stagingDiagnostics.js new file mode 100644 index 000000000..cfcdffa04 --- /dev/null +++ b/js/stagingDiagnostics.js @@ -0,0 +1,38 @@ +/* @flow */ +'use strict'; + +import * as Updates from 'expo-updates'; +import { getOtaDiagnostics } from './otaDiagnostics'; +import { credentialStore } from './secureCredentialStore'; +import { adjusterNetwork } from './adjusterNetworkConfig'; + +// A temporary certification aid for the permanent internal staging client. +// It is gated on the trusted OTA channel, so it can never render on a +// production binary or a production OTA even if this bundle is republished. +export function stagingDiagnosticsEnabled(channel = Updates.channel) { + return channel === 'staging'; +} + +// Opened inside the same ASWebAuthenticationSession the sign-in flow uses. +// An ephemeral session has an empty cookie jar, so the canonical origin must +// report an anonymous visitor. Anything else proves the browser-auth context +// is still sharing the system Safari session. +export function browserSessionProbeUrl( + origin = adjusterNetwork.canonicalOrigin, +) { + return origin ? `${origin}/session/current.json` : null; +} + +export async function collectStagingDiagnostics(updates = Updates) { + const ota = getOtaDiagnostics(updates); + let retainedCredential = 'unknown'; + try { + const origin = adjusterNetwork.canonicalOrigin; + const token = origin ? await credentialStore.readSiteToken(origin) : null; + // Never surface the credential itself, only whether one survived. + retainedCredential = token ? 'present' : 'absent'; + } catch { + retainedCredential = 'unreadable'; + } + return Object.freeze({ ...ota, retainedCredential }); +} diff --git a/scripts/verify-ios-auth-presentation.mjs b/scripts/verify-ios-auth-presentation.mjs index 20c4fd3f8..45add6b37 100644 --- a/scripts/verify-ios-auth-presentation.mjs +++ b/scripts/verify-ios-auth-presentation.mjs @@ -45,6 +45,15 @@ const assertions = [ ), ), ], + [ + 'honours an ephemeral browser-auth context', + source.includes('ephemeral:(BOOL)ephemeral') && + source.includes('session.prefersEphemeralWebBrowserSession = ephemeral;'), + ], + [ + 'never pins a persistent shared-Safari session', + !source.includes('prefersEphemeralWebBrowserSession = NO'), + ], [ 'links AuthenticationServices explicitly', podspec.includes('s.frameworks = "AuthenticationServices", "UIKit"'), diff --git a/testing/native-auth-stale-identity/BACKLOG.md b/testing/native-auth-stale-identity/BACKLOG.md new file mode 100644 index 000000000..d4e2845dc --- /dev/null +++ b/testing/native-auth-stale-identity/BACKLOG.md @@ -0,0 +1,192 @@ +# Non-blocking follow-ups + +Opened alongside the stale-identity authorization fix. None of these gate that +fix, and none may delay its staging or promotion. + +## 1. Misleading reviewer-specific denial copy + +`principal_classification_required` is a generic admission denial, but the +native copy presents it as reviewer-specific. Replace it with accurate generic +copy that does not imply a reviewer-only condition. Reviewer-visible copy is +frozen during Apple review of Build 8; schedule after the freeze lifts. + +## 2. `/native/v1/authorization-profile` returns 500 on a bad User-Api-Key + +A malformed or missing `User-Api-Key` produces a 500 instead of a clean 401. +Server-side fix. It must remain fail-closed: an unauthenticated or malformed +request is denied, only the status and body shape change. Do not weaken +User API authorization while fixing the status code. + +## 3. Legal acceptance screen is too long + +Consolidate into a concise single acknowledgement UI while preserving +per-instrument and per-version acceptance evidence. The recorded evidence +granularity is the constraint; the presentation is not. Reviewer-visible; +schedule after the Apple review freeze. + +## 4. Governed cleanup of qa_test production credentials + +Production User API keys 111 and 112 belong to `qa_test`, plus one stale August +session. These need governed revocation with an audit record. Do not touch +`cert_probe_01`. Do not mutate production server state outside an approved +cleanup window. + +## 5. `POST-SUBMISSION-HOLD.md` names a stale production OTA group + +`testing/native-app-store-readiness/POST-SUBMISSION-HOLD.md` records the +production channel as group `39eb1e9b-8b72-480b-99f1-f52ad6d351fc` +(iOS update `01a01ff3-d501-73b3-b799-2b4cf353efcb`, source +`3fb9de379e499736513d6ade7227b5ce32201ba1`). A read-only channel check on +2026-09-02 shows production has since moved twice and now serves group +`70eebadf-5736-4cd6-a7db-2980a69f0494`, source +`fdb83141879f7b1df60d46a488343563d3bb156e`, published 2026-09-01. + +The stale value matters because that document is the stated rollback target +during the Apple-review freeze. Correct it to the live group. + +That file is untracked in this lineage — it exists only in the canonical +repository's working tree — so this correction is recorded here rather than +applied to it. Documentation only; it does not gate any fix. + +## 6. Founder Production Approval UX / Deployment Gate V2 + +Production promotion is currently a founder-run CLI step with no in-product or +dashboard approval surface, and the governed rollback target lives only in a +document that has already gone stale twice. Design a deployment gate that +records the approver, the exact certified group, the rollback target, and the +freeze state in one authoritative place. + +## 7. Written-but-unwired modules on the certification path + +`js/otaDiagnostics.js` (`getOtaDiagnostics`) and +`js/authorizationConsent.js` (`USER_API_KEY_SCOPE_COPY`) are implemented and +unit-tested but imported by nothing. The missing OTA surface is why proving +which bundle a device had loaded required a laptop and a USB cable, which +directly cost a certification cycle. The staging diagnostics block added in +this lane is deliberately temporary; decide whether a permanent, governed +build-identity surface belongs in the product before removing it. + +## 8. HIGH — Edit Profile shows no profile photo controls on device + +Physical iPhone testing found no way to add, change, or remove the member +avatar in the native Edit Profile screen. Recorded for the immediate +post-certification profile polish pass; not touched during auth certification. + +### Correction to the diagnosis + +The native photo editor is already built. `js/product/NativeProfileScreen.js` +renders, at the top of the editor, the current `MemberAvatar` (or the pending +local preview), a **Change photo** action and a **Remove photo** action. The +supporting pipeline exists and is wired end to end: + +| Requirement | Status | +| ----------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Avatar shown prominently at top of Edit Profile | Built | +| Change photo action | Built | +| Choose from Photo Library | Built (`ImagePicker.launchImageLibraryAsync`, square crop, quality 0.85) | +| Take photo with Camera | **Missing — the only genuine UI gap** | +| Remove/reset photo | Built, with a confirmation alert | +| Permission / error / loading states | Built (`photo_permission_denied`, `profileSaveErrorMessage`, `submitting`) | +| Reuses Discourse's avatar pipeline | Built — `uploadProfilePhoto` posts to `/native/v1/profile/photo` and requires `card.photo.delegatedTo === 'discourse-avatar'`. No parallel image system exists | +| Image type/size validation | Built (`unsupported_profile_photo_type` in `js/product/profileSaveState.js`) | +| Immediate avatar refresh across native UI | Built (`js/product/avatarAuthority.js` publishes the new template) | + +The entire block is gated on the server capability `card?.photo.enabled`, and +`uploadProfilePhoto` additionally fails closed with `photo_capability_disabled` +unless `photo.enabled`, `photo.editable`, and the `discourse-avatar` delegation +are all present. + +**So the controls were absent on device because the server capability is off, +not because the UI is missing.** +`testing/native-app-store-readiness/POST-SUBMISSION-HOLD.md` states this +deliberately: "Keep `structured_profile_photo_enabled=false` during review", +and names profile-photo activation as the first bounded post-freeze +server/privacy certification lane. + +### Actual work remaining + +1. **Server/privacy lane (the real blocker).** Complete the deferred + certification the hold document already scopes — storage and privacy audit, + MIME and size validation, EXIF/GPS stripping, signed-out visibility, + moderation/removal/cache behavior, deletion behavior — then enable + `structured_profile_photo_enabled`. The native UI needs no change to appear. +2. **Camera capture (native, small).** Add a "Take photo" option beside + "Choose from library", using `ImagePicker.requestCameraPermissionsAsync` and + `launchCameraAsync` with the same crop, quality, and + `normalizeProfilePhotoPickerAsset` normalization. `NSCameraUsageDescription` + is already declared. Note that App Store readiness item P1.7 proposes + removing `NSMicrophoneUsageDescription`; the camera string must stay. +3. **Web parity audit (unverified here).** Whether the web member profile + exposes the same capability could not be checked from this repository. Audit + before activation so both surfaces agree, and confirm + `/native/v1/profile/photo` stays compatible with the Discourse avatar + contract rather than diverging. +4. **Re-verify on device after activation** — the capability gate means this + cannot be certified while the flag is false. + +## 9. RESOLVED (implementation pending OTA) — granted_badge notification tap had no destination + +A `granted_badge` notification (observed with Autobiographer) marks itself read +when tapped but produces no navigation: the member is left where they were with +no destination opened. + +Filed during the 2026-09-08 production certification of +`dad0af191716aca8d33c6afe58900e70d2be29e4`. **Not a regression from that +change** — it is a pre-existing native UX defect in notification routing and was +explicitly excluded from that certification. + +`js/DiscourseUtils.js:38-40` does map notification type 12 to +`/badges/{badge_id}/basic?username={username}`, so an endpoint is produced. The +defect is therefore downstream of that mapping: either `data.badge_id` is absent +on the payload, or the resulting web route does not open a native destination. +Confirm which before changing the mapping. + +Marking-as-read succeeding while navigation silently does nothing is the part +that matters: either open a destination or leave the notification unread. + + +### 2026-09-09 — item 9 resolved by native notification intents + +Full history of this item, so none of it is attempted again. + +1. **Original defect.** A `granted_badge` tap marked itself read and then did + nothing. `DiscourseUtils` did produce `/badges/:id/basic`, so the endpoint + existed; the destination was being discarded downstream. +2. **first_party_web (PR #16).** Added a canonical-origin allowlist so valid + member pages opened in the in-app WebView. Device result: the WebView + rendered blank, because `WebViewComponent`'s navigation policy deliberately + blocks internal pages with no native route, to prevent an unauthenticated + Discourse session appearing behind a member's back. +3. **OTP session bootstrap (PR #19, #20).** Minted a one-time password through + the supported `/user-api-key/otp` contract to establish a WebView session. + The first attempt failed with a 400: `require_params_otp` demands + `application_name`, which was omitted. PR #20 corrected the contract - + `/user-api-key/otp.json` plus `application_name` - and the request then + succeeded and the confirmation form rendered. **Finish Login failed with + "Missing, invalid or expired token."** Not debugged further; see 5. +4. **Instrumentation false positive.** The staged diagnostics recorded + `destination_resume: succeeded` for that failed flow. The stage observed + navigation leaving the bootstrap URL, not whether a session had been + established, so it reported success for a failure. Any future success signal + must observe the thing it claims to prove. +5. **Architectural decision (founder).** Abandon the approach. A member already + authenticated in the native app must never perform a second web + authentication merely to open a notification. Notification taps resolve to + native intents; anything without a native destination gets one explicit + bounded state. + +The experiment surface was removed and the strict WebView guard restored. See +`docs/NATIVE-NOTIFICATION-INTENTS.md` for the resulting model and type matrix. + +**Abandoned, uncertified OTA candidates.** These shipped to the production +channel during the experiment and were never certified. Their provenance tags +remain immutable; this record supplies the outcome. + +| SHA | Group | Outcome | +| --- | --- | --- | +| `487f03c3` | `09d99bc9` | first_party_web; device FAIL, blank WebView | +| `65f9d58a` | `08d07007` | OTP bootstrap; device FAIL, 400 on the OTP request | +| `43af7b78` | `eb509d26` | corrected OTP contract; device FAIL at Finish Login | + +Production was rolled back to the certified `dad0af191716` (republished as +group `9098ec70-a3dd-47dd-b623-c51fba68b181`). diff --git a/testing/native-auth-stale-identity/README.md b/testing/native-auth-stale-identity/README.md new file mode 100644 index 000000000..d562005e4 --- /dev/null +++ b/testing/native-auth-stale-identity/README.md @@ -0,0 +1,198 @@ +# Native stale-identity authorization fix + +Recorded: 2026-09-02 (America/New_York) +Base lineage: `fdb83141879f7b1df60d46a488343563d3bb156e` +(`fix/auth-failure-classification-20260830`), the commit currently live on both +the `production` and `staging` OTA channels +Lane: `system` (auth boundary) + +## Base correction + +This fix was first written on `codex/notification-tap-routing` (`879bea6a`). +A read-only channel check before staging showed that branch is not the live +lineage: it forks from the shared ancestor `126ec8e7` and omits the 44 commits +that reached production on 2026-09-01, including the Build 8 submission commit +`2eb3ef6c`, the host-qualified authorization redirect, server-verified +authorization profiles, onboarding gates, and the share-extension work. Staging +that branch would have published a 44-commit regression to the certification +channel. The fix was therefore cherry-picked onto `fdb83141`, and its tests were +updated to the current governed contracts. The defect was confirmed still +present on `fdb83141` before the cherry-pick. + +## Defect + +Server/E2E evidence commit `35d9d27` proved a production native authorization +defect: a stale `ASWebAuthenticationSession` identity survived app delete and +reinstall. Physical TestFlight Build 8 production testing confirmed the auth +session reused `qa_test` instead of allowing `cert_probe_01` to sign in. + +`ASWebAuthenticationSession` defaults to the shared system Safari data store. +Deleting the app clears the app container and `AsyncStorage` (and therefore the +client ID, RSA keys, nonce, and stored User API Key), but it does not clear the +shared Safari cookie jar. The reinstalled app therefore presented an +already-authenticated Discourse session and bound the new User API Key to the +previous account without ever prompting for credentials. + +Web and server lifecycle behavior was already certified; production server +release `228ab5bc8d77c3e89f9b520aba79748bbefa6fb5` is unchanged by this work. + +## Call site + +The flow is **native Swift/Objective-C `ASWebAuthenticationSession`**, not +Expo/JS `WebBrowser.openAuthSessionAsync`. `expo-web-browser` is not a +dependency of this project. + +| Layer | File | +| --------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | +| Product entry points | `js/Discourse.js:344`, `js/Discourse.js:730`, `js/screens/HomeScreen.js:78`, `js/screens/WebViewScreenComponents/WebViewComponent.js:456` | +| Authorization orchestration | `js/site_manager.js` — `requestAuth()` | +| JS bridge wrapper | `js/iosAuthSession.js` — `requestIOSAuth()` | +| Native bridge | `vendor/react-native-safari-web-auth/ios/SafariWebAuth.mm` | + +The native bridge already accepted an `ephemeral` argument and already applied +it as `session.prefersEphemeralWebBrowserSession = ephemeral`. The defect was +entirely in JavaScript: `js/site_manager.js` passed a hardcoded `false`, and +`requestIOSAuth` defaulted the flag to `false`. + +## Fix + +`js/iosAuthSession.js` now exports `EPHEMERAL_AUTH_SESSION = true` and always +passes it to the native bridge. The flag is no longer a caller-supplied +parameter, so no call site can reintroduce a persistent shared-Safari session. +`js/site_manager.js` calls `requestIOSAuth(url, this.customScheme)`. + +Preserved unchanged: + +- the governed `AUTH_REDIRECT` callback contract + (`adjusternetwork://adjusternetwork.org/auth_redirect`) and its + `isSafeAuthCallback` allowlist; +- the one-shot nonce/client-ID binding in `handleAuthPayload`, including its + server-verified `/native/v1/authorization-profile` exact-match check and the + restore-previous-authorization path; +- `generateAuthURL`'s canonical-origin admission check; +- channel-derived, fail-closed production/staging environment resolution; +- scopes, policy, and User API authorization. + +No separate "Use a different account" control is required. Because every +authorization now begins in a fresh ephemeral browser-auth context, the browser +never carries a prior identity into a new attempt, so a member cannot be trapped +behind a stale one. The existing `Log out of this device` control in +`PrivacyAccountScreen` remains the in-session account-switch path; it revokes +the User API Key, removes the stored token, and clears cookies. + +## OTA or binary: OTA + +This is OTA-shippable on the existing runtime `an-ios-android-1.0.0-native-2`. +The ruling is based on inspection of the shipped artifact, not on inference: + +- Build 8 submitted source `2eb3ef6c6783181d30a59a2783e42f7c723a2769` already + contains `ephemeral:(BOOL)ephemeral` and + `session.prefersEphemeralWebBrowserSession = ephemeral` in `SafariWebAuth.mm`. +- The Build 8 archive executable + (`~/Library/Developer/Xcode/Archives/2026-08-20/AdjusterNetwork-1.0-8.xcarchive`) + exports the selector `requestAuth:callbackURLScheme:ephemeral:resolver:rejecter:` + and references `setPrefersEphemeralWebBrowserSession:`. + +The fix therefore changes only a JavaScript argument value inside an unchanged +JS/native interface. Per `docs/NATIVE-OTA-OPERATIONS.md` this stays inside the +existing runtime contract: no native source, dependency, entitlement, +permission, capability, Expo module, or interface change, and no runtime version +bump. `app.config.js` and `eas.json` are untouched. + +## Tests + +`js/__tests__/authEphemeralSession.test.js` (14 tests, green on `fdb83141` +alongside the full suite at 89 suites / 614 tests): + +- the auth session is launched ephemerally, and the `SiteManager` call site + cannot opt out; +- no source path can request a persistent shared-Safari session; +- the shipped native bridge honours the flag on an unchanged interface; +- a replayed or nonce-mismatched prior-account payload binds nothing, and a + rejected payload fails the authorization instead of silently continuing; +- signing in as account B binds the User API Key to B and leaves A untouched; +- the callback stays restricted to the governed `AUTH_REDIRECT`, and the + pre-hardening unqualified `adjusternetwork://auth_redirect` is rejected; +- production/staging environment resolution is unchanged and fail-closed, + including the canonical-origin admission check on `generateAuthURL`. + +`scripts/verify-ios-auth-presentation.mjs` gained two native assertions so a +future native regression away from the ephemeral context fails the verifier. + +## Pre-push audit + +- `yarn format:check`, `yarn lint`: PASS +- `yarn test:unit --runInBand`: 60 suites, 417 tests, PASS +- `yarn verify:ota`: 17/17 PASS +- `yarn verify:ios-auth`: 12/12 PASS +- `scripts/native-devex-native.mjs --configuration=Release`: BUILD SUCCEEDED +- `yarn native:lane` classifies the change as the `system` lane +- `yarn validate:system`: PASS, evidence at + `.local/evidence/native-devex/last-run.json` + +The first system-lane run failed inside `ReactCodegen` with missing +`ios/build/generated/**` inputs. That is the known React Native codegen +script-phase race, not a product regression: the artifacts are present, the +change touches no native source, and an immediate rerun of the identical native +build succeeded. + +The three GitHub workflows (`linting`, `jest-tests`, `ios-tests`) only trigger +on `pull_request` or a push to `main`. This branch is the working trunk, more +than a hundred commits ahead of `main`, and no pull request exists for it, so +the push did not start a run. `yarn lint`, `yarn format:check` and +`yarn test:unit` — the complete `linting` and `jest-tests` job commands — were +run locally and pass. The `ios-tests` Detox simulator suite has not been run; +opening a pull request against `main` to trigger it is a release-picture +decision for the founder, not a CI convenience, and per `AGENTS.md` simulator +evidence could not certify this native API behavior in any case. + +`PHYSICAL_REQUIRED` remains open by design. The system lane requires physical +device evidence, which is the founder's `cert_probe_01` retest below. + +## Founder action required + +Production OTA promotion is not performed. It needs founder authorization, and +`testing/native-app-store-readiness/POST-SUBMISSION-HOLD.md` additionally +freezes production OTA repointing while Apple review of Build 8 is underway. + +Governed promotion path once authorized: + +0. Commit or stash the working tree. `yarn ota:stage` refuses a dirty tree, and + the repository still carries pre-existing uncommitted Build 8 certification + evidence (`testing/native-app-store-readiness/`, + `testing/native-media-attachments/`) that is not this lane's work to commit. + +1. `yarn ota:stage` — publishes to the `staging` channel. A dry run confirms the + exact command it will issue for this fix: + + ``` + npx eas-cli@latest update --branch staging --platform all \ + --message "Staging " --non-interactive + ``` + + with `AN_OTA_CHANNEL=staging` and `AN_OTA_GIT_SHA=`, both derived + automatically. Verify with `yarn ota:stage --dry-run` first. + +2. Physically certify on the permanent staging app with `cert_probe_01`: + install, sign in, sign out, delete the app, reinstall, and confirm the + authorization prompt asks for credentials and does not reuse `qa_test`. +3. `yarn ota:promote --group=` — republishes that + exact certified update group to `production`. Do not rebundle. + +Live channel state read on 2026-09-02, which supersedes the group recorded in +`testing/native-app-store-readiness/POST-SUBMISSION-HOLD.md`: + +| Channel | Group | Source SHA | Runtime | Published | +| ---------- | -------------------------------------- | -------------- | ------------------------------- | ---------- | +| production | `70eebadf-5736-4cd6-a7db-2980a69f0494` | `fdb83141879f` | `an-ios-android-1.0.0-native-2` | 2026-09-01 | +| staging | `4fbf12ad-69f3-47a0-b88a-7238bde5214c` | `fdb83141879f` | `an-ios-android-1.0.0-native-2` | 2026-09-01 | + +The production group above is the rollback target. The hold document's +`39eb1e9b-8b72-480b-99f1-f52ad6d351fc` is stale and should be corrected. + +`cert_probe_01` must remain intact and is not modified by this work. No +production server state is mutated. + +## Non-blocking follow-ups + +Tracked separately in `BACKLOG.md`; none of them gate this fix. diff --git a/testing/native-ratelimit-resilience/README.md b/testing/native-ratelimit-resilience/README.md new file mode 100644 index 000000000..fe5f86d96 --- /dev/null +++ b/testing/native-ratelimit-resilience/README.md @@ -0,0 +1,126 @@ +# Native rate-limit resilience — production certification + +Recorded: 2026-09-08 (America/New_York) +Verdict: **PRODUCTION OTA CERTIFIED** + +## Shipped artifact + +| Field | Value | +| -------------------- | -------------------------------------------------------------------------------------------------------- | +| Certified SHA | `dad0af191716aca8d33c6afe58900e70d2be29e4` | +| Tag | `ota-a4a5e4ee-dad0af19` | +| Production OTA group | `a4a5e4ee-f11d-4804-9756-50c0ede8cd54` | +| iOS update | `01a08212-1e11-7fec-83a3-9c20b9d80de1` | +| Android update | `01a08212-1e11-73a9-af10-65ad49debf68` | +| Runtime | `an-ios-android-1.0.0-native-2` | +| Rollback target | group `a81e298f-1e6f-4b5f-8c7c-ea466833e57b`, iOS `01a078d2-f294-701b-950b-af3ac6864835`, SHA `f662cf3b` | + +Rollback command: `yarn ota:promote --group=a81e298f-1e6f-4b5f-8c7c-ea466833e57b` + +This was a **direct production publish**, not a staging republish, because staging +was deliberately left untouched. Its manifest therefore carries +`expo-channel-name: production` and `useEmbeddedUpdate: true`, unlike earlier +promotions that were republished from staging. + +## Defects corrected + +**Retry-After was silently discarded.** `RequestOrchestrator.beginCooldown` +called `retryAfterDelayMs(response, retryIndex)`, but that helper takes the +Retry-After header _value_, so it returned `null` on its first type check. The +cooldown was then set to `now() + null`, which is `now()`, and expired +instantly. Retry-After was never honored and the limiter never blocked anything. +The production ledger corroborated it exactly: every recorded `cooldown_begin` +carried `durationClass: "short"`, which is what `null < 10000` evaluates to. + +**One ceiling was doing two jobs.** Clamping the cooldown to the per-request +ceiling meant a production `Retry-After: 136` was recorded as 60s, so GET chains +re-entered a known-active window at roughly 60s and 120s. The two concerns are +now separate: `RATE_LIMIT_COOLDOWN_MAX_MS` (180s) bounds the server-directed +cooldown lifetime; `RATE_LIMIT_MAX_MS` (60s) bounds how long one request may +block. When the remaining cooldown exceeds the per-request ceiling, +`waitForBucket` fails fast with the existing bounded `api_rate_limited` error +rather than sending a request into an active window, and leaves the cooldown +intact so later requests keep observing it. + +**Avatars latched the letter initial permanently.** `onError` set `failedUri`, +and the reset only ran when the URI changed — which never happens for a fixed +member and size. Stack screens remount on every navigation and the member-photo +route sends `Cache-Control: private, no-store`, so each mount issued a fresh +authenticated request and a single 429 was terminal for that instance. Recovery +is now bounded: two delayed retries (1.5s, 6s) counted per URI, then the initial +stands. The pending timer is cleared on unmount. + +**Duplicate profile reads.** `/u/:username.json` had no TTL, so remounting +refetched it every navigation. It now coalesces inside a 15s TTL without serving +stale. Chat message loads are deliberately excluded because they are real-time. + +## Corrections to earlier analysis + +Two of our working hypotheses were wrong and are recorded so they are not +repeated: + +- **Message-bus does not carry `User-Api-Key` and does not consume the User API + bucket.** The earlier attribution of ~65% of bucket consumption to + message-bus was incorrect. Native has no message-bus client at all; the + hypothesis that the `WKWebView` Discourse session was consuming the User API + bucket is also disproved. +- **Real pre-change User API pressure** came primarily from `/chat/api/*`, + member-photo image loads, `/u/*.json`, and `/native/v1/*`. + +## Physical device certification + +Founder-led on the production iPhone, with the server lane observing read-only. + +Floor, Discussions, Ask, Lounge, Intel, You, Member Profile, Edit Profile, +back-navigation and remounts, Notifications, background/foreground, +force-close/reopen, and post-reopen You → Member Profile → Edit Profile: all +PASS. Avatar rendered as a photo on every profile surface including across +remounts. No crashes or hangs. + +Server observation over ~6 minutes: 94 device requests, 92 bucket-consuming, +peak rolling 10s = 9, peak rolling 60s = 21. **User API 429s = 0**, other 429s = +0, 5xx = 0. member-photo 16/16 200, `/u/*.json` 5/5 200, `/native/v1/*` 19/19 +200, `/chat/api/*` 14/14 200, `/site.json` 8/8 200, `/latest.json` 8/8 200. No +server-side contradiction to the founder results. + +### Scope limit of this certification + +**No natural 429 occurred during the run.** Retry-After handling, the fail-fast +path above the per-request ceiling, and avatar recovery from a transient failure +therefore remain **code- and CI-validated, not production-trigger validated**. +The run proves the change is healthy under normal load and did not regress +anything; it does not prove the limiter behaviour under a live 429. + +## Validation + +CI green 4/4 on the exact SHA: lint, Jest, iPhone Detox (54m5s), iPad Detox +(54m21s). Locally: 94 suites / 719 tests, `verify:ota` 17/17, +`verify:ios-auth` 12/12, native Release BUILD SUCCEEDED. + +Preserved throughout: private member-photo credential boundary (only +`/renaissance/member-photo/` receives User API credentials), ordinary +`/user_avatar/` unauthenticated, canonical-origin guard, secure-media origin +guard, identity refresh, logout and account-switch cleanup. No server settings, +runtime, or binary change. The 100/min server setting is not to be reopened. + +## Avatar-resolution diagnostic (superseded, not merged) + +Branch `diag/native-avatar-resolution-20260908`, draft PR #12, final SHA +`a4b82b6ddf34183c00b57cbf62ba2417c2735f94`. Never merged and never promoted to +production; retained on the branch for reuse. + +It instrumented the shared `Avatar` with a monotonic per-instance id, a UTC +timestamp with milliseconds, mount/unmount, the React key, source-object +recreation, the navigator kind, the resolved path, the authority key and +presence, the `memberImageSource` classification, whether `failedUri` matched +the resolved URI, the fallback branch taken, and the image lifecycle with the +exact `nativeEvent.error`. Instance ids mattered because `no-store` plus stack +remounting means several Image instances share one URI and their events +interleave. + +It was never run: the fixture iPhone was on the production channel and could not +consume a staging OTA, and no second device was available. The root cause was +instead proven from source plus the server lane's 429 evidence, so the design is +recorded here rather than carried in trunk. The two ideas worth keeping are the +per-instance id for correlating interleaved events, and a UTC wall clock with +milliseconds for aligning a specific client instance against an edge log.