This differential implements a function that encrypts parts of iOS notification payload that are important to user's privacy
Details
Details
- Pull cookie of currently logged in user from MariaDB.
- Call this function in the keyserver code where iOS notification is constructed with hardcoded cookie id.
- Send notification to the physical iOS device. Log content of send notification and ensure relevant fields are encrypted.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
I can't tell where these functions will be used, so it's a little harder to review
| keyserver/src/push/crypto.js | ||
|---|---|---|
| 1 ↗ | (On Diff #26469) | Newline here |
| 38 ↗ | (On Diff #26469) | Can we save space and set this to 1? |
| 42 ↗ | (On Diff #26469) | We can remove the async keyword here |
| 46–48 ↗ | (On Diff #26469) | We can use a shorthand |
| 52 ↗ | (On Diff #26469) | Does encryptIOSNotification need to be exported? |
Comment Actions
- Validate every notification field before encryption attempt
- Encrypt more fields in notification payload
Comment Actions
One request inline; please address before landing, or feel free to re-request review if you disagree or if I'm missing something
| keyserver/src/push/crypto.js | ||
|---|---|---|
| 41 ↗ | (On Diff #26554) | Can we construct a new Notification rather than mutating the existing one? |
Comment Actions
I should have been more specific
| keyserver/src/push/crypto.js | ||
|---|---|---|
| 12 ↗ | (On Diff #26611) | Rather than cloning the existing notification, can you create a new empty one, and fill in its fields as necessary? |
Comment Actions
Return apn.Notifications object in case of encryption failure. Previous apprhac used '...' syntax, and it failed further since the returned object didn't have 'length()' method.
| keyserver/src/push/crypto.js | ||
|---|---|---|
| 12 ↗ | (On Diff #26651) | Let's construct a new apn.Notification instead of cloning the one we're passed in |