-
Notifications
You must be signed in to change notification settings - Fork 30
fix(auth): lookup user role from membership for mesh JWT and API keys #2137
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.
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -473,10 +473,27 @@ async function authenticateRequest( | |
| try { | ||
| const meshJwtPayload = await verifyMeshToken(token); | ||
| if (meshJwtPayload) { | ||
| // Look up user's organization role for admin/owner bypass | ||
| let role: string | undefined; | ||
| if (meshJwtPayload.sub && meshJwtPayload.metadata?.organizationId) { | ||
| const membership = await db | ||
| .selectFrom("member") | ||
| .select(["member.role"]) | ||
| .where("member.userId", "=", meshJwtPayload.sub) | ||
| .where( | ||
| "member.organizationId", | ||
| "=", | ||
| meshJwtPayload.metadata.organizationId, | ||
| ) | ||
| .executeTakeFirst(); | ||
| role = membership?.role; | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DB error in role lookup silently fails valid authenticationThe new database queries for role lookup are placed inside try blocks designed only for JWT/API key validation errors. If the database query throws (connection error, timeout, etc.), the exception is caught by the outer catch block, causing valid authentication to silently fail. For Mesh JWT, there's no logging at all - the code just proceeds to try API key authentication. For API keys, the error is logged with a misleading message. A transient database error would cause authenticated users to unexpectedly lose access. Additional Locations (1) |
||
| } | ||
|
|
||
| return { | ||
| user: { | ||
| id: meshJwtPayload.sub, | ||
| connectionId: meshJwtPayload.metadata?.connectionId, | ||
| role, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing top-level role breaks bypass in two code pathsThe PR adds Additional Locations (1) |
||
| }, | ||
| permissions: meshJwtPayload.permissions, | ||
| organization: meshJwtPayload.metadata?.organizationId | ||
|
|
@@ -505,9 +522,21 @@ async function authenticateRequest( | |
| // API keys have permissions stored directly on them | ||
| const permissions = result.key.permissions as Permission | undefined; | ||
|
|
||
| // Look up user's organization role for admin/owner bypass | ||
| let role: string | undefined; | ||
| if (result.key.userId && orgMetadata?.id) { | ||
| const membership = await db | ||
| .selectFrom("member") | ||
| .select(["member.role"]) | ||
| .where("member.userId", "=", result.key.userId) | ||
| .where("member.organizationId", "=", orgMetadata.id) | ||
| .executeTakeFirst(); | ||
| role = membership?.role; | ||
| } | ||
|
|
||
| return { | ||
| apiKeyId: result.key.id, | ||
| user: { id: result.key.userId }, // Include userId from API key | ||
| user: { id: result.key.userId, role }, // Include userId and role from membership | ||
| permissions, // Store the API key's permissions | ||
| organization: orgMetadata | ||
| ? { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Role lookup query doesn't filter by organization. A user with memberships in multiple organizations could get the wrong role (e.g., 'member' role from Org A instead of 'owner' from Org B), breaking the admin/owner bypass. Add
.where("member.organizationId", "=", meshJwtPayload.metadata?.organizationId)to scope the query.Prompt for AI agents
✅ Addressed in
de852d5