From b59a21066b461478a5281d0f6b86668993c8f5fc Mon Sep 17 00:00:00 2001 From: holmes Date: Thu, 10 Sep 2026 12:30:06 -0700 Subject: [PATCH 1/2] Disable removal and key rotation in protocol 4 --- docs/api.md | 3 + docs/protocol-4-removal-policy.md | 226 ++++++++++++++++ .../src/test/AuthProvider.test.ts | 7 +- packages/auth/src/connection/identity.ts | 6 +- packages/auth/src/connection/message.ts | 12 +- .../connection/test/authentication.test.ts | 6 +- .../auth/src/connection/test/identity.test.ts | 14 +- .../test/invitationAcceptance.test.ts | 8 +- .../connection/test/memberRoleGrant.test.ts | 6 +- .../connection/test/persistAdmission.test.ts | 3 +- .../connection/test/protocolMessages.test.ts | 48 +++- .../auth/src/connection/test/sync.test.ts | 51 ++-- .../test/validateInvitationAcceptance.test.ts | 24 +- .../validateInvitationAcceptance.ts | 2 +- packages/auth/src/team/Team.ts | 33 ++- packages/auth/src/team/disabledActions.ts | 16 ++ packages/auth/src/team/membershipResolver.ts | 3 + packages/auth/src/team/reducer.ts | 6 + .../team/selectors/test/visibleKeys.test.ts | 3 +- .../team/selectors/test/visibleScopes.test.ts | 3 +- .../auth/src/team/test/createTeam.test.ts | 3 +- .../deviceAdmissionConcurrencyDesign.test.ts | 26 +- .../src/team/test/deviceConcurrency.test.ts | 32 +-- .../src/team/test/deviceRegistration.test.ts | 9 +- packages/auth/src/team/test/devices.test.ts | 21 +- .../src/team/test/duplicateAdmission.test.ts | 3 +- .../team/test/forgedDeviceRemovalRace.test.ts | 9 +- .../src/team/test/forgedRegistration.test.ts | 6 +- .../auth/src/team/test/forgedRemoval.test.ts | 15 +- .../src/team/test/invitationRoleGrant.test.ts | 8 +- packages/auth/src/team/test/keys.test.ts | 11 +- .../keysetCommitmentAuthorization.test.ts | 3 +- .../lockboxAuthorizationHardening.test.ts | 15 +- .../team/test/lockboxChannelTakeover.test.ts | 56 ++-- .../test/lockboxPostRotationAdmission.test.ts | 3 +- .../test/lockboxRotationConcurrency.test.ts | 3 +- packages/auth/src/team/test/members.test.ts | 12 +- .../src/team/test/membershipResolver.test.ts | 18 +- .../src/team/test/removalRotationGate.test.ts | 256 ++++++++++++++++++ packages/auth/src/team/test/roles.test.ts | 12 +- .../sameGenerationLockboxReplacement.test.ts | 6 +- .../team/test/securityCandidates.poc.test.ts | 17 +- packages/auth/src/team/test/servers.test.ts | 16 +- 43 files changed, 843 insertions(+), 197 deletions(-) create mode 100644 docs/protocol-4-removal-policy.md create mode 100644 packages/auth/src/team/disabledActions.ts create mode 100644 packages/auth/src/team/test/removalRotationGate.test.ts diff --git a/docs/api.md b/docs/api.md index 593279293..f3db630cb 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1 +1,4 @@ # API + +Quiet protocol 4 disables member, role, device, and server removal and all key rotation. +See the [protocol policy and compatibility limits](protocol-4-removal-policy.md). diff --git a/docs/protocol-4-removal-policy.md b/docs/protocol-4-removal-policy.md new file mode 100644 index 000000000..9d9074ff5 --- /dev/null +++ b/docs/protocol-4-removal-policy.md @@ -0,0 +1,226 @@ +# Protocol 4: removal and rotation disabled + +This Quiet release has no membership revocation or key rotation. The fixed protocol policy +disables `REMOVE_MEMBER`, `REMOVE_MEMBER_ROLE`, `REMOVE_ROLE`, `REMOVE_DEVICE`, `REMOVE_SERVER`, +`CHANGE_MEMBER_KEYS`, `CHANGE_SERVER_KEYS`, and `ROTATE_KEYS`. No graph metadata, peer request, +or runtime configuration can enable them. + +Local dispatch refuses these actions. Key changes are refused before mutating the supplied +keyset. For received graphs, normal signature and authorization validation still runs; a valid +disabled action advances graph history without changing membership, devices, servers, roles, +lockboxes, or rotation requests. The resolver excludes disabled actions from concurrency rules +so a removal cannot suppress another member's valid work or an admission. Automatic deferred +rotations are also disabled, including requests from invalid-admission cleanup. + +Invitations, admission, additions, role grants, and encryption remain supported. Invalid +admission cleanup still rejects identities that were never validly registered; this policy +does not turn invalid admissions into members. The existing lockbox authorization rules still +reject replacement generations smuggled onto supported actions. + +Quiet deletes an entire channel through OrbitDB metadata and channel-store cleanup, without +calling auth's `REMOVE_ROLE`. `memberCanDeleteRole` therefore retains its existing admin and +role checks. `memberCanRemoveMembersFromRole` returns false. Disabling graph role removal does +not authorize ordinary members to delete channel metadata. + +## Compatibility + +`CONNECTION_PROTOCOL_VERSION` increases from 3 to 4. The ready message, signed identity proof, +and encrypted invitation acceptance require the current version, so an ordinary protocol 3 +peer cannot synchronize using removal-enabled semantics. Quiet clients and QSS must use the +same updated auth revision. This is a breaking protocol change, independent of the application +version number. + +This is **not a migration for existing protocol 3 communities**. Replaying a historical removal +under protocol 4 retains the previously removed membership. A graph containing descendants +encrypted under a now-ignored rotation may fail to load with its original keys. Providing old +rotated keys does not restore protocol 3 membership semantics. Use fresh communities or a +separately reviewed migration; this change does not supply one. + +For an incoming graph whose descendant requires a disabled rotation, `Team.merge` fails during +decryption before installing any graph or state. The receiver keeps its prior keys and can +continue merging valid updates. There is no promise of interoperability with a modified peer +that continues writing under forbidden generations, or of recovering legacy rotated history. + +## Test disposition + +`removalRotationGate.test.ts` covers all eight action types through local APIs, correctly signed +modified-peer graphs, live merge, object load, and serialized load. It also checks concurrent +role grants, suppressed deferred rotation, an aliased caller keyset, atomic rejection of an +encrypted descendant, continued updates, invitations, private encryption, and the retained +admin channel-deletion predicate. Device admission/concurrency and malformed rotation-carrier +tests now assert the inert-removal policy. Protocol tests reject the previous version before +graph exchange and reject a protocol 3 identity signature under protocol 4. + +The historical tests listed below require successful removal or rotation. They remain in the +source as explicit `it.skip` specifications for any future revocation protocol, with a policy +comment at each test. They do not establish revocation guarantees for this release. Tests for +supported signature, identity, invitation, and lockbox authorization continue to run. No test +or production switch bypasses the gate. + +85 existing test cases are explicitly retired by this change: + +### [connection/test/authentication.test.ts](../packages/auth/src/connection/test/authentication.test.ts) + +- doesn't connect with a member who has been removed +- won't re-admit a device that was removed, even with a fresh invitation + +### [connection/test/memberRoleGrant.test.ts](../packages/auth/src/connection/test/memberRoleGrant.test.ts) + +- rejects a stale member grant before a non-admin peer admits the invitee +- rejects a stale grant before the server admits the invitee + +### [connection/test/persistAdmission.test.ts](../packages/auth/src/connection/test/persistAdmission.test.ts) + +- refuses a claim whose identity has been removed + +### [connection/test/sync.test.ts](../packages/auth/src/connection/test/sync.test.ts) + +- resolves concurrent duplicate removals +- lets a member remove the founder +- resolves mutual demotions in favor of the senior member +- resolves mutual removals without invalidating the senior member's concurrent actions +- gets both sides of the story in the case of mutual removals +- when a member is demoted and makes concurrent admin-only changes, discards those changes +- when a member is demoted and concurrently adds a device, the new device is kept +- when an invitation is discarded, also discard related admittance actions +- resolves circular concurrent demotions +- Alice promotes Bob then demotes him +- rotates keys after a member is removed +- rotates keys after a member is demoted +- decrypts new links received following a key rotation (upon connecting) +- allows a new member to join after team keys have been rotated +- decrypts new links received following a key rotation (while connected) +- unwinds an invalidated admission +- Eve steals Bob's phone; Bob heals the team + +### [connection/test/validateInvitationAcceptance.test.ts](../packages/auth/src/connection/test/validateInvitationAcceptance.test.ts) + +- rejects a fresh envelope wrapping a pre-removal graph (member) +- rejects a fresh envelope wrapping a pre-removal graph (device) +- rejects a fresh envelope wrapping a pre-removal graph sent by a server +- rejects an older-handshake admission from an active sender when id and claim both match +- rejects an admission that the membership resolver has invalidated +- rejects a graph in which the invitee was admitted and then removed + +### [team/selectors/test/visibleKeys.test.ts](../packages/auth/src/team/selectors/test/visibleKeys.test.ts) + +- after rotating keys, can still see the same scopes + +### [team/selectors/test/visibleScopes.test.ts](../packages/auth/src/team/selectors/test/visibleScopes.test.ts) + +- after rotating keys, can still see the same scopes + +### [team/test/createTeam.test.ts](../packages/auth/src/team/test/createTeam.test.ts) + +- deserializes a team after key rotations + +### [team/test/deviceRegistration.test.ts](../packages/auth/src/team/test/deviceRegistration.test.ts) + +- records when a device was admitted and when it was removed +- won't register a device id that was removed +- rejects a link signed by a device that was removed + +### [team/test/devices.test.ts](../packages/auth/src/team/test/devices.test.ts) + +- Bob can remove Bob's device +- Alice can remove Bob's device +- deviceWasRemoved works as expected +- throws when trying to remove a removed device +- throws when trying to access a removed device +- doesn't throw when deliberately trying to access a removed device +- has an admin rotate shared keys after a non-admin removes a device + +### [team/test/duplicateAdmission.test.ts](../packages/auth/src/team/test/duplicateAdmission.test.ts) + +- keeps exactly one removal tombstone and a usable member selector after merging a duplicate + +### [team/test/forgedDeviceRemovalRace.test.ts](../packages/auth/src/team/test/forgedDeviceRemovalRace.test.ts) + +- discards what the stolen device does while it is being removed +- does not let the stolen device lock the owner out by removing his other device first +- discards what the stolen device does when its owner is concurrently removed + +### [team/test/forgedRegistration.test.ts](../packages/auth/src/team/test/forgedRegistration.test.ts) + +- rejects registering a device id that was removed +- rejects a device admitted against an invitation whose owner has left the team + +### [team/test/forgedRemoval.test.ts](../packages/auth/src/team/test/forgedRemoval.test.ts) + +- rejects links signed by a removed member’s device +- rejects links signed by a removed member’s device even when the device itself was left alone + +### [team/test/invitationRoleGrant.test.ts](../packages/auth/src/team/test/invitationRoleGrant.test.ts) + +- rejects a stale grant before assigning the role + +### [team/test/keys.test.ts](../packages/auth/src/team/test/keys.test.ts) + +- after changing his keys, Bob still has team keys +- has an admin rotate shared keys after a non-admin changes their USER keys +- Every time Alice changes her keys, the admin keys are rotated + +### [team/test/keysetCommitmentAuthorization.test.ts](../packages/auth/src/team/test/keysetCommitmentAuthorization.test.ts) + +- drops a conflicting historical-generation redistribution + +### [team/test/lockboxAuthorizationHardening.test.ts](../packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts) + +- does not carry a retired generic recipient into a subsequent role rotation +- does not let an unrelated member pre-seed a future USER generation +- selects the legitimate USER key change in both concurrent merge orders +- does not let an unrelated member pre-seed a future SERVER generation +- preserves an authenticated ROTATE_KEYS recipient transition + +### [team/test/lockboxChannelTakeover.test.ts](../packages/auth/src/team/test/lockboxChannelTakeover.test.ts) + +- still lets an admin rotate the team key when removing a member +- still lets an admin rotate the channel key when removing a member from it + +### [team/test/lockboxPostRotationAdmission.test.ts](../packages/auth/src/team/test/lockboxPostRotationAdmission.test.ts) + +- lockboxes every team-key generation to the joiner + +### [team/test/lockboxRotationConcurrency.test.ts](../packages/auth/src/team/test/lockboxRotationConcurrency.test.ts) + +- does not poison the graph when a role add lands before a concurrent re-key + +### [team/test/members.test.ts](../packages/auth/src/team/test/members.test.ts) + +- removes a member +- only admins can remove members +- rotates keys after removing a member +- doesn't do anything if asked to remove a nonexistent member + +### [team/test/membershipResolver.test.ts](../packages/auth/src/team/test/membershipResolver.test.ts) + +- discards changes made by a member who is concurrently removed +- discards changes made by a member who is concurrently demoted +- resolves mutual concurrent removals in favor of the team founder +- resolves mutual concurrent removals in favor of the senior member +- resolves mutual concurrent demotions in favor of the team founder +- resolves circular mutual concurrent demotions in favor of the team founder + +### [team/test/roles.test.ts](../packages/auth/src/team/test/roles.test.ts) + +- removes a member from a role +- removes a role +- Alice can remove herself as admin as long as there at least one other admin +- rotates keys when a member is removed from a role + +### [team/test/sameGenerationLockboxReplacement.test.ts](../packages/auth/src/team/test/sameGenerationLockboxReplacement.test.ts) + +- a removed role member cannot rebind the current generation to a key they control +- nor by lying in the manifest about which key the lockbox contains + +### [team/test/securityCandidates.poc.test.ts](../packages/auth/src/team/test/securityCandidates.poc.test.ts) + +- does not deliver the replacement role key to the principal removed by the rotation +- does not deliver replacement USER or TEAM keys to a fully removed member +- can leave the target of a losing concurrent member removal with the winning team key +- can fail to recover a descendant written under a losing concurrent team rotation + +### [team/test/servers.test.ts](../packages/auth/src/team/test/servers.test.ts) + +- can't be re-added after being removed +- has its keys rotated by an admin diff --git a/packages/auth-provider-automerge-repo/src/test/AuthProvider.test.ts b/packages/auth-provider-automerge-repo/src/test/AuthProvider.test.ts index 624249128..16bdc6aa6 100644 --- a/packages/auth-provider-automerge-repo/src/test/AuthProvider.test.ts +++ b/packages/auth-provider-automerge-repo/src/test/AuthProvider.test.ts @@ -287,8 +287,11 @@ describe('auth provider for automerge-repo', () => { await authenticated(alice, charlie) await synced(alice, charlie) // ✅ - // Alice boots charlie - aliceTeam.remove(charlie.user.userId) + // Protocol 4 retains membership and keys when removal is requested. Persistence must still + // support later invitations and reconnecting with this unchanged keyring. + expect(() => aliceTeam.remove(charlie.user.userId)).toThrow(/disabled/i) + expect(aliceTeam.has(charlie.user.userId)).toBe(true) + expect(aliceTeam.teamKeys().generation).toBe(0) // Alice sends Bob an invitation const { seed: bobInvite } = aliceTeam.inviteMember() diff --git a/packages/auth/src/connection/identity.ts b/packages/auth/src/connection/identity.ts index 0941c323c..471c13172 100644 --- a/packages/auth/src/connection/identity.ts +++ b/packages/auth/src/connection/identity.ts @@ -17,9 +17,9 @@ export const challenge = (identityClaim: KeyScope): Challenge => ({ }) /** - * Canonical v3 payload signed when proving an established peer's identity. Including the protocol - * version in the authenticated tuple prevents a relay from bridging a legacy REQUEST_IDENTITY - * shape into v3 while forwarding the legacy proof and then synchronizing incompatible replicas. + * Canonical payload signed when proving an established peer's identity. Including the protocol + * version in the authenticated tuple prevents a relay from translating REQUEST_IDENTITY while + * forwarding an old proof and then synchronizing replicas with different authorization rules. */ export const identityProofPayload = (challenge: Challenge): Payload => [ diff --git a/packages/auth/src/connection/message.ts b/packages/auth/src/connection/message.ts index b7f63df14..929c3748a 100644 --- a/packages/auth/src/connection/message.ts +++ b/packages/auth/src/connection/message.ts @@ -5,11 +5,11 @@ import type { InvitationKind } from 'invitation/index.js' import type { ErrorMessage, LocalErrorMessage } from './errors.js' /** - * Peers at this version authorize lockboxes by a commitment to the complete encrypted keyset. - * Negotiating this version before any identity or graph exchange prevents replicas with different - * manifest-authorization rules from synchronizing. + * Protocol 4 disables removals and key rotation, in addition to protocol 3's complete-keyset + * lockbox commitments. Negotiate before identity or graph exchange: a protocol 3 replica would + * apply actions that protocol 4 deliberately leaves inert. */ -export const CONNECTION_PROTOCOL_VERSION = 3 as const +export const CONNECTION_PROTOCOL_VERSION = 4 as const export type ReadyMessage = { type: 'REQUEST_IDENTITY' @@ -28,8 +28,8 @@ export type ReadyMessage = { * Runtime validation for the first protocol message, which arrives as untyped wire data. * * This is also the protocol-version negotiation. The payload is exact so an old implementation - * cannot ignore an unknown version field and continue under different manifest-authorization - * rules. Everything downstream — an invitee's proof of invitation above all — is bound to the + * cannot ignore an unknown version field and continue under different authorization rules. + * Everything downstream — an invitee's proof of invitation above all — is bound to the * identity nonce this message carries. */ export const isReadyMessage = (message: unknown): message is ReadyMessage => { diff --git a/packages/auth/src/connection/test/authentication.test.ts b/packages/auth/src/connection/test/authentication.test.ts index 14de1e3c4..dc769400a 100644 --- a/packages/auth/src/connection/test/authentication.test.ts +++ b/packages/auth/src/connection/test/authentication.test.ts @@ -31,7 +31,8 @@ describe('connection', () => { await disconnect(alice, bob) }) - it("doesn't connect with a member who has been removed", async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip("doesn't connect with a member who has been removed", async () => { const { alice, bob } = setup('alice', 'bob') // 👩🏾 Alice removes Bob @@ -266,7 +267,8 @@ describe('connection', () => { expect(phoneConnection.team!.hasDevice(phone.deviceId)).toBe(true) }) - it("won't re-admit a device that was removed, even with a fresh invitation", async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip("won't re-admit a device that was removed, even with a fresh invitation", async () => { const { alice, bob } = setup('alice', 'bob') await connect(alice, bob) diff --git a/packages/auth/src/connection/test/identity.test.ts b/packages/auth/src/connection/test/identity.test.ts index 92322e559..af4f7b5f1 100644 --- a/packages/auth/src/connection/test/identity.test.ts +++ b/packages/auth/src/connection/test/identity.test.ts @@ -51,7 +51,7 @@ describe('identity', () => { expect(validation).not.toBeValid() }) - it('rejects a legacy identity proof that does not commit to protocol v3', () => { + it('rejects a legacy identity proof that does not commit to the negotiated protocol', () => { const alicesChallenge = challenge({ type: USER, name: 'bob' }) const legacyProof = signatures.sign( alicesChallenge, @@ -62,6 +62,18 @@ describe('identity', () => { expect(verify(alicesChallenge, legacyProof, redactKeys(bob.user.keys))).not.toBeValid() }) + it('rejects a protocol 3 identity proof even if a relay rewrites its ready message', () => { + const currentChallenge = challenge({ type: USER, name: 'bob' }) + const { type, name, nonce, timestamp } = currentChallenge + const previousProof = signatures.sign( + [3, type, name, nonce, timestamp], + bob.user.keys.signature.secretKey, + IDENTITY_CHALLENGE + ) + + expect(verify(currentChallenge, previousProof, redactKeys(bob.user.keys))).not.toBeValid() + }) + it('rejects reused proof of identity', () => { const bobSecretKeys = bob.user.keys const bobPublicKeys = redactKeys(bob.user.keys) diff --git a/packages/auth/src/connection/test/invitationAcceptance.test.ts b/packages/auth/src/connection/test/invitationAcceptance.test.ts index 9686532c1..8e949931d 100644 --- a/packages/auth/src/connection/test/invitationAcceptance.test.ts +++ b/packages/auth/src/connection/test/invitationAcceptance.test.ts @@ -140,17 +140,17 @@ describe('invitation acceptance wire format', () => { } }) - // The outer payload must be *exactly* the v3 schema: no unknown extra fields (which could smuggle + // The outer payload must be *exactly* the v4 schema: no unknown extra fields (which could smuggle // data past the envelope), no missing fields, no other version. This pins the strictness of the // schema check, not every possible malformed payload. - it('rejects outer payloads that are not exactly the v3 schema', async () => { + it('rejects outer payloads that are not exactly the v4 schema', async () => { const fixture = await captureAcceptance() expect(() => openExpectedAcceptance({ ...fixture, payload: { ...fixture.payload, extra: true } }) ).toThrow() expect(() => - openExpectedAcceptance({ ...fixture, payload: { ...fixture.payload, version: 4 } }) + openExpectedAcceptance({ ...fixture, payload: { ...fixture.payload, version: 3 } }) ).toThrow() const { senderPublicKey: _, ...missingSenderKey } = fixture.payload expect(() => @@ -213,7 +213,7 @@ describe('invitation acceptance wire format', () => { }) const ACCEPTANCE_DOMAIN = 'localfirst-auth/invitation-acceptance' -const ACCEPTANCE_VERSION = 3 +const ACCEPTANCE_VERSION = 4 type ExpectedPayload = { version: number diff --git a/packages/auth/src/connection/test/memberRoleGrant.test.ts b/packages/auth/src/connection/test/memberRoleGrant.test.ts index d71b6adcb..e2e9036d8 100644 --- a/packages/auth/src/connection/test/memberRoleGrant.test.ts +++ b/packages/auth/src/connection/test/memberRoleGrant.test.ts @@ -151,7 +151,8 @@ describe('granting the member role on admission', () => { expect(bobConnection.team!.has(charlie.user.userId)).toBe(false) }) - it('rejects a stale member grant before a non-admin peer admits the invitee', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects a stale member grant before a non-admin peer admits the invitee', async () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) alice.team.dispatch({ type: 'SET_METADATA', @@ -184,7 +185,8 @@ describe('granting the member role on admission', () => { expect(bobConnection.team!.has(charlie.user.userId)).toBe(false) }) - it('rejects a stale grant before the server admits the invitee', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects a stale grant before the server admits the invitee', async () => { const alice = createTestUser('stale-alice') const bob = createTestUser('stale-bob') const team = teams.createTeam('stale-server-relayed-member-grant', alice, undefined, { diff --git a/packages/auth/src/connection/test/persistAdmission.test.ts b/packages/auth/src/connection/test/persistAdmission.test.ts index 507382597..e790d7bdc 100644 --- a/packages/auth/src/connection/test/persistAdmission.test.ts +++ b/packages/auth/src/connection/test/persistAdmission.test.ts @@ -535,7 +535,8 @@ describe('connection', () => { ).toBeUndefined() }) - it('refuses a claim whose identity has been removed', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('refuses a claim whose identity has been removed', () => { const { alice, charlie } = setup('alice', { user: 'charlie', member: false }) const { seed } = alice.team.inviteMember() const { proof, claim, possessionProof } = admission(seed, charlie) diff --git a/packages/auth/src/connection/test/protocolMessages.test.ts b/packages/auth/src/connection/test/protocolMessages.test.ts index f68ab9193..de7834330 100644 --- a/packages/auth/src/connection/test/protocolMessages.test.ts +++ b/packages/auth/src/connection/test/protocolMessages.test.ts @@ -21,7 +21,7 @@ import { describe, expect, it, vi } from 'vitest' import type { NumberedMessage } from '../MessageQueue.js' describe('connection protocol messages', () => { - it('accepts only the exact v3 REQUEST_IDENTITY payload', () => { + it('accepts only the exact v4 REQUEST_IDENTITY payload', () => { const identityNonce = randomKey() expect( isReadyMessage({ @@ -55,7 +55,7 @@ describe('connection protocol messages', () => { ).toBe(false) }) - it('connects peers that both negotiate v3', async () => { + it('connects peers that both negotiate v4', async () => { const { alice, bob } = setup('alice', 'bob') await expect(connect(alice, bob)).resolves.toBe(true) }) @@ -87,9 +87,15 @@ describe('connection protocol messages', () => { ['old nonce shape', { acceptorNonce: randomKey() }], ['missing version', { identityNonce: randomKey() }], ['missing identity nonce', { protocolVersion: CONNECTION_PROTOCOL_VERSION }], - ['old version', { protocolVersion: 2, identityNonce: randomKey() }], - ['future version', { protocolVersion: 4, identityNonce: randomKey() }], - ['unknown version', { protocolVersion: '3', identityNonce: randomKey() }], + ['previous removal-enabled version', { protocolVersion: 3, identityNonce: randomKey() }], + [ + 'future version', + { protocolVersion: CONNECTION_PROTOCOL_VERSION + 1, identityNonce: randomKey() }, + ], + [ + 'unknown version', + { protocolVersion: String(CONNECTION_PROTOCOL_VERSION), identityNonce: randomKey() }, + ], [ 'extra field', { protocolVersion: CONNECTION_PROTOCOL_VERSION, identityNonce: randomKey(), extra: true }, @@ -133,8 +139,17 @@ describe('connection protocol messages', () => { return rest }, ], - ['old version', (payload: Record) => ({ ...payload, version: 2 })], - ['future version', (payload: Record) => ({ ...payload, version: 4 })], + [ + 'previous removal-enabled version', + (payload: Record) => ({ ...payload, version: 3 }), + ], + [ + 'future version', + (payload: Record) => ({ + ...payload, + version: CONNECTION_PROTOCOL_VERSION + 1, + }), + ], ['extra field', (payload: Record) => ({ ...payload, extra: true })], ])('rejects an invitation acceptance with a %s before sync', async (_label, rewrite) => { const { alice, bob } = setup('alice', { user: 'bob', member: false }) @@ -177,8 +192,17 @@ describe('connection protocol messages', () => { return rest }, ], - ['old version', (envelope: Record) => ({ ...envelope, version: 2 })], - ['future version', (envelope: Record) => ({ ...envelope, version: 4 })], + [ + 'previous removal-enabled version', + (envelope: Record) => ({ ...envelope, version: 3 }), + ], + [ + 'future version', + (envelope: Record) => ({ + ...envelope, + version: CONNECTION_PROTOCOL_VERSION + 1, + }), + ], ['extra field', (envelope: Record) => ({ ...envelope, extra: true })], ])('rejects an encrypted invitation envelope with a %s before sync', async (_label, mutate) => { const { alice, bob } = setup('alice', { user: 'bob', member: false }) @@ -238,9 +262,9 @@ class RewriteAcceptanceChannel extends TestChannel { } /** - * Models a relay translating the legacy and v3 REQUEST_IDENTITY field names in both directions, + * Models a relay translating the legacy and current REQUEST_IDENTITY field names in both directions, * while one peer still produces the pre-v3 identity signature over the bare challenge. Syntax - * translation gets both requests through, but the authenticated v3 identity payload must not. + * translation gets both requests through, but the authenticated current identity payload must not. */ class LegacyRequestBridgeChannel extends TestChannel { sawSync = false @@ -259,7 +283,7 @@ class LegacyRequestBridgeChannel extends TestChannel { if (numbered.type === 'REQUEST_IDENTITY') { // The intermediate legacy representation is intentionally not delivered: the relay maps it - // straight back to the exact v3 payload understood by the current endpoint. + // straight back to the exact current payload understood by the current endpoint. const legacyRequest = { acceptorNonce: numbered.payload.identityNonce } const translated = { ...numbered, diff --git a/packages/auth/src/connection/test/sync.test.ts b/packages/auth/src/connection/test/sync.test.ts index a001f69c3..554b8d86b 100644 --- a/packages/auth/src/connection/test/sync.test.ts +++ b/packages/auth/src/connection/test/sync.test.ts @@ -346,7 +346,8 @@ describe('connection', () => { expectEveryoneToKnowEveryone(alice, charlie, bob, dwight) }) - it('resolves concurrent duplicate removals', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('resolves concurrent duplicate removals', async () => { const { alice, bob, charlie } = setup('alice', 'bob', 'charlie') // 👳🏽‍♂️ Charlie is a member @@ -371,7 +372,8 @@ describe('connection', () => { expect(bob.team.has(charlie.userId)).toBe(false) }) - it('lets a member remove the founder', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('lets a member remove the founder', async () => { const { alice, bob } = setup('alice', 'bob') // 👩🏾<->👨🏻‍🦲 Alice and Bob connect @@ -387,7 +389,8 @@ describe('connection', () => { expect(bob.team.has(alice.userId)).toBe(false) }) - it('resolves mutual demotions in favor of the senior member', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('resolves mutual demotions in favor of the senior member', async () => { const { alice, bob } = setup('alice', 'bob') await connect(alice, bob) @@ -423,7 +426,8 @@ describe('connection', () => { expect(bob.getState(alice.deviceId)).toEqual('connected') }) - it("resolves mutual removals without invalidating the senior member's concurrent actions", async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip("resolves mutual removals without invalidating the senior member's concurrent actions", async () => { const { alice, bob, charlie } = setup('alice', 'bob', 'charlie') // 👨🏻‍🦲 Bob removes 👩🏾 Alice @@ -449,7 +453,8 @@ describe('connection', () => { expect(charlie.team.hasRole('MANAGERS')).toBe(true) }) - it('gets both sides of the story in the case of mutual removals', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('gets both sides of the story in the case of mutual removals', async () => { const { alice, bob, charlie } = setup('alice', 'bob', 'charlie') // 👨🏻‍🦲 Bob removes 👩🏾 Alice @@ -479,7 +484,8 @@ describe('connection', () => { await disconnection(bob, charlie) }) - it('when a member is demoted and makes concurrent admin-only changes, discards those changes', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('when a member is demoted and makes concurrent admin-only changes, discards those changes', async () => { const { alice, bob, charlie } = setup('alice', 'bob', { user: 'charlie', admin: false, @@ -500,7 +506,8 @@ describe('connection', () => { expect(bob.team.memberHasRole(charlie.userId, ADMIN)).toBe(false) }) - it('when a member is demoted and concurrently adds a device, the new device is kept', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('when a member is demoted and concurrently adds a device, the new device is kept', async () => { const { alice, bob } = setup('alice', 'bob') // 👩🏾 Alice removes 👨🏻‍🦲 Bob from admin role @@ -528,7 +535,8 @@ describe('connection', () => { expect(alice.team.members(bob.userId).devices).toHaveLength(2) }) - it('when an invitation is discarded, also discard related admittance actions', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('when an invitation is discarded, also discard related admittance actions', async () => { const { alice, bob, charlie } = setup('alice', 'bob', { user: 'charlie', member: false, @@ -551,7 +559,8 @@ describe('connection', () => { expect(bob.team.has(charlie.userId)).toBe(false) }) - it('resolves circular concurrent demotions', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('resolves circular concurrent demotions', async () => { const { alice, bob, charlie, dwight } = setup('alice', 'bob', 'charlie', 'dwight') // Bob demotes Charlie @@ -582,7 +591,8 @@ describe('connection', () => { expect(isAdmin(charlie.userId)).toBe(true) }) - it('Alice promotes Bob then demotes him', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('Alice promotes Bob then demotes him', async () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) await connect(alice, bob) @@ -606,7 +616,8 @@ describe('connection', () => { expect(bob.team.memberIsAdmin(bob.userId)).toBe(false) }) - it('rotates keys after a member is removed', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rotates keys after a member is removed', async () => { const { alice, bob } = setup('alice', 'bob') await connect(alice, bob) @@ -628,7 +639,8 @@ describe('connection', () => { expect(alice.team.teamKeys().generation).toBe(1) }) - it('rotates keys after a member is demoted', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rotates keys after a member is demoted', async () => { const { alice, bob } = setup('alice', 'bob') await connect(alice, bob) @@ -654,7 +666,8 @@ describe('connection', () => { expect(alice.team.teamKeys().generation).toBe(0) }) - it('decrypts new links received following a key rotation (upon connecting)', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('decrypts new links received following a key rotation (upon connecting)', async () => { const { alice, bob, charlie } = setup('alice', 'bob', 'charlie') await connect(alice, bob) @@ -676,7 +689,8 @@ describe('connection', () => { expect(charlie.team.hasRole('managers')).toBe(true) }) - it('allows a new member to join after team keys have been rotated', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('allows a new member to join after team keys have been rotated', async () => { const { alice, bob, charlie } = setup(['alice', 'bob', { user: 'charlie', member: false }]) await connect(alice, bob) @@ -700,7 +714,8 @@ describe('connection', () => { expect(charlie.team.hasRole('managers')).toBe(true) }) - it('decrypts new links received following a key rotation (while connected)', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('decrypts new links received following a key rotation (while connected)', async () => { const { alice, bob, charlie } = setup('alice', 'bob', 'charlie') await connect(alice, bob) @@ -725,7 +740,8 @@ describe('connection', () => { expect(charlie.team.hasRole('managers')).toBe(true) }) - it('unwinds an invalidated admission', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('unwinds an invalidated admission', async () => { const { alice, bob, charlie } = setup('alice', 'bob', { user: 'charlie', member: false, @@ -758,7 +774,8 @@ describe('connection', () => { }) describe('post-compromise recovery', () => { - it("Eve steals Bob's phone; Bob heals the team", async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip("Eve steals Bob's phone; Bob heals the team", async () => { const { alice, bob, charlie } = setup('alice', 'bob', 'charlie') await connect(alice, bob) await connect(bob, charlie) diff --git a/packages/auth/src/connection/test/validateInvitationAcceptance.test.ts b/packages/auth/src/connection/test/validateInvitationAcceptance.test.ts index f2df742a4..a66a88890 100644 --- a/packages/auth/src/connection/test/validateInvitationAcceptance.test.ts +++ b/packages/auth/src/connection/test/validateInvitationAcceptance.test.ts @@ -91,10 +91,10 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { expect(device.outcome.kind).toBe('joined') }) - // The connection must send the welcome exactly once, and only in the encrypted v3 envelope. + // The connection must send the welcome exactly once, and only in the encrypted v4 envelope. // Counting on the wire guards against a duplicate send or a residual plaintext/legacy-format // path that an attacker could trigger as a downgrade. - it('sends exactly one welcome per handshake, always in the encrypted v3 envelope', async () => { + it('sends exactly one welcome per handshake, always in the encrypted v4 envelope', async () => { const { alice, bob } = setup('alice', { user: 'bob', member: false }) const { seed, teamId } = alice.team.inviteMember() const result = await connectInvitee({ @@ -103,7 +103,7 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { }) expect(result.channel.acceptanceCount).toBe(1) - expect(result.channel.acceptance).toHaveProperty('version', 3) + expect(result.channel.acceptance).toHaveProperty('version', 4) expect(result.channel.acceptance).toHaveProperty('encryptedAcceptance', expect.any(Uint8Array)) }) @@ -317,7 +317,8 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { * local write failed. A fresh invitee has no anti-rollback anchor, so no exception here can be * made safe; retry coherence lives on the admitting side instead. */ - it('rejects a fresh envelope wrapping a pre-removal graph (member)', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects a fresh envelope wrapping a pre-removal graph (member)', async () => { const { alice, bob } = setup('alice', { user: 'bob', member: false }) const { seed, teamId } = alice.team.inviteMember() @@ -343,7 +344,8 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { }) }) - it('rejects a fresh envelope wrapping a pre-removal graph (device)', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects a fresh envelope wrapping a pre-removal graph (device)', async () => { const { alice, bob } = setup('alice', 'bob') const { seed, teamId } = bob.team.inviteDevice() alice.team.merge(bob.team.graph) @@ -374,7 +376,8 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { // The sync server is the likeliest holder of a stale graph, and it signs welcomes with its own // server identity rather than a device, so the rule has to hold on that path too. - it('rejects a fresh envelope wrapping a pre-removal graph sent by a server', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects a fresh envelope wrapping a pre-removal graph sent by a server', async () => { const { alice, bob } = setup('alice', { user: 'bob', member: false }) const serverWithSecrets = createServer({ host: 'example.com', seed: 'example.com' }) alice.team.addServer(redactServer(serverWithSecrets)) @@ -408,7 +411,8 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { * the proof differs, because that admission was made in an earlier handshake. It must still be * rejected, or every other rule can be satisfied by an old graph. */ - it('rejects an older-handshake admission from an active sender when id and claim both match', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects an older-handshake admission from an active sender when id and claim both match', async () => { const { alice, bob } = setup('alice', { user: 'bob', member: false }) const { seed, teamId } = alice.team.inviteMember() @@ -531,7 +535,8 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { // Validation must consult what is effective after resolution, not what is present in the graph — // otherwise a demoted or removed admin could still usher people in. This is one representative // resolver interaction; the resolver's own ordering rules have their own suites. - it('rejects an admission that the membership resolver has invalidated', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects an admission that the membership resolver has invalidated', async () => { const { alice, bob, charlie } = setup('alice', 'bob', { user: 'charlie', member: false, @@ -563,7 +568,8 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { // only the final-state rule (the claimed identity must be uniquely active when the graph is // fully reduced) rejects. Without it, an invitee could "successfully join" a team they are // already off of. - it('rejects a graph in which the invitee was admitted and then removed', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects a graph in which the invitee was admitted and then removed', async () => { const { alice, bob } = setup('alice', { user: 'bob', member: false }) const { seed, teamId } = alice.team.inviteMember() const acceptorTeam = cloneTeam(alice.team, alice.connectionContext) diff --git a/packages/auth/src/connection/validateInvitationAcceptance.ts b/packages/auth/src/connection/validateInvitationAcceptance.ts index 3297deb1a..49fc063d0 100644 --- a/packages/auth/src/connection/validateInvitationAcceptance.ts +++ b/packages/auth/src/connection/validateInvitationAcceptance.ts @@ -25,7 +25,7 @@ import type { AcceptInvitationPayload, InvitationAcceptanceEnvelope } from './me * this session, as exactly the identity they claimed. The checks run in order, each mapped to a * failure reason the connection machine reports distinctly: * - * 1. the outer and inner schemas are exactly v3 — else PROTOCOL_VERSION_UNSUPPORTED + * 1. the outer and inner schemas match the current protocol — else PROTOCOL_VERSION_UNSUPPORTED * 2. the envelope opened and is bound to this handshake — else ACCEPTANCE_INVALID * 3. the graph's root hash is the expected team id — else WRONG_TEAM * 4. the graph validates and contains this invitation with the claimed kind — else WRONG_TEAM diff --git a/packages/auth/src/team/Team.ts b/packages/auth/src/team/Team.ts index 1a0c316e5..34fe08339 100644 --- a/packages/auth/src/team/Team.ts +++ b/packages/auth/src/team/Team.ts @@ -37,6 +37,7 @@ import { type LocalContext } from 'team/context.js' import { KeyType, scopesMatch } from 'util/index.js' import { ADMIN_SCOPE, ALL, TEAM_SCOPE, initialState } from './constants.js' import { decryptTeamGraph } from './decryptTeamGraph.js' +import { isDisabledAction } from './disabledActions.js' import { membershipResolver as resolver } from './membershipResolver.js' import { redactUser } from './redactUser.js' import { reducer } from './reducer.js' @@ -256,6 +257,7 @@ export class Team extends EventEmitter { /** Add a link to the graph, then recompute team state from the new graph */ public dispatch(action: TeamAction, teamKeys: KeysetWithSecrets = this.teamKeys()) { + assert(!isDisabledAction(action), `Removal and key rotation are disabled: ${action.type}`) this.store.dispatch(action, teamKeys) this.state = this.store.getState() this.emit('updated', { head: this.graph.head }) @@ -310,7 +312,7 @@ export class Team extends EventEmitter { }) } - /** Remove a member from the team */ + /** Member removal is disabled in protocol 4. */ public remove = (userId: string) => { // Create new keys & lockboxes for any keys this person had access to const lockboxes = this.rotateKeys({ type: USER, name: userId }, { excludeMemberId: userId }) @@ -393,7 +395,7 @@ export class Team extends EventEmitter { }) } - /** Remove a role from the team */ + /** Graph role removal is disabled in protocol 4. Quiet channel deletion uses OrbitDB metadata. */ public removeRole = (roleName: string) => { this._isRoleRemovable(roleName, true) @@ -461,7 +463,7 @@ export class Team extends EventEmitter { ) } - /** Remove a role from a member */ + /** Removing a member from a role is disabled in protocol 4. */ public removeMemberRole = (userId: string, roleName: string) => { if (roleName === ADMIN) { const adminCount = this.membersInRole(ADMIN).length @@ -505,8 +507,9 @@ export class Team extends EventEmitter { return canUserAddMemberToRole(roleName, memberId, undefined, this.state) } - /** Check if member has permissions to reevoke membership from a role */ + /** Role membership removal is unavailable in protocol 4, including for administrators. */ public memberCanRemoveMembersFromRole(roleName: string, memberId: string): boolean { + if (isDisabledAction({ type: 'REMOVE_MEMBER_ROLE' })) return false if (!this._memberHasPrivelegeToPerformAction(memberId, 'REMOVE_MEMBER_ROLE')) { return false } @@ -518,7 +521,7 @@ export class Team extends EventEmitter { return this._memberHasPrivelegeToPerformAction(memberId, 'ADD_ROLE') } - /** Check if member has permissions to delete a specific role */ + /** Authorization for Quiet's channel metadata deletion; does not enable graph REMOVE_ROLE. */ public memberCanDeleteRole(roleName: string, memberId: string): boolean { if (!this._memberHasPrivelegeToPerformAction(memberId, 'REMOVE_ROLE')) { return false @@ -540,7 +543,7 @@ export class Team extends EventEmitter { return select.device(this.state, deviceId, options) } - /** Remove a member's device */ + /** Device removal is disabled in protocol 4. */ public removeDevice = (deviceId: string) => { if (!this.hasDevice(deviceId)) throw new Error(`Device ${deviceId} not found`) @@ -949,7 +952,7 @@ export class Team extends EventEmitter { * `188.26.221.135`). * * The expected usage is for the application to add a server or servers immediately after the team - * is created. However, the application can add or remove servers at any time. + * is created. More servers can be added later; server removal is disabled in protocol 4. * * Just before adding a server, the application should send it the latest graph and the team keys * (so it can decrypt the team graph). No invitation or authentication is necessary in this phase, @@ -975,7 +978,7 @@ export class Team extends EventEmitter { }) } - /** Removes a server from the team. */ + /** Server removal is disabled in protocol 4. */ public removeServer = (serverId: string) => { const lockboxes = this.rotateKeys( { type: SERVER, name: serverId }, @@ -1162,8 +1165,7 @@ export class Team extends EventEmitter { public adminKeys = (generation?: number) => this.roleKeys(ADMIN, generation) /** - * Replaces a member's or a server's secret keyset with the one provided. (An admin can do this - * for someone else; anyone can do it for themselves.) + * Member and server key replacement is disabled in protocol 4. * * Only these two kinds of keys rotate. Device keys and a server's identity keys are what their * ids are fingerprints of, and what their past links were signed with — rotating them would @@ -1177,6 +1179,12 @@ export class Team extends EventEmitter { ) const isForServer = type === SERVER + const actionType = isForServer ? 'CHANGE_SERVER_KEYS' : 'CHANGE_MEMBER_KEYS' + // Refuse before mutating the supplied keyset, which may alias the caller's current keys. + assert( + !isDisabledAction({ type: actionType }), + `Removal and key rotation are disabled: ${actionType}` + ) const currentKeys = isForServer ? this.servers(name).keys : this.members(name).keys newKeys.generation = currentKeys.generation + 1 @@ -1185,7 +1193,7 @@ export class Team extends EventEmitter { // Post the new public keys to the graph this.dispatch({ - type: isForServer ? 'CHANGE_SERVER_KEYS' : 'CHANGE_MEMBER_KEYS', + type: actionType, payload: { keys: redactKeys(newKeys), lockboxes }, }) @@ -1217,6 +1225,9 @@ export class Team extends EventEmitter { } private checkForPendingKeyRotations() { + // Invalid admission cleanup can request rotations even without a REMOVE_* action. Do not + // let an update callback dispatch a disabled action after a successful graph mutation. + if (isDisabledAction({ type: 'ROTATE_KEYS' })) return // Only admins can rotate keys if (!this.memberIsAdmin(this.userId)) { return diff --git a/packages/auth/src/team/disabledActions.ts b/packages/auth/src/team/disabledActions.ts new file mode 100644 index 000000000..9a5130bfa --- /dev/null +++ b/packages/auth/src/team/disabledActions.ts @@ -0,0 +1,16 @@ +import type { TeamAction } from './types.js' + +/** Quiet has no removal or key-rotation protocol in this release. This is fixed local policy, + * never a graph-supplied capability or a switch a peer can enable. Channel deletion uses + * OrbitDB metadata, not REMOVE_ROLE; its admin permission predicate remains available. */ +export const isDisabledAction = ({ type }: Pick): boolean => + [ + 'REMOVE_MEMBER', + 'REMOVE_MEMBER_ROLE', + 'REMOVE_ROLE', + 'REMOVE_DEVICE', + 'REMOVE_SERVER', + 'CHANGE_MEMBER_KEYS', + 'CHANGE_SERVER_KEYS', + 'ROTATE_KEYS', + ].includes(type) diff --git a/packages/auth/src/team/membershipResolver.ts b/packages/auth/src/team/membershipResolver.ts index ef0278397..93dda55a5 100644 --- a/packages/auth/src/team/membershipResolver.ts +++ b/packages/auth/src/team/membershipResolver.ts @@ -25,6 +25,7 @@ import { } from 'team/types.js' import { arraysAreEqual } from 'util/arraysAreEqual.js' import { isAdminOnlyAction } from './isAdminOnlyAction.js' +import { isDisabledAction } from './disabledActions.js' /** * This is a custom resolver, used to flatten a graph of team membership operations into a strictly @@ -41,6 +42,8 @@ export const membershipResolver: Resolver = graph => { const bubbles = getConcurrentBubbles(graph).map(hashes => hashes.map(hash => graph.links[hash])) const invalidLinks: TeamLink[] = [] for (let bubble of bubbles) { + // Ignored removals must not invalidate their targets' concurrent work or admissions. + bubble = bubble.filter(link => !isDisabledAction(link.body)) for (const ruleName in membershipRules) { // Apply this rule to find any links that need to be invalidated const rule = membershipRules[ruleName] diff --git a/packages/auth/src/team/reducer.ts b/packages/auth/src/team/reducer.ts index 1046ef1f4..b3c9b65b6 100644 --- a/packages/auth/src/team/reducer.ts +++ b/packages/auth/src/team/reducer.ts @@ -3,6 +3,7 @@ import { Logger } from '@localfirst/shared' import { ADMIN } from 'role/index.js' import { clone, composeTransforms } from 'util/index.js' import { invalidLinkReducer } from './invalidLinkReducer.js' +import { isDisabledAction } from './disabledActions.js' import { setHead } from './setHead.js' import { addInvitedDevice, @@ -77,6 +78,11 @@ export const reducer: Reducer = ( throw validation.error } + // Preserve authenticated graph history without applying a disabled operation or its keys. + // Retaining its lockboxes would still allow a modified peer to force key rotation. A later + // link encrypted under such a rotation is rejected before Team.merge installs any graph. + if (isDisabledAction(link.body)) return setHead(link)(state) + // Apply the semantic state transition before collecting its lockbox deliveries. A delivery // policy can then compare the plan with both the state that authorized the action and the state // the action creates (especially important for removal and key-change transitions). diff --git a/packages/auth/src/team/selectors/test/visibleKeys.test.ts b/packages/auth/src/team/selectors/test/visibleKeys.test.ts index e696d649b..7e375317d 100644 --- a/packages/auth/src/team/selectors/test/visibleKeys.test.ts +++ b/packages/auth/src/team/selectors/test/visibleKeys.test.ts @@ -40,7 +40,8 @@ describe('visibleKeys', () => { expect(keysAdminSees.map(getScope)).toEqual([{ type: ROLE, name: 'MANAGERS' }]) }) - it('after rotating keys, can still see the same scopes', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('after rotating keys, can still see the same scopes', () => { const { alice } = setup('alice') const getUserKeys = () => select.visibleKeys(alice.team.state, alice.user.keys).map(getScope) diff --git a/packages/auth/src/team/selectors/test/visibleScopes.test.ts b/packages/auth/src/team/selectors/test/visibleScopes.test.ts index 479bddc75..1d7c829ab 100644 --- a/packages/auth/src/team/selectors/test/visibleScopes.test.ts +++ b/packages/auth/src/team/selectors/test/visibleScopes.test.ts @@ -52,7 +52,8 @@ describe('visibleScopes', () => { expect(adminScopes).toEqual([{ type: ROLE, name: 'MANAGERS' }]) }) - it('after rotating keys, can still see the same scopes', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('after rotating keys, can still see the same scopes', () => { const { alice } = setup('alice') const { type, name } = alice.user.keys diff --git a/packages/auth/src/team/test/createTeam.test.ts b/packages/auth/src/team/test/createTeam.test.ts index 4300f57c7..6969bca39 100644 --- a/packages/auth/src/team/test/createTeam.test.ts +++ b/packages/auth/src/team/test/createTeam.test.ts @@ -29,7 +29,8 @@ describe('Team', () => { expect(restoredTeam.teamName).toBe('Spies Я Us') }) - it('deserializes a team after key rotations', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('deserializes a team after key rotations', () => { const { alice, bob } = setup('alice', 'bob') // We start with generation 0 keys diff --git a/packages/auth/src/team/test/deviceAdmissionConcurrencyDesign.test.ts b/packages/auth/src/team/test/deviceAdmissionConcurrencyDesign.test.ts index 43ec0a005..d5730cf37 100644 --- a/packages/auth/src/team/test/deviceAdmissionConcurrencyDesign.test.ts +++ b/packages/auth/src/team/test/deviceAdmissionConcurrencyDesign.test.ts @@ -9,11 +9,11 @@ import { forge } from './forgeHelpers.js' import { deviceAdmission } from './helpers.js' /** - * These are executable design targets for concurrency cases the current reducer does not yet - * satisfy. They intentionally fail until their invariants are implemented. + * Protocol 4 ignores removals, including their effects on concurrent device admissions. These + * signed graph branches must preserve both existing registrations and new valid admissions. */ describe('device admission concurrency design targets', () => { - it('converges to one valid removal when two admins concurrently remove the same device', () => { + it('keeps the device when two admins concurrently request its removal', () => { const { alice, bob } = setup('alice', 'bob') const { seed } = bob.team.inviteDevice() bob.team.admitDevice(...deviceAdmission(seed, bob.phone!)) @@ -44,13 +44,13 @@ describe('device admission concurrency design targets', () => { alice.team.teamKeyring() ) - expect(loaded.hasDevice(bob.phone!.deviceId)).toBe(false) + expect(loaded.hasDevice(bob.phone!.deviceId)).toBe(true) expect( loaded.state.removedDevices.filter(device => device.deviceId === bob.phone!.deviceId) - ).toHaveLength(1) + ).toHaveLength(0) }) - it('lets member removal win over a concurrent admission for that member device', () => { + it('retains a device admission concurrent with a disabled member removal', () => { const { alice, bob } = setup('alice', 'bob') const { seed } = bob.team.inviteDevice() alice.team.merge(bob.team.graph) @@ -81,7 +81,7 @@ describe('device admission concurrency design targets', () => { teamKeys: alice.team.teamKeys(), }) - // Exercise the ordering that currently applies the removal before the invalid admission. + // Exercise removal before admission in the deterministic link ordering. if (removal.head[0] < admission.head[0]) { branches = { admission, removal } break @@ -97,11 +97,11 @@ describe('device admission concurrency design targets', () => { alice.team.teamKeyring() ) - expect(loaded.has(bob.userId)).toBe(false) - expect(loaded.hasDevice(bob.phone!.deviceId)).toBe(false) + expect(loaded.has(bob.userId)).toBe(true) + expect(loaded.hasDevice(bob.phone!.deviceId)).toBe(true) }) - it('retains the owner when a device admission is invalidated with its invitation', () => { + it('keeps an invitation and its device admission despite a concurrent device removal', () => { const { alice, bob } = setup('alice', 'bob') const { seed: phoneSeed } = bob.team.inviteDevice() bob.team.admitDevice(...deviceAdmission(phoneSeed, bob.phone!)) @@ -141,7 +141,9 @@ describe('device admission concurrency design targets', () => { device => device.deviceId === tablet.deviceId ) - expect(tombstone?.userId).toBe(bob.userId) - expect(loaded.state.pendingKeyRotations).toContain(bob.userId) + expect(tombstone).toBeUndefined() + expect(loaded.hasDevice(tablet.deviceId)).toBe(true) + expect(loaded.hasDevice(bob.deviceId)).toBe(true) + expect(loaded.state.pendingKeyRotations).toEqual([]) }) }) diff --git a/packages/auth/src/team/test/deviceConcurrency.test.ts b/packages/auth/src/team/test/deviceConcurrency.test.ts index 65fd160f1..f0e7adcb6 100644 --- a/packages/auth/src/team/test/deviceConcurrency.test.ts +++ b/packages/auth/src/team/test/deviceConcurrency.test.ts @@ -8,9 +8,8 @@ import { describe, expect, it } from 'vitest' import { deviceAdmission } from './helpers.js' /** - * When two devices act at the same time and one of them is being removed, someone has to lose. The - * resolver decides that the same way on every replica, from the graph alone — these tests pin down - * who loses and why. + * Protocol 4 keeps disabled removals inert during concurrency resolution. Registered devices + * retain their authority and their concurrent actions are not censored by a removal request. */ describe('membershipResolver', () => { describe('devices', () => { @@ -35,7 +34,7 @@ describe('membershipResolver', () => { const removeDevice = (deviceId: string) => ({ type: 'REMOVE_DEVICE', payload: { deviceId } }) as TeamAction - it('discards what a device does while it is concurrently being removed', () => { + it('retains device actions concurrent with a disabled removal', () => { const { alice, bob, phoneSigner } = setupWithPhone() const keys = alice.team.teamKeys() @@ -55,13 +54,13 @@ describe('membershipResolver', () => { keys, }) as TeamGraph - // ✅ Removal wins: the phone's role never happened + // The removal is inert and cannot censor the concurrent role addition. const merged = merge(aliceBranch, phoneBranch) - expect(graphSummary(merged)).not.toContain('ADD_ROLE:managers') + expect(graphSummary(merged)).toContain('ADD_ROLE:managers') expect(graphSummary(merged)).toContain('REMOVE_DEVICE') }) - it('resolves mutual device removals in favor of the senior device', () => { + it('keeps both devices after mutual removal requests', () => { const { alice, bob, phoneSigner } = setupWithPhone() const keys = alice.team.teamKeys() @@ -81,16 +80,16 @@ describe('membershipResolver', () => { keys, }) as TeamGraph - // ✅ The laptop was registered first, so it wins and the phone goes + // Neither removal applies, regardless of device seniority. const merged = merge(laptopBranch, phoneBranch) as TeamGraph const mergedTeam = teams.load(merged, bob.localContext, bob.team.teamKeyring()) expect(mergedTeam.hasDevice(bob.deviceId)).toBe(true) - expect(mergedTeam.hasDevice(bob.phone!.deviceId)).toBe(false) - expect(mergedTeam.deviceWasRemoved(bob.phone!.deviceId)).toBe(true) + expect(mergedTeam.hasDevice(bob.phone!.deviceId)).toBe(true) + expect(mergedTeam.deviceWasRemoved(bob.phone!.deviceId)).toBe(false) }) - it("discards what any of a member's devices do while the member is concurrently removed", () => { + it("retains all devices' actions concurrent with a disabled member removal", () => { const { alice, bob, phoneSigner } = setupWithPhone() const keys = alice.team.teamKeys() @@ -110,13 +109,12 @@ describe('membershipResolver', () => { keys, }) as TeamGraph - // ✅ Removing the member reaches every device they sign with, because a link is attributed to - // the member who registered its signer + // Disabled member removal cannot invalidate any registered device’s actions. const merged = merge(aliceBranch, phoneBranch) - expect(graphSummary(merged)).not.toContain('ADD_ROLE:managers') + expect(graphSummary(merged)).toContain('ADD_ROLE:managers') }) - it('lets a device retire itself even when it is the only removal in flight', () => { + it('ignores self-removal while retaining unrelated concurrent work', () => { const { alice, bob, phoneSigner } = setupWithPhone() const keys = alice.team.teamKeys() @@ -136,12 +134,12 @@ describe('membershipResolver', () => { keys, }) as TeamGraph - // ✅ A device retiring itself is the removal, not something overridden by one + // Self-removal is also disabled; the unrelated role still exists. const merged = merge(aliceBranch, phoneBranch) as TeamGraph expect(graphSummary(merged)).toContain('REMOVE_DEVICE') const mergedTeam = teams.load(merged, bob.localContext, bob.team.teamKeyring()) - expect(mergedTeam.hasDevice(bob.phone!.deviceId)).toBe(false) + expect(mergedTeam.hasDevice(bob.phone!.deviceId)).toBe(true) }) }) }) diff --git a/packages/auth/src/team/test/deviceRegistration.test.ts b/packages/auth/src/team/test/deviceRegistration.test.ts index 8fc2f595a..bde8c47be 100644 --- a/packages/auth/src/team/test/deviceRegistration.test.ts +++ b/packages/auth/src/team/test/deviceRegistration.test.ts @@ -25,7 +25,8 @@ describe('Team', () => { expect(Object.keys(alice.team.graph.links)).toHaveLength(1) }) - it('records when a device was admitted and when it was removed', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('records when a device was admitted and when it was removed', () => { const { alice, bob } = setup('alice', 'bob') const [bobsLaptop] = alice.team.members(bob.userId).devices! @@ -51,7 +52,8 @@ describe('Team', () => { expect(admitTwice).toThrow(/already in use/) }) - it("won't register a device id that was removed", () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip("won't register a device id that was removed", () => { const { bob } = setup('alice', 'bob') // 👨🏻‍🦲 Bob admits his phone, then it's stolen and 👩🏾 Alice removes it @@ -100,7 +102,8 @@ describe('Team', () => { expect(load).toThrow(/not registered/) }) - it('rejects a link signed by a device that was removed', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects a link signed by a device that was removed', () => { const { alice, bob } = setup('alice', 'bob') // 👩🏾 Alice removes 👨🏻‍🦲 Bob's laptop diff --git a/packages/auth/src/team/test/devices.test.ts b/packages/auth/src/team/test/devices.test.ts index 6b7b872bf..8a3be1926 100644 --- a/packages/auth/src/team/test/devices.test.ts +++ b/packages/auth/src/team/test/devices.test.ts @@ -20,13 +20,15 @@ describe('Team', () => { expect(alice.team.members(bob.userId).devices).toHaveLength(1) }) - it(`Bob can remove Bob's device`, () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip(`Bob can remove Bob's device`, () => { const { bob } = setup() bob.team.removeDevice(bob.device.deviceId) expect(bob.team.members(bob.userId)?.devices ?? []).toHaveLength(0) }) - it("Alice can remove Bob's device", () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip("Alice can remove Bob's device", () => { const { alice, bob } = setup() alice.team.removeDevice(bob.device.deviceId) expect(alice.team.members(bob.userId).devices).toHaveLength(0) @@ -42,7 +44,8 @@ describe('Team', () => { expect(tryToRemoveDevice).toThrowError() }) - it('deviceWasRemoved works as expected', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('deviceWasRemoved works as expected', () => { const { alice, bob } = setup() alice.team.removeDevice(bob.device.deviceId) expect(alice.team.deviceWasRemoved(alice.device.deviceId)).toBe(false) // Device still exists @@ -50,7 +53,8 @@ describe('Team', () => { expect(alice.team.deviceWasRemoved(bob.phone!.deviceId)).toBe(false) // Device never existed }) - it('throws when trying to remove a removed device', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('throws when trying to remove a removed device', () => { const { alice, bob } = setup() const bobDevice = alice.team.members(bob.userId).devices![0].deviceId alice.team.removeDevice(bobDevice) @@ -67,7 +71,8 @@ describe('Team', () => { expect(remove).toThrow() }) - it('throws when trying to access a removed device', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('throws when trying to access a removed device', () => { const { alice, bob } = setup() const bobDevice = alice.team.members(bob.userId).devices![0].deviceId alice.team.removeDevice(bobDevice) @@ -76,7 +81,8 @@ describe('Team', () => { expect(getDevice).toThrow() }) - it("doesn't throw when deliberately trying to access a removed device", () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip("doesn't throw when deliberately trying to access a removed device", () => { const { alice, bob } = setup() const bobDevice = alice.team.members(bob.userId).devices![0].deviceId alice.team.removeDevice(bobDevice) @@ -106,7 +112,8 @@ describe('Team', () => { expect(getDevice).toThrow() }) - it('has an admin rotate shared keys after a non-admin removes a device', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('has an admin rotate shared keys after a non-admin removes a device', () => { const { alice, bob } = setup() // Keys have never been rotated diff --git a/packages/auth/src/team/test/duplicateAdmission.test.ts b/packages/auth/src/team/test/duplicateAdmission.test.ts index b8cb16116..4c0036d2e 100644 --- a/packages/auth/src/team/test/duplicateAdmission.test.ts +++ b/packages/auth/src/team/test/duplicateAdmission.test.ts @@ -74,7 +74,8 @@ describe('Team', () => { }) // Design target: keep this red until duplicate tombstone cleanup is idempotent. - it('keeps exactly one removal tombstone and a usable member selector after merging a duplicate', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('keeps exactly one removal tombstone and a usable member selector after merging a duplicate', () => { const { alice, bob, charlie } = setup('alice', 'bob', { user: 'charlie', member: false }) const { seed } = alice.team.inviteMember() diff --git a/packages/auth/src/team/test/forgedDeviceRemovalRace.test.ts b/packages/auth/src/team/test/forgedDeviceRemovalRace.test.ts index d652bdc64..44c159ce5 100644 --- a/packages/auth/src/team/test/forgedDeviceRemovalRace.test.ts +++ b/packages/auth/src/team/test/forgedDeviceRemovalRace.test.ts @@ -37,7 +37,8 @@ describe('a removed device racing its removal', () => { return { alice, bob, thief: deviceSigner(bob.phone!), keys: alice.team.teamKeys() } } - it('discards what the stolen device does while it is being removed', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('discards what the stolen device does while it is being removed', () => { const { alice, bob, thief, keys } = stolenPhone() const shared = clone(alice.team.graph) @@ -69,7 +70,8 @@ describe('a removed device racing its removal', () => { expect(loaded.deviceWasRemoved(bob.phone!.deviceId)).toBe(true) }) - it('does not let the stolen device lock the owner out by removing his other device first', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('does not let the stolen device lock the owner out by removing his other device first', () => { const { alice, bob, thief, keys } = stolenPhone() const shared = clone(alice.team.graph) @@ -99,7 +101,8 @@ describe('a removed device racing its removal', () => { expect(loaded.deviceWasRemoved(bob.phone!.deviceId)).toBe(true) }) - it('discards what the stolen device does when its owner is concurrently removed', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('discards what the stolen device does when its owner is concurrently removed', () => { const { alice, bob, thief, keys } = stolenPhone() const shared = clone(alice.team.graph) diff --git a/packages/auth/src/team/test/forgedRegistration.test.ts b/packages/auth/src/team/test/forgedRegistration.test.ts index ef21ad5cc..f686a2532 100644 --- a/packages/auth/src/team/test/forgedRegistration.test.ts +++ b/packages/auth/src/team/test/forgedRegistration.test.ts @@ -57,7 +57,8 @@ describe('forged registration', () => { }) }) - it('rejects registering a device id that was removed', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects registering a device id that was removed', () => { const { alice, bob } = setup('alice', 'bob') const removed = { ...alice.team.device(bob.deviceId) } alice.team.removeDevice(bob.deviceId) @@ -199,7 +200,8 @@ describe('forged registration', () => { expect(alice.team.hasDevice(eve.deviceId)).toBe(false) }) - it('rejects a device admitted against an invitation whose owner has left the team', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects a device admitted against an invitation whose owner has left the team', () => { const { alice, bob } = setup('alice', 'bob') // 👨🏻‍🦲 Bob invites his phone and is then removed from the team. A device invitation is the diff --git a/packages/auth/src/team/test/forgedRemoval.test.ts b/packages/auth/src/team/test/forgedRemoval.test.ts index 13e1d2969..40f91025e 100644 --- a/packages/auth/src/team/test/forgedRemoval.test.ts +++ b/packages/auth/src/team/test/forgedRemoval.test.ts @@ -73,7 +73,8 @@ describe('forged removal', () => { expect(charlie.team.memberWasRemoved(alice.userId)).toBe(false) }) - it('rejects links signed by a removed member’s device', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects links signed by a removed member’s device', () => { const { alice, bob } = setup('alice', 'bob') // 👩🏾 Alice removes 👨🏻‍🦲 Bob... @@ -99,7 +100,8 @@ describe('forged removal', () => { }) }) - it('rejects links signed by a removed member’s device even when the device itself was left alone', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects links signed by a removed member’s device even when the device itself was left alone', () => { const { alice, bob } = setup('alice', 'bob') // Only the member is removed this time; his device record is untouched and still carries a @@ -121,7 +123,7 @@ describe('forged removal', () => { }) }) - it('CONTROL: an admin’s own device can remove a member, and the result survives a round trip', () => { + it('CONTROL: a signed admin removal is inert on merge and round trip', () => { const { alice, bob, charlie } = setup('alice', 'bob', 'charlie') const honest = forge({ @@ -132,14 +134,15 @@ describe('forged removal', () => { }) charlie.team.merge(honest) - expect(charlie.team.has(bob.userId)).toBe(false) - expect(charlie.team.memberWasRemoved(bob.userId)).toBe(true) + expect(charlie.team.has(bob.userId)).toBe(true) + expect(charlie.team.memberWasRemoved(bob.userId)).toBe(false) const reloaded = teams.load( serializeTeamGraph(honest), charlie.localContext, alice.team.teamKeyring() ) - expect(reloaded.memberWasRemoved(bob.userId)).toBe(true) + expect(reloaded.has(bob.userId)).toBe(true) + expect(reloaded.memberWasRemoved(bob.userId)).toBe(false) }) }) diff --git a/packages/auth/src/team/test/invitationRoleGrant.test.ts b/packages/auth/src/team/test/invitationRoleGrant.test.ts index 6d2ae6a75..5f52128ff 100644 --- a/packages/auth/src/team/test/invitationRoleGrant.test.ts +++ b/packages/auth/src/team/test/invitationRoleGrant.test.ts @@ -10,12 +10,11 @@ import { describe, expect, it } from 'vitest' const MEMBER = 'MEMBER' describe('invitation role grants', () => { - it('lets an invitation holder self-assign the declared role with every key generation', () => { + it('lets an invitation holder self-assign the declared role with its current key generation', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) alice.team.addRole(MEMBER) alice.team.addMemberRole(alice.userId, MEMBER) - alice.team.removeMemberRole(alice.userId, MEMBER) - alice.team.addMemberRole(alice.userId, MEMBER) + expect(alice.team.roleKeys(MEMBER).generation).toBe(0) const { seed } = alice.team.inviteMember({ roleNames: [MEMBER] }) const acceptanceKeys = invitation.generateStarterKeys(seed) @@ -44,7 +43,8 @@ describe('invitation role grants', () => { ) }) - it('rejects a stale grant before assigning the role', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rejects a stale grant before assigning the role', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) alice.team.addRole(MEMBER) alice.team.addMemberRole(alice.userId, MEMBER) diff --git a/packages/auth/src/team/test/keys.test.ts b/packages/auth/src/team/test/keys.test.ts index 99037aafe..1f93d10f0 100644 --- a/packages/auth/src/team/test/keys.test.ts +++ b/packages/auth/src/team/test/keys.test.ts @@ -43,7 +43,8 @@ describe('Team', () => { expect(adminKeys).toLookLikeKeyset() }) - it('after changing his keys, Bob still has team keys', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('after changing his keys, Bob still has team keys', () => { const { bob } = setup('alice', 'bob') // Bob has team keys @@ -61,7 +62,8 @@ describe('Team', () => { expect(teamKeys2.generation).toBe(1) // The team keys were rotated, so these are new }) - it('has an admin rotate shared keys after a non-admin changes their USER keys', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('has an admin rotate shared keys after a non-admin changes their USER keys', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) bob.team.changeKeys(createKeyset({ type: USER, name: bob.userId })) @@ -85,7 +87,7 @@ describe('Team', () => { alice.team.changeKeys(newKeys) } - expect(tryToChangeBobsKeys).toThrow("Can't change another user's keys.") + expect(tryToChangeBobsKeys).toThrow(/removal and key rotation are disabled/i) expect(alice.team.members(bob.userId).keys.generation).toBe(0) expect(bob.team.members(bob.userId).keys.generation).toBe(0) @@ -100,7 +102,8 @@ describe('Team', () => { expect(bob.team.verify(forgedAsBob)).toBe(false) }) - it('Every time Alice changes her keys, the admin keys are rotated', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('Every time Alice changes her keys, the admin keys are rotated', () => { const { alice } = setup('alice') const changeKeys = () => { const newKeys = { type: KeyType.USER, name: alice.userId } diff --git a/packages/auth/src/team/test/keysetCommitmentAuthorization.test.ts b/packages/auth/src/team/test/keysetCommitmentAuthorization.test.ts index c1e9998ce..bc8207bc7 100644 --- a/packages/auth/src/team/test/keysetCommitmentAuthorization.test.ts +++ b/packages/auth/src/team/test/keysetCommitmentAuthorization.test.ts @@ -31,7 +31,8 @@ describe('complete-keyset manifest authorization', () => { expect(lockbox.open(redistributed, bob.user.keys)).toEqual(established) }) - it('drops a conflicting historical-generation redistribution', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('drops a conflicting historical-generation redistribution', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) alice.team.addRole(CHANNEL) alice.team.addMemberRole(bob.userId, CHANNEL) diff --git a/packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts b/packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts index 4c4dabd4d..86c7251d5 100644 --- a/packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts +++ b/packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts @@ -37,7 +37,8 @@ describe('lockbox authorization hardening', () => { ).toBe(false) }) - it('does not carry a retired generic recipient into a subsequent role rotation', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('does not carry a retired generic recipient into a subsequent role rotation', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) alice.team.addRole(CHANNEL) alice.team.addMemberRole(bob.userId, CHANNEL) @@ -121,7 +122,8 @@ describe('lockbox authorization hardening', () => { ).toBe(false) }) - it('does not let an unrelated member pre-seed a future USER generation', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('does not let an unrelated member pre-seed a future USER generation', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) const nextKeys = createKeyset( { type: KeyType.USER, name: alice.userId }, @@ -162,7 +164,8 @@ describe('lockbox authorization hardening', () => { expect(alice.team.adminKeys().generation).toBe(1) }) - it('selects the legitimate USER key change in both concurrent merge orders', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('selects the legitimate USER key change in both concurrent merge orders', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) const base = alice.team.graph const nextKeys = createKeyset( @@ -206,7 +209,8 @@ describe('lockbox authorization hardening', () => { } }) - it('does not let an unrelated member pre-seed a future SERVER generation', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('does not let an unrelated member pre-seed a future SERVER generation', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) const serverWithSecrets = createServer({ host: 'sync.example', seed: 'registered-server' }) alice.team.addServer(redactServer(serverWithSecrets)) @@ -294,7 +298,8 @@ describe('lockbox authorization hardening', () => { ).toBe(true) }) - it('preserves an authenticated ROTATE_KEYS recipient transition', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('preserves an authenticated ROTATE_KEYS recipient transition', () => { const { alice } = setup('alice') // This is the same action `checkForPendingKeyRotations` emits after a resolver marks a diff --git a/packages/auth/src/team/test/lockboxChannelTakeover.test.ts b/packages/auth/src/team/test/lockboxChannelTakeover.test.ts index abd0a5e21..0fd77404c 100644 --- a/packages/auth/src/team/test/lockboxChannelTakeover.test.ts +++ b/packages/auth/src/team/test/lockboxChannelTakeover.test.ts @@ -83,35 +83,39 @@ const rotationRecipientsFor = ( return { expected, removed: removedRecipients[0] } } -const dispatchDeclaredRotation = ( +const mergeDeclaredRotation = ( alice: UserStuff, removed: UserStuff, scope: (typeof ATTACK_SCOPES)[number], lockboxes: lockbox.Lockbox[] ) => { - if (scope.type === KeyType.TEAM) { - alice.team.dispatch({ - type: 'REMOVE_MEMBER', - payload: { userId: removed.userId, lockboxes }, + const action: TeamAction = + scope.type === KeyType.TEAM + ? { type: 'REMOVE_MEMBER', payload: { userId: removed.userId, lockboxes } } + : { + type: 'REMOVE_MEMBER_ROLE', + payload: { userId: removed.userId, roleName: scope.name, lockboxes }, + } + // Bypass only the local producer, as a modified peer would, then use the real remote path. + alice.team.merge( + append({ + graph: alice.team.graph, + action, + signer: alice.signer, + keys: alice.team.teamKeys(), }) - return - } - - alice.team.dispatch({ - type: 'REMOVE_MEMBER_ROLE', - payload: { userId: removed.userId, roleName: scope.name, lockboxes }, - }) + ) } -const expectDeclaredRemovalApplied = ( +const expectDeclaredRemovalIgnored = ( alice: UserStuff, removed: UserStuff, scope: (typeof ATTACK_SCOPES)[number] ) => { if (scope.type === KeyType.TEAM) { - expect(alice.team.has(removed.userId)).toBe(false) + expect(alice.team.has(removed.userId)).toBe(true) } else { - expect(alice.team.memberHasRole(removed.userId, scope.name)).toBe(false) + expect(alice.team.memberHasRole(removed.userId, scope.name)).toBe(true) } } @@ -268,10 +272,10 @@ describe('honest lockbox private-channel takeover (#61)', () => { const lockboxes = expected.map(recipient => lockbox.create(skippedKeys, recipient)) expect(lockboxes).toHaveLength(expected.length) - // REMOVE_MEMBER and REMOVE_MEMBER_ROLE are declared rotation carriers. Every recipient is - // correct, so this batch is rejected specifically because generation 2 skips generation 1. - expect(() => dispatchDeclaredRotation(alice, bob, scope, lockboxes)).not.toThrow() - expectDeclaredRemovalApplied(alice, bob, scope) + // These signed removal carriers were able to rotate keys in protocol 3. Protocol 4 keeps + // both removal and rotation inert, even with a malformed future generation. + expect(() => mergeDeclaredRotation(alice, bob, scope, lockboxes)).not.toThrow() + expectDeclaredRemovalIgnored(alice, bob, scope) expectNoGeneration(alice, scope, 2) }) @@ -289,8 +293,8 @@ describe('honest lockbox private-channel takeover (#61)', () => { const lockboxes = expected.slice(0, -1).map(recipient => lockbox.create(nextKeys, recipient)) expect(lockboxes).toHaveLength(expected.length - 1) - expect(() => dispatchDeclaredRotation(alice, bob, scope, lockboxes)).not.toThrow() - expectDeclaredRemovalApplied(alice, bob, scope) + expect(() => mergeDeclaredRotation(alice, bob, scope, lockboxes)).not.toThrow() + expectDeclaredRemovalIgnored(alice, bob, scope) expectNoGeneration(alice, scope, 1) }) @@ -308,13 +312,14 @@ describe('honest lockbox private-channel takeover (#61)', () => { const lockboxes = [...expected, removed].map(recipient => lockbox.create(nextKeys, recipient)) expect(lockboxes).toHaveLength(expected.length + 1) - expect(() => dispatchDeclaredRotation(alice, bob, scope, lockboxes)).not.toThrow() - expectDeclaredRemovalApplied(alice, bob, scope) + expect(() => mergeDeclaredRotation(alice, bob, scope, lockboxes)).not.toThrow() + expectDeclaredRemovalIgnored(alice, bob, scope) expectNoGeneration(alice, scope, 1) }) } - it('still lets an admin rotate the team key when removing a member', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('still lets an admin rotate the team key when removing a member', () => { const { alice, bob, charlie } = setupSharedScopes() const teamKeys = alice.team.teamKeys() @@ -333,7 +338,8 @@ describe('honest lockbox private-channel takeover (#61)', () => { expect(() => bob.team.decrypt(envelope)).toThrow() }) - it('still lets an admin rotate the channel key when removing a member from it', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('still lets an admin rotate the channel key when removing a member from it', () => { const { alice, bob, charlie } = setup( 'alice', { user: 'bob', admin: false }, diff --git a/packages/auth/src/team/test/lockboxPostRotationAdmission.test.ts b/packages/auth/src/team/test/lockboxPostRotationAdmission.test.ts index 77fd91abc..e93d9e7b3 100644 --- a/packages/auth/src/team/test/lockboxPostRotationAdmission.test.ts +++ b/packages/auth/src/team/test/lockboxPostRotationAdmission.test.ts @@ -17,7 +17,8 @@ import { memberAdmission } from './helpers.js' * has, so it is distribution, not rotation, and every generation survives. */ describe('admission after a team-key rotation (185b650)', () => { - it('lockboxes every team-key generation to the joiner', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('lockboxes every team-key generation to the joiner', () => { const { alice, bob, charlie } = setup('alice', 'bob', { user: 'charlie', member: false }) // 👩🏾 Alice removes 👨🏻‍🦲 Bob, which rotates the team keys to generation 1. diff --git a/packages/auth/src/team/test/lockboxRotationConcurrency.test.ts b/packages/auth/src/team/test/lockboxRotationConcurrency.test.ts index db8150adc..2f138a5d5 100644 --- a/packages/auth/src/team/test/lockboxRotationConcurrency.test.ts +++ b/packages/auth/src/team/test/lockboxRotationConcurrency.test.ts @@ -8,7 +8,8 @@ import { describe, expect, it } from 'vitest' const CHANNEL = 'private-channel-role' describe('lockbox rotation under honest concurrency (#63)', () => { - it('does not poison the graph when a role add lands before a concurrent re-key', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('does not poison the graph when a role add lands before a concurrent re-key', () => { const { alice, bob, charlie, dave } = setup( 'alice', 'bob', diff --git a/packages/auth/src/team/test/members.test.ts b/packages/auth/src/team/test/members.test.ts index cd187b881..298b29ab0 100644 --- a/packages/auth/src/team/test/members.test.ts +++ b/packages/auth/src/team/test/members.test.ts @@ -69,7 +69,8 @@ describe('Team', () => { expect(addBobAgain).not.toThrow() }) - it('removes a member', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('removes a member', () => { const { alice, bob, charlie } = setup('alice', 'bob', { user: 'charlie', member: false }) expect(alice.team.has(bob.userId)).toBe(true) @@ -84,7 +85,8 @@ describe('Team', () => { expect(alice.team.memberWasRemoved(charlie.userId)).toBe(false) // Charlie was never a member }) - it('only admins can remove members', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('only admins can remove members', () => { const { alice, bob, charlie } = setup('alice', { user: 'bob', admin: false }, 'charlie') // Bob can't remove Charlie because Bob's not an admin @@ -94,7 +96,8 @@ describe('Team', () => { expect(() => alice.team.remove(charlie.userId)).not.toThrow() }) - it('rotates keys after removing a member', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rotates keys after removing a member', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: true }) // Keys have never been rotated @@ -109,7 +112,8 @@ describe('Team', () => { expect(alice.team.adminKeys().generation).toBe(1) }) - it("doesn't do anything if asked to remove a nonexistent member", () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip("doesn't do anything if asked to remove a nonexistent member", () => { const { alice } = setup('alice') // Try removing bob although he hasn't been added diff --git a/packages/auth/src/team/test/membershipResolver.test.ts b/packages/auth/src/team/test/membershipResolver.test.ts index 2392814db..26db82296 100644 --- a/packages/auth/src/team/test/membershipResolver.test.ts +++ b/packages/auth/src/team/test/membershipResolver.test.ts @@ -61,7 +61,8 @@ describe('membershipResolver', () => { ]) }) - it('discards changes made by a member who is concurrently removed', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('discards changes made by a member who is concurrently removed', () => { // 👩🏾 🡒 👨🏻‍🦲 Alice creates a graph and shares it with Bob let { aGraph, bGraph, keys } = setup() @@ -91,7 +92,8 @@ describe('membershipResolver', () => { expectMergedResult(aGraph, bGraph, 'ROOT,ADD:bob,REMOVE:bob') }) - it('discards changes made by a member who is concurrently demoted', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('discards changes made by a member who is concurrently demoted', () => { // 👩🏾 🡒 👨🏻‍🦲 Alice creates a graph and shares it with Bob let { aGraph, bGraph, keys } = setup() @@ -121,7 +123,8 @@ describe('membershipResolver', () => { expectMergedResult(aGraph, bGraph, 'ROOT,ADD:bob,REMOVE:admin:bob') }) - it('resolves mutual concurrent removals in favor of the team founder', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('resolves mutual concurrent removals in favor of the team founder', () => { // 👩🏾 🡒 👨🏻‍🦲 Alice creates a graph and shares it with Bob let { aGraph, bGraph, keys } = setup() @@ -149,7 +152,8 @@ describe('membershipResolver', () => { expectMergedResult(aGraph, bGraph, 'ROOT,ADD:bob,REMOVE:bob') }) - it('resolves mutual concurrent removals in favor of the senior member', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('resolves mutual concurrent removals in favor of the senior member', () => { // 👩🏾 Alice creates a graph and adds Charlie let { aGraph, keys } = setup() @@ -188,7 +192,8 @@ describe('membershipResolver', () => { expectMergedResult(bGraph, cGraph, 'ROOT,ADD:bob,ADD:charlie,REMOVE:charlie') }) - it('resolves mutual concurrent demotions in favor of the team founder', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('resolves mutual concurrent demotions in favor of the team founder', () => { // 👩🏾 🡒 👨🏻‍🦲 Alice creates a graph and shares it with Bob let { aGraph, bGraph, keys } = setup() @@ -216,7 +221,8 @@ describe('membershipResolver', () => { expectMergedResult(aGraph, bGraph, 'ROOT,ADD:bob,REMOVE:admin:bob') }) - it('resolves circular mutual concurrent demotions in favor of the team founder', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('resolves circular mutual concurrent demotions in favor of the team founder', () => { // 👩🏾 🡒 👨🏻‍🦲 Alice creates a graph and adds Charlie as admin let { aGraph, keys } = setup() diff --git a/packages/auth/src/team/test/removalRotationGate.test.ts b/packages/auth/src/team/test/removalRotationGate.test.ts new file mode 100644 index 000000000..10373ab87 --- /dev/null +++ b/packages/auth/src/team/test/removalRotationGate.test.ts @@ -0,0 +1,256 @@ +import { createKeyset, merge, redactKeys, type KeyScope } from '@localfirst/crdx' +import * as lockbox from 'lockbox/index.js' +import { pack, unpack } from 'msgpackr' +import { ADMIN } from 'role/index.js' +import { createServer, redactServer } from 'server/index.js' +import * as teams from 'team/index.js' +import { serializeTeamGraph } from 'team/serialize.js' +import type { TeamAction, TeamState } from 'team/types.js' +import { KeyType } from 'util/index.js' +import { setup, type UserStuff } from 'util/testing/index.js' +import { describe, expect, it, vi } from 'vitest' +import { forge } from './forgeHelpers.js' + +const CHANNEL = 'private-channel' + +const fixture = () => { + const users = setup('alice', { user: 'bob', admin: false }, 'charlie') + const { alice, bob } = users + alice.team.addRole(CHANNEL) + alice.team.addMemberRole(alice.userId, CHANNEL) + alice.team.addMemberRole(bob.userId, CHANNEL) + const server = createServer({ host: 'relay.example', seed: 'removal-gate-relay' }) + alice.team.addServer(redactServer(server)) + for (const peer of Object.values(users)) peer.team.merge(alice.team.graph) + return { alice, bob, charlie: users.charlie, server } +} + +type Fixture = ReturnType +type Operation = { peer: UserStuff; run: () => void; legacyAction?: () => TeamAction } + +const keyChangeOperation = (peer: UserStuff, scope: KeyScope): Operation => ({ + peer, + run: () => peer.team.changeKeys(createKeyset(scope)), + legacyAction() { + const keys = createKeyset(scope) + keys.generation = 1 + // Protocol 3's key-change producer; the protocol 4 API refuses before changing a keyset. + // @ts-expect-error Exercise the old lockbox producer without publishing its result. + const lockboxes = peer.team.rotateKeys(keys) + return { + type: scope.type === KeyType.SERVER ? 'CHANGE_SERVER_KEYS' : 'CHANGE_MEMBER_KEYS', + payload: { keys: redactKeys(keys), lockboxes }, + } + }, +}) + +const operations: Record Operation> = { + REMOVE_MEMBER: ({ alice, bob }) => ({ peer: alice, run: () => alice.team.remove(bob.userId) }), + REMOVE_MEMBER_ROLE: ({ alice, bob }) => ({ + peer: alice, + run: () => alice.team.removeMemberRole(bob.userId, CHANNEL), + }), + REMOVE_ROLE: ({ alice }) => ({ peer: alice, run: () => alice.team.removeRole(CHANNEL) }), + REMOVE_DEVICE: ({ bob }) => ({ peer: bob, run: () => bob.team.removeDevice(bob.deviceId) }), + REMOVE_SERVER: ({ alice, server }) => ({ + peer: alice, + run: () => alice.team.removeServer(server.serverId), + }), + CHANGE_MEMBER_KEYS: ({ bob }) => + keyChangeOperation(bob, { type: KeyType.USER, name: bob.userId }), + CHANGE_SERVER_KEYS: ({ alice, server }) => + keyChangeOperation(alice, { type: KeyType.SERVER, name: server.serverId }), + ROTATE_KEYS: ({ alice, bob }) => ({ + peer: alice, + run() { + // The exact payload an unpatched admin emits for a deferred rotation. + // @ts-expect-error Exercise the existing producer without publishing its result. + const lockboxes = alice.team.rotateKeys({ type: KeyType.USER, name: alice.userId }) + alice.team.dispatch({ type: 'ROTATE_KEYS', payload: { userId: bob.userId, lockboxes } }) + }, + }), +} + +/** Capture a legacy producer's payload before it can alter local state, then sign it via CRDX. + * This models a modified peer without an off-switch in the production protocol gate. */ +const modifiedPeerGraph = ({ peer, run, legacyAction }: Operation) => { + let action = legacyAction?.() + if (action === undefined) { + const captured = new Error('captured modified-peer action') + const dispatch = vi.spyOn(peer.team, 'dispatch').mockImplementation(nextAction => { + action = nextAction + throw captured + }) + try { + expect(run).toThrow(captured) + } finally { + dispatch.mockRestore() + } + } + expect(action).toBeDefined() + return forge({ + graph: peer.team.graph, + action: action!, + signer: peer.signer, + teamKeys: peer.team.teamKeys(), + }) +} + +const withoutHead = ({ head: _head, ...state }: TeamState) => unpack(pack(state)) + +describe('removal and rotation are disabled on the Quiet protocol', () => { + it.each(Object.entries(operations))( + 'refuses to emit %s without changing state or graph', + (_type, operation) => { + const f = fixture() + const { peer, run } = operation(f) + const state = unpack(pack(peer.team.state)) + const graph = peer.team.save() + const keys = unpack(pack(peer.user.keys)) + + expect(run).toThrow(/removal and key rotation are disabled/i) + expect(unpack(pack(peer.team.state))).toEqual(state) + expect(peer.team.save()).toEqual(graph) + expect(peer.user.keys).toEqual(keys) + } + ) + + it.each(Object.entries(operations))( + 'ignores a signed modified-peer %s on merge and both load paths', + (_type, operation) => { + const f = fixture() + const graph = modifiedPeerGraph(operation(f)) + const { charlie } = f + const state = withoutHead(charlie.team.state) + const keyring = charlie.team.teamKeyring() + + charlie.team.merge(graph) + expect(withoutHead(charlie.team.state)).toEqual(state) + expect(charlie.team.graph.head).toEqual(graph.head) + expect(charlie.team.teamKeys().generation).toBe(0) + expect(charlie.team.roleKeys(CHANNEL).generation).toBe(0) + + for (const source of [graph, serializeTeamGraph(graph)]) { + const loaded = teams.load(source, charlie.localContext, keyring) + expect(withoutHead(loaded.state)).toEqual(state) + } + // Accepted inert history remains loadable using only the original keys. + expect( + withoutHead(teams.load(charlie.team.save(), charlie.localContext, keyring).state) + ).toEqual(state) + } + ) + + it.each(['REMOVE_MEMBER', 'REMOVE_DEVICE', 'REMOVE_MEMBER_ROLE'])( + '%s cannot censor the target or a concurrent role grant', + type => { + const f = fixture() + const { alice, bob, charlie } = f + const removal = modifiedPeerGraph( + type === 'REMOVE_MEMBER_ROLE' + ? { peer: alice, run: () => alice.team.removeMemberRole(charlie.userId, ADMIN) } + : type === 'REMOVE_DEVICE' + ? { peer: alice, run: () => alice.team.removeDevice(charlie.deviceId) } + : { peer: alice, run: () => alice.team.remove(charlie.userId) } + ) + charlie.team.addRole('concurrent-channel') + charlie.team.addMemberRole(bob.userId, 'concurrent-channel') + const concurrent = charlie.team.graph + const keyring = alice.team.teamKeyring() + + const replicas = [ + teams.load(alice.team.save(), alice.localContext, keyring), + teams.load(alice.team.save(), alice.localContext, keyring), + ] + replicas[0].merge(removal).merge(concurrent) + replicas[1].merge(concurrent).merge(removal) + expect(replicas[0].state).toEqual(replicas[1].state) + for (const replica of replicas) { + expect(replica.memberIsAdmin(charlie.userId)).toBe(true) + expect(replica.hasDevice(charlie.deviceId)).toBe(true) + expect(replica.memberHasRole(bob.userId, 'concurrent-channel')).toBe(true) + expect(replica.state.pendingKeyRotations).toEqual([]) + } + const loaded = teams.load( + serializeTeamGraph(merge(removal, concurrent)), + alice.localContext, + keyring + ) + expect(loaded.memberHasRole(bob.userId, 'concurrent-channel')).toBe(true) + } + ) + + it('does not publish deferred rotations even when invalid admission cleanup requests one', () => { + const { alice, bob } = fixture() + // The invalid-link reducer can request rotations independently of REMOVE_* actions. + alice.team.state.pendingKeyRotations.push(bob.userId) + const before = alice.team.save() + expect(() => alice.team.emit('updated', { head: alice.team.graph.head })).not.toThrow() + expect(alice.team.save()).toEqual(before) + expect(alice.team.teamKeys().generation).toBe(0) + }) + + it('does not mutate a supplied keyset that aliases the caller’s current keys', () => { + const { bob } = fixture() + const keys = unpack(pack(bob.user.keys)) + expect(() => bob.team.changeKeys(bob.user.keys)).toThrow(/disabled/i) + expect(bob.user.keys).toEqual(keys) + }) + + it('rejects a descendant encrypted under an ignored rotation atomically, then accepts ordinary updates', () => { + const f = fixture() + const { alice, charlie } = f + const rotate = operations.ROTATE_KEYS + const rotation = modifiedPeerGraph(rotate(f)) + const action = rotation.links[rotation.head[0]].body + expect(action.type).toBe('ROTATE_KEYS') + if (action.type !== 'ROTATE_KEYS') throw new Error('expected rotation') + const box = action.payload.lockboxes.find( + box => box.contents.type === KeyType.TEAM && box.recipient.name === charlie.userId + )! + // Recover the modified writer's next TEAM generation as an old peer would. The honest + // receiver is never given this key and must not learn it from the disabled action. + const rotatedKeys = lockbox.open(box, charlie.user.keys) + expect(rotatedKeys.generation).toBe(1) + const descendant = forge({ + graph: rotation, + action: { type: 'SET_TEAM_NAME', payload: { teamName: 'only readable after rotating' } }, + signer: alice.signer, + teamKeys: rotatedKeys, + }) + const before = charlie.team.save() + const state = unpack(pack(charlie.team.state)) + expect(() => charlie.team.merge(descendant)).toThrow() + expect(charlie.team.save()).toEqual(before) + expect(unpack(pack(charlie.team.state))).toEqual(state) + expect(Object.values(charlie.team.teamKeyring()).map(key => key.generation)).toEqual([0]) + expect(() => + teams.load(serializeTeamGraph(descendant), charlie.localContext, charlie.team.teamKeyring()) + ).toThrow() + + alice.team.setTeamName('still usable') + charlie.team.merge(alice.team.graph) + expect(charlie.team.teamName).toBe('still usable') + expect( + teams.load(charlie.team.save(), charlie.localContext, charlie.team.teamKeyring()).teamName + ).toBe('still usable') + }) + + it('retains invitations, additions, private encryption and the admin channel-delete predicate', () => { + const { alice, bob, charlie } = fixture() + expect(alice.team.memberCanRemoveMembersFromRole(CHANNEL, alice.userId)).toBe(false) + expect(alice.team.memberCanRemoveMembersFromRole(CHANNEL, bob.userId)).toBe(false) + expect(alice.team.memberCanDeleteRole(CHANNEL, alice.userId)).toBe(true) + expect(alice.team.memberCanDeleteRole(CHANNEL, bob.userId)).toBe(false) + expect(() => alice.team.removeRole(CHANNEL)).toThrow(/removal and key rotation are disabled/i) + + const { id } = alice.team.inviteMember() + expect(alice.team.getInvitation(id).revoked).not.toBe(true) + alice.team.addMemberRole(charlie.userId, CHANNEL) + charlie.team.merge(alice.team.graph) + expect(charlie.team.decrypt(alice.team.encrypt('private message', CHANNEL))).toBe( + 'private message' + ) + expect(charlie.team.memberCanDeleteRole(CHANNEL, charlie.userId)).toBe(true) + }) +}) diff --git a/packages/auth/src/team/test/roles.test.ts b/packages/auth/src/team/test/roles.test.ts index 1cb3f7a07..2776c8481 100644 --- a/packages/auth/src/team/test/roles.test.ts +++ b/packages/auth/src/team/test/roles.test.ts @@ -119,7 +119,8 @@ describe('Team', () => { expect(alice.team.memberHasRole(alice.userId, FOOBAR)).toBe(false) }) - it('removes a member from a role', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('removes a member from a role', () => { const { alice, bob } = setup('alice', 'bob') // Alice creates manager role and add 👨🏻‍🦲 Bob to it @@ -256,7 +257,8 @@ describe('Team', () => { expect(bob.team.memberHasRole(charlie.userId, 'MEMBER')).toBe(false) }) - it('removes a role', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('removes a role', () => { const { alice } = setup('alice') // 👩🏾 Alice adds the managers role @@ -398,7 +400,8 @@ describe('Team', () => { expect(remove).toThrow() }) - it('Alice can remove herself as admin as long as there at least one other admin', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('Alice can remove herself as admin as long as there at least one other admin', () => { const { alice } = setup('alice', 'bob') const remove = () => { @@ -408,7 +411,8 @@ describe('Team', () => { expect(remove).not.toThrow() }) - it('rotates keys when a member is removed from a role', async () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('rotates keys when a member is removed from a role', async () => { const COOLKIDS = 'coolkids' const { alice, bob, charlie } = setup( diff --git a/packages/auth/src/team/test/sameGenerationLockboxReplacement.test.ts b/packages/auth/src/team/test/sameGenerationLockboxReplacement.test.ts index d8b27b2a9..3f1ace24b 100644 --- a/packages/auth/src/team/test/sameGenerationLockboxReplacement.test.ts +++ b/packages/auth/src/team/test/sameGenerationLockboxReplacement.test.ts @@ -81,7 +81,8 @@ describe('same-generation lockbox replacement', () => { return { alice, legitimateKeys, evilKeys, recipient: currentRoleLockbox!.recipient, attackWith } } - it('a removed role member cannot rebind the current generation to a key they control', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('a removed role member cannot rebind the current generation to a key they control', () => { const { alice, legitimateKeys, evilKeys, recipient, attackWith } = setupRemovedRoleMember() // The forged lockbox is honest about what it carries: its manifest advertises Bob's public @@ -99,7 +100,8 @@ describe('same-generation lockbox replacement', () => { ) }) - it('nor by lying in the manifest about which key the lockbox contains', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('nor by lying in the manifest about which key the lockbox contains', () => { const { alice, legitimateKeys, evilKeys, recipient, attackWith } = setupRemovedRoleMember() // Authorization reads the public manifest; only recipients can decrypt the payload. So this diff --git a/packages/auth/src/team/test/securityCandidates.poc.test.ts b/packages/auth/src/team/test/securityCandidates.poc.test.ts index 2ef15d030..bfa808a3b 100644 --- a/packages/auth/src/team/test/securityCandidates.poc.test.ts +++ b/packages/auth/src/team/test/securityCandidates.poc.test.ts @@ -102,7 +102,8 @@ describe('security candidate PoCs: team and lockbox state', () => { expect(() => symmetric.decryptBytes(message.contents, attackerRoleKeys.secretKey)).toThrow() }) - it('does not deliver the replacement role key to the principal removed by the rotation', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('does not deliver the replacement role key to the principal removed by the rotation', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) alice.team.addRole(CHANNEL) alice.team.addMemberRole(bob.userId, CHANNEL) @@ -176,7 +177,8 @@ describe('security candidate PoCs: team and lockbox state', () => { ).toBe(false) }) - it('does not deliver replacement USER or TEAM keys to a fully removed member', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('does not deliver replacement USER or TEAM keys to a fully removed member', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) const beforeRemoval = serializeTeamGraph(alice.team.graph) bob.team = teams.load(beforeRemoval, bob.localContext, createKeyring(alice.team.teamKeys())) @@ -196,7 +198,8 @@ describe('security candidate PoCs: team and lockbox state', () => { expect(() => symmetric.decryptBytes(futureMessage.contents, oldTeamKeys.secretKey)).toThrow() }) - it('can leave the target of a losing concurrent member removal with the winning team key', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('can leave the target of a losing concurrent member removal with the winning team key', () => { const { alice, bob, dave, charlie } = setup( 'alice', { user: 'bob', admin: false }, @@ -257,10 +260,7 @@ describe('security candidate PoCs: team and lockbox state', () => { it('rejects an admin-authored cross-user key replacement on live merge and cold load', () => { const { alice, bob } = setup('alice', 'bob') const teamKeys = alice.team.teamKeys() - const replacement = createKeyset( - { type: KeyType.USER, name: bob.userId }, - 'alice-rotates-bob' - ) + const replacement = createKeyset({ type: KeyType.USER, name: bob.userId }, 'alice-rotates-bob') replacement.generation = bob.team.members(bob.userId).keys.generation + 1 // Build the exact graph a modified administrator would publish, bypassing the public API's @@ -286,7 +286,8 @@ describe('security candidate PoCs: team and lockbox state', () => { }) }) - it('can fail to recover a descendant written under a losing concurrent team rotation', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('can fail to recover a descendant written under a losing concurrent team rotation', () => { const { alice, bob } = setup('alice', 'bob') const base = serializeTeamGraph(alice.team.graph) const baseKeys = createKeyring(alice.team.teamKeys()) diff --git a/packages/auth/src/team/test/servers.test.ts b/packages/auth/src/team/test/servers.test.ts index cba280b4a..98d54ac64 100644 --- a/packages/auth/src/team/test/servers.test.ts +++ b/packages/auth/src/team/test/servers.test.ts @@ -28,7 +28,7 @@ import { describe('Team', () => { describe('a server', () => { - it('can be added and removed by an admin', () => { + it('can be added by an admin but removal is disabled', () => { const { alice } = setupHumans('alice') // Add server @@ -43,10 +43,10 @@ describe('Team', () => { // The host is a label, not an identity, so looking one up by host gives a list expect(alice.team.serversByHost(host)).toHaveLength(1) - // Remove server - alice.team.removeServer(server.serverId) - expect(alice.team.servers().length).toBe(0) - expect(alice.team.serverWasRemoved(server.serverId)).toBe(true) + // Protocol 4 retains the server even when its admin requests removal. + expect(() => alice.team.removeServer(server.serverId)).toThrow(/disabled/i) + expect(alice.team.servers().length).toBe(1) + expect(alice.team.serverWasRemoved(server.serverId)).toBe(false) }) it("throws if a named server doesn't exist on the team", () => { @@ -54,7 +54,8 @@ describe('Team', () => { expect(() => alice.team.servers('foo.com')).toThrow() }) - it("can't be re-added after being removed", () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip("can't be re-added after being removed", () => { const { alice } = setupHumans('alice') // Add server @@ -264,7 +265,8 @@ describe('Team', () => { expect(alice.team.members(bob.userId).devices).toHaveLength(2) }) - it('has its keys rotated by an admin', () => { + // Protocol 4 disables removal/rotation; retained as a historical revocation specification. + it.skip('has its keys rotated by an admin', () => { const { alice } = setupHumans('alice', 'bob') const { server, serverWithSecrets } = createServer(host) alice.team.addServer(server) From 6f534c89bceb875e8c71997943e5e76e48ccbd88 Mon Sep 17 00:00:00 2001 From: holmes Date: Thu, 10 Sep 2026 12:34:07 -0700 Subject: [PATCH 2/2] Retain admission and key delivery checks under the removal gate --- docs/protocol-4-removal-policy.md | 5 +---- .../test/validateInvitationAcceptance.test.ts | 8 +------ .../lockboxAuthorizationHardening.test.ts | 22 ++++++------------- 3 files changed, 9 insertions(+), 26 deletions(-) diff --git a/docs/protocol-4-removal-policy.md b/docs/protocol-4-removal-policy.md index 9d9074ff5..5d0fa4bb5 100644 --- a/docs/protocol-4-removal-policy.md +++ b/docs/protocol-4-removal-policy.md @@ -57,7 +57,7 @@ comment at each test. They do not establish revocation guarantees for this relea supported signature, identity, invitation, and lockbox authorization continue to run. No test or production switch bypasses the gate. -85 existing test cases are explicitly retired by this change: +82 existing test cases are explicitly retired by this change: ### [connection/test/authentication.test.ts](../packages/auth/src/connection/test/authentication.test.ts) @@ -98,7 +98,6 @@ or production switch bypasses the gate. - rejects a fresh envelope wrapping a pre-removal graph (member) - rejects a fresh envelope wrapping a pre-removal graph (device) - rejects a fresh envelope wrapping a pre-removal graph sent by a server -- rejects an older-handshake admission from an active sender when id and claim both match - rejects an admission that the membership resolver has invalidated - rejects a graph in which the invitee was admitted and then removed @@ -167,9 +166,7 @@ or production switch bypasses the gate. ### [team/test/lockboxAuthorizationHardening.test.ts](../packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts) - does not carry a retired generic recipient into a subsequent role rotation -- does not let an unrelated member pre-seed a future USER generation - selects the legitimate USER key change in both concurrent merge orders -- does not let an unrelated member pre-seed a future SERVER generation - preserves an authenticated ROTATE_KEYS recipient transition ### [team/test/lockboxChannelTakeover.test.ts](../packages/auth/src/team/test/lockboxChannelTakeover.test.ts) diff --git a/packages/auth/src/connection/test/validateInvitationAcceptance.test.ts b/packages/auth/src/connection/test/validateInvitationAcceptance.test.ts index a66a88890..ad05edb1f 100644 --- a/packages/auth/src/connection/test/validateInvitationAcceptance.test.ts +++ b/packages/auth/src/connection/test/validateInvitationAcceptance.test.ts @@ -411,8 +411,7 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { * the proof differs, because that admission was made in an earlier handshake. It must still be * rejected, or every other rule can be satisfied by an old graph. */ - // Protocol 4 disables removal/rotation; retained as a historical revocation specification. - it.skip('rejects an older-handshake admission from an active sender when id and claim both match', async () => { + it('rejects an older-handshake admission from an active sender when id and claim both match', async () => { const { alice, bob } = setup('alice', { user: 'bob', member: false }) const { seed, teamId } = alice.team.inviteMember() @@ -424,11 +423,6 @@ describe('invitee validation of the welcome (ACCEPT_INVITATION)', () => { assert(staleAdmission !== undefined && staleAdmission.body.type === 'ADMIT_MEMBER') const stalePayload = staleAdmission.body.payload - // The team removes Bob after that admission, so the delivered graph is now a rollback. - const current = cloneTeam(stale, alice.connectionContext) - current.remove(bob.userId) - expect(current.memberWasRemoved(bob.userId)).toBe(true) - let checked = false const result = await connectInvitee({ acceptor: withTeam(alice.connectionContext, cloneTeam(alice.team, alice.connectionContext)), diff --git a/packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts b/packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts index 86c7251d5..3c8aa6aa3 100644 --- a/packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts +++ b/packages/auth/src/team/test/lockboxAuthorizationHardening.test.ts @@ -122,8 +122,7 @@ describe('lockbox authorization hardening', () => { ).toBe(false) }) - // Protocol 4 disables removal/rotation; retained as a historical revocation specification. - it.skip('does not let an unrelated member pre-seed a future USER generation', () => { + it('does not let an unrelated member pre-seed a future USER generation', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) const nextKeys = createKeyset( { type: KeyType.USER, name: alice.userId }, @@ -157,11 +156,9 @@ describe('lockbox authorization hardening', () => { ) ).toBe(false) - alice.team.changeKeys(nextKeys) - expect(alice.team.members(alice.userId).keys.generation).toBe(1) - expect(alice.team.members(alice.userId).keys.encryption).toBe(nextKeys.encryption.publicKey) - expect(alice.team.teamKeys().generation).toBe(1) - expect(alice.team.adminKeys().generation).toBe(1) + expect(alice.team.members(alice.userId).keys.generation).toBe(0) + expect(alice.team.teamKeys().generation).toBe(0) + expect(alice.team.adminKeys().generation).toBe(0) }) // Protocol 4 disables removal/rotation; retained as a historical revocation specification. @@ -209,8 +206,7 @@ describe('lockbox authorization hardening', () => { } }) - // Protocol 4 disables removal/rotation; retained as a historical revocation specification. - it.skip('does not let an unrelated member pre-seed a future SERVER generation', () => { + it('does not let an unrelated member pre-seed a future SERVER generation', () => { const { alice, bob } = setup('alice', { user: 'bob', admin: false }) const serverWithSecrets = createServer({ host: 'sync.example', seed: 'registered-server' }) alice.team.addServer(redactServer(serverWithSecrets)) @@ -245,12 +241,8 @@ describe('lockbox authorization hardening', () => { ) ).toBe(false) - alice.team.changeKeys(nextKeys) - expect(alice.team.servers(serverWithSecrets.serverId).keys.generation).toBe(1) - expect(alice.team.servers(serverWithSecrets.serverId).keys.encryption).toBe( - nextKeys.encryption.publicKey - ) - expect(alice.team.teamKeys().generation).toBe(1) + expect(alice.team.servers(serverWithSecrets.serverId).keys.generation).toBe(0) + expect(alice.team.teamKeys().generation).toBe(0) }) it('does not let an ADMIT_MEMBER link pre-poison the USER commitment before join', () => {