fix: return 401 in getAllPermissions when user is not authenticated#6131
fix: return 401 in getAllPermissions when user is not authenticated#6131octo-patch wants to merge 1 commit intoFlowiseAI:mainfrom
Conversation
…ixes FlowiseAI#6105) When GET /api/v1/auth/resolve is called without authentication, the request is whitelisted (passthrough) and matches the GET /api/v1/auth/:type route with type='resolve'. This calls getAllPermissions() where req.user is undefined, causing a 500 error: 'Cannot read properties of undefined (reading isOrganizationAdmin)'. Add an early guard to return 401 Unauthorized if no user is present, preventing the crash and returning a proper error response.
There was a problem hiding this comment.
Code Review
This pull request introduces an authentication check in the getAllPermissions controller to return a 401 Unauthorized status if the user is not logged in. Feedback suggests reordering the logic to perform this check before serializing permissions and using loose equality (== null) for the nullish check to align with standard idioms.
| const allPermissions = appServer.identityManager.getPermissions().toJSON() | ||
| const user = req.user as LoggedInUser | ||
|
|
||
| if (!user) { | ||
| return res.status(StatusCodes.UNAUTHORIZED).json({ message: 'Unauthorized' }) | ||
| } |
There was a problem hiding this comment.
Moving the authentication check before the permissions serialization avoids unnecessary processing for unauthenticated requests. This is particularly beneficial for endpoints that might be exposed to unauthenticated traffic. Additionally, using loose equality (== null) is the standard idiom for nullish checks in this project as per the general rules.
const user = req.user as LoggedInUser
if (user == null) {
return res.status(StatusCodes.UNAUTHORIZED).json({ message: 'Unauthorized' })
}
const allPermissions = appServer.identityManager.getPermissions().toJSON()References
- In JavaScript/TypeScript, use loose equality (== null) as a standard idiom for a 'nullish' check that covers both null and undefined.
Fixes #6105
Problem
GET /api/v1/auth/resolvereturns a 500 Internal Server Error with message:Root cause:
/api/v1/auth/resolveis added to the whitelist (unauthenticated passthrough), intended for thePOST /api/v1/auth/resolvehandler. However, aGETrequest to the same path also bypasses authentication and matches theGET /api/v1/auth/:typeroute withtype='resolve'. This callsgetAllPermissions()wherereq.userisundefined, leading to the crash at:This causes the login page to fail when the UI or tooling makes an unauthenticated GET to this endpoint.
Solution
Add an early null check in
getAllPermissionsto return401 Unauthorizedifreq.useris not set, instead of crashing with a 500.This is a minimal defensive fix that makes the endpoint robust against unauthenticated requests regardless of how they reach the handler.
Testing
GET /api/v1/auth/resolvenow returns401instead of500GET /api/v1/auth/:typeare unaffectedPOST /api/v1/auth/resolve(used by the UI) is unaffected