wso2 / product-is

Welcome to the WSO2 Identity Server source code! For info on working with the WSO2 Identity Server repository and contributing code, click the link below.
http://wso2.github.io/
Apache License 2.0
741 stars 719 forks source link

Revisit the Username Validation Logic during Self Registration #16500

Open ImalshaG opened 1 year ago

ImalshaG commented 1 year ago

According to the current self registration implementation, we allow users to get registered to any tenant domain if the user has access to a SaaS application of one of the tenant domains. This will cause unintended behaviors in cases if a specific tenant domain intentionally requires users to be registered only through the exposed apps from that tenant domain. But due to the current behaviour, external users can get created in that tenant domain through SaaS apps of other tenants.

Therefore for self registration we should not allow user registration in other tenant domains even through SaaS applications. Users should be allowed to register only in the relevant tenant domain of the application that the user has access to.

In this case, users should only use the tenant aware username (only the username without appending the tenant domain) for both Saas and non-SaaS applications. Therefore the username validation logic should be changed to use the complete username, instead of using the tenantAwareUsername for validation[1][2].

[1] https://github.com/wso2-extensions/identity-governance/blob/master/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/signup/UserSelfRegistrationManager.java#L1290-L1298 [2] https://github.com/wso2-extensions/identity-governance/blob/master/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/signup/UserSelfRegistrationManager.java#L1447-L1449

DMHP commented 1 year ago

In order to create a user in the other tenant domains (other than the application domain) 'self-registration' should be enabled in the other tenants. Anyway restricting the user onboarding if the application domain and the user domain are different can be done.

ImalshaG commented 12 months ago

The expectation of this flow is for the user to provide just the username during self-registration for both SaaS/non-SaaS application without appending the tenant domain to the username. Since emails such as john@wso2.com are also valid usernames in the latest IS versions (even without enabling the email as username config), after this improvement we should follow the same logic for both SaaS and non-SaaS apps. For ex: Current behaviour - in SaaS apps if the user provides john@wso2.com as input, a user with username john will be created at wso2.com tenant. Expected behaviour - If a user provides john@wso2.com as input for any type (saas or non-saas) a user with username john@wso2.com should be created in the app's tenant domain.

To implement this, the current user validation logic will also have to be changed. During self registration, the user object is resolved from the username input at [1]. Since for saas apps the tenant aware name is extracted from the user input [2], we should modify here as well. Since the username validation logic at [3] and [4] are also considering the tenant-aware username (removing the part after the @ mark), this logic will also have to change to validate the whole username.

[1] https://github.com/wso2/identity-apps/blob/master/java/apps/recovery-portal/src/main/webapp/self-registration-with-verification.jsp#L93 [2] https://github.com/wso2/carbon-identity-framework/blob/master/components/identity-mgt/org.wso2.carbon.identity.mgt.endpoint.util/src/main/java/org/wso2/carbon/identity/mgt/endpoint/util/IdentityManagementServiceUtil.java#L262 [3] https://github.com/wso2-extensions/identity-governance/blob/master/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/signup/UserSelfRegistrationManager.java#L1282 [4] https://github.com/wso2-extensions/identity-governance/blob/master/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/signup/UserSelfRegistrationManager.java#L1439

Yasasr1 commented 11 months ago

As mentioned in the above comment resolveUser method [1] can be updated to correctly resolve the complete username (including the substring after @). Resolved user object is then passed to the self signup API /validate-username endpoint where the tenant-aware username resolving logic in [2] [3] needs to be changed. After this improvement, through the myaccount application, users can self-register to the carbon.super domain only.

Note: Username regex validation at [3] currently consider tenant aware and userstore domain aware username for validation. (Removing the substring after @ and the substring before /)

During testing the fix for this improvement, several existing issues could be observed in the password recovery flows when the email as username config is enabled

Screenshot 2023-09-15 at 2 57 04 PM

These issues should be fixed separately

[1] - https://github.com/wso2/carbon-identity-framework/blob/master/components/identity-mgt/org.wso2.carbon.identity.mgt.endpoint.util/src/main/java/org/wso2/carbon/identity/mgt/endpoint/util/IdentityManagementServiceUtil.java#L262 [2] - https://github.com/wso2/carbon-identity-framework/blob/master/components/identity-mgt/org.wso2.carbon.identity.mgt.endpoint.util/src/main/java/org/wso2/carbon/identity/mgt/endpoint/util/IdentityManagementServiceUtil.java#L262 [3] - https://github.com/wso2-extensions/identity-governance/blob/master/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/signup/UserSelfRegistrationManager.java#L1439