-
Notifications
You must be signed in to change notification settings - Fork 549
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
Control JIT provisioning users associating to local users through associating_to_existing_user configuration. #5468
Conversation
PR builder started |
PR builder started |
PR builder completed |
PR builder completed |
8729ebd
to
6eb6868
Compare
PR builder started |
PR builder completed |
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.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/7794722752
…ting to local users otherwise enabled through config.
PR builder started |
PR builder completed |
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.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/7813318849
features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2
Show resolved
Hide resolved
features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2
Show resolved
Hide resolved
...plication/authentication/framework/handler/provisioning/impl/DefaultProvisioningHandler.java
Outdated
Show resolved
Hide resolved
...plication/authentication/framework/handler/provisioning/impl/DefaultProvisioningHandler.java
Outdated
Show resolved
Hide resolved
.../authentication/framework/handler/request/impl/JITProvisioningPostAuthenticationHandler.java
Show resolved
Hide resolved
@@ -413,9 +408,6 @@ private PostAuthnHandlerFlowStatus handleRequestFlow(HttpServletRequest request, | |||
localClaimValues.get(EMAIL_ADDRESS_CLAIM))) { | |||
username = UserCoreUtil.addTenantDomainToEntry(username, context.getTenantDomain()); | |||
} | |||
if (StringUtils.isEmpty(associatedLocalUser)) { | |||
isUsernameExists(context, username); |
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.
What will happen if a customer uses custom jsp file? won't it cause an issue as we are skipping a isUsernameExists function?
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.
To identify if the user is existing, same as the defualt implementation, the user will have to first performa a username validation using the api/identity/user/v1.0/validate-username
endpoint. This will throw an error if the username is already existing.
If the custom jsp skips this check, the federation flow will continue without the user being provisioned. However the user will not be aware of the provisioning failure unless username is validated initially and the existence of the username is handled appropriately.
...plication/authentication/framework/handler/provisioning/impl/DefaultProvisioningHandler.java
Show resolved
Hide resolved
|
||
if (StringUtils.isBlank(associatedUserName)) { | ||
// If a local user is using the same username, association is not allowed unless enabled through config. | ||
if (isAssociationToExistingUserAllowed()) { |
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.
Dont' we need to handle migration for this configuration?
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.
For any cloud deployments, if it does not associate JIT provisioning users to local accounts, rather creates new accounts. Therefore the default behaviour of this change will not affect asgardeo deployment.
For other customers whose behaviour is to associate to local users, they will have to activate this through the configuration.
Silent provisioning flow with configuration disabled (default) silent_provisioning.mov |
Silent provisioning flow with config enabled silent_provisioning_with_config_enabled.mov |
Password consent prompt flow password_consent_prompt.mov |
Username password consent prompt flow username_password_consent_prompt.mov |
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.
LGTM
f0db9d4
to
e1c9a68
Compare
PR builder started |
PR builder completed |
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.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/7868274814
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.
LGTM
Purpose
Adding the
associating_to_existing_user
configuration.This configuration allows the configuration of whether to allow JIT provisioning users to be associated to an already existing local account or not. The default value of this configuration is false.
The existing validations for username existence are removed since these validations were causing the federated authentication flow to break upon failure. The validation is now done inside the DefaultProvisioningHandler and an error is thrown unless local user association is not enabled through the above configuration.