From c63c72e2c272e8cdcbcb31c190b160670c9b60e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 19:13:09 +0000 Subject: [PATCH 01/29] Deny guest/kiosk direct-key reads of ownerless movements The movement item-read rule compared createdBy to auth.token.email. Guest and kiosk tokens carry no email claim, and guest-created movements carry no createdBy, so null === null let any guest or kiosk session read ownerless movement records directly by key. Require a non-null auth.token.email before the owner comparison in the generated item-read rule (personal-access projects only; shared projects are unaffected). Add rule tests covering direct-key reads of ownerless and owned records by a guest. --- tasks/processFirebaseRules.js | 8 +++++++- test/rules/movementWrite.rules.js | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tasks/processFirebaseRules.js b/tasks/processFirebaseRules.js index 6137facd..bb20f8f4 100644 --- a/tasks/processFirebaseRules.js +++ b/tasks/processFirebaseRules.js @@ -42,6 +42,12 @@ function processMovementOwnership(config) { // (their email), and a single-record read only for the record's owner; admins // read everything. Guest/kiosk (no email) read nothing. Shared-access projects // keep the permissive rule. +// +// The owner comparison must first require a non-null `auth.token.email`: +// guest/kiosk tokens carry no email claim, and ownerless (guest-created) +// movements carry no `createdBy`, so without this guard both sides evaluate to +// null and `null === null` would let any guest/kiosk read every ownerless +// movement by key. const readProcessors = { movementListRead: processMovementListRead, movementItemRead: processMovementItemRead, @@ -64,7 +70,7 @@ function processMovementItemRead(config) { if (config.loginForm !== 'email') { return "auth !== null"; } - return "auth !== null && (" + CAN_SEE_ALL_MOVEMENTS + " || data.child('createdBy').val() === auth.token.email)"; + return "auth !== null && (" + CAN_SEE_ALL_MOVEMENTS + " || (auth.token.email !== null && data.child('createdBy').val() === auth.token.email))"; } function newValEquals(val) { diff --git a/test/rules/movementWrite.rules.js b/test/rules/movementWrite.rules.js index 8ad9f9fa..60579c05 100644 --- a/test/rules/movementWrite.rules.js +++ b/test/rules/movementWrite.rules.js @@ -141,6 +141,8 @@ async function testPersonalAccess() { await expect('pilot reads own movement by key', true, get(ref(alice, 'departures/alice_read'))); await expect('pilot cannot read another movement by key', false, get(ref(alice, 'departures/bob_read'))); await expect('guest cannot read movements', false, get(ownQuery(guest, 'departures', 'alice@example.com'))); + await expect('guest cannot read an ownerless movement by key', false, get(ref(guest, 'departures/ownerless_edit'))); + await expect('guest cannot read an owned movement by key', false, get(ref(guest, 'departures/alice_read'))); await expect('admin reads all movements (unbounded query)', true, get(unboundedQuery(admin, 'departures'))); await expect('admin reads any movement by key', true, get(ref(admin, 'departures/bob_read'))); await expect('allMovements operator reads all (unbounded query)', true, get(unboundedQuery(operator, 'departures'))); From 8f9fa415788f661ce7a20b86eeadf56401556adf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 21:06:43 +0000 Subject: [PATCH 02/29] Require admin flag to equal true in database rules Database security rules checked administrator status with .exists() on the /admins/ node, which is satisfied by any stored value (including false, 0 or an empty string). Align the rules with the API layer, which already requires the value to equal true, so only an explicit true grants administrator access. Applies the change to the generated rule constant and to every static occurrence in the rule template. Add rule tests covering a non-true admin record (denied) alongside a genuine admin (allowed). --- firebase-rules-template.json | 38 +++++++++++++++---------------- tasks/processFirebaseRules.js | 2 +- test/rules/movementWrite.rules.js | 12 ++++++++++ 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/firebase-rules-template.json b/firebase-rules-template.json index 5a2cfa52..01a35850 100644 --- a/firebase-rules-template.json +++ b/firebase-rules-template.json @@ -244,14 +244,14 @@ }, "aircrafts": { ".read": "auth !== null", - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".indexOn": [ "type" ] }, "users": { ".read": "auth !== null", - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".indexOn": [ "memberNr" ] @@ -266,27 +266,27 @@ "settings": { "lockDate": { ".read": "auth !== null", - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()" + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true" }, "guestAccessToken": { - ".read": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".read": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".write": false }, "kioskAccessToken": { - ".read": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".read": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".write": false }, "aircrafts": { "homeBase": { ".read": "auth !== null", - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()" + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true" }, "club": { ".read": "auth !== null", - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()" + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true" }, "custom": { - ".read": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".read": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".write": false }, "$other": { @@ -294,27 +294,27 @@ } }, "invoiceRecipients": { - ".read": "auth !== null && root.child('admins/' + auth.uid).exists()", - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()" + ".read": "auth !== null && root.child('admins/' + auth.uid).val() === true", + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true" }, "privacyPolicyUrl": { ".read": true, - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".validate": "newData.isString()" }, "movementRetentionDays": { - ".read": "auth !== null && root.child('admins/' + auth.uid).exists()", - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".read": "auth !== null && root.child('admins/' + auth.uid).val() === true", + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".validate": "newData.isNumber() && newData.val() > 0" }, "messageRetentionDays": { - ".read": "auth !== null && root.child('admins/' + auth.uid).exists()", - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".read": "auth !== null && root.child('admins/' + auth.uid).val() === true", + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".validate": "newData.isNumber() && newData.val() > 0" }, "aerodromeStatusBannerEnabled": { ".read": true, - ".write": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".validate": "newData.isBoolean()" }, "$other": { @@ -322,7 +322,7 @@ } }, "messages": { - ".read": "auth !== null && root.child('admins/' + auth.uid).exists()", + ".read": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".indexOn": [ "negativeTimestamp" ], @@ -373,7 +373,7 @@ "timestamp" ], "$status_id": { - ".write": "auth !== null && root.child('admins/' + auth.uid).exists() && !data.exists() && newData.exists()", + ".write": "auth !== null && root.child('admins/' + auth.uid).val() === true && !data.exists() && newData.exists()", ".validate": "newData.hasChildren(['status', 'details', 'timestamp', 'by'])", "status": { ".validate": "newData.val() === 'open' || newData.val() === 'restricted' || newData.val() === 'closed'" @@ -407,7 +407,7 @@ "card-payments": { "$card_payment_id": { ".read": "auth !== null", - ".write": "auth !== null && newData.exists() && (root.child('admins/' + auth.uid).exists() || (!data.exists() && newData.child('status').val() === 'pending') || (data.exists() && data.child('status').val() === 'pending' && newData.child('status').val() === 'cancelled'))", + ".write": "auth !== null && newData.exists() && (root.child('admins/' + auth.uid).val() === true || (!data.exists() && newData.child('status').val() === 'pending') || (data.exists() && data.child('status').val() === 'pending' && newData.child('status').val() === 'cancelled'))", ".validate": "newData.hasChildren(['amount', 'currency', 'arrivalReference', 'refNr', 'timestamp', 'status'])", "currency": { ".validate": "newData.isString() && newData.val().length > 0" diff --git a/tasks/processFirebaseRules.js b/tasks/processFirebaseRules.js index bb20f8f4..f82f5002 100644 --- a/tasks/processFirebaseRules.js +++ b/tasks/processFirebaseRules.js @@ -14,7 +14,7 @@ const processors = { // projects (e.g. lspv) keep the permissive lockDate-only rule. The // `{movementOwnership}` token in the movement `.write` rule is replaced with // this suffix so the lockDate expression itself stays verbatim in the template. -const IS_ADMIN = "root.child('admins/' + auth.uid).exists()"; +const IS_ADMIN = "root.child('admins/' + auth.uid).val() === true"; const IS_GUEST_OR_KIOSK = "(auth.uid === 'guest' || auth.uid === 'kiosk')"; diff --git a/test/rules/movementWrite.rules.js b/test/rules/movementWrite.rules.js index 60579c05..bb3c275b 100644 --- a/test/rules/movementWrite.rules.js +++ b/test/rules/movementWrite.rules.js @@ -106,6 +106,10 @@ async function testPersonalAccess() { await set(ref(db, 'departures/owned_for_guest'), validDeparture(config, 'alice@example.com')); await set(ref(db, 'departures/alice_read'), validDeparture(config, 'alice@example.com')); await set(ref(db, 'departures/bob_read'), validDeparture(config, 'bob@example.com')); + // A disabled admin recorded as `false` (rather than deleted) must not be + // treated as an admin: the admin predicate requires the value === true. + await set(ref(db, 'admins/stale-admin-uid'), false); + await set(ref(db, 'settings/invoiceRecipients/r1'), { name: 'Acme' }); }); const alice = env.authenticatedContext('alice-uid', { email: 'alice@example.com' }).database(); @@ -113,6 +117,7 @@ async function testPersonalAccess() { const guest = env.authenticatedContext('guest').database(); const admin = env.authenticatedContext('admin-uid').database(); const operator = env.authenticatedContext('operator-uid', { email: 'operator@example.com' }).database(); + const staleAdmin = env.authenticatedContext('stale-admin-uid').database(); const anon = env.unauthenticatedContext().database(); // pilot @@ -145,6 +150,13 @@ async function testPersonalAccess() { await expect('guest cannot read an owned movement by key', false, get(ref(guest, 'departures/alice_read'))); await expect('admin reads all movements (unbounded query)', true, get(unboundedQuery(admin, 'departures'))); await expect('admin reads any movement by key', true, get(ref(admin, 'departures/bob_read'))); + // Admin predicate must require value === true, matching the API layer. A + // `false` record (e.g. an admin disabled by setting the value instead of + // deleting the key) must grant nothing — via the generated see-all rule or a + // templated admin-only node rule. + await expect('admin reads an admin-only settings node', true, get(ref(admin, 'settings/invoiceRecipients'))); + await expect('disabled (false) admin cannot read all movements', false, get(unboundedQuery(staleAdmin, 'departures'))); + await expect('disabled (false) admin cannot read an admin-only settings node', false, get(ref(staleAdmin, 'settings/invoiceRecipients'))); await expect('allMovements operator reads all (unbounded query)', true, get(unboundedQuery(operator, 'departures'))); await expect('allMovements operator reads any movement by key', true, get(ref(operator, 'departures/bob_read'))); await expect('unauthenticated cannot read movements', false, get(ownQuery(anon, 'departures', 'alice@example.com'))); From 90f76802e7aed31ec7ed1cd094aa078f8b04168b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 21:28:20 +0000 Subject: [PATCH 03/29] Pin GitHub Actions to commit SHAs Workflows referenced actions by mutable refs (branch or major-version tag), so the code executed by a run could change without review. Pin every action to a full commit SHA, keeping a version comment for readability and future updates. Each action is pinned to the commit its current ref already resolves to, so runner behaviour is unchanged: actions/checkout -> v4.4.0 actions/setup-node -> v4.4.0 anthropics/claude-code-action -> v1 w9jds/setup-firebase -> main tip (no release tag published) --- .github/workflows/claude.yml | 6 +++--- .github/workflows/firebase-hosting-dev.yml | 12 ++++++------ .github/workflows/firebase-hosting-prod.yml | 12 ++++++------ .github/workflows/test-pull-request.yml | 8 ++++---- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 250e65dc..db869cb3 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -26,12 +26,12 @@ jobs: actions: read # Required for Claude to read CI results on PRs steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: fetch-depth: 0 - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: 24 @@ -40,7 +40,7 @@ jobs: - name: Run Claude Code id: claude - uses: anthropics/claude-code-action@v1 + uses: anthropics/claude-code-action@e63208cb983318a44e3f945e959ef894b707dcfa # v1 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} diff --git a/.github/workflows/firebase-hosting-dev.yml b/.github/workflows/firebase-hosting-dev.yml index 035fda66..d42eb7e6 100644 --- a/.github/workflows/firebase-hosting-dev.yml +++ b/.github/workflows/firebase-hosting-dev.yml @@ -19,13 +19,13 @@ jobs: environment: ${{ matrix.environment }} steps: - run: echo 'Running deplyoment for project ${{ vars.PROJECT }}' - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: 24 - run: npm ci - run: npm run build --project=${{ vars.PROJECT }} - - uses: w9jds/setup-firebase@main + - uses: w9jds/setup-firebase@869785322147e6a53d463a55db0a5af1b4ce4ba6 # main (no release tag; pinned) with: project_id: ${{ vars.FIREBASE_PROJECT }} tools-version: 14 @@ -47,11 +47,11 @@ jobs: environment: ${{ matrix.environment }} steps: - run: echo 'Running functions deplyoment for project ${{ vars.PROJECT }}' - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: 24 - - uses: w9jds/setup-firebase@main + - uses: w9jds/setup-firebase@869785322147e6a53d463a55db0a5af1b4ce4ba6 # main (no release tag; pinned) with: project_id: ${{ vars.FIREBASE_PROJECT }} tools-version: 14 diff --git a/.github/workflows/firebase-hosting-prod.yml b/.github/workflows/firebase-hosting-prod.yml index b5fa4f02..6ea8573a 100644 --- a/.github/workflows/firebase-hosting-prod.yml +++ b/.github/workflows/firebase-hosting-prod.yml @@ -17,13 +17,13 @@ jobs: environment: ${{ matrix.environment }} steps: - run: echo 'Running deplyoment for project ${{ vars.PROJECT }}' - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: 24 - run: npm ci - run: npm run build:prod --project=${{ vars.PROJECT }} - - uses: w9jds/setup-firebase@main + - uses: w9jds/setup-firebase@869785322147e6a53d463a55db0a5af1b4ce4ba6 # main (no release tag; pinned) with: project_id: ${{ vars.FIREBASE_PROJECT }} tools-version: 14 @@ -43,11 +43,11 @@ jobs: environment: ${{ matrix.environment }} steps: - run: echo 'Running functions deplyoment for project ${{ vars.PROJECT }}' - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: 24 - - uses: w9jds/setup-firebase@main + - uses: w9jds/setup-firebase@869785322147e6a53d463a55db0a5af1b4ce4ba6 # main (no release tag; pinned) with: project_id: ${{ vars.FIREBASE_PROJECT }} tools-version: 14 diff --git a/.github/workflows/test-pull-request.yml b/.github/workflows/test-pull-request.yml index 66cfb7ee..ba6bec9a 100644 --- a/.github/workflows/test-pull-request.yml +++ b/.github/workflows/test-pull-request.yml @@ -5,8 +5,8 @@ jobs: if: '${{ github.event.pull_request.head.repo.full_name == github.repository }}' runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: 24 - run: npm ci @@ -18,8 +18,8 @@ jobs: if: '${{ github.event.pull_request.head.repo.full_name == github.repository }}' runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: 24 - run: cd functions && npm ci From 4bcdcd07ab1936538b66a8859a80c3469a6ef0a1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 21:49:34 +0000 Subject: [PATCH 04/29] Check ID token revocation in API auth middleware The API auth middleware verified ID tokens without the revocation check, so a token stayed valid until its natural expiry even after the session was revoked or the account disabled. Pass checkRevoked=true to verifyIdToken, matching the WebAuthn account-management path; the existing catch already returns 401 for the resulting error. Update the middleware unit test for the new call signature and add a case asserting a revoked token yields 401. --- functions/api/fbAuth.js | 5 ++++- functions/api/fbAuth.spec.js | 16 +++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/functions/api/fbAuth.js b/functions/api/fbAuth.js index 75ed748d..b72120cc 100644 --- a/functions/api/fbAuth.js +++ b/functions/api/fbAuth.js @@ -22,7 +22,10 @@ const fbAuth = async (req, res, next) => { } try { - const decodedToken = await admin.auth().verifyIdToken(idToken); + // Pass checkRevoked=true so revoked sessions and disabled accounts are + // rejected immediately (matches the WebAuthn path in webauthnHelpers.js), + // rather than remaining valid until the ID token expires. + const decodedToken = await admin.auth().verifyIdToken(idToken, true); const uid = decodedToken.uid; console.log('Authenticated user:', uid); diff --git a/functions/api/fbAuth.spec.js b/functions/api/fbAuth.spec.js index 24f97ab7..e450c93d 100644 --- a/functions/api/fbAuth.spec.js +++ b/functions/api/fbAuth.spec.js @@ -49,7 +49,7 @@ describe('functions', () => { await fbAuth(req, res, next); - expect(admin.auth().verifyIdToken).toHaveBeenCalledWith('valid-token'); + expect(admin.auth().verifyIdToken).toHaveBeenCalledWith('valid-token', true); expect(req.fbUserId).toBe('user123'); expect(req.fbUserEmail).toBe('user@test.com'); expect(next).toHaveBeenCalled(); @@ -65,6 +65,20 @@ describe('functions', () => { expect(res.send).toHaveBeenCalledWith('Unauthorized'); expect(next).not.toHaveBeenCalled(); }); + + it('returns 401 when the token has been revoked', async () => { + req.headers.authorization = 'Bearer revoked-token'; + const revokedError = new Error('Token revoked'); + revokedError.code = 'auth/id-token-revoked'; + admin.auth().verifyIdToken.mockRejectedValue(revokedError); + + await fbAuth(req, res, next); + + expect(admin.auth().verifyIdToken).toHaveBeenCalledWith('revoked-token', true); + expect(res.status).toHaveBeenCalledWith(401); + expect(res.send).toHaveBeenCalledWith('Unauthorized'); + expect(next).not.toHaveBeenCalled(); + }); }); describe('fbAdminAuth', () => { From 7bf2362f76654fe59837d7426c47f752beddffa7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 14:44:52 +0000 Subject: [PATCH 05/29] Avoid logging movement details on save failure On a save failure the client logged the entire movement object to the browser console, including name, email, phone, remarks and member number. On a shared or kiosk device that PII could persist in the console for a later user. Log only non-identifying context (collection path and record key), matching the payment-method save handler. --- src/modules/movements/sagas.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/modules/movements/sagas.ts b/src/modules/movements/sagas.ts index 207bd345..08aedbf1 100644 --- a/src/modules/movements/sagas.ts +++ b/src/modules/movements/sagas.ts @@ -564,7 +564,10 @@ export function* saveMovement() { } catch(e) { if (console && typeof console.error === 'function') { console.error('Failed to save movement', e); - console.error('movement', movement); + // Log only non-identifying context; the movement object holds PII + // (name, email, phone, remarks, member number) that must not persist + // in the browser console on shared/kiosk devices. + console.error('movement path', path, 'key', key); } yield put(actions.saveMovementFailed(e)) } From 0d435b6496bbc1f1fa50e26921c54d4198a6fec3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 17:46:53 +0000 Subject: [PATCH 06/29] Load aerodrome status via API and restrict raw status reads The public status page and the start-page banner read the /status node directly from the database, which required a world-readable rule and exposed the full status history (including author name and email) to unauthenticated callers. Serve both from the existing public status endpoint instead, polling for updates, and restrict the raw /status database read to admins (the only remaining direct reader is the admin settings view). The endpoint's output is unchanged, so external consumers are unaffected. --- firebase-rules-template.json | 2 +- .../settings/aerodromeStatus/remote.spec.ts | 21 ++++- .../settings/aerodromeStatus/remote.ts | 8 ++ .../settings/aerodromeStatus/sagas.spec.ts | 92 ++++++++++--------- src/modules/settings/aerodromeStatus/sagas.ts | 58 ++++++++---- 5 files changed, 117 insertions(+), 64 deletions(-) diff --git a/firebase-rules-template.json b/firebase-rules-template.json index 01a35850..18983f62 100644 --- a/firebase-rules-template.json +++ b/firebase-rules-template.json @@ -368,7 +368,7 @@ } }, "status": { - ".read": "true", + ".read": "auth !== null && root.child('admins/' + auth.uid).val() === true", ".indexOn": [ "timestamp" ], diff --git a/src/modules/settings/aerodromeStatus/remote.spec.ts b/src/modules/settings/aerodromeStatus/remote.spec.ts index 81ab639e..108f192b 100644 --- a/src/modules/settings/aerodromeStatus/remote.spec.ts +++ b/src/modules/settings/aerodromeStatus/remote.spec.ts @@ -9,7 +9,7 @@ jest.mock('firebase/database', () => ({ import firebase from '../../../util/firebase'; import {get, push} from 'firebase/database'; -import {loadLatest, save} from './remote'; +import {loadLatest, save, fetchCurrentStatus} from './remote'; describe('modules', () => { describe('settings/aerodromeStatus/remote', () => { @@ -43,5 +43,24 @@ describe('modules', () => { await expect(save({status: 'open'})).rejects.toThrow('Save failed'); }); }); + + describe('fetchCurrentStatus', () => { + beforeEach(() => { + (global as any).__FIREBASE_PROJECT_ID__ = 'test-project'; + (global as any).fetch = jest.fn(); + }); + + it('fetches the public status endpoint and resolves the parsed JSON', async () => { + const body = {status: 'open', message: 'hi', last_update_date: '2020-03-17T11:15:00.000Z'}; + (global.fetch as jest.Mock).mockResolvedValue({json: () => Promise.resolve(body)}); + + const result = await fetchCurrentStatus(); + + expect(result).toEqual(body); + expect(global.fetch).toHaveBeenCalledWith( + 'https://europe-west1-test-project.cloudfunctions.net/api/aerodrome/status' + ); + }); + }); }); }); diff --git a/src/modules/settings/aerodromeStatus/remote.ts b/src/modules/settings/aerodromeStatus/remote.ts index db4f319d..c2f0870a 100644 --- a/src/modules/settings/aerodromeStatus/remote.ts +++ b/src/modules/settings/aerodromeStatus/remote.ts @@ -8,3 +8,11 @@ export function loadLatest() { export function save(status: unknown) { return push(firebase('/status'), status as any).then(() => undefined); } + +// Public, unauthenticated status endpoint (Cloud Function over the Admin SDK). +// Used instead of a direct RTDB read so the raw /status node can be locked down +// and the read path stays behind a backend-agnostic HTTP contract. +export function fetchCurrentStatus() { + const url = `https://europe-west1-${__FIREBASE_PROJECT_ID__}.cloudfunctions.net/api/aerodrome/status`; + return fetch(url).then(response => response.json()); +} diff --git a/src/modules/settings/aerodromeStatus/sagas.spec.ts b/src/modules/settings/aerodromeStatus/sagas.spec.ts index 217fcef6..843122f9 100644 --- a/src/modules/settings/aerodromeStatus/sagas.spec.ts +++ b/src/modules/settings/aerodromeStatus/sagas.spec.ts @@ -1,15 +1,14 @@ -import {call, put, select, take} from 'redux-saga/effects'; +import {call, put, select, delay} from 'redux-saga/effects'; import * as actions from './actions'; import * as sagas from './sagas'; import * as remote from './remote'; import FakeFirebaseSnapshot from '../../../../test/FakeFirebaseSnapshot'; import ImmutableItemsArray from "../../../util/ImmutableItemsArray" -import firebase from '../../../util/firebase'; -import {onValue} from 'firebase/database'; jest.mock('../../../util/firebase'); jest.mock('firebase/database', () => ({ - onValue: jest.fn(), + get: jest.fn(), + push: jest.fn(), query: jest.fn(r => r), orderByChild: jest.fn(), limitToLast: jest.fn(), @@ -176,56 +175,65 @@ describe('modules', () => { }); }); - describe('watchCurrentAerodromeStatus', () => { - beforeEach(() => { - jest.clearAllMocks(); - (firebase as jest.Mock).mockReturnValue({}); + describe('mapCurrentStatus', () => { + it('maps the API response to the widget shape', () => { + expect(sagas.mapCurrentStatus({ + status: 'restricted', + message: 'Eine Landung pro Pilot pro Tag.', + last_update_date: '2020-03-17T11:15:00.000Z', + })).toEqual({ + status: 'restricted', + details: 'Eine Landung pro Pilot pro Tag.', + timestamp: new Date('2020-03-17T11:15:00.000Z').getTime(), + }); }); - it('should wait for WATCH_CURRENT_AERODROME_STATUS and then call onValue', () => { - const channel = { put: jest.fn() }; - const generator = sagas.watchCurrentAerodromeStatus(channel); - - expect(generator.next().value).toEqual(take(actions.WATCH_CURRENT_AERODROME_STATUS)); - expect(generator.next().done).toEqual(true); - - expect(firebase).toHaveBeenCalledWith('/status'); - expect(onValue).toHaveBeenCalledWith(expect.anything(), expect.any(Function)); + it('returns null when there is no current status', () => { + expect(sagas.mapCurrentStatus({})).toBeNull(); + expect(sagas.mapCurrentStatus(null)).toBeNull(); }); + }); - it('should call channel.put with status when snapshot has data', () => { - const channel = { put: jest.fn() }; - const generator = sagas.watchCurrentAerodromeStatus(channel); - - generator.next(); // take - generator.next(); // onValue call + done - - const callback = (onValue as jest.Mock).mock.calls[0][1]; + describe('pollCurrentAerodromeStatus', () => { + it('fetches the status API, dispatches the mapped status, then delays', () => { + const generator = sagas.pollCurrentAerodromeStatus(); - const statusItem = { status: 'open', details: '' }; - const snapshot = { val: () => ({ key1: statusItem }) }; - callback(snapshot); + expect(generator.next().value).toEqual(call(remote.fetchCurrentStatus)); - expect(channel.put).toHaveBeenCalledWith( - actions.setCurrentAerodromeStatus(statusItem) + const response = { + status: 'restricted', + message: 'Eine Landung pro Pilot pro Tag.', + last_update_date: '2020-03-17T11:15:00.000Z', + }; + + expect(generator.next(response).value).toEqual( + put(actions.setCurrentAerodromeStatus({ + status: 'restricted', + details: 'Eine Landung pro Pilot pro Tag.', + timestamp: new Date('2020-03-17T11:15:00.000Z').getTime(), + })) ); - }); - it('should call channel.put with null when snapshot is empty', () => { - const channel = { put: jest.fn() }; - const generator = sagas.watchCurrentAerodromeStatus(channel); + expect(generator.next().value).toEqual(delay(sagas.POLL_INTERVAL_MS)); + }); - generator.next(); // take - generator.next(); // onValue call + done + it('dispatches null when the API returns no current status', () => { + const generator = sagas.pollCurrentAerodromeStatus(); - const callback = (onValue as jest.Mock).mock.calls[0][1]; + expect(generator.next().value).toEqual(call(remote.fetchCurrentStatus)); + expect(generator.next({}).value).toEqual( + put(actions.setCurrentAerodromeStatus(null)) + ); + expect(generator.next().value).toEqual(delay(sagas.POLL_INTERVAL_MS)); + }); - const nullSnapshot = { val: () => null }; - callback(nullSnapshot); + it('keeps polling after a fetch error', () => { + const generator = sagas.pollCurrentAerodromeStatus(); - expect(channel.put).toHaveBeenCalledWith( - actions.setCurrentAerodromeStatus(null) - ); + expect(generator.next().value).toEqual(call(remote.fetchCurrentStatus)); + // The thrown fetch error is caught; the saga still delays and loops. + expect(generator.throw(new Error('network')).value) + .toEqual(delay(sagas.POLL_INTERVAL_MS)); }); }); }); diff --git a/src/modules/settings/aerodromeStatus/sagas.ts b/src/modules/settings/aerodromeStatus/sagas.ts index 39670be9..689e6e1a 100644 --- a/src/modules/settings/aerodromeStatus/sagas.ts +++ b/src/modules/settings/aerodromeStatus/sagas.ts @@ -1,15 +1,17 @@ -import {all, call, fork, put, select, take, takeEvery} from 'redux-saga/effects'; -import {onValue, query, orderByChild, limitToLast} from 'firebase/database'; +import {all, call, put, select, takeEvery, takeLeading, delay} from 'redux-saga/effects'; import * as actions from './actions'; import * as remote from './remote'; import ImmutableItemsArray from "../../../util/ImmutableItemsArray" -import createChannel, {monitor} from '../../../util/createChannel'; -import firebase from '../../../util/firebase'; export const authSelector = (state: any) => state.auth.data; export const profileSelector = (state: any) => (state.profile && state.profile.profile) || {}; +// How often the public status page / banner refetch the current status. The +// former Firebase onValue subscription pushed updates instantly; polling trades +// that for a small, bounded delay in exchange for not reading the DB directly. +export const POLL_INTERVAL_MS = 60000; + export function* loadAerodromeStatus() { try { yield put(actions.aerodromeStatusLoading()); @@ -78,27 +80,43 @@ export function* saveAerodromeStatus(action: any) { } } -export function* watchCurrentAerodromeStatus(channel: any) { - yield take(actions.WATCH_CURRENT_AERODROME_STATUS); - const queryRef = query( - firebase('/status'), - orderByChild('timestamp'), - limitToLast(1) - ); - onValue(queryRef, (snapshot) => { - const map = snapshot.val(); - const arr = map ? Object.values(map) : []; - const status = arr.length > 0 ? arr[0] : null; - channel.put(actions.setCurrentAerodromeStatus(status)); - }); +// Map the public status API response onto the shape the page/banner render +// (status code, message text, and a numeric timestamp). +export function mapCurrentStatus(response: any) { + if (!response || !response.status) { + return null; + } + return { + status: response.status, + details: response.message, + timestamp: response.last_update_date + ? new Date(response.last_update_date).getTime() + : undefined, + }; +} + +// Poll the public status API instead of subscribing to /status in the database +// directly, so the raw node can be restricted to admins and the read path is +// backend-agnostic. Started once (takeLeading) even though both the public +// status page and the start-page banner dispatch the watch action. +export function* pollCurrentAerodromeStatus() { + while (true) { + try { + const response = yield call(remote.fetchCurrentStatus); + yield put(actions.setCurrentAerodromeStatus(mapCurrentStatus(response))); + } catch (e) { + if (console && typeof console.error === 'function') { + console.error('Failed to load aerodrome status', e); + } + } + yield delay(POLL_INTERVAL_MS); + } } export default function* sagas() { - const aerodromeStatusChannel = createChannel(); yield all([ takeEvery(actions.LOAD_AERODROME_STATUS, loadAerodromeStatus), takeEvery(actions.SAVE_AERODROME_STATUS, saveAerodromeStatus), - fork(monitor, aerodromeStatusChannel), - fork(watchCurrentAerodromeStatus, aerodromeStatusChannel) + takeLeading(actions.WATCH_CURRENT_AERODROME_STATUS, pollCurrentAerodromeStatus), ]) } From 743f8636c2c0d2b6ad63ca4712cc46893993e3a1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 20:06:45 +0000 Subject: [PATCH 07/29] Reconcile card payment amount before completing arrival The settlement trigger marked an arrival paid whenever a card payment flipped to success, without checking the paid amount. Compare the paid amount against the arrival's recorded fee (feeTotalGross) and refuse to complete on a mismatch or when no fee is recorded, keeping the existing not-already-completed guard. Note: the recorded fee is still client-supplied; server-computed fees are tracked as follow-up. This closes the amount-mismatch settlement path. Adds tests for matching, mismatching and missing-fee cases. --- functions/updateArrivalPaymentStatus.js | 20 ++++++ functions/updateArrivalPaymentStatus.spec.js | 65 +++++++++++++++++--- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/functions/updateArrivalPaymentStatus.js b/functions/updateArrivalPaymentStatus.js index 8a8f92f8..8bce00ca 100644 --- a/functions/updateArrivalPaymentStatus.js +++ b/functions/updateArrivalPaymentStatus.js @@ -43,6 +43,26 @@ const handleUpdate = async (change) => { return } + // Reconcile the paid amount against the arrival's recorded fee before + // marking it paid, so a payment for an arbitrary (e.g. one cent) amount + // cannot settle an arrival that owes more. The card-payment amount is in + // cents; the arrival fee (feeTotalGross) is in currency units. + // NOTE: the arrival fee is itself client-supplied today — full fee + // integrity (server-computed fees) is tracked as separate follow-up work; + // this check closes the amount-mismatch settlement path. + const paidAmount = afterValue.amount; + const expectedFee = arrivalValues.feeTotalGross; + const expectedAmount = typeof expectedFee === 'number' + ? Math.round(expectedFee * 100) + : null; + + if (expectedAmount === null || paidAmount !== expectedAmount) { + logger.warn( + `Refusing to complete arrival ${afterValue.arrivalReference}: paid amount ${paidAmount} does not match expected fee ${expectedAmount} (card payment ${cardPaymentKey})` + ); + return + } + if (arrivalValues.paymentMethod && arrivalValues.paymentMethod.status === 'pending') { logger.info( `Setting payment status of arrival ${afterValue.arrivalReference} to completed (card payment ${cardPaymentKey})` diff --git a/functions/updateArrivalPaymentStatus.spec.js b/functions/updateArrivalPaymentStatus.spec.js index 1b4a5289..5c34dfd2 100644 --- a/functions/updateArrivalPaymentStatus.spec.js +++ b/functions/updateArrivalPaymentStatus.spec.js @@ -102,10 +102,10 @@ describe('functions', () => { expect(mockRef.update).not.toHaveBeenCalled(); }); - it('updates arrival payment status when status changes to success', async () => { + it('updates arrival payment status when status changes to success and amount matches', async () => { const mockRef = { once: jest.fn().mockResolvedValue({ - val: () => ({ paymentMethod: { status: 'pending', method: 'card' } }) + val: () => ({ paymentMethod: { status: 'pending', method: 'card' }, feeTotalGross: 16 }) }), update: jest.fn().mockResolvedValue() }; @@ -118,7 +118,7 @@ describe('functions', () => { const change = makeChange( { status: 'pending' }, - { status: 'success', arrivalReference: 'arr1' } + { status: 'success', arrivalReference: 'arr1', amount: 1600 } ); await mockCapturedHandler({ data: change }); @@ -128,10 +128,61 @@ describe('functions', () => { }); }); + it('does not complete when the paid amount does not match the arrival fee', async () => { + const mockRef = { + once: jest.fn().mockResolvedValue({ + val: () => ({ paymentMethod: { status: 'pending', method: 'card' }, feeTotalGross: 16 }) + }), + update: jest.fn() + }; + + mockAdmin.database.mockReturnValue({ + ref: jest.fn().mockReturnValue({ + child: jest.fn().mockReturnValue(mockRef) + }) + }); + + // 1 cent paid against a 16.00 fee (1600 cents) + const change = makeChange( + { status: 'pending' }, + { status: 'success', arrivalReference: 'arr1', amount: 1 } + ); + + await mockCapturedHandler({ data: change }); + + expect(mockRef.update).not.toHaveBeenCalled(); + expect(mockLogger.warn).toHaveBeenCalled(); + }); + + it('does not complete when the arrival has no recorded fee', async () => { + const mockRef = { + once: jest.fn().mockResolvedValue({ + val: () => ({ paymentMethod: { status: 'pending', method: 'card' } }) + }), + update: jest.fn() + }; + + mockAdmin.database.mockReturnValue({ + ref: jest.fn().mockReturnValue({ + child: jest.fn().mockReturnValue(mockRef) + }) + }); + + const change = makeChange( + { status: 'pending' }, + { status: 'success', arrivalReference: 'arr1', amount: 1600 } + ); + + await mockCapturedHandler({ data: change }); + + expect(mockRef.update).not.toHaveBeenCalled(); + expect(mockLogger.warn).toHaveBeenCalled(); + }); + it('does not update when arrival payment method is not pending', async () => { const mockRef = { once: jest.fn().mockResolvedValue({ - val: () => ({ paymentMethod: { status: 'completed' } }) + val: () => ({ paymentMethod: { status: 'completed' }, feeTotalGross: 16 }) }), update: jest.fn() }; @@ -144,7 +195,7 @@ describe('functions', () => { const change = makeChange( { status: 'pending' }, - { status: 'success', arrivalReference: 'arr1' } + { status: 'success', arrivalReference: 'arr1', amount: 1600 } ); await mockCapturedHandler({ data: change }); @@ -155,7 +206,7 @@ describe('functions', () => { it('throws and logs error when database update fails', async () => { const mockRef = { once: jest.fn().mockResolvedValue({ - val: () => ({ paymentMethod: { status: 'pending' } }) + val: () => ({ paymentMethod: { status: 'pending' }, feeTotalGross: 16 }) }), update: jest.fn().mockRejectedValue(new Error('DB error')) }; @@ -168,7 +219,7 @@ describe('functions', () => { const change = makeChange( { status: 'pending' }, - { status: 'success', arrivalReference: 'arr1' } + { status: 'success', arrivalReference: 'arr1', amount: 1600 } ); await expect(mockCapturedHandler({ data: change })).rejects.toThrow('DB error'); From 1959422af7c8907ea1b81a0e221fe15d3841a75b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 20:27:42 +0000 Subject: [PATCH 08/29] Neutralize spreadsheet formulas in CSV exports User-controlled fields (registration, name, remarks, location, etc.) were written to CSV exports unescaped, so a cell beginning with =, +, -, @ (or tab/CR) would be executed as a formula when an admin opens the report in a spreadsheet. Prefix such cells with a single quote so they render as literal text. Applied in the shared CSV writer (covers the movement report) and in the manually built landings report. --- src/util/LandingsReport.ts | 3 ++- src/util/neutralizeCsvValue.spec.ts | 31 +++++++++++++++++++++++++++++ src/util/neutralizeCsvValue.ts | 12 +++++++++++ src/util/writeCsv.spec.ts | 15 ++++++++++++++ src/util/writeCsv.ts | 18 ++++++++++++++++- 5 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 src/util/neutralizeCsvValue.spec.ts create mode 100644 src/util/neutralizeCsvValue.ts diff --git a/src/util/LandingsReport.ts b/src/util/LandingsReport.ts index cc5874be..68b03bb9 100644 --- a/src/util/LandingsReport.ts +++ b/src/util/LandingsReport.ts @@ -5,6 +5,7 @@ import { firebaseToLocal } from './movements'; import { fetch as fetchAircrafts } from './aircrafts'; import dates from '../util/dates'; import moment from 'moment'; +import neutralizeCsvValue from './neutralizeCsvValue'; class LandingsReport { @@ -108,7 +109,7 @@ class LandingsReport { }; return LandingsReport.header - .map(header => csvRecord[header]) + .map(header => neutralizeCsvValue(csvRecord[header])) .join(this.delimiter); } } diff --git a/src/util/neutralizeCsvValue.spec.ts b/src/util/neutralizeCsvValue.spec.ts new file mode 100644 index 00000000..8aa77978 --- /dev/null +++ b/src/util/neutralizeCsvValue.spec.ts @@ -0,0 +1,31 @@ +import neutralizeCsvValue from './neutralizeCsvValue'; + +describe('util', () => { + describe('neutralizeCsvValue', () => { + it('prefixes values starting with a formula character', () => { + expect(neutralizeCsvValue('=1+1')).toEqual("'=1+1"); + expect(neutralizeCsvValue('+1')).toEqual("'+1"); + expect(neutralizeCsvValue('-1')).toEqual("'-1"); + expect(neutralizeCsvValue('@SUM(A1)')).toEqual("'@SUM(A1)"); + expect(neutralizeCsvValue('=HYPERLINK("http://evil")')).toEqual("'=HYPERLINK(\"http://evil\")"); + }); + + it('prefixes values starting with a tab or carriage return', () => { + expect(neutralizeCsvValue('\t=1')).toEqual("'\t=1"); + expect(neutralizeCsvValue('\r=1')).toEqual("'\r=1"); + }); + + it('leaves safe strings unchanged', () => { + expect(neutralizeCsvValue('HBABC')).toEqual('HBABC'); + expect(neutralizeCsvValue('normal text')).toEqual('normal text'); + expect(neutralizeCsvValue('a=b')).toEqual('a=b'); + }); + + it('leaves non-strings unchanged', () => { + expect(neutralizeCsvValue(42)).toEqual(42); + expect(neutralizeCsvValue(0)).toEqual(0); + expect(neutralizeCsvValue(undefined)).toEqual(undefined); + expect(neutralizeCsvValue(null)).toEqual(null); + }); + }); +}); diff --git a/src/util/neutralizeCsvValue.ts b/src/util/neutralizeCsvValue.ts new file mode 100644 index 00000000..79b944a9 --- /dev/null +++ b/src/util/neutralizeCsvValue.ts @@ -0,0 +1,12 @@ +// Prevent CSV / spreadsheet formula injection. Spreadsheet apps (Excel, Google +// Sheets, LibreOffice) interpret a cell whose text begins with `=`, `+`, `-` or +// `@` (and, in some parsers, a leading tab or carriage return) as a formula. If +// that text came from user input it can execute or trigger a dangerous-content +// prompt when an admin opens an exported report. Prefixing such a value with a +// single quote forces the spreadsheet to treat it as literal text. +export default function neutralizeCsvValue(value: unknown): unknown { + if (typeof value === 'string' && /^[=+\-@\t\r]/.test(value)) { + return `'${value}`; + } + return value; +} diff --git a/src/util/writeCsv.spec.ts b/src/util/writeCsv.spec.ts index 77d66c5d..4c203742 100644 --- a/src/util/writeCsv.spec.ts +++ b/src/util/writeCsv.spec.ts @@ -56,5 +56,20 @@ describe('util', () => { expect(csv).toEqual(expectedCsv); }); }); + + it('neutralizes cells that would be interpreted as spreadsheet formulas', () => { + const records = [ + ['header1', 'header2'], + ['=1+1', '@SUM(A1)'], + ]; + + const expectedCsv = + 'header1,header2\n' + + "'=1+1,'@SUM(A1)\n"; + + return writeCsv(records).then(csv => { + expect(csv).toEqual(expectedCsv); + }); + }); }); }); diff --git a/src/util/writeCsv.ts b/src/util/writeCsv.ts index de9fe947..2a0c282b 100644 --- a/src/util/writeCsv.ts +++ b/src/util/writeCsv.ts @@ -1,8 +1,24 @@ import {stringify} from 'csv-stringify/browser/esm' +import neutralizeCsvValue from './neutralizeCsvValue' + +function neutralizeRecord(record) { + if (Array.isArray(record)) { + return record.map(neutralizeCsvValue); + } + if (record && typeof record === 'object') { + const out = {}; + Object.keys(record).forEach(key => { + out[key] = neutralizeCsvValue(record[key]); + }); + return out; + } + return neutralizeCsvValue(record); +} function writeCsv(records, options={}) { + const safeRecords = Array.isArray(records) ? records.map(neutralizeRecord) : records; return new Promise((resolve, reject) => { - stringify(records, options, function(err, csv){ + stringify(safeRecords, options, function(err, csv){ if (err) { reject(err); } else { From a26f84a3aab861662b21c5026930b5b6d83f0405 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 20:39:44 +0000 Subject: [PATCH 09/29] Use null-prototype maps for user-keyed report grouping Report grouping indexed plain objects with user-controlled strings (invoice recipient name, aircraft registration). A value such as '__proto__' or 'constructor' then interacted with the object prototype instead of being an ordinary key, corrupting the grouping or throwing and breaking report generation. Build these accumulators with Object.create(null) so untrusted keys are always plain data. --- src/util/InvoicesReport.spec.ts | 15 +++++++++++++++ src/util/InvoicesReport.ts | 10 ++++++++-- src/util/LandingsReport.spec.ts | 13 +++++++++++++ src/util/LandingsReport.ts | 5 ++++- 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/util/InvoicesReport.spec.ts b/src/util/InvoicesReport.spec.ts index 39137238..7211a182 100644 --- a/src/util/InvoicesReport.spec.ts +++ b/src/util/InvoicesReport.spec.ts @@ -73,6 +73,21 @@ describe('util', () => { return new InvoicesReport(year, month, options); } + describe('groupArrivalsByRecipient', () => { + it('groups a __proto__ invoice recipient without corrupting the report', () => { + const report = makeReport(); + const arrivals = [ + { paymentMethod: { method: 'invoice', invoiceRecipientName: '__proto__' } }, + { paymentMethod: { method: 'invoice', invoiceRecipientName: '__proto__' } }, + ]; + + const grouped = report.groupArrivalsByRecipient(arrivals); + + expect(Object.keys(grouped)).toContain('__proto__'); + expect(grouped['__proto__']).toHaveLength(2); + }); + }); + describe('constructor', () => { it('pads single-digit month', () => { const report = makeReport(2023, 3); diff --git a/src/util/InvoicesReport.ts b/src/util/InvoicesReport.ts index 52c5204a..840a240f 100644 --- a/src/util/InvoicesReport.ts +++ b/src/util/InvoicesReport.ts @@ -172,7 +172,10 @@ class InvoicesReport { } groupArrivalsByRecipient(arrivals) { - const recipients = {} + // Null-prototype: invoiceRecipientName is user-controlled, so a value like + // '__proto__' or 'constructor' must be an ordinary key, not touch the + // prototype chain (which would corrupt grouping / crash the report). + const recipients = Object.create(null) arrivals.forEach(arrival => { const invoiceRecipientName = arrival.paymentMethod.method === 'invoice' @@ -196,7 +199,10 @@ class InvoicesReport { } groupCustomsDeclarationsByRecipient(customsDeclarations) { - const recipients = {} + // Null-prototype: invoiceRecipientName is user-controlled, so a value like + // '__proto__' or 'constructor' must be an ordinary key, not touch the + // prototype chain (which would corrupt grouping / crash the report). + const recipients = Object.create(null) customsDeclarations.forEach(customsDeclaration => { const invoiceRecipientName = customsDeclaration.invoiceRecipientName diff --git a/src/util/LandingsReport.spec.ts b/src/util/LandingsReport.spec.ts index e8c6da4c..995339ac 100644 --- a/src/util/LandingsReport.spec.ts +++ b/src/util/LandingsReport.spec.ts @@ -108,6 +108,19 @@ describe('util', () => { expect(summary[0].landingCount).toBe(5); }); + it('handles a __proto__ immatriculation without corrupting the summary', () => { + const report = new LandingsReport(2023, 6); + + const arrivals = makeArrivalsSnapshot([ + {date: '2023-06-01', time: '10:00', immatriculation: '__proto__', mtow: 750, landingCount: 2}, + ]); + + const summary = report.getAircraftsSummary(arrivals); + expect(summary).toHaveLength(1); + expect(summary[0].immatriculation).toBe('__proto__'); + expect(summary[0].landingCount).toBe(2); + }); + it('marks invalidMtow when mtow differs across records', () => { const report = new LandingsReport(2023, 6); diff --git a/src/util/LandingsReport.ts b/src/util/LandingsReport.ts index 68b03bb9..e4125eda 100644 --- a/src/util/LandingsReport.ts +++ b/src/util/LandingsReport.ts @@ -63,7 +63,10 @@ class LandingsReport { } getAircraftsSummary(arrivals) { - const map: Record = {}; + // Null-prototype: immatriculation is user-controlled, so a value like + // '__proto__' must be an ordinary key rather than mutating the prototype + // chain (which would corrupt the summary / crash the report). + const map: Record = Object.create(null); arrivals.forEach(record => { const arrival = firebaseToLocal(record.val()); From bf22240e09ed6ca336bfc347f9c328f544e8baea Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 20:59:32 +0000 Subject: [PATCH 10/29] Restrict customs completion URL to the configured customs origin The customs completion URL stored on a movement was validated only as a string, so an arbitrary (e.g. phishing) URL could be stored and later opened from the "Open customs" action. Constrain it in the database rules to begin with the configured customs baseUrl (falling back to the previous behaviour when customs is not configured), which rejects the write at the source. As defense in depth, the client now opens only https completion URLs. --- firebase-rules-template.json | 4 ++-- src/modules/customs/sagas.spec.ts | 19 +++++++++++++++++++ src/modules/customs/sagas.ts | 15 +++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/firebase-rules-template.json b/firebase-rules-template.json index 18983f62..be56c6bc 100644 --- a/firebase-rules-template.json +++ b/firebase-rules-template.json @@ -91,7 +91,7 @@ ".validate": "newData.isString()" }, "customsFormUrl": { - ".validate": "newData.isString()" + ".validate": "newData.isString() && (!root.child('settings/customsDeclarationApp/baseUrl').exists() || newData.val().beginsWith(root.child('settings/customsDeclarationApp/baseUrl').val()))" }, "privacyPolicyAcceptedAt": { ".validate": "newData.isString() && newData.val().matches(/^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d{3}Z$/)" @@ -228,7 +228,7 @@ ".validate": "newData.isString()" }, "customsFormUrl": { - ".validate": "newData.isString()" + ".validate": "newData.isString() && (!root.child('settings/customsDeclarationApp/baseUrl').exists() || newData.val().beginsWith(root.child('settings/customsDeclarationApp/baseUrl').val()))" }, "privacyPolicyAcceptedAt": { ".validate": "newData.isString() && newData.val().matches(/^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d{3}Z$/)" diff --git a/src/modules/customs/sagas.spec.ts b/src/modules/customs/sagas.spec.ts index 659ba772..a168dc28 100644 --- a/src/modules/customs/sagas.spec.ts +++ b/src/modules/customs/sagas.spec.ts @@ -66,6 +66,25 @@ describe('modules', () => { }); }); + describe('openCompletionUrl', () => { + it('opens https completion URLs', () => { + const openMock = jest.fn(); + window.open = openMock; + sagas.openCompletionUrl('https://customs.example/forms/abc'); + expect(openMock).toHaveBeenCalledWith('https://customs.example/forms/abc', '_blank', 'noopener,noreferrer'); + }); + + it('refuses non-https and invalid completion URLs', () => { + const openMock = jest.fn(); + window.open = openMock; + sagas.openCompletionUrl('javascript:alert(1)'); + sagas.openCompletionUrl('http://evil.example'); + sagas.openCompletionUrl('data:text/html,x'); + sagas.openCompletionUrl('not a url'); + expect(openMock).not.toHaveBeenCalled(); + }); + }); + describe('startCustoms', () => { it('should open completion URL and return early if customsFormId and customsFormUrl exist', () => { const openMock = jest.fn(); diff --git a/src/modules/customs/sagas.ts b/src/modules/customs/sagas.ts index 50681298..00957da1 100644 --- a/src/modules/customs/sagas.ts +++ b/src/modules/customs/sagas.ts @@ -113,6 +113,21 @@ export const saveCustomsFormData = async (movementData: any, customsFormId: stri } export const openCompletionUrl = (url: string) => { + // Defense in depth: only ever navigate to an https URL. The stored + // customsFormUrl is constrained to the customs baseUrl by the database + // rules, but this also guards values stored before that rule and blocks + // dangerous schemes (javascript:, data:, http:). + let parsed + try { + parsed = new URL(url) + } catch (e) { + console.warn('Refusing to open invalid completion URL') + return + } + if (parsed.protocol !== 'https:') { + console.warn('Refusing to open non-https completion URL') + return + } const newWindow = window.open(url, '_blank', 'noopener,noreferrer') if (!newWindow) { console.warn('Popup blocked for completion URL:', url) From 9e689549d6d6438ce081c51a68fd93135da9e682 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 21:15:22 +0000 Subject: [PATCH 11/29] Add baseline security headers to hosting responses Hosting sent only cache headers. Add X-Frame-Options: DENY and a Content-Security-Policy with frame-ancestors 'none' (clickjacking), X-Content-Type-Options: nosniff, a strict Referrer-Policy, and a Permissions-Policy disabling device APIs the app does not use. The CSP is intentionally limited to frame-ancestors so it cannot break resource loading; a full content policy is a separate, larger change. --- firebase.json | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/firebase.json b/firebase.json index 9af05257..164e59ba 100644 --- a/firebase.json +++ b/firebase.json @@ -52,6 +52,16 @@ "headers": [ { "key": "Cache-Control", "value": "public, max-age=31536000, immutable" } ] + }, + { + "source": "**", + "headers": [ + { "key": "X-Frame-Options", "value": "DENY" }, + { "key": "Content-Security-Policy", "value": "frame-ancestors 'none'" }, + { "key": "X-Content-Type-Options", "value": "nosniff" }, + { "key": "Referrer-Policy", "value": "strict-origin-when-cross-origin" }, + { "key": "Permissions-Policy", "value": "geolocation=(), camera=(), microphone=(), payment=()" } + ] } ] }, From efebc4609987c2d7cba9016e2cce95bb9f591a2c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 21:32:42 +0000 Subject: [PATCH 12/29] Remove unused user import (server sync endpoint and CSV import) The user directory is not maintained through the app: the server-side sync endpoint (POST /api/users/import) and the admin CSV user import were unused. Remove both, along with their now-dead supporting code. Server: drop the /users/import route, the syncUsers module, the Basic-auth middleware (its only consumer) and the member-management route gating. Client: drop the imports module, the User import admin tab/form, and the importUsers/importCsv/parseCsv helpers. This also removes the unawaited-deletes bug in the sync path and the Basic-auth-protected destructive route entirely, rather than fixing them. The API_SERVICEUSER credentials and generated member-management flag written by the deploy workflow are now unused and can be removed separately. --- functions/api/basicAuth.js | 28 -- functions/api/basicAuth.spec.js | 135 --------- functions/api/index.js | 35 --- functions/api/index.spec.js | 48 --- functions/api/syncUsers.js | 86 ------ functions/api/syncUsers.spec.js | 225 -------------- src/components/AdminPage/AdminNavigation.tsx | 1 - src/components/AdminPage/AdminPage.tsx | 7 - .../AdminPage/subpages/AdminImportPage.tsx | 17 -- .../UserImportForm/UserImportForm.tsx | 56 ---- src/components/UserImportForm/index.tsx | 3 - src/containers/UserImportFormContainer.tsx | 64 ---- src/containers/containers.spec.tsx | 15 - src/modules/imports/actions.ts | 70 ----- src/modules/imports/index.ts | 8 - src/modules/imports/reducer.spec.ts | 166 ----------- src/modules/imports/reducer.ts | 82 ------ src/modules/imports/sagas.spec.ts | 91 ------ src/modules/imports/sagas.ts | 47 --- src/modules/index.ts | 3 - src/util/importCsv.spec.ts | 276 ------------------ src/util/importCsv.ts | 144 --------- src/util/importUsers.ts | 17 -- src/util/parseCsv.spec.ts | 20 -- src/util/parseCsv.ts | 14 - 25 files changed, 1658 deletions(-) delete mode 100644 functions/api/basicAuth.js delete mode 100644 functions/api/basicAuth.spec.js delete mode 100644 functions/api/index.spec.js delete mode 100644 functions/api/syncUsers.js delete mode 100644 functions/api/syncUsers.spec.js delete mode 100644 src/components/AdminPage/subpages/AdminImportPage.tsx delete mode 100644 src/components/UserImportForm/UserImportForm.tsx delete mode 100644 src/components/UserImportForm/index.tsx delete mode 100644 src/containers/UserImportFormContainer.tsx delete mode 100644 src/modules/imports/actions.ts delete mode 100644 src/modules/imports/index.ts delete mode 100644 src/modules/imports/reducer.spec.ts delete mode 100644 src/modules/imports/reducer.ts delete mode 100644 src/modules/imports/sagas.spec.ts delete mode 100644 src/modules/imports/sagas.ts delete mode 100644 src/util/importCsv.spec.ts delete mode 100644 src/util/importCsv.ts delete mode 100644 src/util/importUsers.ts delete mode 100644 src/util/parseCsv.spec.ts delete mode 100644 src/util/parseCsv.ts diff --git a/functions/api/basicAuth.js b/functions/api/basicAuth.js deleted file mode 100644 index 9a13bd52..00000000 --- a/functions/api/basicAuth.js +++ /dev/null @@ -1,28 +0,0 @@ -const expectedUsername = process.env.API_SERVICEUSER_USERNAME -const expectedPassword = process.env.API_SERVICEUSER_PASSWORD - -const basicAuth = (req, res, next) => { - if (!expectedUsername || !expectedPassword) { - console.info( - "Set API_SERVICEUSER_USERNAME and API_SERVICEUSER_PASSWORD env vars for the API auth" - ) - res.status(401).send('Unauthorized') - return - } - - const authHeader = req.headers.authorization || '' - const [type, credentials] = authHeader.split(' ') - - if (type === 'Basic' && credentials) { - const decoded = Buffer.from(credentials, 'base64').toString('utf-8') - const [username, password] = decoded.split(':') - - if (username === expectedUsername && password === expectedPassword) { - return next() - } - } - - res.status(401).send('Unauthorized') -} - -module.exports = basicAuth diff --git a/functions/api/basicAuth.spec.js b/functions/api/basicAuth.spec.js deleted file mode 100644 index 18e02a2a..00000000 --- a/functions/api/basicAuth.spec.js +++ /dev/null @@ -1,135 +0,0 @@ -'use strict'; - -// basicAuth.js captures process.env at require time, so each describe -// block resets modules and requires the module with the relevant env. - -const makeReq = (authHeader) => ({ - headers: { authorization: authHeader || '' }, -}); - -const makeRes = () => ({ - status: jest.fn().mockReturnThis(), - send: jest.fn().mockReturnThis(), -}); - -describe('functions/api/basicAuth', () => { - let consoleInfoSpy; - - beforeEach(() => { - consoleInfoSpy = jest.spyOn(console, 'info').mockImplementation(() => {}); - }); - - afterEach(() => { - consoleInfoSpy.mockRestore(); - delete process.env.API_SERVICEUSER_USERNAME; - delete process.env.API_SERVICEUSER_PASSWORD; - }); - - describe('when env vars are missing', () => { - let basicAuth; - - beforeEach(() => { - jest.resetModules(); - basicAuth = require('./basicAuth'); - }); - - it('returns 401 when both env vars are absent', () => { - const next = jest.fn(); - const res = makeRes(); - basicAuth(makeReq(), res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(res.send).toHaveBeenCalledWith('Unauthorized'); - expect(next).not.toHaveBeenCalled(); - }); - }); - - describe('when username is missing', () => { - let basicAuth; - - beforeEach(() => { - jest.resetModules(); - process.env.API_SERVICEUSER_PASSWORD = 'pass'; - basicAuth = require('./basicAuth'); - }); - - it('returns 401', () => { - const next = jest.fn(); - const res = makeRes(); - basicAuth(makeReq(), res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); - }); - }); - - describe('when password is missing', () => { - let basicAuth; - - beforeEach(() => { - jest.resetModules(); - process.env.API_SERVICEUSER_USERNAME = 'user'; - basicAuth = require('./basicAuth'); - }); - - it('returns 401', () => { - const next = jest.fn(); - const res = makeRes(); - basicAuth(makeReq(), res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); - }); - }); - - describe('with valid env vars', () => { - let basicAuth; - - beforeEach(() => { - jest.resetModules(); - process.env.API_SERVICEUSER_USERNAME = 'admin'; - process.env.API_SERVICEUSER_PASSWORD = 's3cret'; - basicAuth = require('./basicAuth'); - }); - - it('calls next() when credentials are valid', () => { - const next = jest.fn(); - const res = makeRes(); - const credentials = Buffer.from('admin:s3cret').toString('base64'); - basicAuth(makeReq(`Basic ${credentials}`), res, next); - expect(next).toHaveBeenCalled(); - expect(res.status).not.toHaveBeenCalled(); - }); - - it('returns 401 when password is wrong', () => { - const next = jest.fn(); - const res = makeRes(); - const credentials = Buffer.from('admin:wrongpass').toString('base64'); - basicAuth(makeReq(`Basic ${credentials}`), res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); - }); - - it('returns 401 when username is wrong', () => { - const next = jest.fn(); - const res = makeRes(); - const credentials = Buffer.from('wronguser:s3cret').toString('base64'); - basicAuth(makeReq(`Basic ${credentials}`), res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); - }); - - it('returns 401 when no Authorization header is provided', () => { - const next = jest.fn(); - const res = makeRes(); - basicAuth(makeReq(''), res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); - }); - - it('returns 401 when auth type is Bearer instead of Basic', () => { - const next = jest.fn(); - const res = makeRes(); - basicAuth(makeReq('Bearer sometoken'), res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/functions/api/index.js b/functions/api/index.js index ea9f2275..09a50096 100644 --- a/functions/api/index.js +++ b/functions/api/index.js @@ -3,24 +3,10 @@ const admin = require('firebase-admin') const express = require('express') const cors = require('cors')({origin: true, credentials: true}) const fetchAerodromeStatus = require('./fetchAerodromeStatus') -const basicAuth = require('./basicAuth') -const syncUsers = require('./syncUsers') const fetchUserInvoiceRecipients = require('./fetchUserInvoiceRecipients') const {fetchInvoices, fetchCheckouts, postPrepopulatedForm, isCustomsDeclarationAppAvailable} = require('./customs/fetchFromCustoms') const {fbAuth, fbAdminAuth} = require('./fbAuth') -// The user-import (member management) endpoint is only relevant to projects with -// member management enabled (currently lspv). The deploy workflow writes this -// generated flag from the project's `memberManagement` config; it is absent in -// local dev / tests, where the endpoint stays disabled. Fail closed: if the -// flag is missing or false, the route is not registered at all (404). -let memberManagementEnabled = false -try { - memberManagementEnabled = require('../member-management.generated.js') -} catch (e) { - if (e.code !== 'MODULE_NOT_FOUND') throw e -} - const api = express() api.use(cors) @@ -37,25 +23,6 @@ api.get('(/api)?/aerodrome/status', async (req, res) => { res.send(status) }) -if (memberManagementEnabled) { - api.post('(/api)?/users/import', basicAuth, async (req, res) => { - try { - const users = req.body.users - if (!Array.isArray(users)) { - return res.status(400).send('Invalid users format') - } - - const db = admin.database() - await syncUsers(db, users) - - res.status(200).send({ message: 'Users imported successfully' }) - } catch (e) { - console.error('Failed to import users', e) - res.status(500).send({ error: 'Failed to import users' }) - } - }) -} - api.get('(/api)?/customs/invoices', fbAdminAuth, async (req, res) => { try { const db = admin.database() @@ -115,5 +82,3 @@ api.get('(/api)?/users/me/invoice-recipients', fbAuth, async (req, res) => { }) module.exports = onRequest({ region: 'europe-west1' }, api) -// Exposed for tests (route registration depends on the member-management flag). -module.exports.app = api diff --git a/functions/api/index.spec.js b/functions/api/index.spec.js deleted file mode 100644 index 37da5f72..00000000 --- a/functions/api/index.spec.js +++ /dev/null @@ -1,48 +0,0 @@ -'use strict'; - -// The /users/import (member management) route is registered only when the -// generated member-management flag is truthy. The flag file is absent in tests, -// so it is mocked per-case (virtual module). - -const FLAG_PATH = '../member-management.generated.js'; - -const loadApi = (flag) => { - jest.resetModules(); - jest.doMock(FLAG_PATH, () => flag, { virtual: true }); - jest.doMock('firebase-functions/v2/https', () => ({ - onRequest: (opts, app) => app, - })); - return require('./index').app; -}; - -const hasUsersImportRoute = (app) => - app._router.stack.some( - layer => layer.route && String(layer.route.path).includes('/users/import') - ); - -describe('functions', () => { - describe('api/index member-management gating', () => { - afterEach(() => { - jest.dontMock(FLAG_PATH); - jest.resetModules(); - }); - - it('does not register /users/import when member management is disabled', () => { - const app = loadApi(false); - expect(hasUsersImportRoute(app)).toBe(false); - }); - - it('does not register /users/import when the flag file is missing', () => { - // Simulate the generated file being absent (MODULE_NOT_FOUND -> disabled). - jest.resetModules(); - jest.doMock('firebase-functions/v2/https', () => ({ onRequest: (opts, app) => app })); - const app = require('./index').app; - expect(hasUsersImportRoute(app)).toBe(false); - }); - - it('registers /users/import when member management is enabled', () => { - const app = loadApi(true); - expect(hasUsersImportRoute(app)).toBe(true); - }); - }); -}); diff --git a/functions/api/syncUsers.js b/functions/api/syncUsers.js deleted file mode 100644 index 53e0e381..00000000 --- a/functions/api/syncUsers.js +++ /dev/null @@ -1,86 +0,0 @@ -'use strict'; - -const syncUsers = async (firebase, users) => { - const usersRef = firebase.ref('users') - - const currentUsers = await fetchCurrentUsers(usersRef) - const importedUsers = mapUsersByMemberNr(users) - - const {updates, deletes} = buildUpdatesAndDeletes(currentUsers, importedUsers) - await applyUpdatesAndDeletes(usersRef, updates, deletes) -} - -async function fetchCurrentUsers(usersRef) { - const snapshot = await usersRef.once('value') - const users = snapshot.val() || {} - - // Transform users to a map where memberNr is the key for easy lookup - const userMap = {} - Object.keys(users).forEach(id => { - const user = users[id] - if (user.memberNr) { - userMap[user.memberNr] = {...user, id} - } - }) - return userMap -} - -function mapUsersByMemberNr(users) { - const userMap = {} - users.forEach(user => { - if (user.memberNr) { - userMap[user.memberNr] = user - } - }) - return userMap -} - -function buildUpdatesAndDeletes(currentUsers, importedUsers) { - const updates = {} - const deletes = [] - - // Add new users and update existing ones - Object.keys(importedUsers).forEach(memberNr => { - const importedUser = importedUsers[memberNr] - if (currentUsers[memberNr]) { - // Existing user - update by ID - updates[currentUsers[memberNr].id] = importedUser - } else { - // New user - add to the list of new users - updates[`new-${memberNr}`] = importedUser // Placeholder for new - } - }) - - // Delete users that are not present in the imported array - Object.keys(currentUsers).forEach(memberNr => { - if (!importedUsers.hasOwnProperty(memberNr)) { - deletes.push(currentUsers[memberNr].id) - } - }) - - return {updates, deletes} -} - -async function applyUpdatesAndDeletes(usersRef, updates, deletes) { - // Apply new and updated users - const batch = {} - for (const key in updates) { - if (key.startsWith('new-')) { - // Generate new ID for new user - await usersRef.push(updates[key]) - } else { - // Update existing user - batch[key] = updates[key] - } - } - - // Remove users - deletes.forEach(async id => { - await usersRef.child(id).remove() - }) - - // Apply updates - await usersRef.update(batch) -} - -module.exports = syncUsers diff --git a/functions/api/syncUsers.spec.js b/functions/api/syncUsers.spec.js deleted file mode 100644 index 3345d77a..00000000 --- a/functions/api/syncUsers.spec.js +++ /dev/null @@ -1,225 +0,0 @@ -'use strict'; - -const syncUsers = require('./syncUsers'); - -describe('functions/api/syncUsers', () => { - const buildFirebaseMock = (existingUsersObj = {}) => { - const mockPush = jest.fn().mockResolvedValue({ key: 'new-generated-key' }); - const mockRemove = jest.fn().mockResolvedValue(); - const mockUpdate = jest.fn().mockResolvedValue(); - const mockOnce = jest.fn().mockResolvedValue({ - val: () => existingUsersObj - }); - const mockChild = jest.fn(() => ({ remove: mockRemove })); - - const usersRef = { - once: mockOnce, - push: mockPush, - update: mockUpdate, - child: mockChild - }; - - const firebase = { - ref: jest.fn(() => usersRef) - }; - - return { firebase, usersRef, mockPush, mockRemove, mockUpdate, mockChild }; - }; - - describe('mapUsersByMemberNr', () => { - it('adds new user when memberNr not in current users', async () => { - const { firebase, mockPush } = buildFirebaseMock({}); - - await syncUsers(firebase, [ - { memberNr: '001', firstname: 'Alice', lastname: 'Smith' } - ]); - - expect(mockPush).toHaveBeenCalledWith( - expect.objectContaining({ memberNr: '001', firstname: 'Alice' }) - ); - }); - - it('ignores imported users without memberNr', async () => { - const { firebase, mockPush, mockUpdate } = buildFirebaseMock({}); - - await syncUsers(firebase, [ - { firstname: 'NoMember', lastname: 'User' } // no memberNr - ]); - - expect(mockPush).not.toHaveBeenCalled(); - }); - }); - - describe('fetchCurrentUsers', () => { - it('builds user map keyed by memberNr from database snapshot', async () => { - const existingUsers = { - 'firebase-id-1': { memberNr: '001', firstname: 'Alice' }, - 'firebase-id-2': { memberNr: '002', firstname: 'Bob' } - }; - const { firebase, mockUpdate } = buildFirebaseMock(existingUsers); - - // Import same users - should result in updates (same memberNr) - await syncUsers(firebase, [ - { memberNr: '001', firstname: 'Alice Updated' }, - { memberNr: '002', firstname: 'Bob Updated' } - ]); - - // Should update by firebase ID (not push new ones) - expect(mockUpdate).toHaveBeenCalledWith( - expect.objectContaining({ - 'firebase-id-1': expect.objectContaining({ memberNr: '001', firstname: 'Alice Updated' }), - 'firebase-id-2': expect.objectContaining({ memberNr: '002', firstname: 'Bob Updated' }) - }) - ); - }); - - it('skips existing users without memberNr during lookup build', async () => { - // Users without memberNr in the database are excluded from the map - const existingUsers = { - 'firebase-id-1': { firstname: 'NoMember' } // no memberNr - }; - const { firebase, mockPush } = buildFirebaseMock(existingUsers); - - // New import with memberNr - should push since existing user has no memberNr for lookup - await syncUsers(firebase, [ - { memberNr: '999', firstname: 'New User' } - ]); - - expect(mockPush).toHaveBeenCalledWith( - expect.objectContaining({ memberNr: '999' }) - ); - }); - - it('handles null database value (empty database)', async () => { - const mockOnce = jest.fn().mockResolvedValue({ val: () => null }); - const mockPush = jest.fn().mockResolvedValue({}); - const mockUpdate = jest.fn().mockResolvedValue(); - const mockChild = jest.fn(() => ({ remove: jest.fn() })); - - const usersRef = { once: mockOnce, push: mockPush, update: mockUpdate, child: mockChild }; - const firebase = { ref: jest.fn(() => usersRef) }; - - await syncUsers(firebase, [{ memberNr: '001', firstname: 'Alice' }]); - - expect(mockPush).toHaveBeenCalled(); - }); - }); - - describe('buildUpdatesAndDeletes', () => { - it('updates existing user by firebase ID', async () => { - const existingUsers = { - 'fb-id-1': { memberNr: '001', firstname: 'Alice', lastname: 'Old' } - }; - const { firebase, mockUpdate } = buildFirebaseMock(existingUsers); - - await syncUsers(firebase, [ - { memberNr: '001', firstname: 'Alice', lastname: 'New' } - ]); - - const updateArg = mockUpdate.mock.calls[0][0]; - expect(updateArg['fb-id-1']).toEqual( - expect.objectContaining({ memberNr: '001', lastname: 'New' }) - ); - }); - - it('creates placeholder key for new user', async () => { - const { firebase, mockUpdate, mockPush } = buildFirebaseMock({}); - - await syncUsers(firebase, [ - { memberNr: '123', firstname: 'New User' } - ]); - - // New user should be pushed (not in batch update) - expect(mockPush).toHaveBeenCalledWith( - expect.objectContaining({ memberNr: '123' }) - ); - }); - - it('deletes users not in imported list', async () => { - const existingUsers = { - 'fb-id-old': { memberNr: '999', firstname: 'OldUser' } - }; - const { firebase, mockChild, mockRemove } = buildFirebaseMock(existingUsers); - - // Import does NOT include memberNr 999 - await syncUsers(firebase, [ - { memberNr: '001', firstname: 'NewUser' } - ]); - - expect(mockChild).toHaveBeenCalledWith('fb-id-old'); - expect(mockRemove).toHaveBeenCalled(); - }); - - it('handles empty imported users list (deletes all existing)', async () => { - const existingUsers = { - 'fb-id-1': { memberNr: '001', firstname: 'Alice' } - }; - const { firebase, mockChild, mockRemove } = buildFirebaseMock(existingUsers); - - await syncUsers(firebase, []); - - expect(mockChild).toHaveBeenCalledWith('fb-id-1'); - expect(mockRemove).toHaveBeenCalled(); - }); - - it('handles empty existing users (adds all imported)', async () => { - const { firebase, mockPush } = buildFirebaseMock({}); - - await syncUsers(firebase, [ - { memberNr: '001', firstname: 'Alice' }, - { memberNr: '002', firstname: 'Bob' } - ]); - - expect(mockPush).toHaveBeenCalledTimes(2); - }); - }); - - describe('applyUpdatesAndDeletes', () => { - it('calls update with batch of existing user updates', async () => { - const existingUsers = { - 'fb-id-1': { memberNr: '001', firstname: 'Alice' }, - 'fb-id-2': { memberNr: '002', firstname: 'Bob' } - }; - const { firebase, mockUpdate } = buildFirebaseMock(existingUsers); - - await syncUsers(firebase, [ - { memberNr: '001', firstname: 'Alice v2' }, - { memberNr: '002', firstname: 'Bob v2' } - ]); - - expect(mockUpdate).toHaveBeenCalledTimes(1); - const batchArg = mockUpdate.mock.calls[0][0]; - expect(batchArg['fb-id-1']).toBeDefined(); - expect(batchArg['fb-id-2']).toBeDefined(); - }); - - it('calls usersRef.ref with "users" path', async () => { - const { firebase } = buildFirebaseMock({}); - - await syncUsers(firebase, []); - - expect(firebase.ref).toHaveBeenCalledWith('users'); - }); - - it('handles mix of new and existing users', async () => { - const existingUsers = { - 'fb-id-1': { memberNr: '001', firstname: 'Alice' } - }; - const { firebase, mockPush, mockUpdate } = buildFirebaseMock(existingUsers); - - await syncUsers(firebase, [ - { memberNr: '001', firstname: 'Alice Updated' }, // existing - { memberNr: '002', firstname: 'Bob New' } // new - ]); - - // Existing gets updated via batch - const batchArg = mockUpdate.mock.calls[0][0]; - expect(batchArg['fb-id-1']).toBeDefined(); - - // New gets pushed - expect(mockPush).toHaveBeenCalledWith( - expect.objectContaining({ memberNr: '002' }) - ); - }); - }); -}); diff --git a/src/components/AdminPage/AdminNavigation.tsx b/src/components/AdminPage/AdminNavigation.tsx index 926332b4..7894454e 100644 --- a/src/components/AdminPage/AdminNavigation.tsx +++ b/src/components/AdminPage/AdminNavigation.tsx @@ -116,7 +116,6 @@ const AdminNavigation = ({ activeTab, hiddenTabs, onTabChange }) => { { key: 'invoice-recipients', label: t('admin.invoiceRecipients'), icon: 'receipt' }, { key: 'kiosk-access', label: t('admin.kioskAccess'), icon: 'person_add' }, { key: 'guest-access', label: t('admin.guestAccess'), icon: 'person_add' }, - { key: 'import', label: t('admin.import'), icon: 'file_upload' }, { key: 'privacy', label: t('admin.privacy'), icon: 'security' }, ].filter(item => !hiddenTabs.includes(item.key)); diff --git a/src/components/AdminPage/AdminPage.tsx b/src/components/AdminPage/AdminPage.tsx index 842b48ae..2eb6a230 100644 --- a/src/components/AdminPage/AdminPage.tsx +++ b/src/components/AdminPage/AdminPage.tsx @@ -9,7 +9,6 @@ import AdminExportPage from './subpages/AdminExportPage'; import AdminLockMovementsPage from './subpages/AdminLockMovementsPage'; import AdminAerodromeStatusPage from './subpages/AdminAerodromeStatusPage'; import AdminMessagesPage from './subpages/AdminMessagesPage'; -import AdminImportPage from './subpages/AdminImportPage'; import AdminAircraftPage from './subpages/AdminAircraftPage'; import AdminInvoiceRecipientsPage from './subpages/AdminInvoiceRecipientsPage'; import AdminGuestAccessPage from './subpages/AdminGuestAccessPage'; @@ -49,8 +48,6 @@ const renderSubPage = (activeTab: string) => { return ; case 'messages': return ; - case 'import': - return ; case 'aircraft': return ; case 'invoice-recipients': @@ -83,7 +80,6 @@ const AdminPage = ({auth, guestAccessToken, kioskAccessToken}: any) => { const invoicePaymentEnabled = objectToArray(__CONF__.paymentMethods).includes('invoice'); const guestAccessEnabled = guestAccessToken && guestAccessToken.token; const kioskAccessEnabled = kioskAccessToken && kioskAccessToken.token; - const memberManagementEnabled = __CONF__.memberManagement === true; if (!invoicePaymentEnabled) { hiddenTabs.push('invoice-recipients'); @@ -94,9 +90,6 @@ const AdminPage = ({auth, guestAccessToken, kioskAccessToken}: any) => { if (!kioskAccessEnabled) { hiddenTabs.push('kiosk-access'); } - if (!memberManagementEnabled) { - hiddenTabs.push('import'); - } if (__CONF__.privacySettings !== true) { hiddenTabs.push('privacy'); } diff --git a/src/components/AdminPage/subpages/AdminImportPage.tsx b/src/components/AdminPage/subpages/AdminImportPage.tsx deleted file mode 100644 index f986913a..00000000 --- a/src/components/AdminPage/subpages/AdminImportPage.tsx +++ /dev/null @@ -1,17 +0,0 @@ -import React from 'react'; -import LabeledBox from '../../LabeledBox'; -import UserImportForm from '../../../containers/UserImportFormContainer'; -import { useTranslation } from 'react-i18next'; - -const AdminImportPage = () => { - const { t } = useTranslation(); - return ( - <> - - - - - ); -}; - -export default AdminImportPage; diff --git a/src/components/UserImportForm/UserImportForm.tsx b/src/components/UserImportForm/UserImportForm.tsx deleted file mode 100644 index 049b720d..00000000 --- a/src/components/UserImportForm/UserImportForm.tsx +++ /dev/null @@ -1,56 +0,0 @@ -import PropTypes from 'prop-types'; -import React from 'react'; -import CsvImportForm, {Example} from '../CsvImportForm'; -import P from '../P'; -import Em from '../Em'; -import Strong from '../Strong'; -import { useTranslation } from 'react-i18next'; - -const UserImportForm = props => { - const { t } = useTranslation(); - const description = ( -
-

{t('adminImport.userListDesc1')}

-

- {t('adminImport.userListDesc2_pre')} UserName, LastName, FirstName, PhoneMobile {t('adminImport.userListDesc2_mid')} Email {t('adminImport.userListDesc2_mid2')} UTF-8{t('adminImport.userListDesc2_post')} -

-

{t('adminImport.userListSample')}

- - UserName,LastName,FirstName,PhoneMobile,Email
- 11069,Mustermann,Max,+41791234567,max@example.com
- 11293,Musterfrau,Maria,+41768765432,maria@example.com
- -
- ); - - return ( -
- -
- ); -}; - -UserImportForm.propTypes = { - disabled: PropTypes.bool, - importInProgress: PropTypes.bool.isRequired, - importDone: PropTypes.bool.isRequired, - importFailed: PropTypes.bool.isRequired, - selectedFile: PropTypes.object, - selectFile: PropTypes.func.isRequired, - startImport: PropTypes.func.isRequired, - closeDoneDialog: PropTypes.func.isRequired, -}; - -export default UserImportForm; diff --git a/src/components/UserImportForm/index.tsx b/src/components/UserImportForm/index.tsx deleted file mode 100644 index 7c44ef0c..00000000 --- a/src/components/UserImportForm/index.tsx +++ /dev/null @@ -1,3 +0,0 @@ -import UserImportForm from './UserImportForm'; - -export default UserImportForm; diff --git a/src/containers/UserImportFormContainer.tsx b/src/containers/UserImportFormContainer.tsx deleted file mode 100644 index 535e3a9d..00000000 --- a/src/containers/UserImportFormContainer.tsx +++ /dev/null @@ -1,64 +0,0 @@ -import React, { useEffect } from 'react'; -import {connect} from 'react-redux'; -import {initImport, selectImportFile, startImport} from '../modules/imports'; -import UserImportForm from '../components/UserImportForm'; -import {RootState} from '../modules'; - -const IMPORT_NAME = 'users'; - -interface Props { - initialized: boolean; - inProgress: boolean; - importDone: boolean; - importFailed: boolean; - selectedFile?: File; - initImport: () => void; - selectFile: (file: File) => void; - startImport: () => void; -} - -const UserImportFormContainer = (props: Props) => { - useEffect(() => { - props.initImport(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - - return ( - - ); -}; - -const mapStateToProps = (state: RootState) => { - let importObj = (state.imports as any)[IMPORT_NAME]; - let initialized = true; - - if (!importObj) { - importObj = {}; - initialized = false; - } - - return { - initialized, - selectedFile: importObj.file, - inProgress: importObj.inProgress === true, - importDone: importObj.done === true, - importFailed: importObj.failed === true, - }; -}; - -const mapDispatchToProps = (dispatch: any) => ({ - initImport: () => dispatch(initImport(IMPORT_NAME)), - selectFile: (file: File) => dispatch(selectImportFile(IMPORT_NAME, file)), - startImport: () => dispatch(startImport(IMPORT_NAME)), -}); - -export default connect(mapStateToProps, mapDispatchToProps)(UserImportFormContainer); diff --git a/src/containers/containers.spec.tsx b/src/containers/containers.spec.tsx index c67d4a9d..c144cf96 100644 --- a/src/containers/containers.spec.tsx +++ b/src/containers/containers.spec.tsx @@ -90,11 +90,6 @@ jest.mock('../components/ItemList', () => ({ default: () =>
, })); -jest.mock('../components/UserImportForm', () => ({ - __esModule: true, - default: () =>
, -})); - jest.mock('../components/YearlySummaryReportForm', () => ({ __esModule: true, default: () =>
, @@ -126,7 +121,6 @@ describe('container mount dispatches', () => { let AircraftDropdownContainer: any; let AircraftsItemListContainer: any; let ArrivalFinishContainer: any; - let UserImportFormContainer: any; let YearlySummaryReportFormContainer: any; let LandingsReportFormContainer: any; let AirstatReportFormContainer: any; @@ -142,7 +136,6 @@ describe('container mount dispatches', () => { AircraftsItemListContainer = require('./AircraftsItemListContainer').default; ArrivalFinishContainer = require('./ArrivalFinishContainer').default; - UserImportFormContainer = require('./UserImportFormContainer').default; YearlySummaryReportFormContainer = require( './YearlySummaryReportFormContainer' ).default; @@ -236,14 +229,6 @@ describe('container mount dispatches', () => { expect(countOf(store, 'LOAD_USER_INVOICE_RECIPIENTS')).toBe(1); }); - it('UserImportFormContainer dispatches INIT_IMPORT exactly once with name=users', () => { - const store = makeStore({ imports: {} }); - render(wrap(store, )); - const inits = store.actions.filter(a => a.type === 'INIT_IMPORT'); - expect(inits.length).toBe(1); - expect(inits[0].payload.name).toBe('users'); - }); - it('YearlySummaryReportFormContainer dispatches INIT_REPORT exactly once with name=yearlySummary', () => { const store = makeStore({ reports: {} }); render(wrap(store, )); diff --git a/src/modules/imports/actions.ts b/src/modules/imports/actions.ts deleted file mode 100644 index 10843980..00000000 --- a/src/modules/imports/actions.ts +++ /dev/null @@ -1,70 +0,0 @@ -export const INIT_IMPORT = 'INIT_IMPORT' as const; -export const SELECT_IMPORT_FILE = 'SELECT_IMPORT_FILE' as const; -export const START_IMPORT = 'START_IMPORT' as const; -export const SET_IMPORT_IN_PROGRESS = 'SET_IMPORT_IN_PROGRESS' as const; -export const IMPORT_SUCCESS = 'IMPORT_SUCCESS' as const; -export const IMPORT_FAILURE = 'IMPORT_FAILURE' as const; - -export type ImportsAction = - | { type: typeof INIT_IMPORT; payload: { name: string } } - | { type: typeof SELECT_IMPORT_FILE; payload: { importName: string; file: File } } - | { type: typeof START_IMPORT; payload: { importName: string } } - | { type: typeof SET_IMPORT_IN_PROGRESS; payload: { importName: string; inProgress: boolean } } - | { type: typeof IMPORT_SUCCESS; payload: { importName: string } } - | { type: typeof IMPORT_FAILURE; payload: { importName: string } }; - -export function initImport(name: string) { - return { - type: INIT_IMPORT, - payload: { - name, - }, - }; -} - -export function selectImportFile(importName: string, file: File) { - return { - type: SELECT_IMPORT_FILE, - payload: { - importName, - file, - }, - }; -} - -export function startImport(importName: string) { - return { - type: START_IMPORT, - payload: { - importName, - }, - }; -} - -export function setImportInProgress(importName: string, inProgress: boolean) { - return { - type: SET_IMPORT_IN_PROGRESS, - payload: { - importName, - inProgress, - }, - }; -} - -export function importSuccess(importName: string) { - return { - type: IMPORT_SUCCESS, - payload: { - importName, - }, - }; -} - -export function importFailure(importName: string) { - return { - type: IMPORT_FAILURE, - payload: { - importName, - }, - }; -} diff --git a/src/modules/imports/index.ts b/src/modules/imports/index.ts deleted file mode 100644 index a66ca0e0..00000000 --- a/src/modules/imports/index.ts +++ /dev/null @@ -1,8 +0,0 @@ -import reducer from './reducer'; -import sagas from './sagas'; - -export { initImport, selectImportFile, startImport, setImportInProgress } from './actions'; - -export { sagas }; - -export default reducer; diff --git a/src/modules/imports/reducer.spec.ts b/src/modules/imports/reducer.spec.ts deleted file mode 100644 index fb79e4b5..00000000 --- a/src/modules/imports/reducer.spec.ts +++ /dev/null @@ -1,166 +0,0 @@ -import reducer from './reducer'; -import * as actions from './actions'; - -const INITIAL_STATE = {}; - -describe('modules', () => { - describe('imports', () => { - describe('reducer', () => { - it('should handle initial state', () => { - expect( - reducer(undefined, {} as any) - ).toEqual(INITIAL_STATE); - }); - - describe('INIT_IMPORT', () => { - it('should initialize an import entry', () => { - expect( - reducer({}, actions.initImport('aircrafts')) - ).toEqual({ - aircrafts: { - file: null, - inProgress: false, - done: false, - failed: false, - }, - }); - }); - - it('should add a new import without affecting existing ones', () => { - expect( - reducer({ - aircrafts: { - file: null, - inProgress: false, - done: false, - failed: false, - }, - }, actions.initImport('movements')) - ).toEqual({ - aircrafts: { - file: null, - inProgress: false, - done: false, - failed: false, - }, - movements: { - file: null, - inProgress: false, - done: false, - failed: false, - }, - }); - }); - }); - - describe('SELECT_IMPORT_FILE', () => { - it('should set the file for the given import', () => { - const file = { name: 'aircrafts.csv' } as unknown as File; - expect( - reducer({ - aircrafts: { - file: null, - inProgress: false, - done: false, - failed: false, - }, - }, actions.selectImportFile('aircrafts', file)) - ).toEqual({ - aircrafts: { - file, - inProgress: false, - done: false, - failed: false, - }, - }); - }); - }); - - describe('SET_IMPORT_IN_PROGRESS', () => { - it('should set inProgress to true', () => { - expect( - reducer({ - aircrafts: { - file: { name: 'aircrafts.csv' } as unknown as File, - inProgress: false, - done: false, - failed: false, - }, - }, actions.setImportInProgress('aircrafts', true)) - ).toEqual({ - aircrafts: { - file: { name: 'aircrafts.csv' }, - inProgress: true, - done: false, - failed: false, - }, - }); - }); - - it('should set inProgress to false', () => { - expect( - reducer({ - aircrafts: { - file: { name: 'aircrafts.csv' } as unknown as File, - inProgress: true, - done: false, - failed: false, - }, - }, actions.setImportInProgress('aircrafts', false)) - ).toEqual({ - aircrafts: { - file: { name: 'aircrafts.csv' }, - inProgress: false, - done: false, - failed: false, - }, - }); - }); - }); - - describe('IMPORT_SUCCESS', () => { - it('should set done to true and inProgress to false', () => { - expect( - reducer({ - aircrafts: { - file: { name: 'aircrafts.csv' } as unknown as File, - inProgress: true, - done: false, - failed: false, - }, - }, actions.importSuccess('aircrafts')) - ).toEqual({ - aircrafts: { - file: { name: 'aircrafts.csv' }, - inProgress: false, - done: true, - failed: false, - }, - }); - }); - }); - - describe('IMPORT_FAILURE', () => { - it('should set failed to true and inProgress to false', () => { - expect( - reducer({ - aircrafts: { - file: { name: 'aircrafts.csv' } as unknown as File, - inProgress: true, - done: false, - failed: false, - }, - }, actions.importFailure('aircrafts')) - ).toEqual({ - aircrafts: { - file: { name: 'aircrafts.csv' }, - inProgress: false, - done: false, - failed: true, - }, - }); - }); - }); - }); - }); -}); diff --git a/src/modules/imports/reducer.ts b/src/modules/imports/reducer.ts deleted file mode 100644 index 5f8e2e2d..00000000 --- a/src/modules/imports/reducer.ts +++ /dev/null @@ -1,82 +0,0 @@ -import * as actions from './actions'; -import { ImportsAction } from './actions'; -import reducer from '../../util/reducer'; - -interface ImportItem { - file: File | null; - inProgress: boolean; - done: boolean; - failed: boolean; -} - -interface ImportsState { - [importName: string]: ImportItem; -} - -const INITIAL_STATE: ImportsState = {}; - -function initImport(state: ImportsState, action: ImportsAction & { type: typeof actions.INIT_IMPORT }) { - return Object.assign({}, state, { - [action.payload.name]: { - file: null, - inProgress: false, - done: false, - failed: false, - }, - }); -} - -function setImportFile(state: ImportsState, action: ImportsAction & { type: typeof actions.SELECT_IMPORT_FILE }) { - const { importName, file } = action.payload; - - const newImportObj = Object.assign({}, state[importName], { - file, - }); - return Object.assign({}, state, { - [importName]: newImportObj, - }); -} - -function setImportInProgress(state: ImportsState, action: ImportsAction & { type: typeof actions.SET_IMPORT_IN_PROGRESS }) { - const { importName, inProgress } = action.payload; - - const newImportObj = Object.assign({}, state[importName], { - inProgress, - }); - return Object.assign({}, state, { - [importName]: newImportObj, - }); -} - -function importSuccess(state: ImportsState, action: ImportsAction & { type: typeof actions.IMPORT_SUCCESS }) { - const { importName } = action.payload; - const newImportObj = Object.assign({}, state[importName], { - inProgress: false, - done: true, - }); - return Object.assign({}, state, { - [importName]: newImportObj, - }); -} - -function importFailure(state: ImportsState, action: ImportsAction & { type: typeof actions.IMPORT_FAILURE }) { - const { importName } = action.payload; - const newImportObj = Object.assign({}, state[importName], { - inProgress: false, - failed: true, - }); - return Object.assign({}, state, { - [importName]: newImportObj, - }); -} - -const ACTION_HANDLERS = { - [actions.INIT_IMPORT]: initImport, - [actions.SELECT_IMPORT_FILE]: setImportFile, - [actions.SET_IMPORT_IN_PROGRESS]: setImportInProgress, - [actions.IMPORT_SUCCESS]: importSuccess, - [actions.IMPORT_FAILURE]: importFailure, -}; - -export type { ImportsState }; -export default reducer(INITIAL_STATE, ACTION_HANDLERS); diff --git a/src/modules/imports/sagas.spec.ts b/src/modules/imports/sagas.spec.ts deleted file mode 100644 index 858df128..00000000 --- a/src/modules/imports/sagas.spec.ts +++ /dev/null @@ -1,91 +0,0 @@ -import {call, put, select} from 'redux-saga/effects'; -import * as actions from './actions'; -import * as sagas from './sagas'; -import importUsers from '../../util/importUsers'; - -jest.mock('../../util/importUsers'); -jest.mock('../../util/log'); - -describe('modules', () => { - describe('imports', () => { - describe('sagas', () => { - describe('selectImport', () => { - it('should return a selector for the given import name', () => { - const selector = sagas.selectImport('users'); - const state = { - imports: { - users: { file: null, inProgress: false } - } - }; - expect(selector(state)).toEqual({ file: null, inProgress: false }); - }); - }); - - describe('doImport', () => { - it('should call importUsers for users import', () => { - const csvString = 'name,email\nJohn,john@example.com'; - sagas.doImport('users', csvString); - expect(importUsers).toHaveBeenCalledWith(csvString); - }); - - it('should throw for unknown import name', () => { - expect(() => sagas.doImport('unknown', 'csv')).toThrow('Unknown import unknown'); - }); - }); - - describe('getString', () => { - it('should resolve with file contents as string', async () => { - const fileContent = 'name,email\nJohn,john@example.com'; - const file = new Blob([fileContent], {type: 'text/csv'}); - - const result = await sagas.getString(file as unknown as File); - expect(result).toBe(fileContent); - }); - }); - - describe('importSaga', () => { - it('should run the success path', () => { - const action = actions.startImport('users'); - const generator = sagas.importSaga(action); - - expect(generator.next().value).toEqual(put(actions.setImportInProgress('users', true))); - - // selectImport returns a new closure each call, so we verify the - // select effect's selector works correctly by calling it on test state - const selectStep = generator.next(); - const selectEffect = selectStep.value; - const testState = { imports: { users: { file: null } } }; - expect((selectEffect as any).payload.selector(testState)).toEqual(testState.imports.users); - - const state = { file: new File([''], 'test.csv') }; - expect(generator.next(state).value).toEqual(call(sagas.getString, state.file)); - - const csvString = 'name,email\nJohn,john@example.com'; - expect(generator.next(csvString).value).toEqual(call(sagas.doImport, 'users', csvString)); - - expect(generator.next().value).toEqual(put(actions.importSuccess('users'))); - - expect(generator.next().done).toEqual(true); - }); - - it('should put importFailure on error', () => { - const action = actions.startImport('users'); - const generator = sagas.importSaga(action); - - expect(generator.next().value).toEqual(put(actions.setImportInProgress('users', true))); - - // Advance past the select step - generator.next(); - - const state = { file: new File([''], 'test.csv') }; - expect(generator.next(state).value).toEqual(call(sagas.getString, state.file)); - - const error = new Error('File read error'); - expect(generator.throw(error).value).toEqual(put(actions.importFailure('users'))); - - expect(generator.next().done).toEqual(true); - }); - }); - }); - }); -}); diff --git a/src/modules/imports/sagas.ts b/src/modules/imports/sagas.ts deleted file mode 100644 index cf8138e6..00000000 --- a/src/modules/imports/sagas.ts +++ /dev/null @@ -1,47 +0,0 @@ -import {all, call, put, select, takeEvery} from 'redux-saga/effects'; -import * as actions from './actions'; -import importUsers from '../../util/importUsers'; -import {error} from '../../util/log'; - -export const selectImport = (importName: string) => (state: any) => state.imports[importName]; - -export function doImport(importName: string, csvString: string) { - switch (importName) { - case 'users': - return importUsers(csvString); - default: - throw new Error('Unknown import ' + importName); - } -} - -export function getString(file: File): Promise { - return new Promise((resolve, reject) => { - const reader = new FileReader(); - reader.onload = event => resolve((event.target as FileReader).result as string); - reader.onerror = err => reject(err); - reader.readAsText(file); - }); -} - -export function* importSaga(action: any) { - const importName = action.payload.importName; - try { - yield put(actions.setImportInProgress(importName, true)); - - const state = yield select(selectImport(importName)); - const csvString = yield call(getString, state.file); - - yield call(doImport, importName, csvString); - - yield put(actions.importSuccess(importName)); - } catch(e) { - error('Failed to import data (import: ' + importName + ')', e); - yield put(actions.importFailure(importName)); - } -} - -export default function* sagas() { - yield all([ - takeEvery(actions.START_IMPORT, importSaga), - ]) -} diff --git a/src/modules/index.ts b/src/modules/index.ts index e58290a5..671a047a 100644 --- a/src/modules/index.ts +++ b/src/modules/index.ts @@ -6,7 +6,6 @@ import aerodromes, {sagas as aerodromesSagas} from './aerodromes'; import aircrafts, {sagas as aircraftsSagas} from './aircrafts'; import auth, {sagas as authSagas} from './auth'; import customs, {sagas as customsSagas} from './customs'; -import imports, {sagas as importsSagas} from './imports'; import invoiceRecipients, {sagas as invoiceRecipientsSagas} from './invoiceRecipients'; import movements, {sagas as movementSagas} from './movements'; import settings, {sagas as settingsSagas} from './settings'; @@ -22,7 +21,6 @@ const createRootReducer = () => combineReducers({ aircrafts, auth, customs, - imports, invoiceRecipients, movements, settings, @@ -44,7 +42,6 @@ export const sagas = function* rootSaga() { aerodromesSagas, aircraftsSagas, authSagas, - importsSagas, invoiceRecipientsSagas, movementSagas, settingsSagas, diff --git a/src/util/importCsv.spec.ts b/src/util/importCsv.spec.ts deleted file mode 100644 index dd7c29ec..00000000 --- a/src/util/importCsv.spec.ts +++ /dev/null @@ -1,276 +0,0 @@ -jest.mock('./firebase'); -jest.mock('firebase/database', () => ({ - get: jest.fn(), - child: jest.fn(), - set: jest.fn(), - remove: jest.fn(), - push: jest.fn(), -})); - -import firebase from './firebase'; -import {get, child, set, remove, push} from 'firebase/database'; -import importCsv from './importCsv'; - -describe('util', () => { - describe('importCsv', () => { - let mockRef; - let childRef; - - beforeEach(() => { - jest.clearAllMocks(); - - mockRef = {}; - childRef = {}; - - (firebase as jest.Mock).mockReturnValue(mockRef); - (child as jest.Mock).mockReturnValue(childRef); - - // Default: empty snapshot (no existing firebase entries) - (get as jest.Mock).mockResolvedValue({forEach: () => {}, exists: () => false, val: () => null}); - (set as jest.Mock).mockResolvedValue(undefined); - (remove as jest.Mock).mockResolvedValue(undefined); - (push as jest.Mock).mockResolvedValue(undefined); - }); - - const baseOptions = { - path: '/users', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true }, - { csv: 'LastName', firebase: 'lastname' }, - { csv: 'FirstName', firebase: 'firstname' }, - ], - }; - - const csvString = - 'UserName,LastName,FirstName\n' + - '1001,Smith,John\n' + - '1002,Doe,Jane\n'; - - describe('basic flow', () => { - it('calls firebase with options.path', async () => { - await importCsv(csvString, baseOptions); - expect(firebase).toHaveBeenCalledWith('/users'); - }); - - it('resolves after processing', async () => { - await expect(importCsv(csvString, baseOptions)).resolves.toBeUndefined(); - }); - }); - - describe('updateExisting - removes items absent from CSV', () => { - it('removes firebase entries whose key is not in CSV', async () => { - // memberNr '9999' is not in the CSV (which has '1001' and '1002') - (get as jest.Mock).mockResolvedValue({ - forEach: fn => - fn({key: 'existingFirebaseKey', val: () => ({memberNr: '9999'})}), - }); - - await importCsv(csvString, baseOptions); - - expect(remove).toHaveBeenCalledWith(childRef); - }); - }); - - describe('updateExisting - sets matching items', () => { - it('sets firebase entries that are in CSV', async () => { - // memberNr '1001' is in the CSV - (get as jest.Mock).mockResolvedValue({ - forEach: fn => - fn({key: 'someFirebaseKey', val: () => ({memberNr: '1001'})}), - }); - - await importCsv(csvString, baseOptions); - - expect(set).toHaveBeenCalledWith( - childRef, - expect.objectContaining({ memberNr: '1001', lastname: 'Smith', firstname: 'John' }) - ); - }); - }); - - describe('addNew with isFirebaseKey', () => { - it('uses child(key).set when isFirebaseKey is true', async () => { - const options = { - path: '/users', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true, isFirebaseKey: true }, - { csv: 'LastName', firebase: 'lastname' }, - ], - }; - - const csv = 'UserName,LastName\n1001,Smith\n'; - await importCsv(csv, options); - - expect(child).toHaveBeenCalledWith(mockRef, '1001'); - expect(set).toHaveBeenCalled(); - }); - }); - - describe('addNew without isFirebaseKey', () => { - it('uses push when isFirebaseKey is false or absent', async () => { - const csv = 'UserName,LastName\n1001,Smith\n'; - const options = { - path: '/users', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true }, - { csv: 'LastName', firebase: 'lastname' }, - ], - }; - - await importCsv(csv, options); - - expect(push).toHaveBeenCalled(); - }); - }); - - describe('modifications', () => { - it('applies uppercase modification', async () => { - const csv = 'UserName,LastName\nabc123,smith\n'; - const options = { - path: '/users', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true, isFirebaseKey: true }, - { csv: 'LastName', firebase: 'lastname', modifications: ['uppercase'] }, - ], - }; - - await importCsv(csv, options); - - expect(set).toHaveBeenCalledWith( - childRef, - expect.objectContaining({ lastname: 'SMITH' }) - ); - }); - - it('applies lowercase modification', async () => { - const csv = 'UserName,Email\nabc123,TEST@EXAMPLE.COM\n'; - const options = { - path: '/users', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true, isFirebaseKey: true }, - { csv: 'Email', firebase: 'email', modifications: ['lowercase'] }, - ], - }; - - await importCsv(csv, options); - - expect(set).toHaveBeenCalledWith( - childRef, - expect.objectContaining({ email: 'test@example.com' }) - ); - }); - - it('applies parseint modification', async () => { - const csv = 'UserName,Age\nabc123,42\n'; - const options = { - path: '/users', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true, isFirebaseKey: true }, - { csv: 'Age', firebase: 'age', modifications: ['parseint'] }, - ], - }; - - await importCsv(csv, options); - - expect(set).toHaveBeenCalledWith( - childRef, - expect.objectContaining({ age: 42 }) - ); - }); - - it('throws on unsupported modification', async () => { - const csv = 'UserName,Name\nabc,foo\n'; - const options = { - path: '/users', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true, isFirebaseKey: true }, - { csv: 'Name', firebase: 'name', modifications: ['invalid'] }, - ], - }; - - await expect(importCsv(csv, options)).rejects.toThrow( - 'Unsupported modification "invalid"' - ); - }); - }); - - describe('missing required CSV column', () => { - it('throws when a required column header is missing', async () => { - const csv = 'WrongCol,LastName\n1001,Smith\n'; - - await expect(importCsv(csv, baseOptions)).rejects.toThrow( - 'Required column "UserName" not found in row' - ); - }); - }); - - describe('additionalEntriesPath', () => { - it('merges additional entries from firebase when snapshot exists', async () => { - const options = { - path: '/users', - additionalEntriesPath: '/settings/extra', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true }, - { csv: 'LastName', firebase: 'lastname' }, - ], - }; - - const csv = 'UserName,LastName\n1001,Smith\n'; - - // First get call: additionalEntriesPath snapshot (exists with extra entry) - // Second get call: updateExisting snapshot (empty) - (get as jest.Mock) - .mockResolvedValueOnce({ - exists: () => true, - val: () => ({extra1: {memberNr: 'extra', lastname: 'Extra'}}), - }) - .mockResolvedValue({forEach: () => {}}); - - await importCsv(csv, options); - - // Both CSV entry (1001) and additional entry (extra1) should be pushed - expect(push).toHaveBeenCalledTimes(2); - }); - - it('does not merge when additional entries snapshot does not exist', async () => { - const options = { - path: '/users', - additionalEntriesPath: '/settings/extra', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true }, - { csv: 'LastName', firebase: 'lastname' }, - ], - }; - - const csv = 'UserName,LastName\n1001,Smith\n'; - - // First get call: additionalEntriesPath snapshot (does not exist) - // Second get call: updateExisting snapshot (empty) - (get as jest.Mock) - .mockResolvedValueOnce({exists: () => false, val: () => null}) - .mockResolvedValue({forEach: () => {}}); - - await importCsv(csv, options); - - // Only the CSV entry should be pushed - expect(push).toHaveBeenCalledTimes(1); - }); - }); - - describe('column without firebase mapping', () => { - it('skips columns that have no firebase property', async () => { - const csv = 'UserName,LastName\n1001,Smith\n'; - const options = { - path: '/users', - columns: [ - { csv: 'UserName', isKey: true }, - { csv: 'LastName', firebase: 'lastname' }, - ], - }; - - await importCsv(csv, options); - expect(push).toHaveBeenCalled(); - }); - }); - }); -}); diff --git a/src/util/importCsv.ts b/src/util/importCsv.ts deleted file mode 100644 index 50ed20ce..00000000 --- a/src/util/importCsv.ts +++ /dev/null @@ -1,144 +0,0 @@ -import firebase from './firebase'; -import {get, child, set, remove, push} from 'firebase/database'; -import parseCsv from "./parseCsv"; - -function findIndex(row, name) { - const index = row.indexOf(name); - if (index === -1) { - throw new Error('Required column "' + name + '" not found in row ' + row); - } - return index; -} - -function findKey(columns) { - return columns.find(column => column.isKey === true); -} - -function modify(value, modification) { - switch (modification) { - case 'uppercase': - return value.toUpperCase(); - case 'lowercase': - return value.toLowerCase(); - case 'parseint': - return parseInt(value, 10); - default: - throw new Error('Unsupported modification "' + modification + '"'); - } -} - -function applyModifications(value, modifications) { - let modifiedValue = value; - - if (modifications) { - modifications.forEach(modification => { - modifiedValue = modify(value, modification); - }); - } - - return modifiedValue; -} - -async function getMap(array, options) { - let itemMap = {}; - - const firstRow = array.shift(); - - const indexes = {}; - options.columns.forEach(column => { - indexes[column.csv] = findIndex(firstRow, column.csv); - }); - - const keyColumn = findKey(options.columns); - - array.forEach(item => { - const data = {}; - - options.columns.forEach(column => { - if (column.firebase) { - let value = item[indexes[column.csv]]; - value = applyModifications(value, column.modifications); - data[column.firebase] = value; - } - }); - - const keyColumnIndex = indexes[keyColumn.csv]; - const itemKey = item[keyColumnIndex]; - - itemMap[itemKey] = data; - }); - - if (options.additionalEntriesPath) { - const snapshot = await get(firebase(options.additionalEntriesPath)); - if (snapshot.exists()) { - itemMap = { - ...itemMap, - ...snapshot.val() - } - } - } - - return itemMap; -} - -async function updateExisting(firebaseRef, itemMap, options) { - const snapshot = await get(firebaseRef); - const existing = {}; - const keyColumn = findKey(options.columns); - - snapshot.forEach(firebaseRow => { - const keyValue = firebaseRow.val()[keyColumn.firebase]; - const item = itemMap[keyValue]; - const childRef = child(firebaseRef, firebaseRow.key); - - if (!item) { - remove(childRef); - } else { - set(childRef, item); - } - - existing[keyValue] = true; - }); - - return existing; -} - -function addNew(firebaseRef, itemMap, existing, options) { - const keyColumn = findKey(options.columns); - - for (const key in itemMap) { - if (existing[key] !== true && itemMap.hasOwnProperty(key)) { - const item = itemMap[key]; - if (keyColumn.isFirebaseKey === true) { - set(child(firebaseRef, key), item); - } else { - push(firebaseRef, item); - } - } - } -} - -/** - * @param csvString - * @param options (marked with * is required) - * - path* {String} (i.e. '/users'), - * - additionalEntriesPath (i.e. '/settings/users/') - * - columns* {Array} - * example: - * columns: [ - * { csv: 'UserName', firebase: 'memberNr', isKey: true }, - * { csv: 'LastName', firebase: 'lastname' }, - * { csv: 'FirstName', firebase: 'firstname' }, - * { csv: 'PhoneMobile', firebase: 'phone' }, - * { csv: 'Email', firebase: 'email' }, - * ] - */ -async function importCsv(csvString, options) { - const data = await parseCsv(csvString); - const itemMap = await getMap(data, options); - const dbRef = firebase(options.path); - const existing = await updateExisting(dbRef, itemMap, options); - addNew(dbRef, itemMap, existing, options); -} - -export default importCsv; diff --git a/src/util/importUsers.ts b/src/util/importUsers.ts deleted file mode 100644 index 04b1b5d1..00000000 --- a/src/util/importUsers.ts +++ /dev/null @@ -1,17 +0,0 @@ -import importCsv from './importCsv'; - -function importUsers(csvString) { - const options = { - path: '/users', - columns: [ - { csv: 'UserName', firebase: 'memberNr', isKey: true }, - { csv: 'LastName', firebase: 'lastname' }, - { csv: 'FirstName', firebase: 'firstname' }, - { csv: 'PhoneMobile', firebase: 'phone' }, - { csv: 'Email', firebase: 'email' }, - ], - }; - return importCsv(csvString, options); -} - -export default importUsers; diff --git a/src/util/parseCsv.spec.ts b/src/util/parseCsv.spec.ts deleted file mode 100644 index 4a042d91..00000000 --- a/src/util/parseCsv.spec.ts +++ /dev/null @@ -1,20 +0,0 @@ -import parseCsv from './parseCsv'; - -const TEST_CSV_STRING = - 'UserName,LastName,FirstName,PhoneMobile,Email\n' + - '11069,Mustermann,Max,+41791234567,max@example.com\n' + - '11293,Musterfrau,Maria,+41768765432,maria@example.com\n'; - -describe('util', () => { - describe('parseCsv', () => { - it('should parse a CSV string', () => { - return parseCsv(TEST_CSV_STRING).then(data => { - expect(data).toEqual([ - ['UserName', 'LastName', 'FirstName', 'PhoneMobile', 'Email'], - ['11069', 'Mustermann', 'Max', '+41791234567', 'max@example.com'], - ['11293', 'Musterfrau', 'Maria', '+41768765432', 'maria@example.com'] - ]); - }); - }); - }); -}); diff --git a/src/util/parseCsv.ts b/src/util/parseCsv.ts deleted file mode 100644 index 7b4fc798..00000000 --- a/src/util/parseCsv.ts +++ /dev/null @@ -1,14 +0,0 @@ -import {parse} from 'csv-parse/browser/esm/sync'; - -function parseCsv(csvString) { - try { - const output = parse(csvString, { - skip_empty_lines: true, - }); - return Promise.resolve(output); - } catch (err) { - return Promise.reject(err); - } -} - -export default parseCsv; From 3a39e4f849ac859a79838d7886bfbb60a6b251f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 18:07:39 +0000 Subject: [PATCH 13/29] Escape branding values in sign-in email templates airportName and themeColor come from the public sign-in request body and were interpolated into the email HTML unescaped, allowing markup or CSS to be injected into a message sent from the trusted sender. Escape airportName for the HTML output and validate themeColor against a hex or CSS-keyword format (falling back to a default otherwise). Plain-text output keeps the raw name. --- functions/auth/emailTemplates.js | 47 ++++++++++++++++++++----- functions/auth/emailTemplates.spec.js | 50 +++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 8 deletions(-) diff --git a/functions/auth/emailTemplates.js b/functions/auth/emailTemplates.js index 2b23eb4c..469f736b 100644 --- a/functions/auth/emailTemplates.js +++ b/functions/auth/emailTemplates.js @@ -9,28 +9,59 @@ const replacePlaceholders = (content, replacements) => { }); }; +// airportName and themeColor come from the (public) request body and are +// interpolated into the sign-in email HTML, so they must be neutralized before +// insertion to prevent HTML/CSS injection (e.g. attacker-authored markup or +// phishing links in a mail sent from the trusted Flightbox sender). +const escapeHtml = value => + String(value) + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); + +// Accept only a hex colour (#rgb / #rrggbb / #rrggbbaa) or a plain CSS colour +// keyword; anything else falls back to a safe default so it cannot break out of +// the style attribute or inject CSS. +const SAFE_COLOR_REGEX = /^(#[0-9a-fA-F]{3,8}|[a-zA-Z]+)$/; +const DEFAULT_THEME_COLOR = '#000000'; + +const sanitizeThemeColor = color => + (typeof color === 'string' && SAFE_COLOR_REGEX.test(color)) ? color : DEFAULT_THEME_COLOR; + const readTemplate = (templateName, format) => { const templatePath = path.join(__dirname, 'templates', `${templateName}.${format}`); return fs.readFileSync(templatePath, 'utf8'); }; const getSignInEmailContent = ({ signInCode, airportName, themeColor, language }) => { - const replacements = { - signInCode, - airportName, - themeColor - }; - const templateName = language === 'en' ? 'signin_en' : 'signin'; const subject = language === 'en' ? 'Sign in to Flightbox' : 'Bei Flightbox anmelden'; const htmlTemplate = readTemplate(templateName, 'html'); const textTemplate = readTemplate(templateName, 'txt'); + const safeColor = sanitizeThemeColor(themeColor); + const name = airportName === undefined || airportName === null ? '' : airportName; + + // HTML output: escape the free-text name and use the validated colour. + const htmlReplacements = { + signInCode, + airportName: escapeHtml(name), + themeColor: safeColor + }; + // Plain-text output: no markup context, so raw text is fine. + const textReplacements = { + signInCode, + airportName: name, + themeColor: safeColor + }; + return { subject, - html: replacePlaceholders(htmlTemplate, replacements), - text: replacePlaceholders(textTemplate, replacements) + html: replacePlaceholders(htmlTemplate, htmlReplacements), + text: replacePlaceholders(textTemplate, textReplacements) }; }; diff --git a/functions/auth/emailTemplates.spec.js b/functions/auth/emailTemplates.spec.js index 8e3fd6be..85314748 100644 --- a/functions/auth/emailTemplates.spec.js +++ b/functions/auth/emailTemplates.spec.js @@ -72,6 +72,56 @@ describe('functions', () => { expect(result.html).not.toContain('{{themeColor}}'); }); + it('escapes HTML in airportName to prevent injection', () => { + const result = getSignInEmailContent({ + signInCode: '123456', + airportName: '', + themeColor: '#003863' + }); + expect(result.html).not.toContain('' + }); + expect(result.html).toContain('#000000'); + expect(result.html).not.toContain('