p2-inc / keycloak-orgs

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

Remove DomainExtractor email verification check to fix unverified bug #261

Closed kalda341 closed 4 months ago

kalda341 commented 4 months ago

This mirrors the following PR in sventorben/keycloak-home-idp-discovery: https://github.com/sventorben/keycloak-home-idp-discovery/pull/248

This check is already performed in HomeIDPDiscoverer after extraction, so it's unnecessary:

        if (config.requireVerifiedEmail()
            && "email".equalsIgnoreCase(config.userAttribute())
            && !user.isEmailVerified()) {
            LOG.infof("Email of user %s not verified. Skipping discovery of linked IdPs", user.getId());
            return homeIdps;
        }

Additionally, this means that the HomeIdpDiscoverer works improperly with unverified emails when requireVerifiedEmail=false:

  1. At first login the user doesn't exist, so the email is unverified. Verification is skipped, the user goes through the auth flow as intended, and is now logged in.
  2. The user logs out.
  3. At second login, the user does exist, but the email is unverified. This check runs, even though unverified emails are supposed to be allowed in HomeIdpDiscoverer, and the login fails with the message: "Unexpected error when handling authentication request to identity provider."

This PR fixes this issue.

xgp commented 4 months ago

FYI we're in the process of updating to the most recent version of the home IDP discovery library. We should make sure this issue is also addressed in the PR @rtufisi

rtufisi commented 4 months ago

Sure