This differential encrypts notifications for sufficiently new web clients.
Details
Details
- Change FUTURE_CODE_VERSION flag to 0.
- Open web client.
- Add console.log() to log data received in service-worker and early return to skip this notification.
- Send web notification.
- Examine service-worker logs - ensure that encrypted payload is visible.
- Using mariadb console ensure that version field of olm_session associated with cookie_id of current web client increments with each notication.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
It looks correct, but I don't have enough knowledge of how notifs work so not accepting, to let others see
| keyserver/src/push/send.js | ||
|---|---|---|
| 1079–1081 ↗ | (On Diff #32461) | shouldn't we use hasMinCodeVersion? |
| 1289 ↗ | (On Diff #32461) | Let's keep it consistent after changing param name |
| lib/types/notif-types.js | ||
| 36 ↗ | (On Diff #32461) | why not string? |
| keyserver/src/push/send.js | ||
|---|---|---|
| 1079–1081 ↗ | (On Diff #32461) | Yeah, would be better to use hasMinCodeVersion. In some places in send.js we don't use it because we don't pass in the full platformDetails, but here it would be easiser. (It would probably be best to refactor to always use hasMinCodeVersion so it's easier to grep for code that can be deleted when we bump the minimum supported client in verifyClientSupported, but that's outside the scope of this diff) |
| lib/types/notif-types.js | ||
| 36 ↗ | (On Diff #32461) | Always better to be have more specific types |
| lib/types/notif-types.js | ||
|---|---|---|
| 36 ↗ | (On Diff #32461) | We do similar for Android notifications. This field is just supposed to indicate that the notifications should have been encrypted but the encryption process failed. That said it will never have different value than '1'. |