[WIP] Add new sharing tab to user profile #9568
Conversation
WalkthroughThis pull request introduces a comprehensive user sharing feature that enables administrators to share users across organizations with configurable policies and role assignments. It adds new APIs, React components, data models, and an OpenAPI specification to support sharing, unsharing, and managing user access across organizations with granular role control. Changes
Sequence DiagramsequenceDiagram
participant User as Admin User
participant UI as ShareUserForm
participant API as Admin APIs
participant Backend as Backend Service
participant Org as Organization Store
User->>UI: Select sharing mode & organizations
UI->>UI: Validate form state
User->>UI: Submit sharing action
UI->>API: Call appropriate share/unshare function
API->>API: Build request config with userId & org data
API->>Backend: POST/PATCH /users/share endpoint
Backend->>Backend: Process sharing policy & roles
Backend->>Org: Update organization-user associations
Org-->>Backend: Confirmation
Backend-->>API: Return 202 Processing response
API-->>UI: Resolve with response data
UI->>UI: Dispatch success alert
UI->>UI: Reset form state
UI-->>User: Display confirmation message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@features/admin.users.v1/components/edit-user.tsx`:
- Around line 281-292: Replace the hard-coded menuItem label "Sharing" with a
localized i18n key and add that key to the locale resources; specifically,
update the menuItem property (the object rendering ResourceTab.Pane and
ShareUserForm) to use the translation function (e.g.,
t('users:editUser.tab.menuItems.4')) instead of the literal string, ensuring the
component that contains this code imports/has access to the i18n hook or
translator used elsewhere in this file, and add the corresponding
"users:editUser.tab.menuItems.4" entry to your locale JSON files for all
supported languages.
In `@features/admin.users.v1/components/share-user-form.tsx`:
- Around line 131-140: ShareUserForm currently assumes onUserSharingCompleted
exists and callers invoke it unconditionally; guard this by either giving it a
safe default or checking before calling. In
features/admin.users.v1/components/share-user-form.tsx update the ShareUserForm
props handling to set onUserSharingCompleted = () => {} in the destructuring (or
wrap every invocation of onUserSharingCompleted in an if (typeof
onUserSharingCompleted === "function") check) and ensure any call sites (e.g.,
triggerUserShare / the save/submit handler inside ShareUserForm) use that
guarded reference to avoid runtime crashes when the prop is omitted.
- Around line 827-857: The unshare API call in shareAllorNoRolesWithSelectedOrgs
is not awaited and thus not included in sharedPromises; change the logic to push
the promise returned by unshareUserWithSelectedOrganizations(data) into
sharedPromises (instead of calling it standalone), and chain .then() to remove
entries from tempShareWithFutureChildOrgsMap and .catch() to set
orgSharingSuccess = false and dispatch the addAlert as currently implemented so
that the subsequent await Promise.all(sharedPromises) properly waits for unshare
completion and error handling.
- Around line 145-149: The FEATURE_DICTIONARY in UserManagementConstants is
missing the USER_SHARED_ACCESS_STATUS key, so
UserManagementConstants.FEATURE_DICTIONARY.get("USER_SHARED_ACCESS_STATUS")
returns undefined and isFeatureEnabled falls back to true; add a new entry to
FEATURE_DICTIONARY with the symbol USER_SHARED_ACCESS_STATUS (use the same
constant name) mapping to the appropriate feature flag string/value used across
the app, update UserManagementConstants to export the USER_SHARED_ACCESS_STATUS
constant and include it in FEATURE_DICTIONARY so share-user-form.tsx's lookup
returns the correct config for isFeatureEnabled.
In `@features/admin.users.v1/models/user.ts`:
- Around line 482-581: The interfaces use UserSharingPolicy correctly for policy
but the nested roleSharing.mode fields should use RoleSharingModes (from
`@wso2is/admin.console-settings.v1/models/shared-access`) instead of the
UI-specific RoleShareType or plain string; update the type of mode in
ShareUserWithAllOrganizationsDataInterface.roleSharing,
SharedOrganizationAndRolesInterface.roleSharing,
ShareUserWithSelectedOrganizationsAndRolesDataInterface (and any other interface
with roleSharing.mode) to RoleSharingModes, add the import for RoleSharingModes,
and remove/replace any references to RoleShareType so the mode matches the
OpenAPI values NONE/SELECTED.
In `@features/admin.users.v1/user-sharing-v2.yml`:
- Around line 27-29: Update the OpenAPI security definition for BasicAuth
referenced in the security section so it documents its limitations and required
protections: add a description to the BasicAuth security scheme stating that
credentials are only base64-encoded (not encrypted), that TLS/HTTPS is required
in production, and indicate the intended use-case (e.g., development only or
service-to-service with mTLS). Locate and modify the BasicAuth entry in the
securitySchemes (the BasicAuth scheme referenced under security) and add the
explanatory description text to ensure callers know to use TLS/mTLS and when
BasicAuth is appropriate.
- Around line 472-482: The schemas allow unbounded arrays: add a maxItems
constraint (e.g., maxItems: 100) to UserCriteria.userIds and likewise add
maxItems to UserUnshareSelectedRequestBody.orgIds and
UserShareSelectedRequestBody.organizations to prevent excessively large
payloads; update the YAML entries for the properties named userIds, orgIds, and
organizations respectively to include a sensible maxItems value consistent with
system capacity and validation elsewhere.
🧹 Nitpick comments (7)
features/admin.users.v1/components/roles-share-with-all.tsx (1)
113-117: Return a stable empty array foruserRolesList.
This avoidsundefinedand better matches the declaredRolesV2Interface[]type.♻️ Suggested refactor
- const userRolesList: RolesV2Interface[] = useMemo(() => { - if (originalUserRoles?.Resources?.length > 0) { - return originalUserRoles.Resources; - } - }, [ originalUserRoles ]); + const userRolesList: RolesV2Interface[] = useMemo(() => { + return originalUserRoles?.Resources?.length > 0 + ? originalUserRoles.Resources + : []; + }, [ originalUserRoles ]);features/admin.users.v1/api/users.ts (1)
521-704: Replaceanypayloads with the sharing data interfaces.
The models already define the payload shapes; using them here prevents accidental contract drift.♻️ Suggested refactor (illustrative)
-import { - ResendCodeRequestData, - UserDetailsInterface, - UserListInterface, - UserSessionsInterface -} from "../models/user"; +import { + ResendCodeRequestData, + ShareOrganizationsAndRolesPatchDataInterface, + ShareUserWithAllOrganizationsDataInterface, + ShareUserWithSelectedOrganizationsAndRolesDataInterface, + UnshareOrganizationsDataInterface, + UnshareUserWithAllOrganizationsDataInterface, + UserDetailsInterface, + UserListInterface, + UserSessionsInterface +} from "../models/user"; @@ -export const shareUserWithAllOrganizations = (data: any): Promise<any> => { +export const shareUserWithAllOrganizations = ( + data: ShareUserWithAllOrganizationsDataInterface +): Promise<any> => { @@ -export const unShareUserWithAllOrganizations = (data: any): Promise<any> => { +export const unShareUserWithAllOrganizations = ( + data: UnshareUserWithAllOrganizationsDataInterface +): Promise<any> => { @@ -export const shareUserWithSelectedOrganizationsAndRoles = (data: any): Promise<any> => { +export const shareUserWithSelectedOrganizationsAndRoles = ( + data: ShareUserWithSelectedOrganizationsAndRolesDataInterface +): Promise<any> => { @@ -export const unshareUserWithSelectedOrganizations = (data: any): Promise<any> => { +export const unshareUserWithSelectedOrganizations = ( + data: UnshareOrganizationsDataInterface +): Promise<any> => { @@ -export const editUserRolesOfExistingOrganizations = (data: any): Promise<any> => { +export const editUserRolesOfExistingOrganizations = ( + data: ShareOrganizationsAndRolesPatchDataInterface +): Promise<any> => {features/admin.users.v1/user-sharing-v2.yml (5)
70-91: Missing 401 and 403 response definitions.All mutation endpoints should document authentication and authorization failure responses. This helps API consumers implement proper error handling.
Proposed fix: Add 401 and 403 responses
"500": description: Internal Server Error content: application/json: schema: $ref: "#/components/schemas/Error" + "401": + description: Unauthorized - Authentication required + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "403": + description: Forbidden - Insufficient permissions (requires internal_user_share scope) + content: + application/json: + schema: + $ref: "#/components/schemas/Error"Apply the same pattern to all other endpoints (
/users/share-with-all,/users/unshare,/users/unshare-with-all, and the GET endpoint).
391-450: Missing 404 response for non-existent user.The GET endpoint should document a 404 response for when the specified
userIddoes not exist. This is important for proper error handling by API consumers.Proposed fix: Add 404 response
"400": description: Bad Request content: application/json: schema: $ref: "#/components/schemas/Error" + "404": + description: Not Found - User with the specified ID does not exist + content: + application/json: + schema: + $ref: "#/components/schemas/Error" "500": description: Internal Server Error
510-520: Consider usingenumfor policy values instead of freeform strings.The policy field documents valid values in the description, but using an
enumwould provide schema-level validation and better code generation for API clients.Proposed fix: Use enum for policy
policy: - type: string - description: |- - Sharing scope for this organization. - - Possible values: - - `SELECTED_ORG_ONLY` - - `SELECTED_ORG_WITH_ALL_EXISTING_CHILDREN_ONLY` - - `SELECTED_ORG_WITH_ALL_EXISTING_AND_FUTURE_CHILDREN` - - `SELECTED_ORG_WITH_EXISTING_IMMEDIATE_CHILDREN_ONLY` - - `SELECTED_ORG_WITH_EXISTING_IMMEDIATE_AND_FUTURE_CHILDREN` + type: string + description: Sharing scope for this organization. + enum: + - SELECTED_ORG_ONLY + - SELECTED_ORG_WITH_ALL_EXISTING_CHILDREN_ONLY + - SELECTED_ORG_WITH_ALL_EXISTING_AND_FUTURE_CHILDREN + - SELECTED_ORG_WITH_EXISTING_IMMEDIATE_CHILDREN_ONLY + - SELECTED_ORG_WITH_EXISTING_IMMEDIATE_AND_FUTURE_CHILDRENApply the same pattern to the global policy in
UserShareAllRequestBody(lines 537-544) andSharingMode(lines 700-704).
624-648: Conditional requirement forrolesnot enforced at schema level.The description states
rolesis "Required whenmode = SELECTED" but the schema doesn't enforce this. While OpenAPI 3.0 lacks native conditional required support, consider:
- Adding a
minItems: 1constraint toroleswhen defined- Documenting validation behavior clearly for implementers
- Using
oneOfwith separate schemas forNONEandSELECTEDmodes for stricter validationAlternative: Use discriminated union pattern
RoleAssignment: oneOf: - type: object properties: mode: type: string enum: [NONE] required: [mode] - type: object properties: mode: type: string enum: [SELECTED] roles: type: array minItems: 1 items: $ref: "#/components/schemas/RoleShareConfig" required: [mode, roles] discriminator: propertyName: mode
12-14: Use HTTPS for contact URL.Minor: The contact URL uses
http://which should behttps://for consistency with security best practices.contact: name: WSO2 - url: 'http://wso2.com/products/identity-server/' + url: 'https://wso2.com/products/identity-server/'
| }, | ||
| { | ||
| menuItem: "Sharing", | ||
| render: () => ( | ||
| <ResourceTab.Pane controlledSegmentation attached={ false }> | ||
| <ShareUserForm | ||
| user={ user } | ||
| readOnly={ isReadOnly } | ||
| /> | ||
| </ResourceTab.Pane> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Localize the new "Sharing" tab label.
All other tabs use i18n keys. A hard-coded label will stay English in localized UIs. Consider using a new translation key (e.g., users:editUser.tab.menuItems.4) and adding it to the locale resources.
🔧 Suggested fix
- {
- menuItem: "Sharing",
+ {
+ menuItem: t("users:editUser.tab.menuItems.4"),
render: () => (
<ResourceTab.Pane controlledSegmentation attached={ false }>
<ShareUserForm
user={ user }
readOnly={ isReadOnly }
/>
</ResourceTab.Pane>
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }, | |
| { | |
| menuItem: "Sharing", | |
| render: () => ( | |
| <ResourceTab.Pane controlledSegmentation attached={ false }> | |
| <ShareUserForm | |
| user={ user } | |
| readOnly={ isReadOnly } | |
| /> | |
| </ResourceTab.Pane> | |
| ) | |
| } | |
| }, | |
| { | |
| menuItem: t("users:editUser.tab.menuItems.4"), | |
| render: () => ( | |
| <ResourceTab.Pane controlledSegmentation attached={ false }> | |
| <ShareUserForm | |
| user={ user } | |
| readOnly={ isReadOnly } | |
| /> | |
| </ResourceTab.Pane> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In `@features/admin.users.v1/components/edit-user.tsx` around lines 281 - 292,
Replace the hard-coded menuItem label "Sharing" with a localized i18n key and
add that key to the locale resources; specifically, update the menuItem property
(the object rendering ResourceTab.Pane and ShareUserForm) to use the translation
function (e.g., t('users:editUser.tab.menuItems.4')) instead of the literal
string, ensuring the component that contains this code imports/has access to the
i18n hook or translator used elsewhere in this file, and add the corresponding
"users:editUser.tab.menuItems.4" entry to your locale JSON files for all
supported languages.
| const { | ||
| user, | ||
| triggerUserShare, | ||
| onUserSharingCompleted, | ||
| readOnly, | ||
| isSharingInProgress, | ||
| operationStatus, | ||
| onOperationStarted, | ||
| ["data-componentid"]: componentId = "user-share-form" | ||
| } = props; |
There was a problem hiding this comment.
Guard onUserSharingCompleted to avoid a runtime crash.
ShareUserForm is rendered without this prop in EditUser, but several call sites invoke it unconditionally. This will throw when users click save.
🔧 Suggested fix
- onUserSharingCompleted,
+ onUserSharingCompleted = () => undefined,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { | |
| user, | |
| triggerUserShare, | |
| onUserSharingCompleted, | |
| readOnly, | |
| isSharingInProgress, | |
| operationStatus, | |
| onOperationStarted, | |
| ["data-componentid"]: componentId = "user-share-form" | |
| } = props; | |
| const { | |
| user, | |
| triggerUserShare, | |
| onUserSharingCompleted = () => undefined, | |
| readOnly, | |
| isSharingInProgress, | |
| operationStatus, | |
| onOperationStarted, | |
| ["data-componentid"]: componentId = "user-share-form" | |
| } = props; |
🤖 Prompt for AI Agents
In `@features/admin.users.v1/components/share-user-form.tsx` around lines 131 -
140, ShareUserForm currently assumes onUserSharingCompleted exists and callers
invoke it unconditionally; guard this by either giving it a safe default or
checking before calling. In
features/admin.users.v1/components/share-user-form.tsx update the ShareUserForm
props handling to set onUserSharingCompleted = () => {} in the destructuring (or
wrap every invocation of onUserSharingCompleted in an if (typeof
onUserSharingCompleted === "function") check) and ensure any call sites (e.g.,
triggerUserShare / the save/submit handler inside ShareUserForm) use that
guarded reference to avoid runtime crashes when the prop is omitted.
| const userFeatureConfig: FeatureAccessConfigInterface = useSelector((state: AppState) => | ||
| state?.config?.ui?.features?.users); | ||
| const isUserShareOperationStatusEnabled: boolean = isFeatureEnabled( | ||
| userFeatureConfig, | ||
| UserManagementConstants.FEATURE_DICTIONARY.get("USER_SHARED_ACCESS_STATUS")); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "USER_SHARED_ACCESS_STATUS" -g '*.{ts,tsx}'Repository: wso2/identity-apps
Length of output: 206
🏁 Script executed:
fd user-management-constants -t fRepository: wso2/identity-apps
Length of output: 182
🏁 Script executed:
rg -n "FEATURE_DICTIONARY" -A 20 -g '*.{ts,tsx}' features/admin.users.v1/Repository: wso2/identity-apps
Length of output: 32452
🏁 Script executed:
cat -n features/admin.users.v1/constants/user-management-constants.ts | sed -n '75,105p'Repository: wso2/identity-apps
Length of output: 2006
🏁 Script executed:
fd -e ts -e tsx | xargs rg -l "isFeatureEnabled" | head -5Repository: wso2/identity-apps
Length of output: 399
🏁 Script executed:
rg -n "function isFeatureEnabled|const isFeatureEnabled|export.*isFeatureEnabled" -A 8Repository: wso2/identity-apps
Length of output: 2557
🏁 Script executed:
cat -n modules/access-control/src/helpers/access-control.ts | sed -n '41,80p'Repository: wso2/identity-apps
Length of output: 1591
Add USER_SHARED_ACCESS_STATUS to FEATURE_DICTIONARY in UserManagementConstants.
The key is not registered in the feature dictionary. When get("USER_SHARED_ACCESS_STATUS") is called at line 149, it returns undefined, causing isFeatureEnabled to fall through to its default return value of true (line 58 of access-control.ts). This means the async notification path will always trigger regardless of the actual feature flag configuration.
🤖 Prompt for AI Agents
In `@features/admin.users.v1/components/share-user-form.tsx` around lines 145 -
149, The FEATURE_DICTIONARY in UserManagementConstants is missing the
USER_SHARED_ACCESS_STATUS key, so
UserManagementConstants.FEATURE_DICTIONARY.get("USER_SHARED_ACCESS_STATUS")
returns undefined and isFeatureEnabled falls back to true; add a new entry to
FEATURE_DICTIONARY with the symbol USER_SHARED_ACCESS_STATUS (use the same
constant name) mapping to the appropriate feature flag string/value used across
the app, update UserManagementConstants to export the USER_SHARED_ACCESS_STATUS
constant and include it in FEATURE_DICTIONARY so share-user-form.tsx's lookup
returns the correct config for isFeatureEnabled.
| // Call unshare API for removed organizations | ||
| if (removedOrgIds.length > 0) { | ||
| const data: UnshareOrganizationsDataInterface = { | ||
| orgIds: removedOrgIds, | ||
| userId: user.id | ||
| }; | ||
|
|
||
| unshareUserWithSelectedOrganizations(data) | ||
| .then(() => { | ||
| // Upon sucessful unsharing, Remove the orgs from tempShareWithFutureChildOrgsMap | ||
| // as they are already removed along with the org unsharing and no need of patch operation | ||
| removedOrgIds.forEach((orgId: string) => { | ||
| delete tempShareWithFutureChildOrgsMap[orgId]; | ||
| }); | ||
| }) | ||
| .catch((error: Error) => { | ||
| orgSharingSuccess = false; | ||
|
|
||
| dispatch(addAlert({ | ||
| description: t("users:edit.sections.sharedAccess.notifications.unshare." + | ||
| "error.description", | ||
| { error: error.message }), | ||
| level: AlertLevels.ERROR, | ||
| message: t("users:edit.sections.sharedAccess.notifications.unshare.error.message") | ||
| })); | ||
| }); | ||
| } | ||
|
|
||
| // Wait for sharing/unsharing to complete | ||
| await Promise.all(sharedPromises); | ||
|
|
There was a problem hiding this comment.
Await unshare operations in shareAllorNoRolesWithSelectedOrgs.
The unshare request isn’t included in Promise.all, so success notifications can fire before the unshare completes (and errors can be missed).
🔧 Suggested fix
- unshareUserWithSelectedOrganizations(data)
- .then(() => {
+ sharedPromises.push(
+ unshareUserWithSelectedOrganizations(data)
+ .then(() => {
// Upon sucessful unsharing, Remove the orgs from tempShareWithFutureChildOrgsMap
// as they are already removed along with the org unsharing and no need of patch operation
removedOrgIds.forEach((orgId: string) => {
delete tempShareWithFutureChildOrgsMap[orgId];
});
- })
- .catch((error: Error) => {
+ })
+ .catch((error: Error) => {
orgSharingSuccess = false;
dispatch(addAlert({
description: t("users:edit.sections.sharedAccess.notifications.unshare." +
"error.description",
{ error: error.message }),
level: AlertLevels.ERROR,
message: t("users:edit.sections.sharedAccess.notifications.unshare.error.message")
}));
- });
+ })
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Call unshare API for removed organizations | |
| if (removedOrgIds.length > 0) { | |
| const data: UnshareOrganizationsDataInterface = { | |
| orgIds: removedOrgIds, | |
| userId: user.id | |
| }; | |
| unshareUserWithSelectedOrganizations(data) | |
| .then(() => { | |
| // Upon sucessful unsharing, Remove the orgs from tempShareWithFutureChildOrgsMap | |
| // as they are already removed along with the org unsharing and no need of patch operation | |
| removedOrgIds.forEach((orgId: string) => { | |
| delete tempShareWithFutureChildOrgsMap[orgId]; | |
| }); | |
| }) | |
| .catch((error: Error) => { | |
| orgSharingSuccess = false; | |
| dispatch(addAlert({ | |
| description: t("users:edit.sections.sharedAccess.notifications.unshare." + | |
| "error.description", | |
| { error: error.message }), | |
| level: AlertLevels.ERROR, | |
| message: t("users:edit.sections.sharedAccess.notifications.unshare.error.message") | |
| })); | |
| }); | |
| } | |
| // Wait for sharing/unsharing to complete | |
| await Promise.all(sharedPromises); | |
| // Call unshare API for removed organizations | |
| if (removedOrgIds.length > 0) { | |
| const data: UnshareOrganizationsDataInterface = { | |
| orgIds: removedOrgIds, | |
| userId: user.id | |
| }; | |
| sharedPromises.push( | |
| unshareUserWithSelectedOrganizations(data) | |
| .then(() => { | |
| // Upon sucessful unsharing, Remove the orgs from tempShareWithFutureChildOrgsMap | |
| // as they are already removed along with the org unsharing and no need of patch operation | |
| removedOrgIds.forEach((orgId: string) => { | |
| delete tempShareWithFutureChildOrgsMap[orgId]; | |
| }); | |
| }) | |
| .catch((error: Error) => { | |
| orgSharingSuccess = false; | |
| dispatch(addAlert({ | |
| description: t("users:edit.sections.sharedAccess.notifications.unshare." + | |
| "error.description", | |
| { error: error.message }), | |
| level: AlertLevels.ERROR, | |
| message: t("users:edit.sections.sharedAccess.notifications.unshare.error.message") | |
| })); | |
| }) | |
| ); | |
| } | |
| // Wait for sharing/unsharing to complete | |
| await Promise.all(sharedPromises); |
🤖 Prompt for AI Agents
In `@features/admin.users.v1/components/share-user-form.tsx` around lines 827 -
857, The unshare API call in shareAllorNoRolesWithSelectedOrgs is not awaited
and thus not included in sharedPromises; change the logic to push the promise
returned by unshareUserWithSelectedOrganizations(data) into sharedPromises
(instead of calling it standalone), and chain .then() to remove entries from
tempShareWithFutureChildOrgsMap and .catch() to set orgSharingSuccess = false
and dispatch the addAlert as currently implemented so that the subsequent await
Promise.all(sharedPromises) properly waits for unshare completion and error
handling.
|
|
||
| /** | ||
| * Interface for role audiences in user sharing. | ||
| */ | ||
| export interface RoleAudiencesInterface { | ||
| display: string; | ||
| type: string; | ||
| } | ||
|
|
||
| /** | ||
| * Interface for role sharing in user sharing. | ||
| */ | ||
| export interface RoleSharingInterface { | ||
| displayName: string; | ||
| audience: RoleAudiencesInterface; | ||
| } | ||
|
|
||
| /** | ||
| * Interface for sharing the user with all organizations. | ||
| */ | ||
| export interface ShareUserWithAllOrganizationsDataInterface { | ||
| userId: string; | ||
| policy: string; | ||
| roleSharing: { | ||
| mode: string; | ||
| roles?: RoleSharingInterface[]; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Interface for unsharing the user with all organizations. | ||
| */ | ||
| export interface UnshareUserWithAllOrganizationsDataInterface { | ||
| userId: string; | ||
| } | ||
|
|
||
| /** | ||
| * Interface for shared organization and roles for users. | ||
| */ | ||
| export interface SharedOrganizationAndRolesInterface { | ||
| orgId: string; | ||
| policy: string; | ||
| roleSharing: { | ||
| mode: string; | ||
| roles: RoleSharingInterface[]; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Interface for sharing the user with selected organization and roles. | ||
| */ | ||
| export interface ShareUserWithSelectedOrganizationsAndRolesDataInterface { | ||
| userId: string; | ||
| organizations: SharedOrganizationAndRolesInterface[]; | ||
| } | ||
|
|
||
| /** | ||
| * Interface for the patch operation to share the user with selected organizations and roles. | ||
| */ | ||
| export interface ShareOrganizationsAndRolesPatchOperationInterface { | ||
| op: string; | ||
| path: string; | ||
| value: RoleSharingInterface[]; | ||
| } | ||
|
|
||
| /** | ||
| * Interface for the patch data to share the user with selected organizations and roles. | ||
| */ | ||
| export interface ShareOrganizationsAndRolesPatchDataInterface { | ||
| userId: string; | ||
| Operations: ShareOrganizationsAndRolesPatchOperationInterface[]; | ||
| } | ||
|
|
||
| /** | ||
| * Interface for unsharing the user from organizations. | ||
| */ | ||
| export interface UnshareOrganizationsDataInterface { | ||
| userId: string; | ||
| orgIds: string[]; | ||
| } | ||
|
|
||
| /** | ||
| * Interface for User. | ||
| */ | ||
| export interface UserInterface extends UserBasicInterface { | ||
| id: string; | ||
| associatedRoles?: { | ||
| allowedAudience?: string; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * User sharing policy enum. | ||
| */ | ||
| export enum UserSharingPolicy { | ||
| ALL_EXISTING_ORGS_ONLY = "ALL_EXISTING_ORGS_ONLY", | ||
| ALL_EXISTING_AND_FUTURE_ORGS = "ALL_EXISTING_AND_FUTURE_ORGS", | ||
| SELECTED_ORG_ONLY = "SELECTED_ORG_ONLY", | ||
| SELECTED_ORG_WITH_ALL_EXISTING_AND_FUTURE_CHILDREN = "SELECTED_ORG_WITH_ALL_EXISTING_AND_FUTURE_CHILDREN" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate the user.ts file and verify the current state
fd 'user\.ts' --type f | grep -E 'admin\.users\.v1.*models'Repository: wso2/identity-apps
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Search for RoleShareType definition in constants
fd 'user-roles' --type f -o -name '*constants*' | xargs grep -l 'RoleShareType' 2>/dev/null || echo "RoleShareType not found"Repository: wso2/identity-apps
Length of output: 333
🏁 Script executed:
#!/bin/bash
# Check what's imported or defined in constants
fd '\.ts$' --type f | xargs grep -l 'RoleShareType\|RoleShare' | head -20Repository: wso2/identity-apps
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Search for actual enum/constant definitions related to sharing modes
rg 'export\s+(enum|const)\s+.*Share|export\s+(enum|const).*Mode' --type ts | head -30Repository: wso2/identity-apps
Length of output: 3416
🏁 Script executed:
#!/bin/bash
# Look for OpenAPI/spec files related to user sharing
fd '\.(yml|yaml|json)$' --type f | xargs grep -l 'shareWithAll\|UserSharing\|roleSharing' 2>/dev/null | head -10Repository: wso2/identity-apps
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Read user-roles.ts to see RoleShareType definition
cat -n features/admin.users.v1/constants/user-roles.ts | head -100Repository: wso2/identity-apps
Length of output: 1525
🏁 Script executed:
#!/bin/bash
# Check user-sharing-v2.yml for the API contract
cat -n features/admin.users.v1/user-sharing-v2.yml | head -200Repository: wso2/identity-apps
Length of output: 8703
🏁 Script executed:
#!/bin/bash
# Search for usage of ShareUserWithAllOrganizationsDataInterface to see if strings are actually used
rg 'ShareUserWithAllOrganizationsDataInterface|policy:|roleSharing' features/admin.users.v1 -A 2 -B 2 | head -100Repository: wso2/identity-apps
Length of output: 7532
🏁 Script executed:
#!/bin/bash
# Find RoleSharingModes definition
rg 'RoleSharingModes|enum.*RoleSha' features/admin.users.v1 -A 5 | head -50Repository: wso2/identity-apps
Length of output: 4368
🏁 Script executed:
#!/bin/bash
# Check how mode values are actually sent in API calls
cat -n features/admin.users.v1/components/share-user-form.tsx | sed -n '1,50p;200,300p;400,500p;600,700p' | head -150Repository: wso2/identity-apps
Length of output: 7036
🏁 Script executed:
#!/bin/bash
# Check the schema definitions in the OpenAPI spec for policy and mode enums
cat -n features/admin.users.v1/user-sharing-v2.yml | tail -500 | head -200Repository: wso2/identity-apps
Length of output: 8343
🏁 Script executed:
#!/bin/bash
# Find RoleSharingModes definition in console-settings
fd 'shared-access' --type f | xargs grep -A 10 'enum RoleSharingModes\|export.*RoleSharingModes'Repository: wso2/identity-apps
Length of output: 451
🏁 Script executed:
#!/bin/bash
# Check the schema definitions at the end of the OpenAPI spec for enum values
tail -300 features/admin.users.v1/user-sharing-v2.yml | grep -A 30 'properties:' | head -80Repository: wso2/identity-apps
Length of output: 2832
🏁 Script executed:
#!/bin/bash
# Find the RoleAssignment schema definition in the OpenAPI spec
rg 'RoleAssignment:' features/admin.users.v1/user-sharing-v2.yml -A 30Repository: wso2/identity-apps
Length of output: 1024
🏁 Script executed:
#!/bin/bash
# Check if RoleSharingModes is actually used in the data interfaces or just the component
rg 'RoleSharingModes|roleSharing.*mode' features/admin.users.v1/models/user.ts -B 2 -A 2Repository: wso2/identity-apps
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Let me check the current interface definitions more carefully to see the mismatch
cat -n features/admin.users.v1/models/user.ts | sed -n '482,581p'Repository: wso2/identity-apps
Length of output: 3145
🏁 Script executed:
#!/bin/bash
# Check what values are actually being used in API calls
rg 'roleSharing|policy:' features/admin.users.v1/api/users.ts -B 3 -A 3 | head -100Repository: wso2/identity-apps
Length of output: 367
Use enums for sharing policy, but verify the correct mode enum.
Using UserSharingPolicy for the policy field is correct and aligns with the API contract. However, the mode field should use RoleSharingModes (from @wso2is/admin.console-settings.v1/models/shared-access) instead of RoleShareType, as the OpenAPI spec defines mode values as NONE and SELECTED (matching RoleSharingModes), not the camelCase values in RoleShareType which is a UI-specific enum.
♻️ Corrected typing upgrade
import { UserRoleInterface } from "@wso2is/admin.core.v1/models/users";
import { SCIMConfigs } from "@wso2is/admin.extensions.v1/configs/scim";
import { GroupsInterface, GroupsMemberInterface } from "@wso2is/admin.groups.v1/models/groups";
import { LinkInterface, MultiValueAttributeInterface, NameInterface, RolesInterface } from "@wso2is/core/models";
+import { RoleSharingModes } from "@wso2is/admin.console-settings.v1/models/shared-access";
import React, { ReactElement } from "react";
import { BulkImportResponseOperationTypes, BulkUserImportStatus } from "../constants";
@@
export interface ShareUserWithAllOrganizationsDataInterface {
userId: string;
- policy: string;
+ policy: UserSharingPolicy;
roleSharing: {
- mode: string;
+ mode: RoleSharingModes;
roles?: RoleSharingInterface[];
};
}
@@
export interface SharedOrganizationAndRolesInterface {
orgId: string;
- policy: string;
+ policy: UserSharingPolicy;
roleSharing: {
- mode: string;
+ mode: RoleSharingModes;
roles: RoleSharingInterface[];
};
}🤖 Prompt for AI Agents
In `@features/admin.users.v1/models/user.ts` around lines 482 - 581, The
interfaces use UserSharingPolicy correctly for policy but the nested
roleSharing.mode fields should use RoleSharingModes (from
`@wso2is/admin.console-settings.v1/models/shared-access`) instead of the
UI-specific RoleShareType or plain string; update the type of mode in
ShareUserWithAllOrganizationsDataInterface.roleSharing,
SharedOrganizationAndRolesInterface.roleSharing,
ShareUserWithSelectedOrganizationsAndRolesDataInterface (and any other interface
with roleSharing.mode) to RoleSharingModes, add the import for RoleSharingModes,
and remove/replace any references to RoleShareType so the mode matches the
OpenAPI values NONE/SELECTED.
| security: | ||
| - OAuth2: [] | ||
| - BasicAuth: [] |
There was a problem hiding this comment.
BasicAuth security scheme allows cleartext credentials.
The static analysis correctly identifies that BasicAuth transmits credentials as base64-encoded (essentially cleartext). While this may be acceptable for development or when TLS is enforced at infrastructure level, consider:
- Documenting that HTTPS/TLS is required when using BasicAuth
- Adding a description to the BasicAuth scheme clarifying its intended use case (e.g., service-to-service calls with mutual TLS)
Proposed fix: Add description to BasicAuth scheme
BasicAuth:
type: http
scheme: basic
+ description: |-
+ Basic authentication. **Important:** Only use over HTTPS/TLS connections.
+ Recommended for service-to-service calls or development environments only.Also applies to: 454-456
🤖 Prompt for AI Agents
In `@features/admin.users.v1/user-sharing-v2.yml` around lines 27 - 29, Update the
OpenAPI security definition for BasicAuth referenced in the security section so
it documents its limitations and required protections: add a description to the
BasicAuth security scheme stating that credentials are only base64-encoded (not
encrypted), that TLS/HTTPS is required in production, and indicate the intended
use-case (e.g., development only or service-to-service with mTLS). Locate and
modify the BasicAuth entry in the securitySchemes (the BasicAuth scheme
referenced under security) and add the explanatory description text to ensure
callers know to use TLS/mTLS and when BasicAuth is appropriate.
| UserCriteria: | ||
| type: object | ||
| description: Criteria for selecting users to share/unshare. | ||
| properties: | ||
| userIds: | ||
| type: array | ||
| description: List of user IDs. | ||
| items: | ||
| type: string | ||
| required: | ||
| - userIds |
There was a problem hiding this comment.
Add maxItems constraint to prevent unbounded requests.
The userIds array lacks a maximum item limit. Unbounded arrays can lead to performance degradation or denial-of-service if clients submit excessively large payloads. Consider adding a reasonable limit based on your processing capacity.
Proposed fix: Add maxItems constraint
UserCriteria:
type: object
description: Criteria for selecting users to share/unshare.
properties:
userIds:
type: array
description: List of user IDs.
+ minItems: 1
+ maxItems: 100
items:
type: string
required:
- userIdsSimilarly, consider adding maxItems to orgIds in UserUnshareSelectedRequestBody (line 561-565) and organizations in UserShareSelectedRequestBody (line 494-498).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UserCriteria: | |
| type: object | |
| description: Criteria for selecting users to share/unshare. | |
| properties: | |
| userIds: | |
| type: array | |
| description: List of user IDs. | |
| items: | |
| type: string | |
| required: | |
| - userIds | |
| UserCriteria: | |
| type: object | |
| description: Criteria for selecting users to share/unshare. | |
| properties: | |
| userIds: | |
| type: array | |
| description: List of user IDs. | |
| minItems: 1 | |
| maxItems: 100 | |
| items: | |
| type: string | |
| required: | |
| - userIds |
🧰 Tools
🪛 Checkov (3.2.334)
[medium] 477-482: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🤖 Prompt for AI Agents
In `@features/admin.users.v1/user-sharing-v2.yml` around lines 472 - 482, The
schemas allow unbounded arrays: add a maxItems constraint (e.g., maxItems: 100)
to UserCriteria.userIds and likewise add maxItems to
UserUnshareSelectedRequestBody.orgIds and
UserShareSelectedRequestBody.organizations to prevent excessively large
payloads; update the YAML entries for the properties named userIds, orgIds, and
organizations respectively to include a sensible maxItems value consistent with
system capacity and validation elsewhere.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9568 +/- ##
=======================================
Coverage 55.88% 55.88%
=======================================
Files 42 42
Lines 1020 1020
Branches 231 247 +16
=======================================
Hits 570 570
+ Misses 450 416 -34
- Partials 0 34 +34
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
1 participant
Purpose
$subject
Related Issues
Related PRs
Checklist
Security checks
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.