Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up when a member's admission is reversed because of conflicts #33

Closed
5 of 6 tasks
HerbCaudill opened this issue Dec 13, 2021 · 0 comments · Fixed by #41
Closed
5 of 6 tasks

Clean up when a member's admission is reversed because of conflicts #33

HerbCaudill opened this issue Dec 13, 2021 · 0 comments · Fixed by #41

Comments

@HerbCaudill
Copy link
Member

HerbCaudill commented Dec 13, 2021

Suppose Bob is removed from the group, and concurrently he invites Charlie, and Charlie joins the group. Charlie's admission is now invalid, because he was invited while concurrently Bob was being removed.

We currently handle this scenario correctly in that Charlie is no longer a member of the group. However we need to do some cleanup, and we currently don't have a way to do it. From #32 :

Currently if Charlie joins and then his invitation is annulled because of this kind of conflict, everyone else lives happily ever after, as if Charlie had never joined. But Charlie did join and so he has the signature chain, so his client app should be told to purge it (assuming it's well-behaved); and someone needs to rotate any keys he might have seen.

None of this is insoluble, but it does require me to rethink some things specific to this implementation.

  1. Currently when we're calculating the group's state from the chain, any actions that we've invalidated because of concurrency issues are simply discarded, and can't enter into the calculation. Instead, I think I'll need to mark those actions as invalid but still feed them through the reducer, so that e.g. an invalidated JOIN can be treated as a REMOVE.

  2. We don't have a way of alerting the group that certain keys need to be rotated. Currently key rotation only happens as part of a conscious REMOVE action by an admin. In the case where someone is removed not by an admin's action but as a mechanical result of the conflict resolution logic, there's not necessarily an admin around to rotate the keys. So we need a completely new mechanism to add an alert to the group's state that certain keys need to be rotated, and to react appropriately to that alert.

The plan:

  • In herbcaudill/crdx, rather than filtering out invalid links, mark them as invalid.
  • In the reducer (here in localfirst/auth) ignore invalid links altogether for now. Behavior should now be exactly same as before, and all tests should pass.

Now we're in a position to detect this scenario and surface it in team state.

  • In the reducer, when a member's admission is reversed because of conflicts (i.e. we see an ADMIT action that's marked invalid), add them to the list of removed users in team state. Now in this test we can test that Charlie knows he's been removed.
  • Add a pendingKeyRotations element to team state. When the reducer sees an invalid ADMIT as above, it should populate this with the keys Charlie could see, using the same logic as when we manually remove someone. Once these keys are rotated, the reducer should remove them from the list.
  • When state is updated and an admin sees anything in pendingKeyRotations, they should perform the key rotation.
  • Stagger key rotation when unwinding invalidated invitation #39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant