Skip to content

User Update Internal access api - service-to-service calls #872

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.

Sign up for GitHub

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

Open
komalm wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

User Update Internal access api - service-to-service calls #872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/.env.sample
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ KAFKA_TOPIC = 'topic'
# Kafka topic to push notification data
NOTIFICATION_KAFKA_TOPIC = notificationtopic

# Internal access token for communicationcation between services via network call
INTERNAL_ACCESS_TOKEN = 'internal-access-token'
# Internal access token for service-to-service / data-pipeline calls (e.g. internal user update). Set a strong secret.
INTERNAL_ACCESS_TOKEN=<set-your-secure-secret>


# JWT Access Token expiry In Days
ACCESS_TOKEN_EXPIRY = '1'
Expand Down
3 changes: 3 additions & 0 deletions src/constants/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ module.exports = {
DEFAULT_PAGE_SIZE: 100,
},
getPaginationOffset,
// Header used by data-pipeline / internal services to specify target user for internal user update (no user auth token)
INTERNAL_USER_ID_HEADER: 'x-internal-user-id',
internalAccessUrls: [
'/user/v1/profile/details',
'/user/v1/user/profileById',
'/user/v1/user/update',
Comment on lines +17 to +22
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Exposing /user/v1/user/update as an internal-access URL is a significant privilege escalation surface — ensure the token bypass issue is fixed first.

This endpoint allows modifying user data. Combined with the undefined === undefined bypass noted in envVariables.js, any caller can hit this endpoint without authentication if the env var is unset. The constants themselves are fine once the token validation is hardened.

🤖 Prompt for AI Agents 
In `@src/constants/common.js` around lines 17 - 22, Remove the privileged endpoint
from the internal access list and harden the token bypass check: remove
'/user/v1/user/update' from internalAccessUrls (and keep INTERNAL_USER_ID_HEADER
unchanged), then fix the token validation logic in envVariables.js so it does
not rely on a truthy/undefined comparison (the current "undefined === undefined"
style check) — instead validate the presence and value of the expected env
var/token explicitly and only allow requests into internalAccessUrls when that
validation succeeds; once the token validation is corrected, you may re-add
'/user/v1/user/update' if truly internal-only.

'/user/v1/account/list',
'/user/v1/user/read',
'/user/v1/admin/create',
Expand Down
5 changes: 5 additions & 0 deletions src/envVariables.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ let enviromentVariables = {
optional: true,
default: 86400,
},
INTERNAL_ACCESS_TOKEN: {
message:
'Secret token for internal/service-to-service and data-pipeline API calls (e.g. user update without user auth token). Keep secure.',
optional: true,
},
Comment on lines +119 to +123
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: optional: true with no default allows internal-access bypass when the env var is unset.

In authenticator.js Line 82, the check is req.headers.internal_access_token === process.env.INTERNAL_ACCESS_TOKEN. If INTERNAL_ACCESS_TOKEN is not configured, both sides are undefined, and undefined === undefined is true — any request without the header will pass the internal access gate.

Either make this variable required (optional: false), or add an explicit guard in the authenticator that rejects internal access when the token is not configured (e.g., process.env.INTERNAL_ACCESS_TOKEN && req.headers.internal_access_token === process.env.INTERNAL_ACCESS_TOKEN).

Option A – make it required 
 INTERNAL_ACCESS_TOKEN: {
     message:
         'Secret token for internal/service-to-service and data-pipeline API calls (e.g. user update without user auth token). Keep secure.',
-    optional: true,
+    optional: false,
 },
Option B – guard in the authenticator (src/middlewares/authenticator.js) 
-				if (req.headers.internal_access_token === process.env.INTERNAL_ACCESS_TOKEN) return true
+				if (process.env.INTERNAL_ACCESS_TOKEN && req.headers.internal_access_token === process.env.INTERNAL_ACCESS_TOKEN) return true
🤖 Prompt for AI Agents 
In `@src/envVariables.js` around lines 119 - 123, The INTERNAL_ACCESS_TOKEN env
spec is marked optional which allows undefined === undefined to pass the
internal access check; update either the env spec to required by setting
INTERNAL_ACCESS_TOKEN.optional = false in src/envVariables.js or (safer) add an
explicit guard in the authenticator middleware (authenticator.js) so the check
becomes something like: ensure process.env.INTERNAL_ACCESS_TOKEN is truthy and
compare req.headers.internal_access_token === process.env.INTERNAL_ACCESS_TOKEN,
and reject internal access when the env var is missing; reference the symbols
INTERNAL_ACCESS_TOKEN, process.env.INTERNAL_ACCESS_TOKEN and
req.headers.internal_access_token to locate the change.

REDIS_HOST: {
message: 'Redis Host Url',
optional: false,
Expand Down
83 changes: 72 additions & 11 deletions src/middlewares/authenticator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,36 @@ const tenantDomainQueries = require('@database/queries/tenantDomain')
const organizationQueries = require('@database/queries/organization')

async function checkPermissions(roleTitle, requestPath, requestMethod) {
const parts = requestPath.match(/[^/]+/g)
const api_path = [`/${parts[0]}/${parts[1]}/${parts[2]}/*`]

if (parts[4]) api_path.push(`/${parts[0]}/${parts[1]}/${parts[2]}/${parts[3]}*`)
else
api_path.push(
`/${parts[0]}/${parts[1]}/${parts[2]}/${parts[3]}`,
`/${parts[0]}/${parts[1]}/${parts[2]}/${parts[3]}*`
)
const parts = requestPath.match(/[^/]+/g) || []
let api_path

if (parts.length >= 4) {
api_path = [`/${parts[0]}/${parts[1]}/${parts[2]}/*`]
if (parts[4]) api_path.push(`/${parts[0]}/${parts[1]}/${parts[2]}/${parts[3]}*`)
else
api_path.push(
`/${parts[0]}/${parts[1]}/${parts[2]}/${parts[3]}`,
`/${parts[0]}/${parts[1]}/${parts[2]}/${parts[3]}*`
)
} else if (parts.length === 3) {
api_path = [`/${parts[0]}/${parts[1]}/${parts[2]}`, `/${parts[0]}/${parts[1]}/${parts[2]}/*`]
} else if (parts.length === 2) {
api_path = [`/${parts[0]}/${parts[1]}`, `/${parts[0]}/${parts[1]}/*`]
} else if (parts.length === 1) {
api_path = [`/${parts[0]}`, `/${parts[0]}/*`]
} else {
return false
}

if (Array.isArray(roleTitle) && !roleTitle.includes(common.PUBLIC_ROLE)) {
roleTitle.push(common.PUBLIC_ROLE)
}

const filter = { role_title: roleTitle, module: parts[2], api_path: { [Op.in]: api_path } }
const filter = {
role_title: roleTitle,
...(parts[2] != null && { module: parts[2] }),
api_path: { [Op.in]: api_path },
}
const attributes = ['request_type', 'api_path', 'module']
const allowedPermissions = await rolePermissionMappingQueries.findAll(filter, attributes)

Expand Down Expand Up @@ -98,7 +113,53 @@ module.exports = async function (req, res, next) {
}
})

if (internalAccess && !authHeader) return next()
// Internal access without user token (existing internal URLs unchanged; pipeline uses internal_access_token only)
if (internalAccess && !authHeader) {
// New path only: data-pipeline user update — resolve target user from headers, set decodedToken; no change to other internal URLs
const internalUserUpdatePath = '/user/v1/user/update'
if (req.path.includes(internalUserUpdatePath)) {
// Express normalizes headers to lowercase
const userId = (req.headers[common.INTERNAL_USER_ID_HEADER] || '').trim()
const tenantCode = (req.headers[common.TENANT_CODE_HEADER] || req.headers['x-tenant-code'] || '').trim()
if (!userId || !tenantCode) {
throw responses.failureResponse({
message: 'INTERNAL_UPDATE_REQUIRES_USER_ID_AND_TENANT',
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}
const user = await userQueries.findUserWithOrganization(
{ id: userId, tenant_code: tenantCode },
{},
false
)
if (!user) {
throw responses.failureResponse({
message: 'USER_NOT_FOUND',
statusCode: httpStatusCode.unauthorized,
responseCode: 'UNAUTHORIZED',
})
}
const org = user.organizations?.[0]
if (!org) {
throw responses.failureResponse({
message: 'USER_HAS_NO_ORGANIZATION',
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})
}
req.decodedToken = {
id: user.id,
name: user.name,
session_id: null,
tenant_code: user.tenant_code,
organization_id: org.id,
organization_code: org.code,
roles: org.roles || [],
}
}
return next()
}
Comment on lines +116 to +162
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, find and examine the authenticator.js file around the specified lines
fd -t f "authenticator.js" | head -5

Repository: ELEVATE-Project/user

Length of output: 96

🏁 Script executed:

# Check the file structure and get context around lines 116-162
cat -n src/middlewares/authenticator.js | sed -n '110,170p'

Repository: ELEVATE-Project/user

Length of output: 2556

🏁 Script executed:

# Look for common.TENANT_CODE_HEADER definition
rg -n "TENANT_CODE_HEADER" --type=js -B2 -A2

Repository: ELEVATE-Project/user

Length of output: 4617

🏁 Script executed:

# Check session_id usage around line 239 and in general
rg -n "session_id" src/middlewares/authenticator.js -B2 -A2

Repository: ELEVATE-Project/user

Length of output: 620

🏁 Script executed:

# Find and examine findUserWithOrganization function
fd -t f "*.js" | xargs rg -l "findUserWithOrganization" | head -5

Repository: ELEVATE-Project/user

Length of output: 490

🏁 Script executed:

# Check for permission checks in the middleware
rg -n "checkPermission" src/middlewares/authenticator.js -B3 -A3

Repository: ELEVATE-Project/user

Length of output: 855

🏁 Script executed:

# Find and examine findUserWithOrganization function definition
rg -n "findUserWithOrganization" -A 10 --type=js | head -40

Repository: ELEVATE-Project/user

Length of output: 2366

🏁 Script executed:

# Check how userId/id is typically validated elsewhere in the codebase
rg -n "parseInt.*userId|UUID.*userId|validate.*userId" -B2 -A2 --type=js | head -50

Repository: ELEVATE-Project/user

Length of output: 46

🏁 Script executed:

# Look at the userQueries to see the function signature
find src -name "*queries*" -o -name "*user*" | grep -i query

Repository: ELEVATE-Project/user

Length of output: 46

🏁 Script executed:

# Check context around line 239 to understand if the internal path actually reaches that code
cat -n src/middlewares/authenticator.js | sed -n '225,245p'

Repository: ELEVATE-Project/user

Length of output: 861

🏁 Script executed:

# Verify if common.INTERNAL_USER_ID_HEADER is defined and what it contains
rg -n "INTERNAL_USER_ID_HEADER" --type=js -B2 -A2

Repository: ELEVATE-Project/user

Length of output: 951

🏁 Script executed:

# Look at the userQueries file to see findUserWithOrganization implementation
find src -path "*database*" -name "*.js" | xargs rg -l "findUserWithOrganization" 2>/dev/null | head -3

Repository: ELEVATE-Project/user

Length of output: 93

🏁 Script executed:

# Check database queries to see how id is used in findUserWithOrganization
rg -n "findUserWithOrganization" -A 15 src/database/queries/user.js 2>/dev/null | head -50

Repository: ELEVATE-Project/user

Length of output: 46

🏁 Script executed:

# Check if there are other places where userId from headers is validated
rg -n "INTERNAL_USER_ID_HEADER\|x-internal-user-id" -B5 -A5 --type=js

Repository: ELEVATE-Project/user

Length of output: 46

🏁 Script executed:

# Verify the control flow - does the internal path actually reach line 239?
cat -n src/middlewares/authenticator.js | sed -n '115,165p'

Repository: ELEVATE-Project/user

Length of output: 2118

🏁 Script executed:

# Try to find and read the users.js file in database queries
find src -name "users.js" -o -name "user.js" | grep -i database

Repository: ELEVATE-Project/user

Length of output: 122

🏁 Script executed:

# Get the actual findUserWithOrganization implementation
rg -n "findUserWithOrganization\s*=" src/database/queries/users.js -A 20 2>/dev/null || rg -n "findUserWithOrganization" src/database/queries/users.js -A 20

Repository: ELEVATE-Project/user

Length of output: 757

🏁 Script executed:

# Check if there's validation elsewhere in the middleware or at the entry point for userId
rg -n "INTERNAL_USER_ID_HEADER" -B10 -A10 --type=js

Repository: ELEVATE-Project/user

Length of output: 3059

🏁 Script executed:

# Verify the complete control flow - check if line 239 is reachable from the internal path
cat -n src/middlewares/authenticator.js | sed -n '115,250p'

Repository: ELEVATE-Project/user

Length of output: 5581

🏁 Script executed:

# Verify if /user/v1/user/update-profile or similar would match req.path.includes()
node <<'EOF'
const path1 = '/user/v1/user/update';
const path2 = '/user/v1/user/update';
const path3 = '/user/v1/user/update-profile';
const path4 = '/user/v1/user/update/extra';

console.log('path.includes test:');
console.log(`path2 includes path1: ${path2.includes(path1)}`);
console.log(`path3 includes path1: ${path3.includes(path1)}`);
console.log(`path4 includes path1: ${path4.includes(path1)}`);
EOF

Repository: ELEVATE-Project/user

Length of output: 164

🏁 Script executed:

# Check what happens if userId is invalid - look at Sequelize's behavior with where clause
rg -n "where:" src/database/queries/users.js -A 5 | head -20

Repository: ELEVATE-Project/user

Length of output: 390

🏁 Script executed:

# Confirm that the internal path returns before reaching line 239 by checking the flow
cat -n src/middlewares/authenticator.js | sed -n '160,165p'

Repository: ELEVATE-Project/user

Length of output: 162

Multiple security concerns in the internal-access update flow.

  1. Line 120 – unintended path matches: req.path.includes(internalUserUpdatePath) matches /user/v1/user/update-profile and similar paths. Use exact comparison: req.path === internalUserUpdatePath || req.path.startsWith(internalUserUpdatePath + '/').

  2. Line 123 – redundant fallback: common.TENANT_CODE_HEADER already resolves to 'x-tenant-code' by default, so the || req.headers['x-tenant-code'] fallback is a no-op. Remove it.

  3. Line 122 – no userId validation: userId from the header is passed directly to the database query without type checking. Validate it as a valid integer or UUID to prevent Sequelize type coercion issues or unexpected query behavior.

  4. Line 154 – null session_id in decodedToken: While the internal path returns before reaching downstream code that accesses session_id (line 239), explicitly null values in token objects complicate future refactoring. Either set a valid placeholder or document the invariant clearly.

  5. Lines 116–162 – no permission check: The internal update path bypasses checkPermissions entirely. If this internal token is meant to act as a superuser credential, document the security implications explicitly.

Proposed fixes for items 1–3 
-		if (req.path.includes(internalUserUpdatePath)) {
+		if (req.path === internalUserUpdatePath || req.path.startsWith(internalUserUpdatePath + '/')) {
 			// Express normalizes headers to lowercase
 			const userId = (req.headers[common.INTERNAL_USER_ID_HEADER] || '').trim()
-			const tenantCode = (req.headers[common.TENANT_CODE_HEADER] || req.headers['x-tenant-code'] || '').trim()
-			if (!userId || !tenantCode) {
+			const tenantCode = (req.headers[common.TENANT_CODE_HEADER] || '').trim()
+			if (!userId || !tenantCode || !/^\d+$/.test(userId)) {
 				throw responses.failureResponse({
 					message: 'INTERNAL_UPDATE_REQUIRES_USER_ID_AND_TENANT',
 					statusCode: httpStatusCode.bad_request,
 					responseCode: 'CLIENT_ERROR',
 				})
 			}
🤖 Prompt for AI Agents 
In `@src/middlewares/authenticator.js` around lines 116 - 162, The internal-access
update flow improperly matches paths, redundantly reads tenant header, lacks
userId validation, sets session_id to null, and bypasses permission checks; fix
by changing the path match for internalUserUpdatePath to exact equality or a
strict prefix check (use req.path === internalUserUpdatePath ||
req.path.startsWith(internalUserUpdatePath + '/')), remove the redundant
fallback that references 'x-tenant-code' since common.TENANT_CODE_HEADER already
normalizes it, validate the userId header (parse/validate as the expected
type—integer or UUID) before calling userQueries.findUserWithOrganization to
avoid injection/coercion, replace req.decodedToken.session_id = null with either
a documented placeholder value or a clearly commented invariant provider (or set
to undefined) in req.decodedToken, and ensure a permission check or explicit
superuser assertion is performed (or documented) before returning next() so the
internal path does not silently bypass checkPermissions.


if (!authHeader) {
try {
Expand Down