Provide support for selective role sharing with application sharing #512

sahandilshan · 2025-05-16T06:49:18Z

Purpose

$subject

TODOs:

Implementation for Get shared application with hierarchical order
Need to test out edge cases
Need to add unit tests

…haring

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java

...so2/carbon/identity/organization/management/application/util/OrganizationShareProcessor.java

.../org/wso2/carbon/identity/organization/management/application/OrgApplicationManagerImpl.java

...2/carbon/identity/organization/management/application/util/OrgApplicationShareProcessor.java

- Provide the support to get application with filtering option. - Add new error message for application sharing - Moved application share operations models to a new package

# Conflicts: # components/org.wso2.carbon.identity.organization.management.application/src/main/java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManagerImpl.java # components/org.wso2.carbon.identity.organization.management.application/src/main/java/org/wso2/carbon/identity/organization/management/application/constant/OrgApplicationMgtConstants.java # components/org.wso2.carbon.identity.organization.management.application/src/main/java/org/wso2/carbon/identity/organization/management/application/internal/OrgApplicationMgtDataHolder.java # components/org.wso2.carbon.identity.organization.management.application/src/main/java/org/wso2/carbon/identity/organization/management/application/internal/OrgApplicationMgtServiceComponent.java

AnuradhaSK · 2025-06-04T17:16:32Z

components/org.wso2.carbon.identity.organization.management.application/pom.xml

                            org.wso2.carbon.identity.role.v2.mgt.core.*;
                            version="${carbon.identity.package.import.version.range}",
                            org.wso2.carbon; version="${carbon.kernel.package.import.version.range}",
+                            *;resolution:=optional,


remove before merging, by adding all required imports

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java

sahandilshan · 2025-06-03T09:07:43Z

components/org.wso2.carbon.identity.organization.management.application/pom.xml

                            org.wso2.carbon.identity.role.v2.mgt.core.*;
                            version="${carbon.identity.package.import.version.range}",
                            org.wso2.carbon; version="${carbon.kernel.package.import.version.range}",
+                            *;resolution:=optional,


Properly fix the imports

sahandilshan · 2025-06-09T06:01:30Z

.../org/wso2/carbon/identity/organization/management/application/OrgApplicationManagerImpl.java

+        // This is done to ensure that the sharing is done in a way that respects the organization hierarchy.
+        List<SelectiveShareApplicationOperation> orgsThatNeedToBeShared = processAndSortOrganizationShares(
+                filteredChildOrgConfigs, mainOrganizationId);
+        if (CollectionUtils.isEmpty(orgsThatNeedToBeShared)) {


Update the APIs with this change

...main/java/org/wso2/carbon/identity/organization/management/handler/SharedRoleMgtHandler.java

.../org/wso2/carbon/identity/organization/management/application/OrgApplicationManagerImpl.java

- Convert logs to debug - Convert the ShareRoleMgt behaviour to previous behavior

- Improve logging - Add unit tests - Reduce using start tenant flows - Optimize to execute less DB queries

codecov · 2025-06-11T18:51:49Z

Codecov Report

Attention: Patch coverage is 26.86145% with 776 lines in your changes missing coverage. Please review.

Project coverage is 52.16%. Comparing base (2569253) to head (73f2fca).
Report is 77 commits behind head on main.

Files with missing lines	Patch %	Lines
...ation/management/handler/SharedRoleMgtHandler.java	37.75%	202 Missing and 42 partials ⚠️
...application/util/OrgApplicationShareProcessor.java	35.10%	106 Missing and 16 partials ⚠️
...lication/listener/OrganizationCreationHandler.java	0.00%	97 Missing ⚠️
...management/application/util/FilterQueriesUtil.java	0.00%	79 Missing ⚠️
...istener/ApplicationSharingManagerListenerImpl.java	0.00%	43 Missing ⚠️
...application/dao/impl/OrgApplicationMgtDAOImpl.java	0.00%	39 Missing ⚠️
...plication/util/OrgApplicationScimFilterParser.java	0.00%	24 Missing ⚠️
...ation/model/SharedApplicationOrganizationNode.java	0.00%	20 Missing ⚠️
...del/operation/ApplicationShareUpdateOperation.java	0.00%	19 Missing ⚠️
.../management/application/OrgApplicationManager.java	0.00%	14 Missing ⚠️
... and 10 more

❌ Your patch status has failed because the patch coverage (26.86%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #512      +/-   ##
============================================
- Coverage     55.49%   52.16%   -3.34%     
- Complexity     1980     2080     +100     
============================================
  Files           184      196      +12     
  Lines         11213    12840    +1627     
  Branches       1639     1936     +297     
============================================
+ Hits           6223     6698     +475     
- Misses         4460     5519    +1059     
- Partials        530      623      +93

Flag	Coverage Δ
unit	`36.06% <26.86%> (-0.37%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

AnuradhaSK · 2025-06-12T11:18:37Z

...carbon/identity/organization/management/application/util/OrgApplicationScimFilterParser.java

+        } else {
+            // The string does not match the strict pattern.
+            throw new OrganizationManagementClientException("Invalid filter string format",
+                    ERROR_CODE_INVALID_ORGANIZATION_PATH_FILTER.getDescription(),
+                    ERROR_CODE_INVALID_ORGANIZATION_PATH_FILTER.getCode());
+        }


Suggested change

} else {

// The string does not match the strict pattern.

throw new OrganizationManagementClientException("Invalid filter string format",

ERROR_CODE_INVALID_ORGANIZATION_PATH_FILTER.getDescription(),

ERROR_CODE_INVALID_ORGANIZATION_PATH_FILTER.getCode());

}

}

// The string does not match the strict pattern.

throw new OrganizationManagementClientException("Invalid filter string format",

ERROR_CODE_INVALID_ORGANIZATION_PATH_FILTER.getDescription(),

ERROR_CODE_INVALID_ORGANIZATION_PATH_FILTER.getCode());

AnuradhaSK · 2025-06-12T11:19:51Z

...carbon/identity/organization/management/application/util/OrgApplicationScimFilterParser.java

+ * The class enforces a strict format and provides a method to extract the organization ID
+ * and an optional path attribute (e.g., ".roles").
+ */
+public class OrgApplicationScimFilterParser {


change the class name excluding SCIM, it's fine to have in the comment

AnuradhaSK · 2025-06-12T11:20:47Z

...wso2/carbon/identity/organization/management/application/util/OrgApplicationManagerUtil.java

+                                                                                      error) {
+        return new OrganizationManagementClientException(error.getMessage(), error.getDescription(), error.getCode());


missing new line after method signature

AnuradhaSK · 2025-06-12T11:21:27Z

...wso2/carbon/identity/organization/management/application/util/OrgApplicationManagerUtil.java

+     *
+     * @param error The error enum.
+     * @param e     The error.
+     * @return OrganizationManagementServerException


complete the comment

AnuradhaSK · 2025-06-12T11:22:50Z

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java

 import org.wso2.carbon.identity.application.common.model.ApplicationBasicInfo;
 import org.wso2.carbon.identity.application.common.model.ServiceProvider;
 import org.wso2.carbon.identity.organization.management.application.model.SharedApplication;
+import org.wso2.carbon.identity.organization.management.application.model.SharedApplicationOrganizationNodePage;


Change the year range in copy right header.
Check other classes as well

Yasasr1 · 2025-06-12T14:11:51Z

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java

+            throws OrganizationManagementException {
+
+        throw new NotImplementedException(
+                "selectiveShareOrganizationApplication method is not implemented in " + this.getClass().getName());


Suggested change

"selectiveShareOrganizationApplication method is not implemented in " + this.getClass().getName());

"shareApplicationWithSelectedOrganizations method is not implemented in " + this.getClass().getName());

Yasasr1 · 2025-06-12T14:12:10Z

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java

+            throws OrganizationManagementException {
+
+        throw new NotImplementedException(
+                "generalOrganizationApplicationShare method is not implemented in " + this.getClass().getName());


Suggested change

"generalOrganizationApplicationShare method is not implemented in " + this.getClass().getName());

"shareApplicationWithAllOrganizations method is not implemented in " + this.getClass().getName());

Yasasr1 · 2025-06-12T14:22:50Z

.../org/wso2/carbon/identity/organization/management/application/OrgApplicationManagerImpl.java

+                if (!parentOrgId.equals(ownerOrgId)) {
+                    Optional<String> parentOrgSharedAppId = resolveSharedApp(
+                            mainApplication.getApplicationResourceId(), ownerOrgId, parentOrgId);
+                    if (!parentOrgSharedAppId.isPresent()) {


Is it possible for an application to be shared with the immediate parent organization, but not with higher-level parent organizations in the hierarchy?

AnuradhaSK · 2025-06-17T04:23:24Z

...arbon/identity/organization/management/application/listener/OrganizationCreationHandler.java

+                        parentOrgId);
+                if (!sharedAppIdOptional.isPresent()) {
+                    // No shared application found for the main application.
+                    // TODO: Should we throw an error here and fail org creation?


fix this todo

there are few more todos in the code

Copilot

Pull Request Overview

This pull request adds support for selective role sharing in application sharing, along with new operations and modifications to the DAO, listener, and manager interfaces to integrate resource sharing policies. Key changes include the introduction of SelectiveShareApplicationOperation, updates to role-sharing configuration classes and events, and enhancements in SQL query building for shared applications.

Reviewed Changes

Copilot reviewed 20 out of 33 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
ApplicationShareRolePolicy.java	Added builder and enum changes for role sharing policies.
OrganizationCreationHandler.java	Updated event handling to support new resource sharing exceptions and role sharing logic.
OrgApplicationMgtDAOImpl.java	Modified DAO methods to support filtering and new shared application retrieval queries.
OrgApplicationManager.java & ApplicationSharingManagerListener*.java	Introduced new methods for selective and policy-based application sharing.
Other files (various listener, model, constants, pom.xml)	Updated imports and integrated resource sharing policy support across components.

Copilot · 2025-06-19T16:09:44Z

...arbon/identity/organization/management/application/listener/OrganizationCreationHandler.java

+            if (ApplicationShareRolePolicy.Mode.SELECTED.ordinal() == roleSharingMode.ordinal()) {
+
+                // That mean the original application is available in the parent org. Not a fragment application.
+                for (SharedResourceAttribute sharedResourceAttribute : sharedResourceAttributes) {
+                    if (SharedAttributeType.ROLE.ordinal() != sharedResourceAttribute.getSharedAttributeType()
+                            .ordinal()) {


Avoid using ordinal() for enum comparisons as it can be error-prone if the enum order changes. It is recommended to use equals() for comparing enum values.

Suggested change

if (ApplicationShareRolePolicy.Mode.SELECTED.ordinal() == roleSharingMode.ordinal()) {

// That mean the original application is available in the parent org. Not a fragment application.

for (SharedResourceAttribute sharedResourceAttribute : sharedResourceAttributes) {

if (SharedAttributeType.ROLE.ordinal() != sharedResourceAttribute.getSharedAttributeType()

.ordinal()) {

if (ApplicationShareRolePolicy.Mode.SELECTED == roleSharingMode) {

// That mean the original application is available in the parent org. Not a fragment application.

for (SharedResourceAttribute sharedResourceAttribute : sharedResourceAttributes) {

if (SharedAttributeType.ROLE != sharedResourceAttribute.getSharedAttributeType()) {

Copilot · 2025-06-19T16:09:45Z

...2/carbon/identity/organization/management/application/dao/impl/OrgApplicationMgtDAOImpl.java

+        String filterQuery = filterQueryBuilder.getFilterQuery();
+        Map<String, String> filterAttributeValue = filterQueryBuilder.getFilterAttributeValue();
+        String placeholders = IntStream.range(0, sharedOrgIds.size())
+                .mapToObj(i -> ":" + SHARED_ORG_ID_PLACEHOLDER_PREFIX + i + ";")


The placeholder construction appends a semicolon to the parameter name, which may not match with the parameter keys when setting them. Remove the trailing semicolon to ensure the parameter names remain consistent.

Suggested change

.mapToObj(i -> ":" + SHARED_ORG_ID_PLACEHOLDER_PREFIX + i + ";")

.mapToObj(i -> ":" + SHARED_ORG_ID_PLACEHOLDER_PREFIX + i)

AnuradhaSK · 2025-06-22T13:06:05Z

...carbon/identity/organization/management/application/constant/OrgApplicationMgtConstants.java

+
+
+


remove unwanted newlines

HasiniSama · 2025-06-24T03:43:16Z

.../org/wso2/carbon/identity/organization/management/application/OrgApplicationManagerImpl.java

+        // Filter only valid child org configs that exist in the owner's child orgs using the map (O(1) lookups).
+        List<SelectiveShareApplicationOperation> filteredChildOrgConfigs = selectiveShareApplicationList.stream()
+                .filter(config -> subOrganizationIdsSet.contains(config.getOrganizationId()))
+                .collect(Collectors.toList());


It's better if we can add a debug log for invalid child org IDs sent.

HasiniSama · 2025-06-24T04:04:02Z

...2/carbon/identity/organization/management/application/util/OrgApplicationShareProcessor.java

+        // 3. Collect valid organization IDs and validate complete hierarchy paths.
+        Set<String> availableOrgIds = new HashSet<>(selectiveShareApplicationList.size());
+        Map<String, SelectiveShareApplicationOperation> orgIdToConfigMap = new HashMap<>(
+                selectiveShareApplicationList.size());


Instead of using a set for availableOrgIds we can use, orgIdToConfigMap.keySet() which returns the list of available organization IDs.

Map.keySet() returns a live view of the map’s keys. It's backed directly by the map — no copying or new allocation.

HasiniSama · 2025-06-24T05:08:10Z

...2/carbon/identity/organization/management/application/util/OrgApplicationShareProcessor.java

+                    parentConfig.getPolicy(),
+                    parentConfig.getRoleSharing()
+                );
+                effectiveConfigs.put(descendantId, inheritedConfig);


How can there be descendantId in the effectiveConfigs if we are iterating the Ids in the bfsOrder?

HasiniSama · 2025-06-24T06:19:50Z

.../org/wso2/carbon/identity/organization/management/application/OrgApplicationManagerImpl.java

+        // Find organizations that are no longer in the share config and need to be unshared.
+        List<SharedApplicationDO> orgsThatOmitted = sharedApplications.stream()
+                .filter(sharedApp -> !configOrgIds.contains(sharedApp.getOrganizationId()))
+                .collect(Collectors.toList());


This will omit all orgs not sent in the request and gets unshared along with it's child orgs. And if it is present in the orgsThatNeedToBeShared list, get reshared again. Can't we remove it from both lists if the config is unchanged? It could reduce the overhead, as a whole subtree would not get unshared/reshared.

HasiniSama · 2025-07-06T16:22:42Z

.../org/wso2/carbon/identity/organization/management/application/OrgApplicationManagerImpl.java

+//                    mainApplication.getApplicationResourceId(), sharedOrgId);
+//        }
+//        return sharedApplicationId;
+//    }


Was there a particular reason this part is commented out?

These methods can be removed

sahandilshan added 5 commits May 16, 2025 12:09

Add applciation configs for sharing policy

df33662

Add new application sharing model classes

22a9aec

Add new util classes required for new application sharing

cee390c

Update application sharing implementation to support selective role s…

41b8ddc

…haring

Update listeners according to new sharing mode

3322727

AnuradhaSK reviewed May 16, 2025

View reviewed changes

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java Show resolved Hide resolved

AnuradhaSK reviewed May 16, 2025

View reviewed changes

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed May 16, 2025

View reviewed changes

...so2/carbon/identity/organization/management/application/util/OrganizationShareProcessor.java Show resolved Hide resolved

sahandilshan force-pushed the selective-role-sharing branch from 7d1ddeb to 08e73a2 Compare May 28, 2025 05:45

sahandilshan commented May 30, 2025

View reviewed changes

Refactored the application sharing

732dfc9

- Provide the support to get application with filtering option. - Add new error message for application sharing - Moved application share operations models to a new package

sahandilshan force-pushed the selective-role-sharing branch from 08e73a2 to 732dfc9 Compare June 1, 2025 13:22

Refactor rolemgt listners and handlers

a56c8bf

sahandilshan marked this pull request as ready for review June 1, 2025 15:42

sahandilshan added 4 commits June 1, 2025 21:27

Add async status for app sharing with policy

45b232b

Use new application share methods instead of deprecated methods

2fa656e

Bump organization.management.core version

d93fcad

AnuradhaSK reviewed Jun 4, 2025

View reviewed changes

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java Show resolved Hide resolved

AnuradhaSK reviewed Jun 4, 2025

View reviewed changes

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Jun 4, 2025

View reviewed changes

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Jun 5, 2025

View reviewed changes

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java Show resolved Hide resolved

AnuradhaSK reviewed Jun 9, 2025

View reviewed changes

...java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java Outdated Show resolved Hide resolved

sahandilshan commented Jun 9, 2025

View reviewed changes

sahandilshan mentioned this pull request Jun 11, 2025

Selctive role sharing wso2/identity-api-server#906

Merged

Improve role sharing of application

dfdd654

- Convert logs to debug - Convert the ShareRoleMgt behaviour to previous behavior

sahandilshan force-pushed the selective-role-sharing branch from 045e793 to dfdd654 Compare June 11, 2025 11:25

Improve application sharing services

1c19eb4

- Improve logging - Add unit tests - Reduce using start tenant flows - Optimize to execute less DB queries

AnuradhaSK reviewed Jun 12, 2025

View reviewed changes

Yasasr1 reviewed Jun 12, 2025

View reviewed changes

Fix issue flows

73f2fca

sahandilshan force-pushed the selective-role-sharing branch from f99fb05 to 73f2fca Compare June 15, 2025 16:10

AnuradhaSK reviewed Jun 17, 2025

View reviewed changes

AnuradhaSK requested a review from Copilot June 19, 2025 16:09

Copilot AI reviewed Jun 19, 2025

View reviewed changes

AnuradhaSK reviewed Jun 22, 2025

View reviewed changes

HasiniSama reviewed Jun 24, 2025

View reviewed changes

HasiniSama reviewed Jul 6, 2025

View reviewed changes

This was referenced Jul 9, 2025

Provide API support for selective role sharing with application sharing wso2/identity-api-server#929

Merged

Provide service for selective role sharing with application sharing #525

Merged

HasiniSama merged commit 73f2fca into wso2-extensions:main Jul 22, 2025
2 of 4 checks passed

		error) {
		return new OrganizationManagementClientException(error.getMessage(), error.getDescription(), error.getCode());

	"selectiveShareOrganizationApplication method is not implemented in " + this.getClass().getName());
	"shareApplicationWithSelectedOrganizations method is not implemented in " + this.getClass().getName());

	"generalOrganizationApplicationShare method is not implemented in " + this.getClass().getName());
	"shareApplicationWithAllOrganizations method is not implemented in " + this.getClass().getName());

	.mapToObj(i -> ":" + SHARED_ORG_ID_PLACEHOLDER_PREFIX + i + ";")
	.mapToObj(i -> ":" + SHARED_ORG_ID_PLACEHOLDER_PREFIX + i)

Provide support for selective role sharing with application sharing #512

Provide support for selective role sharing with application sharing #512

Uh oh!

Conversation

sahandilshan commented May 16, 2025

Purpose

TODOs:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HasiniSama Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

codecov bot commented Jun 11, 2025 •

edited

Loading

HasiniSama Jun 24, 2025 •

edited

Loading