search

p2-inc / keycloak-orgs

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

no invitations found with InvitationAuthenticator #26

Closed tomschulze closed 1 year ago

tomschulze commented 1 year ago

I wanted to try the invitation flow and configured the invitation authenticator according to the docs (added invitation as a step after the username+password form). The invitation action is never triggered and the login flow continues as if invitations are not configured. The following logs appear:

2022-11-24 09:42:44,296 INFO  [io.phasetwo.service.auth.invitation.InvitationAuthenticator] (executor-thread-20) InvitationAuthenticator.configuredFor called for realm myrealm and user xxxx@xxx.xxx
2022-11-24 09:42:44,307 INFO  [io.phasetwo.service.auth.invitation.InvitationAuthenticator] (executor-thread-20) Found 0 invites for xxxx@xxx.xxx
2022-11-24 09:42:44,307 INFO  [io.phasetwo.service.auth.invitation.InvitationAuthenticator] (executor-thread-20) InvitationAuthenticator.authenticate called

I run the quay.io/phasetwo/phasetwo-keycloak:20.0.1-alpha docker image and started keycloak with start-dev. I use the default H2 database. I did query the H2 database file and the realm, organization and invitation data was there.

After debugging, I found that the JpaOrganizationProvider uses realm.getName() as a parameter to setup the named query getInvitationsByRealmAndEmail. When swapping the parameter with realm.getId(), the query returns my invitation.

xgp commented 1 year ago

Looks like a bug that was introduced in the recent version change, as keycloak changed the meaning of id and name.

xgp commented 1 year ago

The culprit lies in the use of realm.getId() when creating an organization: https://github.com/p2-inc/keycloak-orgs/blob/main/src/main/java/io/phasetwo/service/model/jpa/JpaOrganizationProvider.java#L35

In previous versions of Keycloak realm.getId() and realm.getName() used to return the same thing. They finally "fixed" realm.getId() to return the UUID rather than the name. Two alternatives:

  1. in order to maintain backwards compatibility, switch that to realm.getName()
  2. switch everything to use realm.getId() (UUID)
xgp commented 1 year ago

I'm going to go with 2, but will make a note if you have backwards compatibility issues

tomschulze commented 1 year ago

thanks so much for the quick fix. it works now.

xgp commented 1 year ago

No problem. Good catch. I'm trying to figure out exactly what happened with the change. Anyone that had an existing installation that upgrades to this will need to update their realm_id column in the organization table to be the UUID. However, I can't exactly figure out where that is coming from in the DB: https://github.com/keycloak/keycloak/discussions/15690