Introduce User Sharing API v2 #1044
Conversation
WalkthroughAdds a new v2 organization user-sharing module: Maven module registration and POM, OpenAPI v3 specification, generated API types and service layer (core, factory, impl), common-holder/constants updates to support V2, and version/property bumps in parent POMs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as UsersApiServiceImpl
participant Factory as UsersApiServiceCoreFactory
participant Core as UsersApiServiceCore
participant Handler as UserSharingPolicyHandlerServiceV2
participant Holder as UserSharingMgtServiceHolder
note over Factory,Holder: eager static initialization at class load
Factory->>Holder: getUserSharingPolicyHandlerServiceV2()
Holder-->>Factory: UserSharingPolicyHandlerServiceV2
Factory->>Core: new UsersApiServiceCore(handler)
Client->>API: HTTP request (e.g., POST /users/share)
API->>Core: delegate request
Core->>Handler: validate & execute operation
Handler-->>Core: result / error
Core->>API: build HTTP response (202/200/error)
API-->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![wso2-engineering[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java
Outdated
Show resolved
Hide resolved
...api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java (1)
30-49: Static initializer hides the intendedIllegalStateExceptionfrom callersThrowing
IllegalStateExceptionin the static block will surface to callers asExceptionInInitializerError, so theIllegalStateExceptioncatch inUsersApiServiceImplwill never see it. If you want the API layer to consistently getIllegalStateExceptionfrom the factory, consider moving the null‑check and construction intogetUsersApiServiceCore()(lazy init) instead of the static block.One possible refactor:
- private static final UsersApiServiceCore SERVICE; - - static { - UserSharingPolicyHandlerService userSharingPolicyHandlerService = UserSharingMgtServiceHolder - .getUserSharingPolicyHandlerService(); - if (userSharingPolicyHandlerService == null) { - throw new IllegalStateException("UserSharingPolicyHandlerService is not available from the OSGi context."); - } - SERVICE = new UsersApiServiceCore(userSharingPolicyHandlerService); - } + private static volatile UsersApiServiceCore service; @@ - public static UsersApiServiceCore getUsersApiServiceCore() { - - return SERVICE; - } + public static UsersApiServiceCore getUsersApiServiceCore() { + + if (service == null) { + synchronized (UsersApiServiceCoreFactory.class) { + if (service == null) { + UserSharingPolicyHandlerService userSharingPolicyHandlerService = + UserSharingMgtServiceHolder.getUserSharingPolicyHandlerService(); + if (userSharingPolicyHandlerService == null) { + throw new IllegalStateException( + "UserSharingPolicyHandlerService is not available from the OSGi context."); + } + service = new UsersApiServiceCore(userSharingPolicyHandlerService); + } + } + } + return service; + }components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java (1)
43-50: Constructor’sIllegalStateExceptioncatch won’t see static‑init failures from the factoryGiven the current factory implementation, a missing
UserSharingPolicyHandlerServicewill throwIllegalStateExceptionfrom a static initializer, which reaches this constructor asExceptionInInitializerError, bypassing theIllegalStateExceptioncatch.If you don’t refactor the factory, consider broadening the catch here; if you do refactor as suggested in the factory comment, this constructor will then work as intended.
Example if you keep the static initializer:
- try { - this.usersApiServiceCore = UsersApiServiceCoreFactory.getUsersApiServiceCore(); - } catch (IllegalStateException e) { - throw new RuntimeException(ERROR_INITIATING_USERS_API_SERVICE.getMessage(), e); - } + try { + this.usersApiServiceCore = UsersApiServiceCoreFactory.getUsersApiServiceCore(); + } catch (IllegalStateException | ExceptionInInitializerError e) { + throw new RuntimeException(ERROR_INITIATING_USERS_API_SERVICE.getMessage(), e); + }components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml (3)
455-465: UnboundeduserIdsarray may be problematic; consider addingmaxItems
UserCriteria.userIdsis required but unconstrained in size. For bulk operations like share/unshare, this can invite very large payloads and DoS‑type scenarios. If the backend already enforces a sensible cap, it’s worth reflecting that here with amaxItems(and optionallyminItems) so clients have a clear contract.For example (adjust the limit to match server behavior):
userIds: type: array description: List of user IDs. items: type: string + maxItems: 1000
435-449: Clarify that BasicAuth/OAuth2 credentials must only be used over TLSStatic analysis flagged the securitySchemes because HTTP auth can expose credentials over cleartext if used on plain HTTP. Your examples and OAuth2 URLs already use
https, but the spec itself doesn’t state the TLS requirement.I’d recommend explicitly documenting that these endpoints are only supported over HTTPS (and that HTTP is not allowed in production), to satisfy tooling and avoid misconfiguration.
753-765: AlignError.codeexample prefix with actual error prefixThe
Error.codeexample uses"US-00000", whileUserSharingMgtConstants.ERROR_PREFIXis"USM-". To avoid confusion for integrators, it’s better if the example matches the real prefix.- code: - type: string - example: "US-00000" + code: + type: string + example: "USM-00000"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (20)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/UsersApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/UsersApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceFactory.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/Error.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/Link.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/ProcessSuccessResponse.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/RoleAssignment.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/RoleShareConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/RoleShareConfigAudience.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/SharingMode.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserCriteria.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserOrgShareConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserShareAllRequestBody.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserShareSelectedRequestBody.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserSharedOrganization.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserSharedOrganizationsResponse.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserSharingPatchOperation.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserSharingPatchRequest.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserUnshareAllRequestBody.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserUnshareSelectedRequestBody.javais excluded by!**/gen/**
📒 Files selected for processing (6)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/pom.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java (3)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (1)
UsersApiServiceCore(35-82)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java (1)
UsersApiServiceCoreFactory(28-50)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (1)
UserSharingMgtConstants(24-101)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml
[medium] 460-465: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 438-441: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/pom.xml (1)
31-35: V2 module wiring in parent POM looks consistentAdding the v2 module alongside v1 and
commonmatches the expected multi‑module structure for versioned APIs.components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/pom.xml (1)
31-172: POM configuration for the v2 module looks fine; just ensure generated sources are presentDependencies and build plugins align with other JAX‑RS/OpenAPI modules (provided scopes,
build-helper-maven-pluginforsrc/gen/java, Java 8 compiler settings). From a POM perspective this looks good.Just make sure that the generated sources for
src/gen/java(e.g.,UsersApiServiceinterface) are either checked in or produced by your build pipeline, since the openapi‑generator plugin here is commented out.
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR introduces a new versioned User Sharing API (v2) under /api/server/v2/users/** to address limitations of the existing v1 endpoints. The new API follows the Application Sharing pattern and enables incremental role assignment with selective sharing across hierarchical organizations.
Key Changes:
- Added 6 new REST endpoints for user sharing operations (share, unshare, patch, and query)
- Introduced role assignment modes (NONE, SELECTED) with SCIM-style PATCH semantics for incremental updates
- Implemented paginated retrieval of shared organizations with per-organization role visibility
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml (parent) | Added v2 module to the parent POM build configuration |
| organization-user-share-v2.yaml | OpenAPI 3.0 specification defining the v2 API endpoints, schemas, and examples |
| UsersApiServiceImpl.java | Implementation delegating API calls to the core service layer |
| UsersApiServiceCore.java | Core business logic with placeholder methods for all v2 operations |
| UsersApiServiceCoreFactory.java | Factory pattern for initializing the core service with OSGi dependencies |
| Model classes (20 files) | Auto-generated DTOs from OpenAPI spec for request/response handling |
| UsersApi.java | JAX-RS resource class with endpoint mappings and validation |
| UsersApiService.java | Service interface defining the API contract |
| UsersApiServiceFactory.java | Factory for service instantiation |
| pom.xml (v2 module) | Maven configuration with dependencies and build plugins |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserCriteria.java
Outdated
Show resolved
Hide resolved
...arbon/identity/api/server/organization/user/sharing/management/v2/model/RoleShareConfig.java
Outdated
Show resolved
Hide resolved
...api/server/organization/user/sharing/management/v2/model/UserUnshareSelectedRequestBody.java
Outdated
Show resolved
Hide resolved
...pi/server/organization/user/sharing/management/v2/model/UserSharedOrganizationsResponse.java
Outdated
Show resolved
Hide resolved
...dentity/api/server/organization/user/sharing/management/v2/model/UserSharedOrganization.java
Outdated
Show resolved
Hide resolved
...dentity/api/server/organization/user/sharing/management/v2/model/ProcessSuccessResponse.java
Outdated
Show resolved
Hide resolved
...org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/Error.java
Outdated
Show resolved
Hide resolved
...o2/carbon/identity/api/server/organization/user/sharing/management/v2/model/SharingMode.java
Outdated
Show resolved
Hide resolved
...entity/api/server/organization/user/sharing/management/v2/model/RoleShareConfigAudience.java
Outdated
Show resolved
Hide resolved
...wso2/carbon/identity/api/server/organization/user/sharing/management/v2/UsersApiService.java
Outdated
Show resolved
Hide resolved
...org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/Error.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...r.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...wso2/carbon/identity/api/server/organization/user/sharing/management/v2/UsersApiService.java
Outdated
Show resolved
Hide resolved
...va/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/UsersApi.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pom.xml (1)
967-968: Duplicate property definition forcarbon.identity.framework.version.Lines 967 and 968 both define
carbon.identity.framework.version. Maven will use the last declared value (7.8.610), making the SNAPSHOT declaration on line 967 effectively dead code. If the intent is to use7.8.610, remove the duplicate SNAPSHOT line to avoid confusion.🔎 Proposed fix
- <carbon.identity.framework.version>7.8.596-SNAPSHOT</carbon.identity.framework.version> <carbon.identity.framework.version>7.8.610</carbon.identity.framework.version>
🧹 Nitpick comments (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (1)
296-300: Consider sanitizing or omitting userId from error logs.The userId is logged directly in the error message on line 298. Depending on your logging policy and whether userId could contain PII or be used for correlation attacks, consider using a hashed/masked value or omitting it from production logs.
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml (1)
478-484: Consider addingmaxItemsconstraint to prevent unbounded array requests.The
userIdsarray (and other arrays likeorganizations,orgIds,Operations) don't specify a maximum number of items. This could allow clients to submit extremely large payloads, potentially causing performance issues or DoS.🔎 Proposed fix for UserCriteria
UserCriteria: type: object description: Criteria for selecting users to share/unshare. properties: userIds: type: array description: List of user IDs. + maxItems: 100 items: type: string required: - userIdsApply similar constraints to other arrays (
organizations,orgIds,Operations,roles) based on expected usage limits.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (20)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/UsersApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/UsersApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceFactory.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/Error.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/Link.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/ProcessSuccessResponse.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/RoleAssignment.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/RoleShareConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/RoleShareConfigAudience.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/SharingMode.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserCriteria.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserOrgShareConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserShareAllRequestBody.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserShareSelectedRequestBody.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserSharedOrganization.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserSharedOrganizationsResponse.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserSharingPatchOperation.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserSharingPatchRequest.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserUnshareAllRequestBody.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/UserUnshareSelectedRequestBody.javais excluded by!**/gen/**
📒 Files selected for processing (9)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/UserSharingMgtServiceHolder.java(3 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java(2 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/pom.xml(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml(1 hunks)pom.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/pom.xml
🧰 Additional context used
🧬 Code graph analysis (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/UserSharingMgtServiceHolder.java (1)
UserSharingMgtServiceHolder(28-67)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (1)
UsersApiServiceCore(92-869)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java (3)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (1)
UsersApiServiceCore(92-869)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java (1)
UsersApiServiceCoreFactory(30-58)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (1)
UserSharingMgtConstants(24-107)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml
[medium] 479-484: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 457-460: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (9)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/pom.xml (1)
21-26: Parent version update looks consistent.The parent version update to
1.3.229-SNAPSHOTaligns with the multi-module build structure for the new v2 API module.components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (1)
33-34: New constants for patch operation are well-structured.The
RESPONSE_DETAIL_USER_SHARE_PATCHconstant andINVALID_USER_SHARE_PATCH_REQUEST_BODYerror enum follow the existing patterns and maintain sequential error codes. The error messages are clear and actionable.Also applies to: 65-68
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/UserSharingMgtServiceHolder.java (1)
41-46: V2 service holder follows the established V1 pattern.The implementation mirrors the existing V1 pattern using the initialization-on-demand holder idiom, ensuring thread-safe lazy initialization. The structure is consistent with the existing codebase.
Note that if the OSGi service is not registered,
getUserSharingPolicyHandlerServiceV2()will returnnull. Ensure callers handle this case appropriately (the factory should validate this).Also applies to: 57-66
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (2)
92-105: Constructor and dependency injection look good.The constructor properly accepts the V2 service dependency and assigns it to a final field, following good practices for immutability.
709-735: Pagination link building is well-implemented.The cursor-based pagination with Base64-encoded cursors follows good practices. The fallback to relative URLs when
UriInfois unavailable ensures robustness.components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml (2)
456-468: BasicAuth security scheme is acceptable with HTTPS.The static analysis flagged BasicAuth (CKV_OPENAPI_3), but since the server URL explicitly uses
https://, credentials are encrypted in transit. This is acceptable, though OAuth2 is the preferred mechanism for production use.
1-30: OpenAPI specification structure and versioning look good.The v2 API spec is well-structured with clear descriptions, proper versioning in the server URL, and appropriate security schemes.
pom.xml (1)
1001-1002: Version bump to 2.0.43 should be verified against the artifact repository and V2 API requirements.The update to
org.wso2.carbon.identity.organization.management.versionfrom its previous version to 2.0.43 requires verification that:
- Version 2.0.43 is a released version in the WSO2 repository
- It includes the V2 service interfaces referenced in the code changes
Additionally, review the pom.xml for the duplicate
carbon.identity.framework.versionproperty definition mentioned in the original analysis.components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java (1)
59-95: LGTM: Clean delegation pattern.The implementation correctly delegates all API operations to the core service, maintaining a clear separation between the API layer and business logic.
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java
Show resolved
Hide resolved
...r.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (1)
65-75: Consider aligning error naming conventions for consistency.The first error constant (line 65) follows the established
INVALID_prefix pattern used by all existing client validation errors (60000-60004). However, the two subsequent errors (lines 69, 72) introduce anERROR_prefix, creating inconsistency.Additionally,
ERROR_MISSING_USER_CRITERIAis generic and doesn't indicate its specific context (patch operations). For clarity and consistency with operation-specific error names (e.g.,INVALID_SELECTIVE_USER_SHARE_REQUEST_BODY), consider:
INVALID_USER_SHARE_PATCH_MISSING_USER_CRITERIAINVALID_USER_SHARE_PATCH_PATHThis maintains the
INVALID_prefix pattern and makes the context explicit.Proposed naming alignment
- ERROR_MISSING_USER_CRITERIA("60006", - "Missing user criteria in the request body.", - "The user criteria is missing in the request body. Please provide the user criteria to proceed."), - ERROR_UNSUPPORTED_USER_SHARE_PATCH_PATH("60007", - "Unsupported user share patch path.", + INVALID_USER_SHARE_PATCH_MISSING_USER_CRITERIA("60006", + "Missing user criteria in patch request body.", + "The user criteria is missing in the patch request body. Please provide the user criteria to proceed."), + INVALID_USER_SHARE_PATCH_PATH("60007", + "Invalid user share patch path.", "The provided patch path to update attributes of shared user is not supported. " + "Please provide a valid patch path."),components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml (2)
382-390: Add enum constraint toattributesparameter for validation.The
attributesparameter description states "Supported values:roles,sharingMode" but the schema lacks an enum constraint. Without this, the OpenAPI spec accepts any string value (e.g.,attributes=invalidValue), deferring validation to runtime.A previous review suggested adding both values to an enum, which would provide client-side validation and better API documentation.
🔎 Proposed fix
- in: query name: attributes required: false schema: type: string + enum: + - roles + - sharingMode example: "roles" description: |- Specifies the required parameters in the response. Supported values: `roles`, `sharingMode`.
472-482: Consider adding maxItems constraint to userIds array.Static analysis flags that the
userIdsarray lacks amaxItemsconstraint, which could allow arbitrarily large batch requests that might cause performance issues or DoS vulnerabilities.While backend validation likely enforces reasonable limits, adding
maxItemsto the schema provides:
- Early client-side validation
- Clear documentation of API limits
- Defense-in-depth
Example constraint
properties: userIds: type: array description: List of user IDs. + maxItems: 100 items: type: stringAdjust the limit based on your backend's batch processing capabilities.
Based on static analysis hints.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/UsersApi.javais excluded by!**/gen/**
📒 Files selected for processing (4)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java(2 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java(1 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/impl/UsersApiServiceImpl.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-19T07:42:30.680Z
Learnt from: BimsaraBodaragama
Repo: wso2/identity-api-server PR: 1044
File: components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java:35-47
Timestamp: 2025-12-19T07:42:30.680Z
Learning: In the organization user sharing management API modules, factory classes use eager static initialization (static blocks) to fail fast if required OSGi services are not available. This is an intentional design pattern for consistency across API service factories in this module.
Applied to files:
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (3)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/Link.java (1)
Link(33-118)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/RoleShareConfigAudience.java (1)
RoleShareConfigAudience(33-124)components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/SharingMode.java (1)
SharingMode(34-157)
🪛 Checkov (3.2.334)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml
[medium] 477-482: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 455-458: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
🔇 Additional comments (5)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (1)
33-34: LGTM! Consistent success message constant.The new constant follows the existing naming and messaging pattern for async operation responses.
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (2)
104-107: LGTM! Clean dependency injection and error handling.The constructor-based dependency injection and consistent error handling pattern across all public methods effectively distinguish client errors (400) from server errors (500). The validation logic and exception handling align well with REST API best practices.
Also applies to: 118-139, 148-168
714-779: Well-designed pagination link builder with proper fallback.The pagination implementation correctly handles both absolute URI construction (using JAX-RS
UriInfo) and falls back to relative link building when URI context is unavailable. The cursor encoding and proper query parameter handling ensure robust pagination support.components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/resources/organization-user-share-v2.yaml (2)
454-466: BasicAuth security scheme requires HTTPS enforcement.Static analysis correctly flags that BasicAuth transmits credentials in cleartext (base64-encoded). While the server URL specifies
https://(line 20), the OpenAPI spec doesn't enforce HTTPS at the scheme level.In production deployments, ensure TLS termination at the API gateway/reverse proxy level is mandatory for endpoints using BasicAuth. Consider documenting this requirement in the API description or deployment guides.
Based on static analysis hints.
1-26: Excellent API design aligned with Application Sharing patterns.The v2 API successfully addresses the stated objectives:
- Clear separation of selective vs. all-org sharing operations
- SCIM-style PATCH operations for incremental role updates
- Cursor-based pagination for shared organization listing
- Consistent 202 Accepted responses for async processing
- Comprehensive request/response schemas with examples
The alignment with the Application Sharing model (mentioned in lines 7, 330) provides consistency across the platform, improving developer experience.
Also applies to: 31-175, 692-753
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (1)
122-125: Consider adding more context to debug logs.The debug logs currently only log the tenant domain. Based on past reviewer feedback, consider including additional context such as the number of users being shared, organizations involved, or operation type to aid troubleshooting.
💡 Example enhancement
if (LOG.isDebugEnabled()) { - LOG.debug("Selective user sharing API v2 invoked form tenantDomain: " + - PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain()); + LOG.debug("Selective user sharing API v2 invoked for tenantDomain: " + + PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain() + + ", targetOrganizations: " + userShareSelectedRequestBody.getOrganizations().size()); }Also applies to: 154-157, 185-188, 217-220, 249-252
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java(2 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: BimsaraBodaragama
Repo: wso2/identity-api-server PR: 1044
File: components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java:35-47
Timestamp: 2025-12-19T07:42:30.680Z
Learning: In the organization user sharing management API modules, factory classes use eager static initialization (static blocks) to fail fast if required OSGi services are not available. This is an intentional design pattern for consistency across API service factories in this module.
🔇 Additional comments (4)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (2)
33-34: LGTM! Well-aligned constant for PATCH operations.The new response constant follows the established naming and messaging patterns, clearly indicating the asynchronous nature of the patch operation.
65-79: LGTM! Comprehensive error coverage for PATCH validation.The four new error enums provide clear, granular validation messages for the V2 PATCH operations. The sequential error codes (60005-60008), descriptive naming, and actionable error descriptions follow the existing conventions consistently.
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (2)
106-109: LGTM: Constructor properly initializes the service dependency.The constructor follows the dependency injection pattern and aligns with the factory's eager initialization approach mentioned in the learnings.
759-792: LGTM: URI building and encoding are implemented correctly.The pagination link construction gracefully handles missing
UriInfoby falling back to relative links, and URL encoding properly handles the guaranteed UTF-8 support with appropriate exception handling.Also applies to: 839-847
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (2)
329-330: Remove self-explanatory comments.Several comments throughout the DTO building methods simply restate what the code does (e.g., "Set user criteria", "Set policy and roles"). These comments don't add value and can be safely removed for cleaner code.
Based on learnings, the reviewer previously requested removal of comments where code is self-explanatory.
Also applies to: 334-334, 360-361, 365-366, 384-385, 389-389, 406-407
811-836: Consider refactoring manual URL construction to reduce fragility.The
buildRelativeSharedOrgsPageLinkmethod constructs URLs through string concatenation and manual query parameter building. WhileurlEncodeis used, this approach is more error-prone than using a proper URL builder. IfUriInfois unavailable in certain contexts, consider alternatives like:
- Ensuring
UriInfois always available through dependency injection- Using
UriBuilderdirectly instead of string concatenation- Centralizing URL construction logic to avoid duplication
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java(2 hunks)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: BimsaraBodaragama
Repo: wso2/identity-api-server PR: 1044
File: components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java:35-47
Timestamp: 2025-12-19T07:42:30.680Z
Learning: In the organization user sharing management API modules, factory classes use eager static initialization (static blocks) to fail fast if required OSGi services are not available. This is an intentional design pattern for consistency across API service factories in this module.
📚 Learning: 2025-12-19T07:42:30.680Z
Learnt from: BimsaraBodaragama
Repo: wso2/identity-api-server PR: 1044
File: components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java:35-47
Timestamp: 2025-12-19T07:42:30.680Z
Learning: In the organization user sharing management API modules, factory classes use eager static initialization (static blocks) to fail fast if required OSGi services are not available. This is an intentional design pattern for consistency across API service factories in this module.
Applied to files:
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
🔇 Additional comments (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (2)
33-34: LGTM!The new response constant follows the established pattern and provides a clear success message for the patch operation.
65-82: LGTM!The new error message enum entries are well-structured and follow the established patterns in this file. The error codes are properly sequential (60005-60009), and the descriptions provide clear, actionable guidance for API consumers.
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (1)
69-85: Consider aligning error enum constant names with existing naming convention.The five new error enum constants use the "ERROR_" prefix, while all existing client error constants in this enum use the "INVALID_" prefix (e.g.,
INVALID_UUID_FORMAT,INVALID_SELECTIVE_USER_SHARE_REQUEST_BODY). For consistency and maintainability, consider renaming:
ERROR_MISSING_USER_CRITERIA→INVALID_REQUEST_MISSING_USER_CRITERIAERROR_UNSUPPORTED_USER_SHARE_PATCH_PATH→INVALID_USER_SHARE_PATCH_PATHERROR_EMPTY_USER_SHARE_PATCH_PATH→INVALID_EMPTY_USER_SHARE_PATCH_PATHERROR_UNSUPPORTED_USER_SHARE_POLICY→INVALID_USER_SHARE_POLICYERROR_MISSING_USER_IDS→INVALID_REQUEST_MISSING_USER_IDS🔎 Proposed naming consistency refactor
- ERROR_MISSING_USER_CRITERIA("60006", + INVALID_REQUEST_MISSING_USER_CRITERIA("60006", "Missing user criteria in the request body.", "The user criteria is missing in the request body. Please provide the user criteria to proceed."), - ERROR_UNSUPPORTED_USER_SHARE_PATCH_PATH("60007", + INVALID_USER_SHARE_PATCH_PATH("60007", "Unsupported user share patch path.", "The provided patch path to update attributes of shared user is not supported. " + "Please provide a valid patch path."), - ERROR_EMPTY_USER_SHARE_PATCH_PATH("60008", + INVALID_EMPTY_USER_SHARE_PATCH_PATH("60008", "Empty user share patch path.", "The provided patch path to update attributes of shared user is empty. " + "Please provide a valid patch path."), - ERROR_UNSUPPORTED_USER_SHARE_POLICY("60009", + INVALID_USER_SHARE_POLICY("60009", "Unsupported user share policy.", "The provided user share policy is not supported. Please provide a valid user share policy."), - ERROR_MISSING_USER_IDS("60010", + INVALID_REQUEST_MISSING_USER_IDS("60010", "Missing user IDs in the request body.", "The user ID is missing in the request body. Please provide the user ID to proceed."),
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.javacomponents/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: BimsaraBodaragama
Repo: wso2/identity-api-server PR: 1044
File: components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java:35-47
Timestamp: 2025-12-19T07:42:41.255Z
Learning: In the organization user sharing management API modules, factory classes use eager static initialization (static blocks) to fail fast if required OSGi services are not available. This is an intentional design pattern for consistency across API service factories in this module.
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (3)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (1)
UserSharingMgtConstants(24-124)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/Link.java (1)
Link(33-118)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/RoleShareConfigAudience.java (1)
RoleShareConfigAudience(33-124)
🔇 Additional comments (5)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (2)
33-34: LGTM! New response detail constant aligns with existing patterns.The constant follows the established naming convention and provides clear feedback for the PATCH operation.
65-68: LGTM! Error message follows established naming convention.The
INVALID_USER_SHARE_PATCH_REQUEST_BODYconstant correctly uses the "INVALID_" prefix consistent with existing client error enum constants.components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (3)
663-665: Add null check before calling toString() on sharedType.Line 664 calls
orgDO.getSharedType().toString()without checking ifgetSharedType()returns null. This will throw aNullPointerExceptionif the shared type is not set.🔎 Proposed fix
- if (orgDO.getSharedType() != null) { - org.setSharedType(orgDO.getSharedType().toString()); - } + if (orgDO.getSharedType() != null) { + org.setSharedType(orgDO.getSharedType().toString()); + }Note: The null check is already present, so this code is actually safe. No change needed.
704-706: Add null check before calling toString() on mode.Line 705 calls
raDO.getMode().toString()without checking ifgetMode()returns null. This will throw aNullPointerExceptionif the mode is not set.🔎 Proposed fix
RoleAssignmentDO raDO = modeDO.getRoleAssignment(); if (raDO != null) { RoleAssignment ra = new RoleAssignment(); - if (raDO.getMode() != null) { - ra.setMode(RoleAssignment.ModeEnum.fromValue(raDO.getMode().toString())); - } + if (raDO.getMode() != null) { + ra.setMode(RoleAssignment.ModeEnum.fromValue(raDO.getMode().toString())); + }Note: The null check is already present at line 704, so this code is actually safe. No change needed.
697-699: Code is correct as written; no issues found.The
setPolicymethod inSharingMode(line 59) expects aStringparameter, not an enum. The code correctly passesmodeDO.getPolicy().getValue(), which returns aString. The null check on line 697 prevents any NPE. NoSharingMode.PolicyEnumclass exists—the nestedModeEnumis specific toRoleAssignmentand is correctly used separately at line 705 withfromValue().Likely an incorrect or invalid review comment.
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (4)
289-291: PII leakage: Remove user ID from debug logs.The debug log includes the
userIdparameter, which is personally identifiable information (PII). Logging user identifiers can violate privacy policies and compliance requirements (GDPR, CCPA).🔎 Proposed fix
if (LOG.isDebugEnabled()) { - LOG.debug("Get user shared organizations API v2 invoked form tenantDomain: " + - PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain() + " of user: " + userId); + LOG.debug("Get user shared organizations API v2 invoked from tenantDomain: " + + PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain()); }
293-296: Validate UUID format, not just null.The validation only checks if
userIdis null, but the error constantERROR_MISSING_USER_IDS(notINVALID_UUID_FORMATas in past comments) suggests missing input. However, a malformed UUID string will pass this check and potentially cause issues downstream when parsed.🔎 Proposed fix
if (userId == null) { return Response.status(Response.Status.BAD_REQUEST) .entity(buildErrorResponse(makeRequestError(ERROR_MISSING_USER_IDS))).build(); } + try { + UUID.fromString(userId); + } catch (IllegalArgumentException e) { + return Response.status(Response.Status.BAD_REQUEST) + .entity(buildErrorResponse(makeRequestError(ERROR_MISSING_USER_IDS))).build(); + }
311-311: PII leakage: Remove user ID from error logs.Similar to the debug log concern, the error log includes
userIdwhich may be PII. Consider logging without the user identifier to comply with privacy requirements.🔎 Proposed fix
- LOG.error("Error occurred while retrieving organizations shared with user: " + userId, e); + LOG.error("Error occurred while retrieving organizations shared with user.", e);
336-346: Add null check before iterating organizations collection.Line 336 iterates over
userShareSelectedRequestBody.getOrganizations()without first checking if it returns null. While line 337 checks individualorgDetailitems, the collection itself could be null, causing aNullPointerException.🔎 Proposed fix
// Set organizations details - policy and roles for each organization. List<SelectiveUserShareOrgDetailsV2DO> organizationsList = new ArrayList<>(); - for (UserOrgShareConfig orgDetail : userShareSelectedRequestBody.getOrganizations()) { + List<UserOrgShareConfig> organizations = userShareSelectedRequestBody.getOrganizations(); + if (organizations != null) { + for (UserOrgShareConfig orgDetail : organizations) { if (orgDetail != null) { SelectiveUserShareOrgDetailsV2DO selectiveUserShareOrgDetailsV2DO = new SelectiveUserShareOrgDetailsV2DO(); selectiveUserShareOrgDetailsV2DO.setOrganizationId(orgDetail.getOrgId()); selectiveUserShareOrgDetailsV2DO.setPolicy(resolvePolicy(orgDetail.getPolicy())); selectiveUserShareOrgDetailsV2DO.setRoleAssignments( buildRoleAssignmentFromRequest(orgDetail.getRoleAssignment())); organizationsList.add(selectiveUserShareOrgDetailsV2DO); } + } }
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (1)
743-755: Add defensive null checks in role mapping.Lines 746-750 assume
roleDOand its properties are non-null. If the backend returns incomplete data, this could throw NPE. Consider adding defensive checks, especially if this data flows from external systems.🔎 Proposed fix
private RoleShareConfig toApiRole(RoleWithAudienceDO roleDO) { RoleShareConfig role = new RoleShareConfig(); - role.setDisplayName(roleDO.getRoleName()); + if (roleDO != null) { + role.setDisplayName(roleDO.getRoleName()); - RoleShareConfigAudience audience = new RoleShareConfigAudience(); - audience.setDisplay(roleDO.getAudienceName()); - audience.setType(roleDO.getAudienceType()); + RoleShareConfigAudience audience = new RoleShareConfigAudience(); + audience.setDisplay(roleDO.getAudienceName()); + audience.setType(roleDO.getAudienceType()); - role.setAudience(audience); + role.setAudience(audience); + } return role; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-19T07:42:41.255Z
Learnt from: BimsaraBodaragama
Repo: wso2/identity-api-server PR: 1044
File: components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java:35-47
Timestamp: 2025-12-19T07:42:41.255Z
Learning: In the organization user sharing management API modules, factory classes use eager static initialization (static blocks) to fail fast if required OSGi services are not available. This is an intentional design pattern for consistency across API service factories in this module.
Applied to files:
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
🧬 Code graph analysis (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.common/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/common/constants/UserSharingMgtConstants.java (1)
UserSharingMgtConstants(24-124)components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/gen/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/model/RoleShareConfigAudience.java (1)
RoleShareConfigAudience(33-124)
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (4)
289-291: Remove user ID from debug logs to prevent PII leakage.The debug log includes the
userIdparameter, which may be considered PII and could violate privacy policies (GDPR, CCPA). Consider logging only non-sensitive context such as tenant domain.
293-296: Validate UUID format, not just null.The validation only checks if
userIdis null, but the error codeERROR_MISSING_USER_IDSsuggests that format validation should also occur. A non-null but malformed UUID will pass this check and may cause issues downstream.🔎 Proposed fix
if (userId == null) { return Response.status(Response.Status.BAD_REQUEST) - .entity(buildErrorResponse(makeRequestError(ERROR_MISSING_USER_IDS))).build(); + .entity(buildErrorResponse(makeRequestError(ERROR_MISSING_USER_IDS))).build(); } + try { + UUID.fromString(userId); + } catch (IllegalArgumentException e) { + return Response.status(Response.Status.BAD_REQUEST) + .entity(buildErrorResponse(makeRequestError(ERROR_MISSING_USER_IDS))).build(); + }
311-311: Remove user ID from error logs to prevent PII leakage.Similar to the debug log concern, the error log includes
userIdwhich may be PII. Remove the user identifier from the log message to comply with privacy requirements.
556-571: Add null check for role audience to prevent NPE.Lines 564-565 directly access
role.getAudience().getDisplay()and.getType()without checking ifgetAudience()returns null. If the audience is null, this will throw aNullPointerException.🔎 Proposed fix
if (roles != null) { for (RoleShareConfig role : roles) { if (role != null) { RoleWithAudienceDO roleDetails = new RoleWithAudienceDO(); roleDetails.setRoleName(role.getDisplayName()); - roleDetails.setAudienceName(role.getAudience().getDisplay()); - roleDetails.setAudienceType(role.getAudience().getType()); + RoleShareConfigAudience audience = role.getAudience(); + if (audience != null) { + roleDetails.setAudienceName(audience.getDisplay()); + roleDetails.setAudienceType(audience.getType()); + } rolesList.add(roleDetails); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-19T07:42:41.255Z
Learnt from: BimsaraBodaragama
Repo: wso2/identity-api-server PR: 1044
File: components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java:35-47
Timestamp: 2025-12-19T07:42:41.255Z
Learning: In the organization user sharing management API modules, factory classes use eager static initialization (static blocks) to fail fast if required OSGi services are not available. This is an intentional design pattern for consistency across API service factories in this module.
Applied to files:
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java (2)
311-311: Remove user ID from error log.The error log includes
userIdwhich may be PII. Consider logging without the user identifier to comply with privacy requirements.🔎 Proposed fix
- LOG.error("Error occurred while retrieving organizations shared with user: " + userId, e); + LOG.error("Error occurred while retrieving organizations shared with user.", e);
290-291: Remove user ID from debug log and fix typo.The debug log includes the
userIdparameter which may be considered PII and should not be logged to comply with privacy requirements. Additionally, "form" should be "from".🔎 Proposed fix
if (LOG.isDebugEnabled()) { - LOG.debug("Get user shared organizations API v2 invoked form tenantDomain: " + - PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain() + " of user: " + userId); + LOG.debug("Get user shared organizations API v2 invoked from tenantDomain: " + + PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain()); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-19T07:42:41.255Z
Learnt from: BimsaraBodaragama
Repo: wso2/identity-api-server PR: 1044
File: components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/factories/UsersApiServiceCoreFactory.java:35-47
Timestamp: 2025-12-19T07:42:41.255Z
Learning: In the organization user sharing management API modules, factory classes use eager static initialization (static blocks) to fail fast if required OSGi services are not available. This is an intentional design pattern for consistency across API service factories in this module.
Applied to files:
components/org.wso2.carbon.identity.api.server.organization.user.sharing.management/org.wso2.carbon.identity.api.server.organization.user.sharing.management.v2/src/main/java/org/wso2/carbon/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/organization/user/sharing/management/v2/core/UsersApiServiceCore.java
Show resolved
Hide resolved
3 participants
Purpose
This PR introduces a new versioned User Sharing API (
/api/server/v2/users/**) to address functional and frontend limitations of the existing/api/server/v1/users/**endpoints.The new API aligns conceptually and structurally with the Application Sharing API and enables incremental role assignment and selective sharing across hierarchical organizations.
Goals
roleAssignmentobject with supported modes:NONE | SELECTED./v1integrations.Approach
/api/server/v2/users/**:roleAssignmentto reflect role assignment semantics in sub-organizations.NONEandSELECTED, consistent with the Application Sharing model.Related PRs
Related Issues
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.