feat(backend): store data to be transmitted to receiver in outgoing payments#3766
Conversation
✅ Deploy Preview for brilliant-pasca-3e80ec canceled.
|
🚀 Performance Test ResultsTest Configuration:
Test Metrics:
📜 Logs |
sanducb
left a comment
There was a problem hiding this comment.
Looks good! I only left a couple of questions.
| "Tenant ID of the outgoing payment." | ||
| tenantId: String | ||
| "Data to be transmitted to receiver." | ||
| dataToTransmit: String |
There was a problem hiding this comment.
Why do we need dataToTransmit here?
There was a problem hiding this comment.
I think we can indeed remove this from this resolver (I don't think the ASE needs to look it up necessarily).
| "Unique key to ensure duplicate or retried requests are processed only once. For more information, refer to [idempotency](https://rafiki.dev/apis/graphql/admin-api-overview/#idempotency)." | ||
| idempotencyKey: String! | ||
| "Data to be encrypted and sent to the receiver." | ||
| dataToTransmit: String |
There was a problem hiding this comment.
Even though dataToTransmit is explicit enough for ourselves to understand it, I think that integrators might benefit from having it named something like senderData or senderDataToTransmit because of "sender's data" being a common phrase in a "payments" context. WDYT? CC @mkurapov
There was a problem hiding this comment.
If "sender data" is an existing concept in payments, then I think senderData would work here as the field name.
There was a problem hiding this comment.
@sanducb good q & I see your point but in the spirit of keeping it more generic, dataToTransmit works better I think. Even though it will usually be the sender's data (or more specifically, the sending customer's data) that is sent, the ASE can choose to pass in some other payment metadata in there
| CARD_SERVICE_URL: 'http://cloud-nine-wallet-card-service:3007' | ||
| CARD_WEBHOOK_SERVICE_URL: 'http://cloud-nine-wallet-card-service:3007/webhook' | ||
| DB_ENCRYPTION_SECRET: 'zO9KogehJECHReHgQr+ZWGkmgOD4AYa4ksUxALSwgM8=' | ||
| DB_ENCRYPTION_IV: 'e9jyNk0CKajCgI93Ga2v23R/1wGZ2lO339QRaOFgxHM=' |
There was a problem hiding this comment.
Should we instead generated a unique IV every time the data is signed and store it together with the data? Otherwise, I think we would get the same ciphertext given the same input/ dataToTransmit
| "Tenant ID of the outgoing payment." | ||
| tenantId: String | ||
| "Data to be transmitted to receiver." | ||
| dataToTransmit: String |
There was a problem hiding this comment.
I think we can indeed remove this from this resolver (I don't think the ASE needs to look it up necessarily).
| state: OutgoingPaymentState.Sending, | ||
| dataToTransmit: | ||
| deps.config.dbEncryptionSecret && dataToTransmit | ||
| ? encryptDbData(dataToTransmit, deps.config.dbEncryptionSecret) |
There was a problem hiding this comment.
I actually quite like this, gives ASE option to encrypt if they choose to do so
| public getDataToTransmit(key?: string): string | null { | ||
| if (!this.dataToTransmit) return null | ||
| if (!key) return this.dataToTransmit | ||
| const { tag, cipherText, iv } = JSON.parse(this.dataToTransmit) |
There was a problem hiding this comment.
Key rotation might be a bit tricky here, since we could end up encrypting with one key, change the key, try to decrypt with the old one and fail. We can focus on this on a follow-up PR.
(maybe we allow to configure a list of keys instead?)
* Revert "feat(backend): store data to be transmitted to receiver in outgoing payments (#3766)" This reverts commit 4917c14. * Revert "feat(backend): add handlePartialPayment for incoming payments (#3772)" This reverts commit 5170538. * chore: drop extraneous migration * chore: revert grant spent amounts tests in outgoing payment service * chore: update grant spent amount migrations timestamps * chore: attempt to fix CVE-2026-23745 * chore: snooze trivy vulnerabilities for now * chore: update frontend packages, add overrides * chore: override lodash version for vulnerability * chore: snooze grype vulnerabilities * chore: Triggering build * chore: update docker files to node:20-alpine3.23 * chore: patch @apollo/server * chore: snooze CVE-2026-24842 * chore: unignore CVE-2025-15284 * chore: unignore CVE-2024-21538 * Revert "chore: unignore CVE-2024-21538" This reverts commit 5eb9206.
Changes proposed in this pull request
DB_ENCRYPTION_SECRETand theDB_ENCRYPTION_IV, to be used in the symmetric encryption of sensitive columns in the databasedataToTransmitfield to the outgoing payment modeldataToTransmitfield to thedepositOutgoingPaymentLiquidityGraphQL resolverContext
Fixes RAF-1182 and fixes RAF-1179.
Checklist
fixes #numberuser-docslabel (if necessary)