fix(pos-app): rename partner API key to customer API key#396
fix(pos-app): rename partner API key to customer API key#396ignaciosantise wants to merge 4 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @ignaciosantise's task in 3m 1s —— View job Code Review
Found 5 issue(s)Issue 1: URL query parameter renamed without fallbackID: brk-use-url-credentials-url-param-rename-b7c4 Context:
Recommendation: Add temporary fallback during transition: const rawCustomerApiKey = params.get("customerApiKey") ?? params.get("partnerApiKey");Issue 2: Env variable renamed without backward compatID: brk-env-example-partner-to-customer-env-var-a3f2 Context:
Recommendation: Add a dev-mode guard in Issue 3: Hardcoded key string instead of constant in migrationID: secure-storage-hardcoded-key-constant-f3a1 Context:
Recommendation: const NEW_KEY = KEYS.CUSTOMER_API_KEY; // was hardcoded "customer_api_key"Issue 4: API key transmitted via URL query parameterID: dcl-use-url-credentials-apikey-url-exposure-c2d9 Context:
Recommendation: Use a short-lived signed token or server-side session handoff instead of embedding the raw key in the URL. If URL delivery is required, use a one-time code that is exchanged server-side for the actual key. Issue 5: Error message text change breaks monitoring rulesID: brk-payment-error-message-text-rename-d1e8 Context:
Recommendation: Update any Sentry/monitoring alert rules that reference the old message text before or immediately after deploying. |
Review Response — All 5 issues discardedIssue 1 (URL param no fallback): Discarded — user explicitly chose a clean break during planning. No backward compatibility for the old Issue 2 (Env var no fallback): Discarded — fallback not needed in this case. Environments using the old var name will update as part of this rename. Issue 3 (Hardcoded key in migration): Discarded — intentional pattern. Migration functions should use hardcoded strings to remain stable regardless of future constant changes. If Issue 4 (API key in URL): Discarded — pre-existing design introduced in commit Issue 5 (Error message monitoring): Discarded — standard consequence of the rename with low risk. No monitoring rules reference the old message text. |
Adds window.postMessage as the primary method for receiving merchantId and customerApiKey on web, keeping URL query params as fallback. postMessage values are plain text (no base64 needed since data stays in JS memory). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR comprehensively renames "partner" API key terminology to "customer" across the POS app codebase, including UI labels, state management, storage keys, error messages, documentation, and tests. It also introduces postMessage as the primary credential injection method for web deployments (with URL parameters as fallback) and implements secure migration from legacy key names (merchant_api_key, partner_api_key) to the new customer_api_key.
Changes:
- Renamed all references to "partner" API key to "customer" across ~14 files including stores, services, hooks, UI components, and tests
- Added postMessage credential injection mechanism for web with fallback to URL query parameters
- Bumped settings store schema from v12 to v13 with migration logic to handle
isPartnerApiKeySet→isCustomerApiKeySettransition - Enhanced fresh-install cleanup to delete legacy storage keys (
merchant_api_key,partner_api_key) in addition to current keys - Incremented Android versionCode from 17 to 18
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
dapps/pos-app/utils/secure-storage.web.ts |
Updated key constants and migration logic to handle both legacy keys |
dapps/pos-app/utils/secure-storage.ts |
Added LEGACY_CUSTOMER_API_KEYS array, updated clearStaleSecureStorage and migration function |
dapps/pos-app/utils/merchant-config.ts |
Renamed env variable from PARTNER_API_KEY to CUSTOMER_API_KEY |
dapps/pos-app/store/useSettingsStore.ts |
Renamed state field and methods, bumped schema to v13, added migration logic |
dapps/pos-app/services/payment.web.ts |
Updated error messages and method calls to use customer terminology |
dapps/pos-app/services/payment.ts |
Updated variable names, JSDoc, and error messages |
dapps/pos-app/hooks/use-url-credentials.ts |
Added postMessage handler, renamed URL param from partnerApiKey to customerApiKey |
dapps/pos-app/hooks/use-merchant-flow.ts |
Renamed all partner-related state, methods, and variables to customer |
dapps/pos-app/app/settings.tsx |
Updated UI labels, state variables, and handlers to use customer terminology |
dapps/pos-app/app/index.tsx |
Updated state selector to use isCustomerApiKeySet |
dapps/pos-app/app.json |
Incremented Android versionCode to 18 |
dapps/pos-app/__tests__/utils/store-helpers.ts |
Updated test helpers to use customer_api_key |
dapps/pos-app/__tests__/utils/secure-storage.test.ts |
Added comprehensive tests for migration and cleanup logic |
dapps/pos-app/__tests__/store/useSettingsStore.test.ts |
Updated all test assertions and method calls |
dapps/pos-app/__tests__/services/payment.test.ts |
Updated error message assertions |
dapps/pos-app/__tests__/hooks/use-url-credentials.test.ts |
Added extensive postMessage tests, updated URL param tests |
dapps/pos-app/AGENTS.md |
Updated documentation to reference CUSTOMER_API_KEY |
dapps/pos-app/.env.example |
Renamed environment variable |
Comments suppressed due to low confidence (1)
dapps/pos-app/store/useSettingsStore.ts:344
- The migrateCustomerApiKey call lacks error handling. If the migration fails due to SecureStore errors, it could prevent the app from initializing properly since the exception would bubble up and potentially crash the rehydration process. Consider wrapping this in a try-catch block and logging the error, similar to how the pinHash migration or other operations handle errors.
// Run customer API key migration before applying defaults
// This ensures existing users keep their API key during the rename
const migrated = await migrateCustomerApiKey();
if (migrated) {
// Migration was performed, sync the flag
state.isCustomerApiKeySet = true;
useLogsStore
.getState()
.addLog(
"info",
"Customer API key migrated from legacy storage key",
"Settings",
"onRehydrateStorage",
);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function handleMessage(event: MessageEvent) { | ||
| if ( | ||
| !event.data || | ||
| typeof event.data !== "object" || | ||
| event.data.type !== "pos-credentials" | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const { merchantId, customerApiKey } = event.data; | ||
| if (!merchantId && !customerApiKey) return; | ||
|
|
||
| applyCredentials( | ||
| merchantId ?? null, | ||
| customerApiKey ?? null, | ||
| "postMessage", | ||
| ); | ||
| } |
There was a problem hiding this comment.
The postMessage handler lacks origin validation, which creates a serious security vulnerability. Any malicious website or iframe can send a message with type "pos-credentials" to inject arbitrary credentials into the POS app. This could enable unauthorized access or man-in-the-middle attacks.
The event.origin property should be validated against a whitelist of trusted origins before processing the message. Consider adding configuration for allowed origins (e.g., via environment variable) or implementing a secure handshake mechanism.
| * | ||
| * Values must be base64-encoded. | ||
| * 1. **postMessage** — parent/opener sends `{ type: "pos-credentials", merchantId?, customerApiKey? }` | ||
| * Values are plain text (no encoding needed). |
There was a problem hiding this comment.
The PR description's postMessage example shows using "*" as the target origin, which is a security anti-pattern. The example should demonstrate best practices by specifying the actual target origin. While this is documentation only and doesn't affect the code's runtime behavior, it could lead developers to implement insecure postMessage calls when integrating with the POS app.
| * Values are plain text (no encoding needed). | |
| * Values are plain text (no encoding needed). | |
| * Example from the parent/opener: | |
| * `posWindow.postMessage({ type: "pos-credentials", merchantId, customerApiKey }, "https://pos.example.com");` | |
| * Do **not** use `"*"` as the target origin when sending credentials; always specify | |
| * the exact origin of the POS app. |
Summary
partnerAPI key terminology tocustomeracross settings UI, hooks, store API, payment services, web URL params, env defaults, docs, and tests.isPartnerApiKeySettoisCustomerApiKeySetand secure-storage key migration from legacy keys tocustomer_api_key.merchant_api_key,partner_api_key) so stale credentials cannot be re-imported during migration.postMessageas the primary credential injection method on web, keeping URL query params as fallback.versionCodeindapps/pos-app/app.jsonfrom 17 to 18.postMessage Protocol
Parent/opener sends credentials to the POS app window via plain text (no base64 needed):
?merchantId=<base64>&customerApiKey=<base64>)Flow
flowchart TD A[App Rehydrate] --> B[clearStaleSecureStorage] B --> C[Delete customer_api_key + legacy keys] A --> D[migrateCustomerApiKey] D --> E{customer_api_key exists?} E -- No --> F[Copy first legacy key to customer_api_key] E -- Yes --> G[Keep existing customer_api_key] F --> H[Delete legacy keys] G --> HValidation
npm test -- --runInBand __tests__/hooks/use-url-credentials.test.ts __tests__/services/payment.test.ts __tests__/store/useSettingsStore.test.ts __tests__/utils/secure-storage.test.tsnpx tsc --noEmit