-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Team: make sure joined device has all team keys #108
Conversation
Without this, I was finding that a joined device only had access to the most recent generation of team keys. When the device would try to instantiate the team from storage, it couldn't decrypt the root link if that link was encrypted with an older generation. It's possible the joined device should hang out and wait for the `admit member` link or some other way of getting all the team keys, but this worked and I've been banging my head for several hours
This reverts commit 207339c.
This lets us save keys that we wouldn't be able to read from the team (e.g., rotated team keys before we became a member)
Consumers will need these to decrypt the emitted team if there's more than one generation of team keys
This will be multiple generations of team keys if we've joined after the team keys were rotated. If we don't save these keys, we'll be unable to decrypt the full team graph.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to cover this change with a test that fails without it - could use this test as a starting point:
auth/packages/auth-provider-automerge-repo/src/test/AuthProvider.test.ts
Lines 263 to 302 in e753f80
it('persists local context and team state', async () => { | |
const { | |
users: { alice, bob }, | |
teardown, | |
} = setup(['alice', 'bob']) | |
const aliceTeam = Auth.createTeam('team A', alice.context) | |
await alice.authProvider.addTeam(aliceTeam) | |
// Alice sends Bob an invitation | |
const { seed: bobInvite } = aliceTeam.inviteMember() | |
// Bob uses the invitation to join | |
await bob.authProvider.addInvitation({ | |
shareId: getShareId(aliceTeam), | |
invitationSeed: bobInvite, | |
}) | |
// they're able to authenticate and sync | |
await authenticated(alice, bob) | |
await synced(alice, bob) // ✅ | |
// Alice and Bob both close and reopen their apps | |
// reconnect via a new channel | |
const channel = new MessageChannel() | |
const { port1: aliceToBob, port2: bobToAlice } = channel | |
// instantiate new authProviders and repos using this channel | |
const alice2 = alice.restart([aliceToBob]) | |
const bob2 = bob.restart([bobToAlice]) | |
// they're able to authenticate and sync | |
await authenticated(alice2, bob2) | |
await synced(alice2, bob2) // ✅ | |
teardown() | |
}) | |
it('removes a share and deletes it from storage', async () => { |
We were handed some keys when joining the team, and there may be additional generations due to updates since then
This test fails on `main` because bob doesn't remember to save the gen0 team key
Thanks for the pointer on the test, it actually uncovered a bug on the other half of the conversation -- users who were present before the keys were rotated weren't saving the gen(N+1) keys and weren't able to reload the whole team from disk. |
I was finding that a joined device only had access to the most recent generation of team keys. When the device would try to instantiate the team from storage, it couldn't decrypt the root link if that link was encrypted with an older generation.
My first approach was to put the full team keys into lockboxes in the
ADD_DEVICE
link, which made sure thatteam.teamKeyring()
would return the full team keys. We discussed in slack and decided a better approach would be to have the device be in charge of storing the keyring separately.So, we emit the full keyring along with the team from the Connection, attach that keyring to the PrivateShare (separate from the team), and then always use the keyring from the private share.
I still don't know of an easy way to test this until member removal is added to xdev... the two demos in this repo both work just fine with removing members and then inviting more. So this can sit until https://github.com/DevResults/xdev/pull/140 merges