feat(auth): allow admin override of tenant/org #838
Conversation
|
Caution Review failedFailed to post review comments WalkthroughAdds three configurable header-name environment variables for org/tenant identification, introduces two English locale keys, and revises authenticator middleware to support admin-only header-based overrides for tenant_code, organization_id, and organization_code with validation and error handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant MW as Authenticator Middleware
participant ENV as Env Config
C->>MW: HTTP request with Authorization header
MW->>MW: Validate/parse JWT -> decodedToken
alt isAdmin
MW->>ENV: Read ORG_ID_HEADER_NAME / ORG_CODE_HEADER_NAME / TENANT_CODE_HEADER_NAME
MW->>MW: Extract & trim header values
alt Any override header present
MW->>MW: Validate all three present
MW->>MW: Validate orgId is positive integer
alt Validation fails
MW-->>C: 400/422 error (invalid/missing headers)
else Validation passes
MW->>MW: Override decodedToken.data { tenant_code, organization_id, organization_code }
MW->>MW: req.decodedToken = decodedToken.data
MW-->>C: next()
end
else No override headers
MW->>MW: req.decodedToken = decodedToken.data
MW-->>C: next()
end
else non-admin
MW->>MW: Standard token handling
MW->>MW: req.decodedToken = decodedToken.data
MW-->>C: next()
end
note over MW: Admin overrides only apply when all headers are provided<br/>and orgId passes integer/positivity checks.
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
✨ 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 |
coderabbitai
bot
changed the title
nevil-mathew
changed the title
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/middlewares/authenticator.js (1)
172-181: Possible crash when organizations/roles missing; guard and default.org may be undefined; org.roles access will throw. Default roles safely.
- decodedToken.data = { - id: decodedToken.data.id, - name: decodedToken.data.name, - session_id: decodedToken.data.session_id, - tenant_code: decodedToken.data.tenant_code, - organization_id, - organization_code, - roles: org.roles, - } + const roles = + Array.isArray(org?.roles) + ? org.roles + : (Array.isArray(decodedToken.data.roles) ? decodedToken.data.roles : []) + decodedToken.data = { + id: decodedToken.data.id, + name: decodedToken.data.name, + session_id: decodedToken.data.session_id, + tenant_code: decodedToken.data.tenant_code, + organization_id, + organization_code, + roles, + }
🧹 Nitpick comments (3)
src/locales/en.json (1)
186-187: Use consistent interpolation placeholders (double curly braces).Elsewhere i18n uses {{var}}. Update to avoid runtime/template mismatch.
- "ADD_ORG_HEADER": "Please provide all required organization headers: {orgIdHeader}, {orgCodeHeader}, and {tenantCodeHeader} for admin override.", + "ADD_ORG_HEADER": "Please provide all required organization headers: {{orgIdHeader}}, {{orgCodeHeader}}, and {{tenantCodeHeader}} for admin override.",src/middlewares/authenticator.js (2)
276-281: Guard role fallback during roleValidation to avoid undefined access.org may be undefined; fall back to decodedToken.data.roles or [].
- const roles = org.roles + const roles = + Array.isArray(org?.roles) + ? org.roles + : (Array.isArray(decodedToken.data.roles) ? decodedToken.data.roles : [])
256-260: Add audit logging when admin override is applied.For traceability, log admin ID, original org/tenant, and overridden values (no PII).
I can wire this using your existing logger (e.g., @generics/logger). Want a quick patch?
Also applies to: 262-263
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/envVariables.js(1 hunks)src/locales/en.json(1 hunks)src/middlewares/authenticator.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/middlewares/**
⚙️ CodeRabbit configuration file
This is security-sensitive middleware. Scrutinize for potential vulnerabilities.
Files:
src/middlewares/authenticator.js
🧠 Learnings (1)
📚 Learning: 2025-07-31T08:44:36.982Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/services/entities.js:18-23
Timestamp: 2025-07-31T08:44:36.982Z
Learning: In the ELEVATE-Project/user codebase, organizationCode and tenantCode parameters passed to service methods always come from req.decodedToken.organization_code and req.decodedToken.tenant_code, which are guaranteed to be present after token validation. Additional validation for these parameters in service methods is unnecessary as the token validation process ensures they are always available.
Applied to files:
src/middlewares/authenticator.js
🧬 Code graph analysis (1)
src/middlewares/authenticator.js (2)
src/services/organization-feature.js (3)
isAdmin(36-36)orgCode(38-38)tenantCode(39-39)src/controllers/v1/organization-feature.js (3)
isAdmin(27-27)orgCode(68-68)tenantCode(69-69)
🔇 Additional comments (2)
src/middlewares/authenticator.js (2)
228-244: failureResponse supports message objects with key and interpolation
The helper extractsmessage.keyas the error message and preservesmessage.interpolation, so passing an object is valid.
216-244: Use environment-configured header names consistently.
Verify thatcommon.ORG_ID_HEADER,common.ORG_CODE_HEADER, andcommon.TENANT_CODE_HEADERare sourced from the correspondingprocess.env.*_HEADER_NAMEvalues (or yourenvVariablesdefaults) so header lookups inauthenticator.js,organization-feature.js, and any other modules remain in sync.
| const orgIdHeaderName = process.env.ORG_ID_HEADER_NAME | ||
| const orgCodeHeaderName = process.env.ORG_CODE_HEADER_NAME | ||
| const tenantCodeHeaderName = process.env.TENANT_CODE_HEADER_NAME | ||
|
|
||
| // Extract and sanitize header values (trim whitespace, case-insensitive header lookup) | ||
| const orgId = (req.headers[orgIdHeaderName.toLowerCase()] || '').trim() | ||
| const orgCode = (req.headers[orgCodeHeaderName.toLowerCase()] || '').trim() | ||
| const tenantCode = (req.headers[tenantCodeHeaderName.toLowerCase()] || '').trim() | ||
|
|
There was a problem hiding this comment.
Avoid toLowerCase on potentially undefined env vars; prefer req.get and add safe defaults.
If env is unset, orgIdHeaderName.toLowerCase() throws. Also use req.get() for case-insensitive retrieval.
- const orgIdHeaderName = process.env.ORG_ID_HEADER_NAME
- const orgCodeHeaderName = process.env.ORG_CODE_HEADER_NAME
- const tenantCodeHeaderName = process.env.TENANT_CODE_HEADER_NAME
-
- // Extract and sanitize header values (trim whitespace, case-insensitive header lookup)
- const orgId = (req.headers[orgIdHeaderName.toLowerCase()] || '').trim()
- const orgCode = (req.headers[orgCodeHeaderName.toLowerCase()] || '').trim()
- const tenantCode = (req.headers[tenantCodeHeaderName.toLowerCase()] || '').trim()
+ const orgIdHeaderName = process.env.ORG_ID_HEADER_NAME || 'X-Org-Id'
+ const orgCodeHeaderName = process.env.ORG_CODE_HEADER_NAME || 'X-Org-Code'
+ const tenantCodeHeaderName = process.env.TENANT_CODE_HEADER_NAME || 'X-Tenant-Code'
+
+ // Extract and sanitize header values (Express req.get is case-insensitive)
+ const orgId = (req.get(orgIdHeaderName) || '').trim()
+ const orgCode = (req.get(orgCodeHeaderName) || '').trim()
+ const tenantCode = (req.get(tenantCodeHeaderName) || '').trim()📝 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 orgIdHeaderName = process.env.ORG_ID_HEADER_NAME | |
| const orgCodeHeaderName = process.env.ORG_CODE_HEADER_NAME | |
| const tenantCodeHeaderName = process.env.TENANT_CODE_HEADER_NAME | |
| // Extract and sanitize header values (trim whitespace, case-insensitive header lookup) | |
| const orgId = (req.headers[orgIdHeaderName.toLowerCase()] || '').trim() | |
| const orgCode = (req.headers[orgCodeHeaderName.toLowerCase()] || '').trim() | |
| const tenantCode = (req.headers[tenantCodeHeaderName.toLowerCase()] || '').trim() | |
| const orgIdHeaderName = process.env.ORG_ID_HEADER_NAME || 'X-Org-Id' | |
| const orgCodeHeaderName = process.env.ORG_CODE_HEADER_NAME || 'X-Org-Code' | |
| const tenantCodeHeaderName = process.env.TENANT_CODE_HEADER_NAME || 'X-Tenant-Code' | |
| // Extract and sanitize header values (Express req.get is case-insensitive) | |
| const orgId = (req.get(orgIdHeaderName) || '').trim() | |
| const orgCode = (req.get(orgCodeHeaderName) || '').trim() | |
| const tenantCode = (req.get(tenantCodeHeaderName) || '').trim() |
🤖 Prompt for AI Agents
In src/middlewares/authenticator.js around lines 219 to 227, the current code
calls toLowerCase() on environment variables and indexes req.headers directly,
which will throw if an env var is undefined and isn't the safest way to do
case-insensitive header lookup; change the code to first default each header
name from process.env to a safe string (e.g. '' or a known header key) before
calling toLowerCase or avoid calling it entirely, and use Express's
req.get(headerName) to retrieve headers case-insensitively, then trim the
returned value or default to '' so orgId, orgCode and tenantCode are always
defined strings.
There was a problem hiding this comment.
We have already added a default values for these new env variables.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the implementation of envVariables.js to understand how defaults are applied
cat src/envVariables.js
Length of output: 15049
🏁 Script executed:
#!/bin/bash
# Check if authenticator.js imports or uses the envVariables module
rg -n "envVariables|@envVariables|require.*envVariables" src/middlewares/authenticator.js
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check how envVariables is typically used in other parts of the codebase
rg -n -C2 "require.*envVariables" --type=js -g '!node_modules/**' | head -50
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Find the application entry point files
fd -e js "^(app|server|index)\.js$" -d 2
Length of output: 10
🏁 Script executed:
#!/bin/bash
# Check if envVariables is imported/required in common entry points
rg -n "envVariables" --type=js src/app.js src/server.js src/index.js app.js server.js index.js 2>/dev/null || echo "No matches found in entry points"
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Search more broadly for where envVariables module is used/invoked
rg -n "require.*envVariables|envVariables\(\)" --type=js -g '!node_modules' -g '!dist' -g '!build'
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Check the startup sequence in app.js to confirm envVariables runs before middleware
cat src/app.js | head -30
Length of output: 934
@nevil-mathew, you're absolutely right! I verified that src/app.js line 22 executes require('./envVariables')() at application startup, which sets these environment variables to their defaults before any middleware loads:
ORG_ID_HEADER_NAME: 'X-Org-Id'
ORG_CODE_HEADER_NAME: 'X-Org-Code'
TENANT_CODE_HEADER_NAME: 'X-Tenant-Code'So toLowerCase() won't throw errors as I initially warned. The current implementation is safe.
That said, using req.get(headerName) instead of req.headers[headerName.toLowerCase()] is still considered better practice since it's the Express-recommended way for case-insensitive header retrieval, but it's not critical given your startup sequence guarantees these values exist. The current code will work correctly.
🧬 Code Graph Analysis Results
[src/services/organization.js] lines 518-521
let filter = {
organization_code: { [Op.in]: [organizationCode, defaultOrganizationCode] },
tenant_code: tenantCode,
}
🧠 Learnings used
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/services/entities.js:18-23
Timestamp: 2025-07-31T08:44:36.982Z
Learning: In the ELEVATE-Project/user codebase, organizationCode and tenantCode parameters passed to service methods always come from req.decodedToken.organization_code and req.decodedToken.tenant_code, which are guaranteed to be present after token validation. Additional validation for these parameters in service methods is unnecessary as the token validation process ensures they are always available.
Summary by CodeRabbit
New Features
Bug Fixes