Fix: use user role for OAuth tokens bearing user context (#18954)
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`) |
This commit is contained in:
+225
@@ -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> = {}): 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' }),
|
||||
);
|
||||
});
|
||||
});
|
||||
+7
-7
@@ -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!,
|
||||
|
||||
@@ -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];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user