Device Linking 2610 - #3400
Conversation
…nagement - Added PendingDeviceAdmission type to track device admission requests. - Enhanced ConnectionsManagerService to handle device linking, including new linkDevice method. - Updated sigchain handling to prevent persisting pending device contexts. - Introduced integration tests for device linking over P2P. - Modified QSS service to skip authentication for pending devices. - Improved error handling and logging for community initialization and device linking. - Updated socket actions and types to support device linking.
- Added support for device link requests and acknowledgements in SocketService. - Created new sagas and actions for linking devices within the community. - Updated JoinCommunity component to handle device invitations without requiring username registration. - Enhanced LinkedDevices component to display device link information and manage visibility. - Implemented e2e tests for device linking scenarios, ensuring message replication across linked devices. - Refactored existing sagas and components to accommodate new device linking features.
…t buffering for auth sync frames in QSSAuthConnectionManager
…or invite-unaware peers
|
holmesworcester
left a comment
holmesworcester
left a comment
There was a problem hiding this comment.
Let's discuss this all again before merging from a security standpoint, but this is what I've got from looking at the code. No big comments.
| expect(runtime.startP2p).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('honors stored P2P ownership without starting QSS or claiming again', async () => { |
There was a problem hiding this comment.
Just noting that it's very awkward that we have to handle cases where users in a QSS community can stay as p2p-only. That's what this is for, right?
| validForMs: number = DEFAULT_DEVICE_INVITATION_VALID_FOR_MS, | ||
| seed?: string | ||
| ): DeviceLinkInvite { | ||
| const expiresAt = (Date.now() + validForMs) as UnixTimestamp |
There was a problem hiding this comment.
validForMs: 0: createUserInvite treats 0 as "never expires"; here 0 silently yields Date.now(), already expired.
Not sure if this matters but this section looked weird and this was an LLM nit.
|
|
||
| // Community | ||
| COMMUNITY_LAUNCH_FAILED = 'Could not launch community', | ||
| COMMUNITY_ALREADY_INITIALIZED = 'A community is already initialized', |
There was a problem hiding this comment.
Is this error user-facing? Is it log-facing?
| userId: string | ||
| userName: string | ||
| } | ||
|
|
There was a problem hiding this comment.
Can we deprecate these older invite formats yet?
| expect(launchCommunitySpy).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('purges an interrupted device link on startup when no admitted sigchain was persisted', async () => { |
There was a problem hiding this comment.
Are we sure this is the behavior we want?
In p2p mode on iOS, e.g., linking a device will be really unstable because it will keep dying before it connects. I think for p2p we want eventual success.
| return | ||
| } | ||
|
|
||
| const communities = (await this.localDbService.getCommunities()) ?? {} |
There was a problem hiding this comment.
one note is that joinCommunity above goes straight to erasePreviousCommunityArtifacts() with no equivalent check, though it has a frontend check.
Should we add a backend check for joinCommunity?
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const PEER_ID_PATTERN = '[a-zA-Z0-9]{52}' | ||
| const ONION_MULTIADDR_REGEX = new RegExp(`^/dns4/[a-z0-9]{56}\\.onion/tcp/(443|80)/ws/p2p/${PEER_ID_PATTERN}$`) | ||
| const LOCAL_MULTIADDR_REGEX = new RegExp( | ||
| `^/ip4/${LOCAL_HOST.replace(/\./g, '\\.')}/tcp/([1-9][0-9]{0,4})/ws/p2p/${PEER_ID_PATTERN}$` |
3 participants
Pull Request Checklist
(Optional) Mobile checklist
Please ensure you completed the following checks if you did any changes to the mobile package: