f8e3fd110d
## Why
When an application acts on someone's behalf its token carries `userId`
and `userWorkspaceId` alongside `applicationId`, and the application
then received **that person's permissions in full**. The role it
installs with was never consulted, so it was not a bound on what the
application could do for them. It was meant to be an intersection.
`permissions.service.ts` made this visible: the branches are `apiKeyId →
userWorkspaceId → applicationId` and each returns early, so with both
present the user branch won and `application.defaultRoleId` was never
read. The same was true on the object and row-level paths, for different
reasons.
## What had to change
Three independent causes, all of which blocked the intersection from
existing or from being enforced.
**The application was thrown away before anything could use it.**
`workspace-auth-context.middleware.ts` built a `type: 'user'` context
when both principals were present, and `UserWorkspaceAuthContext` had no
slot for an application. It now carries an optional one.
Additive rather than a new union member on purpose. Nothing in the
server exhaustively checks this union (no `assertUnreachable`, one
`switch`, in Sentry tagging), so a sixth member would have compiled fine
and then fallen through actor attribution, that switch, and
`metadata-event-emitter.ts` silently. The additive change leaves all
four type guards returning identical booleans.
**Role resolution returned a single id.** The rule itself now lives in
one place, `resolveRoleIdsForUser`: a user's role, narrowed by the
application's if it declared one, never the same id twice.
`resolveRoleIdsFromAuthContext` and `resolveRolePermissionConfig` build
`{ intersectionOf: [...] }` from it, which `getRepository` already
applied over N roles. A user with no role still resolves to nothing, so
an application can never stand in for a missing user role.
**Row-level security ignored all of it.** RLS was re-derived from a
single role at query time, so it would have been unaffected by any
intersection. Each role is now compiled on its own and the resulting
filters are ANDed.
That last choice matters. Merging the raw predicates and groups first
would have been wrong: `computeRecordGqlOperationFilter` honours only
the first parentless group, so concatenating two roles' groups makes one
role's predicates vanish, **widening** access. Compiling per role and
ANDing needs no synthetic groups, no re-parenting and no `twenty-shared`
type change, and reuses the single-role logic untouched.
Subscriptions go through the same rule. An event stream resolved only
the subscriber's role, so a stream opened by an application acting for
someone was filtered by that person's role alone. The stream now records
the application it was opened by and the publisher intersects both roles
for object permissions, restricted fields and RLS, exactly as a query
does.
Two smaller fixes fall out:
- `getObjectsPermissionsFromRolePermissionConfig` had a `// Multi-role
union/intersection is not ready — use the first assigned role only`
shortcut and now intersects.
- `computePermissionIntersection` hardcoded empty row-level predicate
arrays, which is why RLS-constrained fields were not exempted from the
field-permission check on insert and could fail spuriously. It now
reports the fields **every** role constrains. Reporting fields
constrained by only one role would be worse than the original bug: the
insert guard waives a field-update deny on them, so one role's row-level
rule would cancel another role's deny.
## Behaviour on the edges
**An application that declares no role adds no bound.** `defaultRoleId`
stays null whenever a manifest omits `defaultRoleUniversalIdentifier`,
which is the common case, so denying would have broken a lot of
installed applications. Behaviour changes only for applications that
actually declared a role.
To stop that being permanent, `defaultRoleUniversalIdentifier` should
become required for new applications. Hard-requiring it needs a backfill
for existing installs, so it is not in this PR.
**An application that cannot be found denies.** That is not the same as
one that declared no role, and treating it as such would have let a
token naming a deleted application fall back to the full permissions of
the user it acts for.
**A role that cannot be resolved denies.** `application.defaultRoleId`
is a plain uuid column with no foreign key, and role deletion does not
clear it, so it can dangle. A bound we cannot apply must not let the
remaining roles decide on their own, so the ORM path,
`getObjectsPermissionsFromRolePermissionConfig` and the subscription
publisher all return no permissions in that case rather than falling
back.
## Testing
- Full `twenty-server` unit suite green (896 suites, 7353 tests)
- New spec for `resolveRoleIdsFromAuthContext`: both roles, application
with no declared role, application holding the user's own role, user
with no role, api key, application-only, system
- New spec for multi-role RLS, including the case this fixes (a
restricted role intersected with an unrestricted one keeps the
restriction) and two restricted roles ANDing
- `permissions.service.spec.ts` had **no coverage of the application
branch at all** (`ApplicationEntity` was mocked as `{}`); it now has a
real mock plus user-grants/application-denies, the reverse, both-grant,
the null-role fallback, a shared role, and a missing application
- First coverage of non-empty row-level predicates through
`computePermissionIntersection`, including a field constrained by one
role only
- Subscription publisher: application role denies, both allow,
application role dangling, and both roles reaching the RLS filter
- Updated the two specs that asserted the old behaviour: the middleware
dropping the application, and "use the first role when multiple are
provided"
No schema, cache or GraphQL change: `rolesPermissions` is keyed by role
id alone and the intersection is computed per request from cached
per-role entries.
## Not in this PR
`workflow-execution-context.service.ts` falls back to the **admin** role
when an application has no `defaultRoleId`, and to
`shouldBypassPermissionChecks: true` if admin is not found. That is the
inverse of the rule here and an escalation in its own right, but
workflow execution is sensitive, so it is tracked separately in
twentyhq/core-team-issues#2753.
Three resolvers still carry their own principal precedence and do not
use this seam: `rest-api-base.handler.ts`, `mcp-protocol.service.ts`
(which never builds a user or application context at all), and actor
attribution in `actor-from-auth-context.service.ts`.
Separately, `computeRecordGqlOperationFilter` silently discards
predicates under any parentless group after the first, with no test
coverage. That is a latent bug independent of this work and lives in
`twenty-shared`, shared with the front-end filter system.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01C6nCVbcb5ZZrz67uvqvMWF)_
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/23680?utm_source=github"
rel="nofollow noreferrer noopener" target="_blank">``<img alt="Review
in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg">``</a>
238 lines
8.9 KiB
TypeScript
238 lines
8.9 KiB
TypeScript
import { randomUUID } from 'crypto';
|
|
|
|
import { COMPANY_GQL_FIELDS } from 'test/integration/constants/company-gql-fields.constants';
|
|
import { createOneOperationFactory } from 'test/integration/graphql/utils/create-one-operation-factory.util';
|
|
import { destroyOneOperationFactory } from 'test/integration/graphql/utils/destroy-one-operation-factory.util';
|
|
import { findManyOperationFactory } from 'test/integration/graphql/utils/find-many-operation-factory.util';
|
|
import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util';
|
|
import { updateOneOperationFactory } from 'test/integration/graphql/utils/update-one-operation-factory.util';
|
|
import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util';
|
|
import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util';
|
|
import { generateApplicationToken } from 'test/integration/metadata/suites/application/utils/generate-application-token.util';
|
|
import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util';
|
|
import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util';
|
|
import { createOneRole } from 'test/integration/metadata/suites/role/utils/create-one-role.util';
|
|
import { type Manifest } from 'twenty-shared/application';
|
|
import { STANDARD_OBJECTS } from 'twenty-shared/metadata';
|
|
import { RowLevelPermissionPredicateOperand } from 'twenty-shared/types';
|
|
|
|
import { SEED_APPLE_WORKSPACE_ID } from 'src/engine/workspace-manager/dev-seeder/core/constants/seeder-workspaces.constant';
|
|
|
|
const TEST_APP_UNIVERSAL_IDENTIFIER = randomUUID();
|
|
const TEST_ROLE_UNIVERSAL_IDENTIFIER = randomUUID();
|
|
const TEST_OBJECT_PERMISSION_UNIVERSAL_IDENTIFIER = randomUUID();
|
|
const TEST_PREDICATE_UNIVERSAL_IDENTIFIER = randomUUID();
|
|
|
|
const VISIBLE_COMPANY_ID = randomUUID();
|
|
const HIDDEN_COMPANY_ID = randomUUID();
|
|
const VISIBLE_COMPANY_NAME = `Intersection Visible ${VISIBLE_COMPANY_ID}`;
|
|
const HIDDEN_COMPANY_NAME = `Intersection Hidden ${HIDDEN_COMPANY_ID}`;
|
|
|
|
const COMPANY_UNIVERSAL_IDENTIFIER =
|
|
STANDARD_OBJECTS.company.universalIdentifier;
|
|
const COMPANY_NAME_FIELD_UNIVERSAL_IDENTIFIER =
|
|
STANDARD_OBJECTS.company.fields.name.universalIdentifier;
|
|
|
|
// The application declares a role that is strictly narrower than the admin who
|
|
// holds the token: it may read but not update, and only sees companies whose
|
|
// name contains "Intersection Visible". The admin has neither bound, so every
|
|
// assertion below fails if the application's role is dropped.
|
|
const buildApplicationManifest = (): Manifest =>
|
|
buildBaseManifest({
|
|
appId: TEST_APP_UNIVERSAL_IDENTIFIER,
|
|
roleId: TEST_ROLE_UNIVERSAL_IDENTIFIER,
|
|
overrides: {
|
|
roles: [
|
|
{
|
|
universalIdentifier: TEST_ROLE_UNIVERSAL_IDENTIFIER,
|
|
label: 'Intersection Test Role',
|
|
description: 'Role narrower than the user acting through the app',
|
|
canUpdateAllSettings: false,
|
|
canReadAllObjectRecords: true,
|
|
canUpdateAllObjectRecords: false,
|
|
objectPermissions: [
|
|
{
|
|
universalIdentifier: TEST_OBJECT_PERMISSION_UNIVERSAL_IDENTIFIER,
|
|
objectUniversalIdentifier: COMPANY_UNIVERSAL_IDENTIFIER,
|
|
canReadObjectRecords: true,
|
|
canUpdateObjectRecords: false,
|
|
},
|
|
],
|
|
rowLevelPermissionPredicates: [
|
|
{
|
|
universalIdentifier: TEST_PREDICATE_UNIVERSAL_IDENTIFIER,
|
|
objectUniversalIdentifier: COMPANY_UNIVERSAL_IDENTIFIER,
|
|
fieldUniversalIdentifier: COMPANY_NAME_FIELD_UNIVERSAL_IDENTIFIER,
|
|
operand: RowLevelPermissionPredicateOperand.CONTAINS,
|
|
value: 'Intersection Visible',
|
|
},
|
|
],
|
|
},
|
|
],
|
|
},
|
|
});
|
|
|
|
const findApplicationId = async (): Promise<string> => {
|
|
const rows = await globalThis.testDataSource.query(
|
|
`SELECT id FROM core."application"
|
|
WHERE "universalIdentifier" = $1 AND "workspaceId" = $2`,
|
|
[TEST_APP_UNIVERSAL_IDENTIFIER, SEED_APPLE_WORKSPACE_ID],
|
|
);
|
|
|
|
return rows[0]?.id;
|
|
};
|
|
|
|
const findApplicationDefaultRoleId = async (): Promise<string | null> => {
|
|
const rows = await globalThis.testDataSource.query(
|
|
`SELECT "defaultRoleId" FROM core."application"
|
|
WHERE "universalIdentifier" = $1 AND "workspaceId" = $2`,
|
|
[TEST_APP_UNIVERSAL_IDENTIFIER, SEED_APPLE_WORKSPACE_ID],
|
|
);
|
|
|
|
return rows[0]?.defaultRoleId ?? null;
|
|
};
|
|
|
|
const createCompany = async (id: string, name: string) =>
|
|
makeGraphqlAPIRequest(
|
|
createOneOperationFactory({
|
|
objectMetadataSingularName: 'company',
|
|
gqlFields: COMPANY_GQL_FIELDS,
|
|
data: { id, name },
|
|
}),
|
|
);
|
|
|
|
const destroyCompany = async (id: string) =>
|
|
makeGraphqlAPIRequest(
|
|
destroyOneOperationFactory({
|
|
objectMetadataSingularName: 'company',
|
|
gqlFields: 'id',
|
|
recordId: id,
|
|
}),
|
|
);
|
|
|
|
const findCompanyNames = async (token?: string): Promise<string[]> => {
|
|
const response = await makeGraphqlAPIRequest(
|
|
findManyOperationFactory({
|
|
objectMetadataSingularName: 'company',
|
|
objectMetadataPluralName: 'companies',
|
|
gqlFields: 'id name',
|
|
filter: { id: { in: [VISIBLE_COMPANY_ID, HIDDEN_COMPANY_ID] } },
|
|
}),
|
|
token,
|
|
);
|
|
|
|
expect(response.body.errors).toBeUndefined();
|
|
|
|
return response.body.data.companies.edges.map(
|
|
(edge: { node: { name: string } }) => edge.node.name,
|
|
);
|
|
};
|
|
|
|
describe('An application acting for a user is bound by both roles', () => {
|
|
let applicationAccessToken: string;
|
|
|
|
beforeAll(async () => {
|
|
await setupApplicationForSync({
|
|
applicationUniversalIdentifier: TEST_APP_UNIVERSAL_IDENTIFIER,
|
|
name: 'Role Intersection Test Application',
|
|
description: 'App for testing application and user role intersection',
|
|
sourcePath: 'test-role-intersection',
|
|
});
|
|
|
|
// setupApplicationForSync leaves fake timers installed.
|
|
jest.useRealTimers();
|
|
|
|
const { errors } = await syncApplication({
|
|
manifest: buildApplicationManifest(),
|
|
expectToFail: false,
|
|
});
|
|
|
|
expect(errors).toBeUndefined();
|
|
|
|
const applicationId = await findApplicationId();
|
|
|
|
expect(applicationId).toBeTruthy();
|
|
expect(await findApplicationDefaultRoleId()).toBeTruthy();
|
|
|
|
await createCompany(VISIBLE_COMPANY_ID, VISIBLE_COMPANY_NAME);
|
|
await createCompany(HIDDEN_COMPANY_ID, HIDDEN_COMPANY_NAME);
|
|
|
|
// Minted with the admin token, so it carries that admin's userId and
|
|
// userWorkspaceId alongside the applicationId.
|
|
const { data } = await generateApplicationToken({
|
|
applicationId,
|
|
expectToFail: false,
|
|
});
|
|
|
|
applicationAccessToken =
|
|
data.generateApplicationToken.applicationAccessToken.token;
|
|
}, 120000);
|
|
|
|
afterAll(async () => {
|
|
await destroyCompany(VISIBLE_COMPANY_ID);
|
|
await destroyCompany(HIDDEN_COMPANY_ID);
|
|
|
|
await cleanupApplicationAndAppRegistration({
|
|
applicationUniversalIdentifier: TEST_APP_UNIVERSAL_IDENTIFIER,
|
|
});
|
|
}, 120000);
|
|
|
|
it('should let the user see both companies when acting on their own', async () => {
|
|
const names = await findCompanyNames();
|
|
|
|
expect(names).toHaveLength(2);
|
|
expect(names).toEqual(
|
|
expect.arrayContaining([VISIBLE_COMPANY_NAME, HIDDEN_COMPANY_NAME]),
|
|
);
|
|
});
|
|
|
|
it('should apply the application row-level predicate even though the user has none', async () => {
|
|
const names = await findCompanyNames(applicationAccessToken);
|
|
|
|
expect(names).toEqual([VISIBLE_COMPANY_NAME]);
|
|
});
|
|
|
|
it('should refuse an update the application role forbids and the user role allows', async () => {
|
|
const response = await makeGraphqlAPIRequest(
|
|
updateOneOperationFactory({
|
|
objectMetadataSingularName: 'company',
|
|
gqlFields: COMPANY_GQL_FIELDS,
|
|
recordId: VISIBLE_COMPANY_ID,
|
|
data: { name: `${VISIBLE_COMPANY_NAME} edited` },
|
|
}),
|
|
applicationAccessToken,
|
|
);
|
|
|
|
expect(response.body.errors).toBeDefined();
|
|
expect(response.body.data?.updateCompany).toBeFalsy();
|
|
});
|
|
|
|
it('should refuse a settings mutation the application role forbids', async () => {
|
|
const { errors } = await createOneRole({
|
|
expectToFail: true,
|
|
token: applicationAccessToken,
|
|
input: {
|
|
label: `Should Never Exist ${randomUUID()}`,
|
|
description: 'Created through an application whose role forbids it',
|
|
icon: 'IconLock',
|
|
},
|
|
});
|
|
|
|
expect(errors).toBeDefined();
|
|
});
|
|
|
|
it('should let the same update through when the user acts on their own', async () => {
|
|
const response = await makeGraphqlAPIRequest(
|
|
updateOneOperationFactory({
|
|
objectMetadataSingularName: 'company',
|
|
gqlFields: COMPANY_GQL_FIELDS,
|
|
recordId: VISIBLE_COMPANY_ID,
|
|
data: { name: VISIBLE_COMPANY_NAME },
|
|
}),
|
|
);
|
|
|
|
expect(response.body.errors).toBeUndefined();
|
|
expect(response.body.data.updateCompany.id).toBe(VISIBLE_COMPANY_ID);
|
|
});
|
|
});
|