-
Notifications
You must be signed in to change notification settings - Fork 0
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
MS custom idp & more custom execution steps #29
base: main
Are you sure you want to change the base?
Conversation
src/main/java/tech/neon/custom/NeonCleanUnverifiedAuthenticator.java
Outdated
Show resolved
Hide resolved
src/main/java/tech/neon/custom/NeonIdpEmailVerifyAuthenticator.java
Outdated
Show resolved
Hide resolved
src/main/java/tech/neon/custom/NeonIdpEmailVerifyAuthenticator.java
Outdated
Show resolved
Hide resolved
src/main/java/tech/neon/microsoft/MicrosoftUserAttributeMapper.java
Outdated
Show resolved
Hide resolved
|
||
import org.keycloak.broker.oidc.mappers.UserAttributeMapper; | ||
|
||
public class MicrosoftUserAttributeMapper extends UserAttributeMapper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since your MicrosoftIdentityProvider
extends OIDCIdentityProvider
, the builtin UserAttributeMapper might already be selectable for it, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
builtin UserAttributeMapper returns hardcoded COMPATIBLE_PROVIDERS in getCompatibleProviders. This subclass is just overrides this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. I'm saying that UserAttributeMapper
includes OIDCIdentityProviderFactory.PROVIDER_ID
in the compatible providers, and since your provider extends OIDCIdentityProvider
I think the builtin mapper should already treat it as compatible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If not, then it's fine to extend it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. I don't think it takes into account parent classes. I think it failed when I tried, but I will try one more time.
…r.java Co-authored-by: Krzysztof Szafrański <[email protected]>
Co-authored-by: Krzysztof Szafrański <[email protected]>
….java Co-authored-by: Krzysztof Szafrański <[email protected]>
….java Co-authored-by: Krzysztof Szafrański <[email protected]>
Co-authored-by: Krzysztof Szafrański <[email protected]>
….java Co-authored-by: Krzysztof Szafrański <[email protected]>
No description provided.