From bf22373315cbe98c5541d29bfc3fe31c31bc1abe Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:38:01 +0400 Subject: [PATCH] Fix: use user role for OAuth tokens bearing user context (#18954) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit see [discord discussion](https://discord.com/channels/1130383047699738754/1486299347091198054/1486299351520383116) ## Summary When an OAuth application token carries both `applicationId` and `userId`/`userWorkspaceId`, the auth context now uses the **user's role** for permissions instead of the application's `defaultRoleId`. This fixes the case where external clients authenticating via OAuth (e.g. a client's external AI chat) were getting the app's permissions instead of the authenticated user's. ### What changed - **`jwt.auth.strategy.ts`** (`validateApplicationToken`): when an application token includes user info, also resolve `workspaceMemberId` and `workspaceMember` from the workspace cache (same pattern as `validateAccessToken`) - **`workspace-auth-context.middleware.ts`** (`buildAuthContext`): when both `application` and `user` (with `workspaceMemberId`/`workspaceMember`) are present on the request, build a `UserWorkspaceAuthContext` instead of `ApplicationWorkspaceAuthContext`. Falls back to application context if workspace member cannot be resolved. ### Places impacted by this change (no code changes, behavior changes) These places check `isApplicationAuthContext` or resolve roles from auth context. Since hybrid tokens (OAuth with user) now produce a `UserWorkspaceAuthContext`, they naturally flow into the `isUserAuthContext` branches: | File | Impact | |------|--------| | `permissions.service.ts` — `resolveRolePermissionConfigFromAuthContext` | OAuth+user now uses user's role via `isUserAuthContext` branch instead of `application.defaultRoleId` | | `common-api-context-builder.service.ts` — `getObjectsPermissions` | Same — OAuth+user falls into `isUserAuthContext` branch | | `common-base-query-runner.service.ts` — `getRoleIdOrThrow` | Same — OAuth+user falls into `isUserAuthContext` branch | | `actor-from-auth-context.service.ts` — `buildActorMetadata` | Records created via OAuth+user will show the **user's name** as actor instead of the application's name | | `message-find-one.post-query.hook.ts` | OAuth+user now passes the `isUserAuthContext` check (previously would fail unless it was the Twenty standard application) | | `front-component.resolver.ts` | Front component tokens include `userId` — they will now correctly use the user's role, fixing a pre-existing permission escalation where a user could access data through a front component's app role that exceeded their own | | `logic-function-executor.service.ts` | **Not impacted** — only generates tokens with `applicationId` (no `userId`) | --- .../workspace-auth-context.middleware.spec.ts | 225 ++++++++++++++++++ .../workspace-auth-context.middleware.ts | 14 +- .../auth/strategies/jwt.auth.strategy.ts | 14 ++ 3 files changed, 246 insertions(+), 7 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/auth/middlewares/__tests__/workspace-auth-context.middleware.spec.ts diff --git a/packages/twenty-server/src/engine/core-modules/auth/middlewares/__tests__/workspace-auth-context.middleware.spec.ts b/packages/twenty-server/src/engine/core-modules/auth/middlewares/__tests__/workspace-auth-context.middleware.spec.ts new file mode 100644 index 0000000000..425e5a1bfe --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/auth/middlewares/__tests__/workspace-auth-context.middleware.spec.ts @@ -0,0 +1,225 @@ +import { type NextFunction, type Request, type Response } from 'express'; + +import { + AuthException, + AuthExceptionCode, +} from 'src/engine/core-modules/auth/auth.exception'; +import { workspaceAuthContextStorage } from 'src/engine/core-modules/auth/storage/workspace-auth-context.storage'; + +import { WorkspaceAuthContextMiddleware } from '../workspace-auth-context.middleware'; + +const mockWorkspace = { + id: 'workspace-id', + displayName: 'Test Workspace', +} as Request['workspace']; + +const mockUser = { + id: 'user-id', + email: 'test@example.com', + firstName: 'Test', + lastName: 'User', +} as Request['user']; + +const mockApplication = { + id: 'application-id', + name: 'Test App', + defaultRoleId: 'app-role-id', +} as Request['application']; + +const mockApiKey = { + id: 'api-key-id', + name: 'Test API Key', +} as Request['apiKey']; + +const mockWorkspaceMember = { + id: 'workspace-member-id', + name: { firstName: 'Test', lastName: 'User' }, +} as Request['workspaceMember']; + +describe('WorkspaceAuthContextMiddleware', () => { + let middleware: WorkspaceAuthContextMiddleware; + let mockResponse: Response; + let mockNext: NextFunction; + + beforeEach(() => { + middleware = new WorkspaceAuthContextMiddleware(); + mockResponse = {} as Response; + mockNext = jest.fn(); + }); + + const buildRequest = (overrides: Partial = {}): Request => + ({ + workspace: mockWorkspace, + ...overrides, + }) as unknown as Request; + + it('should call next without auth context when workspace is not defined', () => { + const req = buildRequest({ workspace: undefined }); + + middleware.use(req, mockResponse, mockNext); + + expect(mockNext).toHaveBeenCalled(); + expect(workspaceAuthContextStorage.getStore()).toBeUndefined(); + }); + + it('should create an apiKey auth context when apiKey is present', () => { + const req = buildRequest({ apiKey: mockApiKey }); + let capturedContext: unknown; + + (mockNext as jest.Mock).mockImplementation(() => { + capturedContext = workspaceAuthContextStorage.getStore(); + }); + + middleware.use(req, mockResponse, mockNext); + + expect(capturedContext).toEqual( + expect.objectContaining({ type: 'apiKey', apiKey: mockApiKey }), + ); + }); + + it('should create a user auth context when both application and user are present', () => { + const req = buildRequest({ + application: mockApplication, + user: mockUser, + userWorkspaceId: 'user-workspace-id', + workspaceMemberId: 'workspace-member-id', + workspaceMember: mockWorkspaceMember, + }); + let capturedContext: unknown; + + (mockNext as jest.Mock).mockImplementation(() => { + capturedContext = workspaceAuthContextStorage.getStore(); + }); + + middleware.use(req, mockResponse, mockNext); + + expect(capturedContext).toEqual( + expect.objectContaining({ + type: 'user', + user: mockUser, + userWorkspaceId: 'user-workspace-id', + workspaceMemberId: 'workspace-member-id', + workspaceMember: mockWorkspaceMember, + }), + ); + }); + + it('should create an application auth context when application is present without user', () => { + const req = buildRequest({ application: mockApplication }); + let capturedContext: unknown; + + (mockNext as jest.Mock).mockImplementation(() => { + capturedContext = workspaceAuthContextStorage.getStore(); + }); + + middleware.use(req, mockResponse, mockNext); + + expect(capturedContext).toEqual( + expect.objectContaining({ + type: 'application', + application: mockApplication, + }), + ); + }); + + it('should fall back to application auth context when application and user are present but workspaceMember is missing', () => { + const req = buildRequest({ + application: mockApplication, + user: mockUser, + userWorkspaceId: 'user-workspace-id', + }); + let capturedContext: unknown; + + (mockNext as jest.Mock).mockImplementation(() => { + capturedContext = workspaceAuthContextStorage.getStore(); + }); + + middleware.use(req, mockResponse, mockNext); + + expect(capturedContext).toEqual( + expect.objectContaining({ + type: 'application', + application: mockApplication, + }), + ); + }); + + it('should create a user auth context when user is present without application', () => { + const req = buildRequest({ + user: mockUser, + userWorkspaceId: 'user-workspace-id', + workspaceMemberId: 'workspace-member-id', + workspaceMember: mockWorkspaceMember, + }); + let capturedContext: unknown; + + (mockNext as jest.Mock).mockImplementation(() => { + capturedContext = workspaceAuthContextStorage.getStore(); + }); + + middleware.use(req, mockResponse, mockNext); + + expect(capturedContext).toEqual( + expect.objectContaining({ + type: 'user', + user: mockUser, + userWorkspaceId: 'user-workspace-id', + }), + ); + }); + + it('should create a pendingActivationUser auth context when user and userWorkspaceId are present without workspaceMember', () => { + const req = buildRequest({ + user: mockUser, + userWorkspaceId: 'user-workspace-id', + }); + let capturedContext: unknown; + + (mockNext as jest.Mock).mockImplementation(() => { + capturedContext = workspaceAuthContextStorage.getStore(); + }); + + middleware.use(req, mockResponse, mockNext); + + expect(capturedContext).toEqual( + expect.objectContaining({ + type: 'pendingActivationUser', + user: mockUser, + userWorkspaceId: 'user-workspace-id', + }), + ); + }); + + it('should throw AuthException when workspace is present but no auth mechanism is found', () => { + const req = buildRequest(); + + expect(() => middleware.use(req, mockResponse, mockNext)).toThrow( + new AuthException( + 'No authentication context found', + AuthExceptionCode.UNAUTHENTICATED, + ), + ); + }); + + it('should prioritize apiKey over application and user', () => { + const req = buildRequest({ + apiKey: mockApiKey, + application: mockApplication, + user: mockUser, + userWorkspaceId: 'user-workspace-id', + workspaceMemberId: 'workspace-member-id', + workspaceMember: mockWorkspaceMember, + }); + let capturedContext: unknown; + + (mockNext as jest.Mock).mockImplementation(() => { + capturedContext = workspaceAuthContextStorage.getStore(); + }); + + middleware.use(req, mockResponse, mockNext); + + expect(capturedContext).toEqual( + expect.objectContaining({ type: 'apiKey' }), + ); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/auth/middlewares/workspace-auth-context.middleware.ts b/packages/twenty-server/src/engine/core-modules/auth/middlewares/workspace-auth-context.middleware.ts index 51b9c0cbfa..c446be9c32 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/middlewares/workspace-auth-context.middleware.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/middlewares/workspace-auth-context.middleware.ts @@ -38,13 +38,6 @@ export class WorkspaceAuthContextMiddleware implements NestMiddleware { }); } - if (isDefined(req.application)) { - return buildApplicationAuthContext({ - workspace: req.workspace!, - application: req.application, - }); - } - if ( isDefined(req.userWorkspaceId) && isDefined(req.workspaceMemberId) && @@ -60,6 +53,13 @@ export class WorkspaceAuthContextMiddleware implements NestMiddleware { }); } + if (isDefined(req.application)) { + return buildApplicationAuthContext({ + workspace: req.workspace!, + application: req.application, + }); + } + if (isDefined(req.userWorkspaceId) && isDefined(req.user)) { return buildPendingActivationUserAuthContext({ workspace: req.workspace!, diff --git a/packages/twenty-server/src/engine/core-modules/auth/strategies/jwt.auth.strategy.ts b/packages/twenty-server/src/engine/core-modules/auth/strategies/jwt.auth.strategy.ts index e82754a603..61879f5373 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/strategies/jwt.auth.strategy.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/strategies/jwt.auth.strategy.ts @@ -401,6 +401,20 @@ export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') { context.user = userContext.user; context.userWorkspace = userContext.userWorkspace; context.userWorkspaceId = userContext.userWorkspace.id; + + const { flatWorkspaceMemberMaps } = + await this.workspaceCacheService.getOrRecompute(workspace.id, [ + 'flatWorkspaceMemberMaps', + ]); + + const workspaceMemberId = + flatWorkspaceMemberMaps.idByUserId[userContext.user.id]; + + if (isDefined(workspaceMemberId)) { + context.workspaceMemberId = workspaceMemberId; + context.workspaceMember = + flatWorkspaceMemberMaps.byId[workspaceMemberId]; + } } }