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

Role based user provisioning improvements #5915

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sadilchamishka
Copy link
Contributor

@sadilchamishka sadilchamishka commented Sep 5, 2024

Copy link

codecov bot commented Sep 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 105 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@f1e53c8). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...isioning/listener/ProvisioningRoleMgtListener.java 0.00% 43 Missing ⚠️
...tity/provisioning/OutboundProvisioningManager.java 0.00% 36 Missing ⚠️
...ng/internal/IdentityProvisionServiceComponent.java 0.00% 13 Missing ⚠️
...ioning/internal/ProvisioningServiceDataHolder.java 0.00% 6 Missing ⚠️
...carbon/identity/provisioning/ProvisioningUtil.java 0.00% 4 Missing ⚠️
...stener/DefaultInboundUserProvisioningListener.java 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #5915   +/-   ##
=========================================
  Coverage          ?   21.05%           
  Complexity        ?     5626           
=========================================
  Files             ?     1562           
  Lines             ?    99299           
  Branches          ?    15202           
=========================================
  Hits              ?    20904           
  Misses            ?    75374           
  Partials          ?     3021           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/10713577446

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/10713577446
Status: success

Copy link

@jenkins-is-staging jenkins-is-staging left a 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/10713577446


RoleBasicInfo roleBasicInfo = ProvisioningServiceDataHolder.getInstance().getRoleManagementService()
.getRoleBasicInfoById(roleId,tenantDomain);
// Only organization audience roles are supported for role based outbound provisioning
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be backwards compatible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before IS 7.0 all the roles are organization audience roles. Hence previous roles and new roles created with organization audience role can be used for role base outbound provisioning.

The new type of role which is application audience roles came with IS 7.0. How to handle the role based provisioning for those types of roles should be analyzed due to below concerns.

  • Role name are not unique hence we have to register role uuid or role prefixed with its corresponding app name
  • As outbound provisioning is organization wide config by default (we can configure app wise also), the use cases of having application roles for role based outbound provisioning required to be analyzed.

Due to above concerns, outbound provisioning for application audience is not given at first phase.

Hope the explanation is clear enough, please raise if there any concerns. Here backward compatibility is preserved as here the application roles means not the Application/<role-name> role type had in previous IS versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants