Restrict User Info endpoints to specific roles #382
Conversation
WalkthroughRefactors role definitions into BaseRoles and MetaRoles enums with a META_ROLE_PREFIX constant; adds VIEW_USER_INFO role; enforces role-based access control on the POST /findUserByUin route requiring VIEW_USER_INFO, TICKETS_MANAGER, and TICKETS_SCANNER roles. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
💰 Infracost reportMonthly estimate generatedThis comment will be updated when code changes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
src/api/routes/user.ts(2 hunks)src/common/roles.ts(3 hunks)
🧰 Additional context used
🪛 ESLint
src/api/routes/user.ts
[error] 20-20: Unexpected use of file extension "js" for "common/roles.js"
(import/extensions)
src/common/roles.ts
[error] 6-6: Insert ;
(prettier/prettier)
[error] 36-36: Replace typeof·orgRoles with (typeof·orgRoles)
(prettier/prettier)
[error] 38-38: Replace typeof·AllOrganizationNameList[number], with (typeof·AllOrganizationNameList)[number];
(prettier/prettier)
[error] 39-39: Insert ;
(prettier/prettier)
[error] 40-40: Insert ;
(prettier/prettier)
[error] 65-65: Insert ,
(prettier/prettier)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Unit Tests
- GitHub Check: Build Application
🔇 Additional comments (5)
src/common/roles.ts (3)
8-27: LGTM! Clean role definition with new VIEW_USER_INFO.The new VIEW_USER_INFO role follows the existing naming conventions and is appropriately placed in BaseRoles. The value format is consistent with other view permissions.
29-31: Good separation of meta-roles.Moving AT_LEAST_ONE_ORG_MANAGER to a dedicated MetaRoles enum with the prefix improves code organization and makes the distinction between base and meta-roles clear.
33-44: Well-structured role merging pattern.The dual definition of
AppRoles(as both a const object and a type union) is a good TypeScript pattern that provides both runtime access to role values and compile-time type checking. Excluding MetaRoles fromallAppRolesis appropriate since meta-roles have special semantics.src/api/routes/user.ts (2)
20-20: Import extension is correct for ES modules.The
.jsextension in the import statement is correct for TypeScript projects using ES modules. TypeScript doesn't rewrite import paths, so the runtime expects.jsextensions even when importing from.tsfiles. The ESLint warning can be safely ignored.
33-37: Appropriate role-based access control for sensitive endpoint.The role restrictions correctly limit access to the UIN lookup endpoint. Including VIEW_USER_INFO (the new dedicated role), TICKETS_MANAGER, and TICKETS_SCANNER makes sense since these are the legitimate use cases for looking up users by UIN.
| export const runEnvironments = ["dev", "prod"] as const; | ||
| export type RunEnvironment = (typeof runEnvironments)[number]; | ||
| export enum AppRoles { | ||
| export const META_ROLE_PREFIX = "__metaRole:" |
There was a problem hiding this comment.
Add missing semicolon.
The statement is missing a semicolon, which will cause prettier/ESLint failures.
Apply this diff:
-export const META_ROLE_PREFIX = "__metaRole:"
+export const META_ROLE_PREFIX = "__metaRole:";📝 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.
| export const META_ROLE_PREFIX = "__metaRole:" | |
| export const META_ROLE_PREFIX = "__metaRole:"; |
🧰 Tools
🪛 ESLint
[error] 6-6: Insert ;
(prettier/prettier)
🤖 Prompt for AI Agents
In src/common/roles.ts around line 6, the exported constant declaration lacks a
trailing semicolon causing lint/prettier failures; add a semicolon to the end of
the line so the statement reads with a terminating semicolon.
| export type OrgRole = typeof orgRoles[number]; | ||
| export type OrgRoleDefinition = { | ||
| org: typeof AllOrganizationNameList[number], | ||
| role: OrgRole | ||
| } |
There was a problem hiding this comment.
Fix formatting issues in type definitions.
Multiple formatting issues that will cause prettier/ESLint failures.
Apply this diff:
-export type OrgRole = typeof orgRoles[number];
+export type OrgRole = (typeof orgRoles)[number];
export type OrgRoleDefinition = {
- org: typeof AllOrganizationNameList[number],
- role: OrgRole
-}
+ org: (typeof AllOrganizationNameList)[number];
+ role: OrgRole;
+};📝 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.
| export type OrgRole = typeof orgRoles[number]; | |
| export type OrgRoleDefinition = { | |
| org: typeof AllOrganizationNameList[number], | |
| role: OrgRole | |
| } | |
| export type OrgRole = (typeof orgRoles)[number]; | |
| export type OrgRoleDefinition = { | |
| org: (typeof AllOrganizationNameList)[number]; | |
| role: OrgRole; | |
| }; |
🧰 Tools
🪛 ESLint
[error] 36-36: Replace typeof·orgRoles with (typeof·orgRoles)
(prettier/prettier)
[error] 38-38: Replace typeof·AllOrganizationNameList[number], with (typeof·AllOrganizationNameList)[number];
(prettier/prettier)
[error] 39-39: Insert ;
(prettier/prettier)
[error] 40-40: Insert ;
(prettier/prettier)
🤖 Prompt for AI Agents
In src/common/roles.ts around lines 36 to 40, the OrgRoleDefinition type is
incorrectly formatted (uses commas and inconsistent punctuation) which triggers
prettier/ESLint errors; update the type to use proper TypeScript punctuation and
consistent spacing by replacing the commas with semicolons and ensuring the
object type ends with a semicolon (i.e., "org: typeof
AllOrganizationNameList[number]; role: OrgRole;") and keep the export lines
properly terminated.
| [AppRoles.MANAGE_EXTERNAL_MEMBERSHIP_LIST]: "External Membership List Manager", | ||
| [AppRoles.ALL_ORG_MANAGER]: "Organization Definition Manager", | ||
| [AppRoles.AT_LEAST_ONE_ORG_MANAGER]: "Manager of at least one org", | ||
| [AppRoles.VIEW_USER_INFO]: "User Information Viewer" |
There was a problem hiding this comment.
Add missing trailing comma.
The last entry in the AppRoleHumanMapper object is missing a trailing comma, which will cause prettier/ESLint failures.
Apply this diff:
- [AppRoles.VIEW_USER_INFO]: "User Information Viewer"
+ [AppRoles.VIEW_USER_INFO]: "User Information Viewer",📝 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.
| [AppRoles.VIEW_USER_INFO]: "User Information Viewer" | |
| [AppRoles.VIEW_USER_INFO]: "User Information Viewer", |
🧰 Tools
🪛 ESLint
[error] 65-65: Insert ,
(prettier/prettier)
🤖 Prompt for AI Agents
In src/common/roles.ts around line 65, the AppRoleHumanMapper object's final
entry "[AppRoles.VIEW_USER_INFO]: "User Information Viewer"" is missing a
trailing comma; add a trailing comma at the end of that line so the object ends
with a comma to satisfy Prettier/ESLint formatting rules.
Also cleanup role definitions for meta-roles.
Summary by CodeRabbit
Security
Refactor