p2-inc / keycloak-orgs

Single realm, multi-tenancy for SaaS apps
https://phasetwo.io
Other
417 stars 72 forks source link

Invitations: Deletion of an inviter breaks GET /invitations endpoint #179

Closed MorryGun closed 8 months ago

MorryGun commented 9 months ago

Steps to reproduce:

  1. Create an invitation to Organization A with a custom inviter (User B)
  2. Delete User B
  3. Request invitations for Organization A

Expected result: 200 with a list of existing invitations Actual result: 400 with message {"error":"unknown_error"}

xgp commented 9 months ago

@MorryGun Good catch. It's failing because it can no longer look up the inviter by the stored ID.

We could either lazy remove these invites on load, or proactively on a UserModel.UserRemovedEvent event.

Care to PR a fix?

MorryGun commented 9 months ago

I don't think deleting invited is safe, I'd rather delete the inviter ID - as long as an email is sent, it is not needed anymore? BTW, if this is the case, what is the point of storing it in a DB?

xgp commented 9 months ago

The problem is that if the authority to make an invitation rested in a User that no longer exists, we should probably delete the invite.

BTW, if this is the case, what is the point of storing it in a DB?

It's returned in the API, so we can't break backwards compatibility. We, for one, use the inviter in our UI to show if you (or who) created the invitation.

xgp commented 8 months ago

Settled on lazy removing the inviterId. We may add removal of invites in the future.