Skip to content

Using super admin to create resource for other tenants and orgs - #1639

Merged
rakeshSgr merged 7 commits into
ELEVATE-Project:developfrom
borkarsaish65:fix/4562
May 15, 2026
Merged

rakeshSgr merged 7 commits into
ELEVATE-Project:developfrom
borkarsaish65:fix/4562

Conversation

@borkarsaish65

@borkarsaish65 borkarsaish65 commented May 5, 2026 •

Copy link
Copy Markdown
Contributor

Release Notes

  • Add admin override HTTP header configuration for org and tenant targeting: ORG_CODE_HEADER_NAME and TENANT_CODE_HEADER_NAME (defaults: x-org-code, x-tenant-code)
  • Export tenant-related constants: TENANT_ADMIN_ROLE, ORG_CODE_HEADER, TENANT_CODE_HEADER (header names normalized to lowercase with inline fallbacks to avoid startup TypeError)
  • Extend envVariables to include defaults and optional marking for ORG_CODE_HEADER_NAME and TENANT_CODE_HEADER_NAME; update .env.sample
  • Add i18n strings for org/tenant header prompts and validation/error messages
  • Enhance authenticator middleware to allow admin and tenant_admin to act on behalf of other organizations/tenants via header-driven resolution and OrganizationExtensionQueries lookups; updates req.decodedToken accordingly and enforces header validation
  • Include TENANT_ADMIN_ROLE in Keycloak public key allowlist for token decoding (note: reviewer flagged verification of intent for this auth method)
  • Add migration to create tenant-scoped tenant_admin role entries and duplicate admin permissions into tenant_admin role permissions (transactional, with down migration)

Contribution Summary

Author Lines Added Lines Removed
borkarsaish65 226 1

Review Change Stack

@borkarsaish65

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 5, 2026

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

@coderabbitai

coderabbitai Bot commented May 5, 2026 •

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds configurable HTTP headers and constants for org/tenant admin override, middleware logic to validate and resolve overrides via OrganizationExtensionQueries (updating req.decodedToken), i18n strings, expands notification admin role acceptance, propagates tenant-admin in user roles, and adds a migration to provision tenant-scoped admin permissions.

Changes

Admin Override via Headers

Layer / File(s) Summary
Configuration & Constants
src/.env.sample, src/envVariables.js, src/constants/common.js
Environment variables ORG_CODE_HEADER_NAME and TENANT_CODE_HEADER_NAME added with defaults x-org-code and x-tenant-code; exports ORG_CODE_HEADER, TENANT_CODE_HEADER (lowercased) and new TENANT_ADMIN_ROLE.
Internationalization
src/locales/en.json
Added i18n keys: ADD_ORG_OR_TENANT_HEADER, ORG_CODE_REQUIRED_FOR_TENANT_ADMIN, INVALID_ORG_OR_TENANT_CODE, INVALID_ORG_CODE_FOR_TENANT.
Middleware Import
src/middlewares/authenticator.js
Imports OrganizationExtensionQueries to resolve override mappings.
Middleware Implementation
src/middlewares/authenticator.js
Adds header-driven override path for users with ADMIN_ROLE or TENANT_ADMIN_ROLE: reads headers, validates required combos, calls OrganizationExtensionQueries.findOne, returns 400 on invalid/missing inputs, and updates req.decodedToken.organization_id, organization_code, and/or tenant_code. Also adds TENANT_ADMIN_ROLE to Keycloak role extraction allowlist.
Controller role update
src/controllers/v1/notification.js
Includes TENANT_ADMIN_ROLE in allowed roles for NotificationTemplate.template admin actions.
User request role propagation
src/requests/user.js
Adds TENANT_ADMIN_ROLE to validRoles so tenant-admin roles are retained in fetched user details.
DB Migration
src/database/migrations/20260511122947-add-tenant-admin-role-permissions.js
Adds tenant_admin role_extensions per tenant/org and duplicates admin permissions (excluding admin module) into tenant-scoped role_permission_mapping; includes transactional up/down.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Authenticator
  participant OrganizationExtensionQueries
  participant DB
  Client->>Authenticator: request with override headers
  Authenticator->>OrganizationExtensionQueries: findOne(orgCode, tenantCode)
  OrganizationExtensionQueries->>DB: SELECT mapping FROM organization_extension
  DB-->>OrganizationExtensionQueries: mapping row
  OrganizationExtensionQueries-->>Authenticator: mapping result
  Authenticator->>Authenticator: validate and set req.decodedToken
  Authenticator-->>Client: proceed or failureResponse(400)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ELEVATE-Project/mentoring#1365: Modifies role/tenant handling and env variable usage in src/requests/user.js and src/envVariables.js, overlapping domain changes.
  • ELEVATE-Project/mentoring#1438: Adds middleware logic that populates req.decodedToken.tenant_code, closely related to authenticator header-driven population.
  • ELEVATE-Project/mentoring#1357: Uses req.decodedToken.tenant_code in session flows; dependent on authenticator tenant_code propagation added here.

Suggested reviewers

  • nevil-mathew

Poem

🐰 I nudge headers into the auth trail,
Admins whisper codes down the rabbit-tail,
The DB nods and mappings align,
Tokens hop with org and tenant sign,
x-org-code, x-tenant-code — hop, all fine!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main objective of the pull request: enabling super admin users to create resources for other tenants and organizations through header-based overrides.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@borkarsaish65 borkarsaish65 changed the title savepoint-1 Using super admin to create resource for other tenants and orgs May 5, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
src/middlewares/authenticator.js (2)

161-232: 💤 Low value

Consider extracting the override resolution into a helper.

The two branches (isTenantAdmin && !isAdmin and else if (isAdmin)) duplicate the structure of validate-headers → OrganizationExtensionQueries.findOne → mutate req.decodedToken, with only the tenant source and required-headers set differing. A small helper such as resolveAdminOverride(req, { requireTenantHeader, tenantCode }) would centralize the mutation of req.decodedToken (and the string coercion suggested above), reducing the chance of one branch drifting away from the other on future fixes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middlewares/authenticator.js` around lines 161 - 232, Extract the
duplicate override logic into a helper (e.g., resolveAdminOverride(req, {
requireTenantHeader, tenantCodeSource })) that: reads and trims orgCode and
tenantCode from headers using common.ORG_CODE_HEADER /
common.TENANT_CODE_HEADER, validates required headers, calls
OrganizationExtensionQueries.findOne({ organization_code: orgCode },
tenantCode), and mutates req.decodedToken (organization_id, organization_code,
and optionally tenant_code) or throws the existing failureResponses; then
replace the two branches (isTenantAdmin && !isAdmin and isAdmin) to call this
helper with requireTenantHeader=false/true and tenantCodeSource either
req.decodedToken.tenant_code or the header value accordingly.

156-232: ⚡ Quick win

Add audit logging for admin/tenant_admin overrides.

This block silently rewrites the effective organization_id, organization_code, and (for full admins) tenant_code on the decoded token, which downstream business logic and DB queries trust as the caller's identity. For an admin override feature, audit-trail logging (caller id, role, source tenant_code, target org_code / tenant_code, request path, timestamp) is a standard requirement and the only after-the-fact way to investigate misuse. Consider piping this through the existing structured logger so it can be shipped to the observability stack.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middlewares/authenticator.js` around lines 156 - 232, Add structured
audit logging whenever an admin or tenant-admin override changes
req.decodedToken (after the validation blocks in the isTenantAdmin and isAdmin
branches where req.decodedToken.organization_id/organization_code and
req.decodedToken.tenant_code are assigned). Log the caller identifier from
req.decodedToken (e.g., sub or user_id), the roles variable, the original source
tenant_code (req.decodedToken.tenant_code before change), the target
orgCode/tenantCode values (orgCode, tenantCode), the request path (req.path or
req.originalUrl), and a timestamp; send this via the existing structured logger
(e.g., processLogger or the module used elsewhere) at info/audit level so the
event is recorded in the observability stack. Ensure the log is emitted in both
the tenant-admin override branch (where only orgCode is set) and the admin
branch (where tenantCode+orgCode are set).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/locales/en.json`:
- Line 191: Update the locale string for key ADD_ORG_OR_TENANT_HEADER to
correctly describe the two headers and remove the awkward comma; replace the
current text with something like: "Please provide the required organization and
tenant headers: {{orgCodeHeader}} and {{tenantCodeHeader}} for admin override."
Ensure you edit the value for ADD_ORG_OR_TENANT_HEADER only and preserve the
interpolation tokens {{orgCodeHeader}} and {{tenantCodeHeader}} exactly.

In `@src/middlewares/authenticator.js`:
- Around line 171-198: The tenant_admin branch currently uses
effectiveTenantCode = req.decodedToken.tenant_code and calls
OrganizationExtensionQueries.findOne without enforcing a tenant scope, allowing
cross-tenant org resolution when tenant_code is missing; fix this by explicitly
validating that req.decodedToken.tenant_code is present and non-empty before
calling OrganizationExtensionQueries.findOne (when isTenantAdmin && !isAdmin),
and if missing throw the same kind of failureResponse (e.g., CLIENT_ERROR /
bad_request) rather than proceeding, so the lookup always happens with a
guaranteed tenant_code and req.decodedToken.organization_id / organization_code
are only set after a tenant-scoped findOne succeeds.
- Around line 171-198: When handling the branch for isTenantAdmin && !isAdmin,
explicitly reject requests that include a caller-provided tenant header instead
of silently ignoring it: check the parsed tenant header variable (the value read
into tenantCode / the x-tenant-code header) and if it is present throw
responses.failureResponse with a clear client error (use
httpStatusCode.bad_request and a distinct message/key such as
'TENANT_CODE_NOT_ALLOWED_FOR_TENANT_ADMIN' or similar), keeping the existing
overrideOrg lookup and assignment to req.decodedToken.organization_id and
organization_code unchanged for cases where no tenant header is provided.
- Around line 197-229: The override path assigns overrideOrg.organization_id
(from OrganizationExtensionQueries.findOne) directly to
req.decodedToken.organization_id which can be a number; coerce it to a string
(like the earlier normalization and the admin-token branch that uses
organizationId.toString()) so downstream string comparisons/caching remain
correct—update both places where overrideOrg.organization_id is assigned to
req.decodedToken.organization_id to convert the value to a string before
assignment.

---

Nitpick comments:
In `@src/middlewares/authenticator.js`:
- Around line 161-232: Extract the duplicate override logic into a helper (e.g.,
resolveAdminOverride(req, { requireTenantHeader, tenantCodeSource })) that:
reads and trims orgCode and tenantCode from headers using common.ORG_CODE_HEADER
/ common.TENANT_CODE_HEADER, validates required headers, calls
OrganizationExtensionQueries.findOne({ organization_code: orgCode },
tenantCode), and mutates req.decodedToken (organization_id, organization_code,
and optionally tenant_code) or throws the existing failureResponses; then
replace the two branches (isTenantAdmin && !isAdmin and isAdmin) to call this
helper with requireTenantHeader=false/true and tenantCodeSource either
req.decodedToken.tenant_code or the header value accordingly.
- Around line 156-232: Add structured audit logging whenever an admin or
tenant-admin override changes req.decodedToken (after the validation blocks in
the isTenantAdmin and isAdmin branches where
req.decodedToken.organization_id/organization_code and
req.decodedToken.tenant_code are assigned). Log the caller identifier from
req.decodedToken (e.g., sub or user_id), the roles variable, the original source
tenant_code (req.decodedToken.tenant_code before change), the target
orgCode/tenantCode values (orgCode, tenantCode), the request path (req.path or
req.originalUrl), and a timestamp; send this via the existing structured logger
(e.g., processLogger or the module used elsewhere) at info/audit level so the
event is recorded in the observability stack. Ensure the log is emitted in both
the tenant-admin override branch (where only orgCode is set) and the admin
branch (where tenantCode+orgCode are set).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fb1dc4a8-8059-49b4-a553-dabc3cefae88

📥 Commits

Reviewing files that changed from the base of the PR and between 126dac5 and 4ddcd7c.

📒 Files selected for processing (5)
  • src/.env.sample
  • src/constants/common.js
  • src/envVariables.js
  • src/locales/en.json
  • src/middlewares/authenticator.js

Comment thread src/locales/en.json
"PROFILE_NOT_UPDATED": "Please update your profile with all the mandatory fields",
"BULK_SESSIONS_REMOVED": "Bulk session removal process successfully executed.",
"ADD_ORG_HEADER": "{{orgIdHeader}} is mandatory when {{adminHeader}} is passed",
"ADD_ORG_OR_TENANT_HEADER": "Please provide all required organization headers: {{orgCodeHeader}}, and {{tenantCodeHeader}} for admin override.",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Minor wording issue in ADD_ORG_OR_TENANT_HEADER.

Two small issues:

  • The phrase "all required organization headers" is misleading since {{tenantCodeHeader}} is a tenant header, not an organization header.
  • The Oxford comma before "and" with only two items reads awkwardly when interpolated (e.g., x-org-code, and x-tenant-code).
✏️ Proposed wording fix
-	"ADD_ORG_OR_TENANT_HEADER": "Please provide all required organization headers: {{orgCodeHeader}}, and {{tenantCodeHeader}} for admin override.",
+	"ADD_ORG_OR_TENANT_HEADER": "Please provide both {{orgCodeHeader}} and {{tenantCodeHeader}} headers for admin override.",
📝 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.

Suggested change
"ADD_ORG_OR_TENANT_HEADER": "Please provide all required organization headers: {{orgCodeHeader}}, and {{tenantCodeHeader}} for admin override.",
"ADD_ORG_OR_TENANT_HEADER": "Please provide both {{orgCodeHeader}} and {{tenantCodeHeader}} headers for admin override.",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/locales/en.json` at line 191, Update the locale string for key
ADD_ORG_OR_TENANT_HEADER to correctly describe the two headers and remove the
awkward comma; replace the current text with something like: "Please provide the
required organization and tenant headers: {{orgCodeHeader}} and
{{tenantCodeHeader}} for admin override." Ensure you edit the value for
ADD_ORG_OR_TENANT_HEADER only and preserve the interpolation tokens
{{orgCodeHeader}} and {{tenantCodeHeader}} exactly.

Comment on lines +171 to +198
if (isTenantAdmin && !isAdmin) {
const effectiveTenantCode = req.decodedToken.tenant_code
if (!orgCode) {
throw responses.failureResponse({
message: {
key: 'ORG_CODE_REQUIRED_FOR_TENANT_ADMIN',
interpolation: { orgCodeHeader: orgCodeHeaderName },
},
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}

const overrideOrg = await OrganizationExtensionQueries.findOne(
{ organization_code: orgCode },
effectiveTenantCode
)

if (!overrideOrg) {
throw responses.failureResponse({
message: 'INVALID_ORG_CODE_FOR_TENANT',
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}

req.decodedToken.organization_id = overrideOrg.organization_id
req.decodedToken.organization_code = orgCode

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Privilege escalation: tenant_admin can resolve orgs across tenants when token lacks tenant_code.

effectiveTenantCode is taken from req.decodedToken.tenant_code and passed straight to OrganizationExtensionQueries.findOne. Per its implementation in src/database/queries/organisationExtension.js (lines 95–117), when tenantCode is falsy the query is executed without the tenant_code filter:

if (tenantCode) {
    filter.tenant_code = tenantCode
}

If a tenant_admin's decoded token does not carry a tenant_code (e.g., older tokens, alternate auth providers, partial migrations), the lookup degenerates to WHERE organization_code = :orgCode, allowing the tenant_admin to bind their session to any organization with that code in any tenant. The tenant_admin branch is the security-sensitive path that is explicitly meant to be tenant-scoped, so this guard must be enforced in the middleware itself rather than relying on the query helper.

🔒 Suggested guard before the lookup
 				if (isTenantAdmin && !isAdmin) {
 					const effectiveTenantCode = req.decodedToken.tenant_code
+					if (!effectiveTenantCode) {
+						throw createUnauthorizedResponse()
+					}
 					if (!orgCode) {
 						throw responses.failureResponse({
 							message: {
 								key: 'ORG_CODE_REQUIRED_FOR_TENANT_ADMIN',
 								interpolation: { orgCodeHeader: orgCodeHeaderName },
 							},
 							statusCode: httpStatusCode.bad_request,
 							responseCode: 'CLIENT_ERROR',
 						})
 					}

 					const overrideOrg = await OrganizationExtensionQueries.findOne(
 						{ organization_code: orgCode },
 						effectiveTenantCode
 					)
As per coding guidelines: "This is security-sensitive middleware. Scrutinize for potential vulnerabilities."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middlewares/authenticator.js` around lines 171 - 198, The tenant_admin
branch currently uses effectiveTenantCode = req.decodedToken.tenant_code and
calls OrganizationExtensionQueries.findOne without enforcing a tenant scope,
allowing cross-tenant org resolution when tenant_code is missing; fix this by
explicitly validating that req.decodedToken.tenant_code is present and non-empty
before calling OrganizationExtensionQueries.findOne (when isTenantAdmin &&
!isAdmin), and if missing throw the same kind of failureResponse (e.g.,
CLIENT_ERROR / bad_request) rather than proceeding, so the lookup always happens
with a guaranteed tenant_code and req.decodedToken.organization_id /
organization_code are only set after a tenant-scoped findOne succeeds.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

tenantCode header is silently ignored for tenant_admin overrides.

When isTenantAdmin && !isAdmin, a caller-provided x-tenant-code is read at line 166 but never consulted — the tenant is implicitly forced to req.decodedToken.tenant_code. A tenant_admin who passes x-tenant-code expecting to switch tenants will silently get a successful response scoped to their own tenant, which is confusing and hides what is effectively a denied authorization attempt.

Either reject the request with a clear 400 when a tenant_admin sends x-tenant-code, or document that the header is ignored for that role. Rejecting is preferred for an admin-debug feature.

🔒 Suggested explicit rejection
 				if (isTenantAdmin && !isAdmin) {
+					if (tenantCode) {
+						throw responses.failureResponse({
+							message: 'INVALID_ORG_CODE_FOR_TENANT',
+							statusCode: httpStatusCode.bad_request,
+							responseCode: 'CLIENT_ERROR',
+						})
+					}
 					const effectiveTenantCode = req.decodedToken.tenant_code
As per coding guidelines: "This is security-sensitive middleware. Scrutinize for potential vulnerabilities."
📝 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.

Suggested change
if (isTenantAdmin && !isAdmin) {
const effectiveTenantCode = req.decodedToken.tenant_code
if (!orgCode) {
throw responses.failureResponse({
message: {
key: 'ORG_CODE_REQUIRED_FOR_TENANT_ADMIN',
interpolation: { orgCodeHeader: orgCodeHeaderName },
},
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}
const overrideOrg = await OrganizationExtensionQueries.findOne(
{ organization_code: orgCode },
effectiveTenantCode
)
if (!overrideOrg) {
throw responses.failureResponse({
message: 'INVALID_ORG_CODE_FOR_TENANT',
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}
req.decodedToken.organization_id = overrideOrg.organization_id
req.decodedToken.organization_code = orgCode
if (isTenantAdmin && !isAdmin) {
if (tenantCode) {
throw responses.failureResponse({
message: 'INVALID_ORG_CODE_FOR_TENANT',
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}
const effectiveTenantCode = req.decodedToken.tenant_code
if (!orgCode) {
throw responses.failureResponse({
message: {
key: 'ORG_CODE_REQUIRED_FOR_TENANT_ADMIN',
interpolation: { orgCodeHeader: orgCodeHeaderName },
},
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}
const overrideOrg = await OrganizationExtensionQueries.findOne(
{ organization_code: orgCode },
effectiveTenantCode
)
if (!overrideOrg) {
throw responses.failureResponse({
message: 'INVALID_ORG_CODE_FOR_TENANT',
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}
req.decodedToken.organization_id = overrideOrg.organization_id
req.decodedToken.organization_code = orgCode
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middlewares/authenticator.js` around lines 171 - 198, When handling the
branch for isTenantAdmin && !isAdmin, explicitly reject requests that include a
caller-provided tenant header instead of silently ignoring it: check the parsed
tenant header variable (the value read into tenantCode / the x-tenant-code
header) and if it is present throw responses.failureResponse with a clear client
error (use httpStatusCode.bad_request and a distinct message/key such as
'TENANT_CODE_NOT_ALLOWED_FOR_TENANT_ADMIN' or similar), keeping the existing
overrideOrg lookup and assignment to req.decodedToken.organization_id and
organization_code unchanged for cases where no tenant header is provided.

Comment on lines +197 to +229
req.decodedToken.organization_id = overrideOrg.organization_id
req.decodedToken.organization_code = orgCode
} else if (isAdmin) {
if (!orgCode || !tenantCode) {
throw responses.failureResponse({
message: {
key: 'ADD_ORG_OR_TENANT_HEADER',
interpolation: {
orgCodeHeader: orgCodeHeaderName,
tenantCodeHeader: tenantCodeHeaderName,
},
},
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}

const overrideOrg = await OrganizationExtensionQueries.findOne(
{ organization_code: orgCode },
tenantCode
)

if (!overrideOrg) {
throw responses.failureResponse({
message: 'INVALID_ORG_OR_TENANT_CODE',
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}

req.decodedToken.tenant_code = tenantCode
req.decodedToken.organization_id = overrideOrg.organization_id
req.decodedToken.organization_code = orgCode

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Type inconsistency on organization_id after override.

A few lines above (146-149), req.decodedToken.organization_id is explicitly normalized to a string when numeric. After the override at lines 197 and 228, overrideOrg.organization_id is assigned directly from a Sequelize raw: true query and is typically returned as a number for INTEGER columns. Downstream code that was working against the normalized string (e.g., string equality comparisons, header propagation, cache keys) can silently misbehave when it now receives a number.

The existing admin-token branch at line 251 (organizationId.toString()) already guards against this — apply the same coercion here.

🛠 Proposed fix
 					req.decodedToken.organization_id = overrideOrg.organization_id
 					req.decodedToken.organization_code = orgCode
-				} else if (isAdmin) {
+				} else if (isAdmin) {
@@
 					req.decodedToken.tenant_code = tenantCode
 					req.decodedToken.organization_id = overrideOrg.organization_id
 					req.decodedToken.organization_code = orgCode

Replace both assignments to use string coercion:

-					req.decodedToken.organization_id = overrideOrg.organization_id
+					req.decodedToken.organization_id = overrideOrg.organization_id?.toString()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middlewares/authenticator.js` around lines 197 - 229, The override path
assigns overrideOrg.organization_id (from OrganizationExtensionQueries.findOne)
directly to req.decodedToken.organization_id which can be a number; coerce it to
a string (like the earlier normalization and the admin-token branch that uses
organizationId.toString()) so downstream string comparisons/caching remain
correct—update both places where overrideOrg.organization_id is assigned to
req.decodedToken.organization_id to convert the value to a string before
assignment.

@borkarsaish65

Copy link
Copy Markdown
Contributor Author

@coderabbitai I have reverted to your comments. Your thoughts ? approve from your side if everything looks good

@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the main changed files
echo "=== src/middlewares/authenticator.js ==="
cat src/middlewares/authenticator.js

echo ""
echo "=== src/constants/common.js ==="
cat src/constants/common.js

echo ""
echo "=== src/envVariables.js ==="
cat src/envVariables.js

echo ""
echo "=== src/.env.sample (last 20 lines) ==="
tail -20 src/.env.sample

echo ""
echo "=== src/locales/en.json (last 20 lines) ==="
tail -20 src/locales/en.json

Length of output: 52775


@borkarsaish65, thank you for addressing the previous comments! The middleware logic in authenticator.js looks clean and well-structured. However, there is one remaining critical issue in src/constants/common.js that prevents me from approving:


🚨 Crash Risk — TypeError on Startup

In src/constants/common.js, the module evaluates these constants at load time:

ORG_CODE_HEADER: process.env.ORG_CODE_HEADER_NAME.toLowerCase(),
TENANT_CODE_HEADER: process.env.TENANT_CODE_HEADER_NAME.toLowerCase(),

The problem: common.js is required at the top of envVariables.js:

const common = require('@constants/common')  // ← common.js runs HERE

This means common.js is evaluated before envVariables.js gets a chance to apply the default values (x-org-code, x-tenant-code) to process.env. If these variables are absent from the environment (e.g., in a fresh deployment without the .env changes), process.env.ORG_CODE_HEADER_NAME is undefined, and calling .toLowerCase() on it throws:

TypeError: Cannot read properties of undefined (reading 'toLowerCase')

This will crash the application on startup.

Fix — add inline fallbacks:

TENANT_ADMIN_ROLE: 'tenant_admin',
ORG_CODE_HEADER: (process.env.ORG_CODE_HEADER_NAME || 'x-org-code').toLowerCase(),
TENANT_CODE_HEADER: (process.env.TENANT_CODE_HEADER_NAME || 'x-tenant-code').toLowerCase(),

This mirrors the defaults already declared in envVariables.js and makes the constants safe regardless of load order. Once this is addressed, I'm happy to approve. ✅

@borkarsaish65

Copy link
Copy Markdown
Contributor Author

@coderabbitai done the changes , do a final review. Approve from your side if everything is okay.

@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== src/constants/common.js ==="
cat src/constants/common.js

echo ""
echo "=== src/middlewares/authenticator.js (new block only) ==="
grep -n "ORG_CODE_HEADER\|TENANT_CODE_HEADER\|TENANT_ADMIN\|org_code\|tenant_code\|organization_code" src/middlewares/authenticator.js

Length of output: 10302


@borkarsaish65, the fix has been applied correctly! Here's what I confirmed in src/constants/common.js:

TENANT_ADMIN_ROLE: 'tenant_admin',
ORG_CODE_HEADER: (process.env.ORG_CODE_HEADER_NAME || 'x-org-code').toLowerCase(),
TENANT_CODE_HEADER: (process.env.TENANT_CODE_HEADER_NAME || 'x-tenant-code').toLowerCase(),

The inline fallbacks are in place — this eliminates the crash risk on startup when the env variables are absent. The authenticator.js middleware correctly references all three new constants. Everything looks good. ✅

[approve]


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@borkarsaish65

Copy link
Copy Markdown
Contributor Author

@coderabbitai added few more changes, approve if good to proceed

@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== src/constants/common.js ==="
cat src/constants/common.js

echo ""
echo "=== src/middlewares/authenticator.js ==="
cat src/middlewares/authenticator.js

echo ""
echo "=== src/envVariables.js (relevant section) ==="
grep -n "ORG_CODE\|TENANT_CODE\|TENANT_ADMIN\|org_code\|tenant_code" src/envVariables.js

echo ""
echo "=== src/.env.sample (last 25 lines) ==="
tail -25 src/.env.sample

echo ""
echo "=== src/locales/en.json (last 30 lines) ==="
tail -30 src/locales/en.json

echo ""
echo "=== git diff summary vs develop ==="
git diff origin/develop --stat

Length of output: 33231


🏁 Script executed:

#!/bin/bash
echo "=== Check new locale keys in en.json ==="
grep -n "ADD_ORG_OR_TENANT_HEADER\|ORG_CODE_REQUIRED_FOR_TENANT_ADMIN\|INVALID_ORG_OR_TENANT_CODE\|INVALID_ORG_CODE_FOR_TENANT" src/locales/en.json

echo ""
echo "=== OrganizationExtensionQueries.findOne signature ==="
grep -n "findOne\|exports\|module.exports" src/database/queries/organisationExtension.js | head -40

echo ""
echo "=== Full findOne implementation ==="
ast-grep --pattern 'findOne($$$) { $$$ }' src/database/queries/organisationExtension.js

echo ""
echo "=== Check tenant_code usage in authenticator ==="
grep -n "tenant_code\|effectiveTenantCode" src/middlewares/authenticator.js

echo ""
echo "=== envVariables.js entries for new vars ==="
sed -n '45,70p' src/envVariables.js

Length of output: 1960


@borkarsaish65, I've verified all the new changes. Here's a summary of what I confirmed:

Check Status
common.js inline fallbacks ✅ In place
All 4 locale keys in en.json (lines 191–194) with correct interpolation ✅ Present
OrganizationExtensionQueries.findOne(filter, tenantCode) call signature matches implementation ✅ Correct
envVariables.js — both new vars are optional: true with proper defaults ✅ Correct
.env.sample — new env vars documented ✅ Present
authenticator.js — admin/tenant-admin override logic ✅ Clean

Everything looks good. Approving! ✅

[approve]

@borkarsaish65

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

@borkarsaish65

Copy link
Copy Markdown
Contributor Author

@coderabbitai review my PR , if everything looks good approve the PR from your side

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== git diff --stat vs develop ==="
git diff origin/develop --stat

echo ""
echo "=== src/constants/common.js ==="
cat src/constants/common.js

echo ""
echo "=== src/middlewares/authenticator.js ==="
cat src/middlewares/authenticator.js

echo ""
echo "=== src/envVariables.js (new vars) ==="
grep -n "ORG_CODE\|TENANT_CODE\|TENANT_ADMIN" src/envVariables.js

echo ""
echo "=== src/.env.sample (last 25 lines) ==="
tail -25 src/.env.sample

echo ""
echo "=== src/locales/en.json (last 30 lines) ==="
tail -30 src/locales/en.json

echo ""
echo "=== src/controllers/v1/notification.js ==="
cat src/controllers/v1/notification.js

echo ""
echo "=== src/database/migrations/20260511122947-add-tenant-admin-role-permissions.js ==="
cat src/database/migrations/20260511122947-add-tenant-admin-role-permissions.js

Length of output: 38924


@borkarsaish65, I've reviewed all the changed files. Here's a full summary:

File Status Notes
src/constants/common.js ✅ Inline fallbacks for ORG_CODE_HEADER, TENANT_CODE_HEADER in place
src/middlewares/authenticator.js ✅ Admin/tenant-admin override logic is clean and correct
src/envVariables.js ✅ Both new vars are optional: true with correct defaults
src/.env.sample ✅ Both new vars documented
src/locales/en.json ✅ All 4 new locale keys confirmed present
src/controllers/v1/notification.js ✅ TENANT_ADMIN_ROLE correctly added to role access check
Migration 20260511122947-* ✅ Transactional, inserts tenant_admin role + permissions correctly; down is clean

⚠️ Minor Observation (non-blocking)

In authenticator.js, the validRoles set used by keycloakPublicKeyAuthentication does not include TENANT_ADMIN_ROLE:

const validRoles = new Set([
    common.MENTEE_ROLE,
    common.MENTOR_ROLE,
    common.ORG_ADMIN_ROLE,
    common.ADMIN_ROLE,
    common.SESSION_MANAGER_ROLE,
    // ← common.TENANT_ADMIN_ROLE is missing
])

If AUTH_METHOD=keycloak_public_key, a Keycloak user with the tenant_admin role will have it stripped, and the new org/tenant override logic will never trigger for that auth method. If tenant_admin is only intended for native auth, this is fine — but worth confirming.


Everything else looks solid. Approving! ✅

[approve]

@borkarsaish65

Copy link
Copy Markdown
Contributor Author

@coderabbitai review my PR , if everything looks good approve the PR from your side

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== git diff --stat vs develop ==="
git diff origin/develop --stat

echo ""
echo "=== git log --oneline -5 ==="
git log --oneline -5

echo ""
echo "=== src/constants/common.js ==="
cat src/constants/common.js

echo ""
echo "=== src/middlewares/authenticator.js ==="
cat src/middlewares/authenticator.js

echo ""
echo "=== src/controllers/v1/notification.js ==="
cat src/controllers/v1/notification.js

echo ""
echo "=== src/database/migrations/20260511122947-add-tenant-admin-role-permissions.js ==="
cat src/database/migrations/20260511122947-add-tenant-admin-role-permissions.js

echo ""
echo "=== src/envVariables.js (new vars) ==="
grep -n "ORG_CODE\|TENANT_CODE\|TENANT_ADMIN" src/envVariables.js

echo ""
echo "=== src/locales/en.json (last 30 lines) ==="
tail -30 src/locales/en.json

Length of output: 38510


🏁 Script executed:

#!/bin/bash
echo "=== Verify new locale keys in en.json ==="
grep -n "ADD_ORG_OR_TENANT_HEADER\|ORG_CODE_REQUIRED_FOR_TENANT_ADMIN\|INVALID_ORG_OR_TENANT_CODE\|INVALID_ORG_CODE_FOR_TENANT" src/locales/en.json

echo ""
echo "=== Verify TENANT_ADMIN_ROLE in validRoles set (keycloak) ==="
grep -n "TENANT_ADMIN_ROLE\|validRoles" src/middlewares/authenticator.js

Length of output: 892


@borkarsaish65, all checks pass across the latest commits. Here's the final summary:

Check Status
common.js — inline fallbacks for ORG_CODE_HEADER / TENANT_CODE_HEADER ✅
authenticator.js — admin/tenant-admin override logic ✅
authenticator.js — TENANT_ADMIN_ROLE added to validRoles (Keycloak path) ✅ Fixed
notification.js — TENANT_ADMIN_ROLE in role access check ✅
envVariables.js — both new vars optional: true with defaults ✅
.env.sample — new vars documented ✅
en.json — all 4 locale keys at lines 191–194 with correct interpolation ✅
Migration 20260511122947-* — transactional, down is clean ✅

The previously flagged minor issue — TENANT_ADMIN_ROLE missing from the validRoles set in keycloakPublicKeyAuthentication — has been addressed in the new commits. Everything looks good. Approving! ✅

[approve]

Comment thread src/controllers/v1/notification.js Outdated
try {
if (!utilsHelper.validateRoleAccess(req.decodedToken.roles, [common.ADMIN_ROLE, common.ORG_ADMIN_ROLE])) {
if (
!utilsHelper.validateRoleAccess(req.decodedToken.roles, [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@borkarsaish65 can we remove this logic, and add it at API permission layer


try {
// Step 1: Get all unique tenant+org combinations from existing role_extensions
const [orgsByTenant] = await queryInterface.sequelize.query(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@borkarsaish65 variable name can be changed

// Step 1: Get all unique tenant+org combinations from existing role_extensions
const [orgsByTenant] = await queryInterface.sequelize.query(
`SELECT DISTINCT tenant_code, organization_id, organization_code
FROM role_extensions

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@borkarsaish65 if we require orgs by tenant why we are looking into role_extensions ?

@borkarsaish65

Copy link
Copy Markdown
Contributor Author

@coderabbitai review latest changes, approve if everything is okay.

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== git diff --stat vs develop ==="
git diff origin/develop --stat

echo ""
echo "=== git log --oneline -8 ==="
git log --oneline -8

echo ""
echo "=== src/constants/common.js ==="
cat src/constants/common.js

echo ""
echo "=== src/middlewares/authenticator.js ==="
cat src/middlewares/authenticator.js

echo ""
echo "=== src/controllers/v1/notification.js ==="
cat src/controllers/v1/notification.js

echo ""
echo "=== src/requests/user.js ==="
cat src/requests/user.js

echo ""
echo "=== src/database/migrations/20260511122947-add-tenant-admin-role-permissions.js ==="
cat src/database/migrations/20260511122947-add-tenant-admin-role-permissions.js

echo ""
echo "=== src/envVariables.js (new vars) ==="
grep -n "ORG_CODE\|TENANT_CODE\|TENANT_ADMIN" src/envVariables.js

echo ""
echo "=== src/locales/en.json (last 30 lines) ==="
tail -30 src/locales/en.json

echo ""
echo "=== src/.env.sample (last 25 lines) ==="
tail -25 src/.env.sample

echo ""
echo "=== Verify locale keys ==="
grep -n "ADD_ORG_OR_TENANT_HEADER\|ORG_CODE_REQUIRED_FOR_TENANT_ADMIN\|INVALID_ORG_OR_TENANT_CODE\|INVALID_ORG_CODE_FOR_TENANT" src/locales/en.json

echo ""
echo "=== TENANT_ADMIN_ROLE in validRoles (Keycloak path) ==="
grep -n "TENANT_ADMIN_ROLE\|validRoles" src/middlewares/authenticator.js

Length of output: 70603


@borkarsaish65, I've re-verified all the changed files. Everything looks clean. Here's the final checklist:

Check Status
common.js — inline fallbacks for ORG_CODE_HEADER / TENANT_CODE_HEADER ✅
authenticator.js — admin/tenant-admin override logic ✅
authenticator.js — TENANT_ADMIN_ROLE in validRoles (Keycloak path, line 524) ✅
requests/user.js — TENANT_ADMIN_ROLE in validRoles set ✅
notification.js — permission-based access via middleware (no explicit role guard needed) ✅
envVariables.js — both new vars optional: true with correct defaults ✅
.env.sample — ORG_CODE_HEADER_NAME and TENANT_CODE_HEADER_NAME documented ✅
en.json — all 4 locale keys at lines 191–194 with correct interpolation ✅
Migration 20260511122947-* — transactional, down clean, ignoreDuplicates in place ✅

All previously raised issues have been addressed. Approving! ✅

[approve]

@rakeshSgr
rakeshSgr merged commit 7799751 into ELEVATE-Project:develop May 15, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants