-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(ABAC): Add icons #6758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat.abac
Are you sure you want to change the base?
feat(ABAC): Add icons #6758
Conversation
WalkthroughThis pull request adds ABAC (Attribute-Based Access Control) support to the mobile app by introducing shield icons to replace text tags. Changes include database schema extensions, prop propagation across UI components, and icon selection logic in RoomTypeIcon to display team or hash shield icons when ABAC attributes are present. Changes
Sequence DiagramsequenceDiagram
participant DB as Database
participant Sub as Subscription Model
participant Room as Room Components
participant Icon as RoomTypeIcon
participant UI as Shield Icon Display
DB->>Sub: Load room data with abac_attributes
Sub->>Room: Populate component props (RoomHeader/RoomItem)
Room->>Room: Thread abacAttributes through hierarchy
Room->>Icon: Pass abacAttributes prop
alt abacAttributes present
Icon->>Icon: Select shield icon<br/>(team-shield or hash-shield)
else abacAttributes absent
Icon->>Icon: Select standard icon<br/>(based on teamMain)
end
Icon->>UI: Render selected icon
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/containers/RoomTypeIcon/index.tsx (1)
51-63: Optional: Consider extracting icon selection logic per TODO.The TODO comment on line 51 suggests moving the icon selection logic to a separate function. With the added ABAC logic, this refactoring would improve readability and testability. However, this can be deferred to a future PR.
If addressed, the refactored code could look like:
const getIconName = ({ abacAttributes, teamMain, type, isGroupChat }: { abacAttributes?: string[]; teamMain?: boolean; type: string; isGroupChat?: boolean; }): TIconsName => { if (abacAttributes?.length) { return teamMain ? 'team-shield' : 'hash-shield'; } if (teamMain) { return `teams${type === 'p' ? '-private' : ''}`; } if (type === 'discussion') { return 'discussions'; } if (type === 'c') { return 'channel-public'; } if (type === 'd' && isGroupChat) { return 'message'; } return 'channel-private'; }; // Then use it: const icon = getIconName({ abacAttributes, teamMain, type, isGroupChat });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
android/app/src/main/assets/fonts/custom.ttfis excluded by!**/*.ttfapp/containers/RoomHeader/__snapshots__/RoomHeader.test.tsx.snapis excluded by!**/*.snapapp/containers/RoomItem/__snapshots__/RoomItem.test.tsx.snapis excluded by!**/*.snapapp/containers/RoomTypeIcon/__snapshots__/RoomTypeIcon.test.tsx.snapis excluded by!**/*.snapios/custom.ttfis excluded by!**/*.ttf
📒 Files selected for processing (20)
app/containers/CustomIcon/mappedIcons.js(2 hunks)app/containers/RoomHeader/RoomHeader.stories.tsx(1 hunks)app/containers/RoomHeader/RoomHeader.tsx(5 hunks)app/containers/RoomHeader/index.tsx(4 hunks)app/containers/RoomItem/IconOrAvatar.tsx(2 hunks)app/containers/RoomItem/RoomItem.stories.tsx(1 hunks)app/containers/RoomItem/RoomItem.tsx(3 hunks)app/containers/RoomItem/TypeIcon.tsx(1 hunks)app/containers/RoomItem/index.tsx(1 hunks)app/containers/RoomItem/interfaces.ts(3 hunks)app/containers/RoomTypeIcon/RoomTypeIcon.stories.tsx(1 hunks)app/containers/RoomTypeIcon/index.tsx(3 hunks)app/definitions/ISubscription.ts(1 hunks)app/lib/database/model/Subscription.js(2 hunks)app/lib/database/model/migrations.js(1 hunks)app/lib/database/schema/app.js(2 hunks)app/lib/methods/helpers/mergeSubscriptionsRooms.ts(1 hunks)app/views/RoomActionsView/index.tsx(1 hunks)app/views/RoomInfoView/components/RoomInfoViewTitle.tsx(1 hunks)app/views/RoomView/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
app/lib/database/model/Subscription.js (1)
app/lib/database/utils.ts (1)
sanitizer(19-19)
app/containers/RoomHeader/index.tsx (1)
app/definitions/ISubscription.ts (1)
ISubscription(40-120)
app/containers/RoomItem/interfaces.ts (1)
app/definitions/ISubscription.ts (1)
ISubscription(40-120)
app/containers/RoomItem/index.tsx (1)
app/sagas/deepLinking.js (1)
item(71-76)
app/containers/RoomItem/TypeIcon.tsx (1)
app/containers/RoomItem/interfaces.ts (1)
ITypeIconProps(59-71)
app/containers/RoomHeader/RoomHeader.tsx (1)
app/definitions/ISubscription.ts (1)
ISubscription(40-120)
app/containers/RoomTypeIcon/index.tsx (1)
app/definitions/ISubscription.ts (1)
ISubscription(40-120)
🔇 Additional comments (24)
app/containers/CustomIcon/mappedIcons.js (1)
109-109: LGTM! Clean icon additions.The new shield icons are correctly placed in alphabetical order and follow the existing pattern. The numeric codes align with the icon font mappings.
Also applies to: 206-206
app/lib/database/model/migrations.js (1)
335-343: LGTM! Migration follows established patterns.The migration to version 28 correctly adds the optional
abac_attributescolumn to the subscriptions table. The structure matches existing migrations and safely handles existing data.app/lib/database/schema/app.js (1)
4-4: LGTM! Schema update aligns with migration.The schema version bump to 28 and the
abac_attributescolumn definition are consistent with the migration in migrations.js. The optional string type matches the expected serialization pattern for the array field.Also applies to: 73-74
app/containers/RoomTypeIcon/RoomTypeIcon.stories.tsx (1)
21-22: LGTM! Good story coverage for ABAC icons.The new examples demonstrate ABAC attributes both standalone and combined with
teamMain, providing clear visual testing for the shield icon variants.app/containers/RoomItem/RoomItem.stories.tsx (1)
74-75: LGTM! Comprehensive story examples.The new RoomItem examples with ABAC attributes provide good visual coverage, including the interaction with
teamMain.app/containers/RoomHeader/RoomHeader.stories.tsx (1)
58-59: LGTM! Clear header icon examples.The new examples effectively demonstrate how ABAC attributes render in the room header alongside existing icon types.
app/containers/RoomHeader/index.tsx (1)
4-4: LGTM! Clean prop integration with good type safety.The
abacAttributesprop is correctly typed usingISubscription['abacAttributes'](ensuring consistency with the source interface) and properly forwarded to the child component. The use of thetypekeyword for type-only imports is a good practice.Also applies to: 24-24, 42-43, 94-94
app/definitions/ISubscription.ts (1)
119-119: Serialization properly handled—no changes needed.Verification confirms the
abacAttributesfield is correctly configured. The Subscription model uses WatermelonDB's@jsondecorator (line 156 ofapp/lib/database/model/Subscription.js), which automatically handles JSON serialization between the database's string storage format and the TypeScriptstring[]type. The implementation is complete and working as intended.app/containers/RoomItem/IconOrAvatar.tsx (1)
23-47: LGTM! Clean prop threading for ABAC attributes.The addition of
abacAttributesand its forwarding toTypeIconis straightforward and consistent with the existing prop-passing pattern.app/lib/database/model/Subscription.js (2)
156-156: LGTM! Database field addition follows existing conventions.The
abacAttributesfield is properly decorated with@jsonand uses the standardsanitizer, consistent with other JSON fields in this model (e.g.,roles,muted,tags).
224-225: LGTM! Serialization correctly includes the new field.The
abacAttributesfield is properly included in theasPlain()output, ensuring the ABAC attributes are available when the subscription is serialized.app/containers/RoomItem/index.tsx (1)
97-97: LGTM! Proper data flow from subscription to RoomItem.The
abacAttributesfrom the subscription item is correctly threaded through to theRoomItemcomponent, consistent with how other subscription properties are passed.app/views/RoomActionsView/index.tsx (1)
798-798: LGTM! ABAC attributes propagated to room info rendering.The
abacAttributesare correctly passed toRoomTypeIconin the room actions view, enabling ABAC-aware icon display in the room information section.app/containers/RoomTypeIcon/index.tsx (2)
27-31: LGTM! Type-safe ABAC attribute support added.The
abacAttributesprop is properly typed asISubscription['abacAttributes'], ensuring type consistency across the component hierarchy.
53-63: Icon selection logic and icon availability verified.The ABAC attribute precedence is correctly implemented. Both
'team-shield'(code point 59877) and'hash-shield'(code point 59878) are defined inapp/containers/CustomIcon/mappedIcons.js, confirming the icon names are available and the code is ready for merge.app/containers/RoomHeader/RoomHeader.tsx (1)
83-214: LGTM! ABAC attributes properly threaded through RoomHeader.The
abacAttributesprop is correctly:
- Added to the
IRoomHeaderinterface with proper typing- Included in the component's props destructuring
- Forwarded to
RoomTypeIconin both rendering paths (thread view at line 187 and standard view at line 214)This ensures ABAC-aware icon display in room headers across all view modes.
app/views/RoomView/index.tsx (1)
513-513: LGTM! ABAC attributes correctly passed to RoomHeader.The
abacAttributesfrom the subscription are properly forwarded to theRoomHeadercomponent within the navigation header configuration, enabling ABAC-aware icon display in the room view header.app/containers/RoomItem/RoomItem.tsx (2)
56-57: LGTM! ABAC attributes added to RoomItem props.The
abacAttributesprop is correctly destructured and ready for use in the component.
116-158: LGTM! ABAC attributes forwarded in both display modes.The
abacAttributesare properly passed toTypeIconin both rendering branches:
- Expanded view with last message (line 116)
- Collapsed view (line 158)
This ensures ABAC shield icons appear consistently in room list items regardless of the display mode setting.
app/views/RoomInfoView/components/RoomInfoViewTitle.tsx (1)
66-66: LGTM!The abacAttributes prop is correctly passed to RoomTypeIcon with appropriate optional chaining to handle cases where room may be undefined.
app/containers/RoomItem/TypeIcon.tsx (1)
6-20: LGTM!The component correctly threads the new abacAttributes prop through to RoomTypeIcon while maintaining the existing memoization and prop-passing patterns.
app/containers/RoomItem/interfaces.ts (3)
70-70: LGTM!The abacAttributes property is correctly added to ITypeIconProps with the appropriate optional type derived from ISubscription.
128-128: LGTM!The abacAttributes property is correctly added to IRoomItemProps, consistent with the type used in other interfaces.
166-166: LGTM!The abacAttributes property is correctly added to IIconOrAvatar, maintaining consistency across all interface definitions in this file.
| mergedSubscription.jitsiTimeout = room.jitsiTimeout; | ||
| mergedSubscription.usernames = room.usernames; | ||
| mergedSubscription.uids = room.uids; | ||
| mergedSubscription.abacAttributes = ['Classified', 'Top Secret', 'Confidential']; // TODO: remove this once we have the actual abac attributes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mock data applies ABAC attributes to ALL rooms indiscriminately.
The hardcoded mock assigns ABAC attributes to every subscription that has associated room data, regardless of whether the room should actually have ABAC policies. This means all rooms will display shield icons in the UI, which may complicate testing and validation of the ABAC feature's intended behavior.
Consider adding a conditional check or a flag to selectively apply mock attributes to specific test rooms only, allowing the UI to display both ABAC and non-ABAC states during development.
Would you like me to suggest an implementation that conditionally applies mock attributes based on a room property or pattern (e.g., room name contains "ABAC")?
🤖 Prompt for AI Agents
In app/lib/methods/helpers/mergeSubscriptionsRooms.ts around line 37, the code
indiscriminately sets mergedSubscription.abacAttributes = ['Classified', 'Top
Secret', 'Confidential']; for every subscription with room data; change this to
apply mock ABAC attributes only when a room meets a clear condition (e.g.,
room.meta?.isAbac === true, room.name includes "ABAC", or a dedicated test flag
on the room/subscription). Implement a simple conditional: check the chosen
property on the room before assigning the mock array, otherwise leave
mergedSubscription.abacAttributes undefined or an empty array so non-ABAC rooms
render without shields.
Proposed changes
Adds
hash-shieldandteam-shieldicons to ABAC channels.Note: Rooms are currently mocked until we finish backend tasks https://github.com/RocketChat/Rocket.Chat.ReactNative/pull/6758/files#diff-afc917faa092e24a0649adfc17955ceb82a7e32067cb4a696d083d5faa0f022fR37
Issue(s)
https://rocketchat.atlassian.net/browse/ABAC-75
How to test or reproduce
Screenshots
Rooms list
Room header
Room actions
Room info
Types of changes
Checklist
Further comments
Summary by CodeRabbit