-
Notifications
You must be signed in to change notification settings - Fork 48
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
Support new Organization Feature #340
Comments
@sventorben Do you think this is an opportunity to implement the "discoverer" as an SPI as we discussed? |
Maybe, yes. Still not sure what it would look like though. I am not sure as something simple like this would work: public interface Discoverer {
List<IdentityProviderModel> discoverForUser(String username)
} What else would be needed? |
We override that method in |
The config is bound to the authenticator instance that initiates the discovery process. Do you need the additional config parameters for discovery or for other means? |
Here are the additions we made: These are used in the discovery. |
@xgp Can you take a look at #346 please. This is what I came up with - would like some initial feedback. Consider it to be an early draft. The |
@sventorben this meets all of the needs we currently have. Thanks so much for putting this together, and being so thoughtful about developer support and making sure stability is a future priority. If you think it would help you, I can do an implementation of our org discoverer based on your branch and report back if there are any issues. I understand the desire not to "support" others' extensions of your classes, and thus are choosing to make them final. But, it would be nice, at least in our case, to be able to extend |
That would be great.
I am not a big fan of extension in terms of inheritance here. I had another look at your code and would prefer to go with composition instead. Take a look at my latest commit to the branch. Would that work for you? Your implementation should look something like this in the end: public class OrgsHomeIdpDiscoveryAuthenticatorFactory extends AbstractHomeIdpDiscoveryAuthenticatorFactory {
public OrgsHomeIdpDiscoveryAuthenticatorFactory() {
super(new DiscovererConfig() {
@Override
public List<ProviderConfigProperty> getProperties() {
// return your custom properties here
return null;
}
@Override
public String getProviderId() {
return "orgs";
}
});
}
@Override
public String getId() {
return "orgs";
}
// ...
}
public class OrgsDiscovererFactory implements HomeIdpDiscovererFactory {
@Override
public HomeIdpDiscoverer create(KeycloakSession keycloakSession) {
return new EmailHomeIdpDiscoverer(new Users(keycloakSession), new IdentityProviders() {
@Override
public List<IdentityProviderModel> withMatchingDomain(AuthenticationFlowContext context, List<IdentityProviderModel> candidates, Domain domain) {
// filter the candidates here or simply lookup by your own logic via domain parameter
YourCustomConfig config = new YourCustomConfig(context.getAuthenticatorConfig());
return null;
}
});
}
@Override
public String getId() {
return "orgs";
}
// ...
} |
Is there an existing feature request for this?
Is your feature related to a problem? Please describe.
Placeholder to support the Keycloak’s Organization Feature coming with KC 25.
keycloak/keycloak#23948 (comment)
TODOs:
The text was updated successfully, but these errors were encountered: