tenant-admin-flows-added#698
Open
MallanagoudaB wants to merge 2 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR refactors role-based tenant detail retrieval across utility and service files by explicitly differentiating ADMIN_ROLE and TENANT_ADMIN access patterns. It extends TENANT_ADMIN support to organization validation and service headers, introduces organization ID normalization, and adjusts token-passing logic in middleware calls. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
generics/middleware/authenticator.js (1)
558-561: Make token optional behavior explicit in org validation helper.Line 560 omits token, while
validateIfOrgsBelongsToTenantstill forwards a token argument internally. This works implicitly but is easy to regress. Consider making the helper explicitly branch on token presence.Suggested cleanup
- async function validateIfOrgsBelongsToTenant(tenantId, orgId, token) { + async function validateIfOrgsBelongsToTenant(tenantId, orgId, token = '') { let orgIdArr = Array.isArray(orgId) ? orgId : typeof orgId === 'string' ? orgId.split(',') : [] - let orgDetails = await userService.fetchTenantDetails(tenantId, token) + let orgDetails = token + ? await userService.fetchTenantDetails(tenantId, token) + : await userService.fetchTenantDetails(tenantId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generics/middleware/authenticator.js` around lines 558 - 561, The call to validateIfOrgsBelongsToTenant(req.headers['tenantid'], req.headers['orgid']) should make the optional token handling explicit: detect the token at the call site (e.g. const token = req.headers['authorization'] || req.headers['token'] ) and then either call validateIfOrgsBelongsToTenant(tenantId, orgId, token) when token is present or call validateIfOrgsBelongsToTenant(tenantId, orgId) / with null when not, or alternatively adjust validateIfOrgsBelongsToTenant to accept an optional third param and branch internally on its presence; update the call in the authenticator middleware (the invocation shown) to pass the token or undefined explicitly so token-forwarding is no longer implicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@generics/helpers/programSolutionUtilities.js`:
- Around line 56-57: The membership check is case-sensitive: normalize
organization codes from tenantDetails.data.organizations and from
bodyData.organizations (e.g., trim and toLowerCase) before comparing so
casing/whitespace differences don't break validation; update the construction of
validOrgCodes and the isValid check to use the normalized codes (referencing
validOrgCodes, tenantDetails.data.organizations, bodyData.organizations, and
isValid) so you compare normalized values rather than raw strings.
In `@generics/helpers/solutionAndProjectTemplateUtils.js`:
- Around line 330-339: The code may dereference undefined validOrgs when roles
do not include CONSTANTS.common.ADMIN_ROLE or CONSTANTS.common.TENANT_ADMIN;
ensure validOrgs is always defined before checking validOrgs.success by either
initializing validOrgs to a safe default (e.g., { success: false, data: null })
before the role checks or adding an else branch that sets validOrgs
appropriately; update the block around userDetails.userInformation.roles,
CONSTANTS.common.ADMIN_ROLE, CONSTANTS.common.TENANT_ADMIN and the subsequent
validOrgs.success check (and any error handling that follows) to handle the
default case so there are no runtime exceptions when
userService.fetchTenantDetails is not called.
In `@generics/services/survey.js`:
- Around line 62-68: The code calls userDetails.tenantAndOrgInfo.orgId.join(',')
inside the TENANT_ADMIN conditional (when roles includes
CONSTANTS.common.TENANT_ADMIN) which can throw if tenantAndOrgInfo or orgId is
missing or not an array; update each TENANT_ADMIN block (the sections that
assign options.headers with 'x-auth-token' and orgId) to safely guard and
compute the header value by checking userDetails && userDetails.tenantAndOrgInfo
&& Array.isArray(userDetails.tenantAndOrgInfo.orgId) and only call .join(',')
when true, otherwise set a safe default (empty string or omit the header) so it
cannot raise a runtime exception.
In `@module/programs/helper.js`:
- Around line 46-55: The check assumes validOrgs is always assigned; ensure you
handle non-admin roles before dereferencing validOrgs: in the block around
userDetails and the calls to userService.fetchTenantDetails (and the roles
CONSTANTS.common.ADMIN_ROLE / CONSTANTS.common.TENANT_ADMIN), initialize
validOrgs to a safe default (e.g., an object with success=false) or add an else
branch that sets validOrgs appropriately or returns early; then perform the if
(!validOrgs.success) check only after validOrgs is guaranteed to be defined.
---
Nitpick comments:
In `@generics/middleware/authenticator.js`:
- Around line 558-561: The call to
validateIfOrgsBelongsToTenant(req.headers['tenantid'], req.headers['orgid'])
should make the optional token handling explicit: detect the token at the call
site (e.g. const token = req.headers['authorization'] || req.headers['token'] )
and then either call validateIfOrgsBelongsToTenant(tenantId, orgId, token) when
token is present or call validateIfOrgsBelongsToTenant(tenantId, orgId) / with
null when not, or alternatively adjust validateIfOrgsBelongsToTenant to accept
an optional third param and branch internally on its presence; update the call
in the authenticator middleware (the invocation shown) to pass the token or
undefined explicitly so token-forwarding is no longer implicit.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
generics/helpers/programSolutionUtilities.jsgenerics/helpers/solutionAndProjectTemplateUtils.jsgenerics/middleware/authenticator.jsgenerics/services/survey.jsmodule/programs/helper.js
VISHNUDAS-tunerlabs
requested changes
Mar 3, 2026
| // Check if user is Admin or Tenant Admin | ||
| if (UTILS.validateRoles(userDetails.userInformation.roles, adminTenantAdminRole)) { | ||
| // Fetch tenant details to validate organization codes | ||
| if (UTILS.validateRoles(userDetails.userInformation.roles, CONSTANTS.common.ADMIN_ROLE)) { |
Collaborator
There was a problem hiding this comment.
@MallanagoudaB Please update the changes as per discussion
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
These recommendations are intended to promote code quality and team communication during software development. They cover a variety of topics, including ensuring that pull requests are submitted to the correct branch, documenting new methods, preserving consistency across many services, and avoiding typical blunders like accessing APIs or DB queries within loops. Sensitive data should not be uploaded, and changes to environment variables or database models should be executed consistently. Teams may work more effectively and develop higher-quality software by adhering to these standards.
Type of change
Please choose appropriate options.
Checklist
Summary by CodeRabbit
Release Notes