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

Update home IdP discovery to use new SPI #235

Open
xgp opened this issue May 20, 2024 · 10 comments
Open

Update home IdP discovery to use new SPI #235

xgp opened this issue May 20, 2024 · 10 comments
Assignees
Labels
customer Customer request enhancement New feature or request priority

Comments

@xgp
Copy link
Member

xgp commented May 20, 2024

sventorben/keycloak-home-idp-discovery#346

@xgp xgp added enhancement New feature or request priority customer Customer request labels May 20, 2024
@xgp xgp self-assigned this May 20, 2024
@rtufisi
Copy link
Collaborator

rtufisi commented Jul 20, 2024

New Home IdP Discovery

Screenshot from 2024-07-20 08-36-33
Screenshot from 2024-07-20 08-36-50
Screenshot from 2024-07-20 08-36-59
Screenshot from 2024-07-20 08-37-06

rtufisi added a commit to rtufisi/keycloak-orgs that referenced this issue Jul 20, 2024
@rtufisi
Copy link
Collaborator

rtufisi commented Jul 20, 2024

@xgp I was looking at the old form of the Home IdP Discovery config.

Screenshot from 2024-07-20 08-46-29

Is the 'Required a Verified Email' and 'Required a Verified Domain' still required? Are these our implementations?

@rtufisi
Copy link
Collaborator

rtufisi commented Jul 20, 2024

I believe this is the new 'verifiedEmail'
Screenshot from 2024-07-20 08-51-42

rtufisi added a commit to rtufisi/keycloak-orgs that referenced this issue Jul 20, 2024
rtufisi added a commit to rtufisi/keycloak-orgs that referenced this issue Jul 22, 2024
rtufisi added a commit to rtufisi/keycloak-orgs that referenced this issue Jul 22, 2024
@xgp
Copy link
Member Author

xgp commented Jul 22, 2024

@rtufisi

Is the 'Required a Verified Email' and 'Required a Verified Domain' still required? Are these our implementations?

Yes. Those are for our implementations. We will still need 'Required a Verified Domain'

I believe this is the new 'verifiedEmail'

It looks like he added this to the default.

@rtufisi
Copy link
Collaborator

rtufisi commented Jul 23, 2024

Could you please give me a example on how to use the IdpSelectorAuthenticator?

@rtufisi
Copy link
Collaborator

rtufisi commented Jul 23, 2024

@rtufisi

Is the 'Required a Verified Email' and 'Required a Verified Domain' still required? Are these our implementations?

Yes. Those are for our implementations. We will still need 'Required a Verified Domain'

Done

I believe this is the new 'verifiedEmail'

It looks like he added this to the default.

It seems to me that our implementation flag "verifiedEmail" and the source flag "forwardUserWithUnverifiedEmail" have opposite logic @xpg. Does this impact our logic? How could we proceed without affecting the clients?

rtufisi added a commit to rtufisi/keycloak-orgs that referenced this issue Jul 23, 2024
@xgp
Copy link
Member Author

xgp commented Jul 23, 2024

I think we would have to run a "migration" in a postInit method to load all existing instances of our Authenticator and migrate their configurations.

rtufisi added a commit to rtufisi/keycloak-orgs that referenced this issue Jul 23, 2024
@xgp
Copy link
Member Author

xgp commented Jul 24, 2024

Pulling the home idp extension is currently blocked by it not being in Maven Central. See sventorben/keycloak-home-idp-discovery#400

Going to try pulling the library in locally with the Maven system scope:

<dependency>
    <groupId>com.sample</groupId>
    <artifactId>sample</artifactId>
    <version>1.0</version>
    <scope>system</scope>
    <systemPath>${project.basedir}/src/main/resources/Name_Your_JAR.jar</systemPath>
</dependency>

Interim plan if this ^^^ works:

  1. Import the compatible home idp jar into a libs/ directory in the keycloak-orgs repo
  2. In our distribution/docker image phasetwo-containers repo, import it into the libs/ext/ dir
  3. Document in the README that it is necessary to include the home idp jar
  4. Eventually PR his repo with an example of how to release to maven central

rtufisi added a commit to rtufisi/keycloak-orgs that referenced this issue Jul 27, 2024
@rtufisi
Copy link
Collaborator

rtufisi commented Jul 27, 2024

Hei @xgp . I've created a first example of how we could use the home idp extension as a library. There are several things that block us from using it as a library but I've created a PR on how to use it.

Please check: https://github.com/p2-inc/keycloak-orgs/pull/259/files#diff-ca14b1065de1812332188294559f0f7398a15bb862c910f8d94fb83b1a6ce41b

In order to start the project locally use the following config:
Screenshot from 2024-07-27 14-38-42

rtufisi added a commit to rtufisi/keycloak-orgs that referenced this issue Jul 27, 2024
rtufisi added a commit to rtufisi/keycloak-orgs that referenced this issue Jul 28, 2024
@rtufisi
Copy link
Collaborator

rtufisi commented Jul 29, 2024

@xgp from my point of view to be able to fully implement a authenticator using "home IdP discovery" as a library we would need the following:

  • AbstractHomeIdpDiscoveryAuthenticatorFactory to allow a custom authenticator logic
    public final Authenticator create(KeycloakSession session) {
        return new HomeIdpDiscoveryAuthenticator(this.discovererConfig);
    }

In this way we could implement our custom Authenticator behaviour

  • Some classes exposed as public HomeIdpForwarderConfigProperties, HomeIdpAuthenticationFlowContext etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer Customer request enhancement New feature or request priority
Projects
None yet
Development

No branches or pull requests

2 participants