verify space events identities

[nikgraf]

Initially I wanted to use getVerifiedIdentity directly inside applyEvent, but then realized that on frontend we sometimes need to fetch from the API and on backend it's a DB call.

Passing in the entire list of identities was also an option. Didn't feel like a good idea to pass around the whole list from the store on frontend. I ended up adding a getVerifiedIdentity callback with this signature: (accountId: string) => Effect.Effect<PublicIdentity, InvalidIdentityError>

[pcarranzav]

Still reviewing, but leaving this comment for now, will come back to the review in a while

[pcarranzav]

This sounds a bit risky, if we ever change applyEvent to query the identity for a different accountId then this will always return the same identity... I'd suggest passing a getVerifiedIdentity that checks if the accountId is the one we have, and fails otherwise

[nikgraf]

yeah, that's I case I didn't imagine, but makes sense. Let me know if the change I made is what you envisioned

[pcarranzav]

Some suggestions / details but LGTM

[pcarranzav]

I guess technically we could remove this check, right? It's assuming the behavior of applySpaceEvent, in the future we could have a case where we need to check the identity for a different account mentioned in the event. (That being said I think it's okay to leave this as it is and change it in the future if needed)

[nikgraf]

Yeah, you are right that technically we could remove this check.

At the moment this ensures that the server only applies an event created that this account created. This must not be enforced, but I sleep a bit better that we enforce it. At least until we explicitly only allow accounts to send events to a sync server they themselves created

[pcarranzav]

nit: I see there's a few repetitions of this so maybe they could be split out into a utility function like this (in a separate file):

export const getGetVerifiedIdentity = (allowedAccountId: string) => {
  return function(accountIdToFetch: string) {
    // applySpaceEvent is only allowed to be called by the account that is applying the event
    if (accountIdToFetch !== allowedAccountId) {
      return Effect.fail(new Identity.InvalidIdentityError());
    }

    return Effect.gen(function* () {
      const identity = yield* Effect.tryPromise({
        try: () => getIdentity({ accountId: accountIdToFetch }),
        catch: () => new Identity.InvalidIdentityError(),
      });
      return identity;
    });
  }
}

Then you can pass getGetVerifiedIdentity(accountId) to applyEvent?

[nikgraf]

sounds good, will do it in a follow up PR 👍