Use the keyserver keys to create the sessions.
Depends on D10666
Details
Details
- Reviewers
varun marcin inka kamil - Commits
- rCOMMa39e0e4ef7f3: [lib] Create notifs Olm session
Get access token, set it, and check if the session is created correctly... At this point it isn't because of invalid signature, probably due to prekey format - I'm investigating it.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- handlers (branched from palys-swm/handlers)
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/components/keyserver-connection-handler.js | ||
|---|---|---|
| 30–32 ↗ | (On Diff #35729) | Yes, we should do that |
| 63–65 ↗ | (On Diff #35729) | It's even better: after changes that are made earlier in the stack, the value is mandatory, so we don't need this check at all. But overall, the handling of missing keys should depend on the retry mechanism. |
Comment Actions
I am confused by this differential. I have two questions:
- What is the purpose of deleting and recreating notifications sessions at least every time app starts (session creation is not idempotent - it overwrites previous session)? Session is created on login and via socket if the keyserver happens to be missing session.
- In its current state this differential doesn't guarantee that session on the client is in sync with session on the keyserver - initialEncryptedMessage returned from session creator is not used. If this differential is intentionally breaking the app and you plan to fix it later in the stack then it should be explicitly stated in summary or in a comment.
Comment Actions
Currently, this code is gated behind usingCommServicesAccessToken flag, so it won't break the current logic.
- Ultimately, this code will execute as a part of login / retry logic, so it won't be the case that it is run on the app start. Thanks for the heads up about function not being idempotent!
- The session will be sent to a keyserver using a new auth endpoint - that will be implemented as the next part of https://linear.app/comm/project/implement-the-keyserver-connection-handlers-c95c61b19a15. This diff doesn't break the app, because this logic is hidden behind the flag.
We definitely should return something from this lambda since otherwise it is useless to assign it to variable.
If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.
This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.