fix(role): rebind API keys + agents before deleting their role (#20935)
## Customer-reported bug
A customer hit this when using the AI chat:
```json
{
"message": "API key 760d4822-da40-4b3f-9031-40563d7ed6c9 has no role assigned",
"extensions": {
"code": "INTERNAL_SERVER_ERROR",
"userFriendlyMessage": "This API key has no role assigned."
}
}
```
Their integration authenticates via API key. Somewhere along the way,
the role bound to that API key was deleted, leaving the API key
authenticated but role-less. Any request that hits a permission check
(`getRoleIdForApiKeyId`) blows up.
## Root cause
In `RoleService.deleteManyRoles`, the pre-deletion cleanup
(`assignDefaultRoleToMembersWithRoleToDelete`) only rebinds **user
workspaces** to the workspace default role. API keys and agents pointing
at the role are ignored. Because `RoleTargetEntity.role` declares
`onDelete: 'CASCADE'`, the FK then drops the role_target rows for those
API keys / agents — but the API keys themselves stay in `api_key`, now
orphaned in `apiKeyRoleMap`.
A previous read-side workaround
([2767ddac44](https://github.com/twentyhq/twenty/commit/2767ddac44) —
make the `role` ResolveField nullable) handled the API-key-details page,
but did not address the write paths (`getRoleIdForApiKeyId`).
## Fix
- Rename `assignDefaultRoleToMembersWithRoleToDelete` →
`rebindTargetsOfRoleToDeleteToDefaultRole` and extend it to rebind API
keys (via `ApiKeyRoleService.assignRoleToApiKey`) and agents (via
`AiAgentRoleService.assignRoleToAgent`) in the same step, before the
role is deleted.
- If the workspace default role doesn't satisfy `canBeAssignedToApiKeys`
/ `canBeAssignedToAgents`, the inner `assignRoleTo*` validation throws.
We catch that and rethrow as a `PermissionsException` with a
role-deletion-context message and two new codes —
`ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS` /
`ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS` — so the admin sees a clear
"reassign these first" prompt rather than a confusing inner error.
## Scope / non-goals
- **Already-orphaned API keys are not auto-healed.** The customer still
needs to reassign a role to their existing orphan API key via the UI
(Settings > API Keys > [the key] > role). A separate cleanup command for
existing orphans is a follow-up.
- I did not investigate *why* the customer's session was authenticated
via API key in the AI chat — that may be their integration setup. Worth
confirming with them separately.
## Test plan
- [ ] Workspace with default role `Admin` (which has
`canBeAssignedToApiKeys: true`): create an API key with a custom role,
delete the custom role → API key is rebound to Admin, requests keep
working.
- [ ] Workspace with default role `Member` (default, has
`canBeAssignedToApiKeys: false`): create an API key with a custom role,
delete the custom role → role deletion fails with the new
`ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS` error explaining the admin must
reassign first. API key + custom role are both unchanged.
- [ ] Same two scenarios for agents (`canBeAssignedToAgents`).
- [ ] Existing user-workspace rebind behavior is unchanged.
- [ ] Role deletion with no dependent API keys / agents still works.
This commit is contained in:
+45
-2
@@ -1,6 +1,6 @@
|
||||
import { settingsDraftRoleFamilyState } from '@/settings/roles/states/settingsDraftRoleFamilyState';
|
||||
import { useAtomFamilyStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomFamilyStateValue';
|
||||
import { t } from '@lingui/core/macro';
|
||||
import { plural, t } from '@lingui/core/macro';
|
||||
|
||||
type SettingsRoleSettingsDeleteRoleConfirmationModalSubtitleProps = {
|
||||
roleId: string;
|
||||
@@ -14,8 +14,51 @@ export const SettingsRoleSettingsDeleteRoleConfirmationModalSubtitle = ({
|
||||
roleId,
|
||||
);
|
||||
const roleName = settingsDraftRole.label;
|
||||
const memberCount = settingsDraftRole.workspaceMembers.length;
|
||||
const apiKeyCount = settingsDraftRole.apiKeys.length;
|
||||
const agentCount = settingsDraftRole.agents.length;
|
||||
|
||||
const segments: string[] = [];
|
||||
|
||||
if (memberCount > 0) {
|
||||
segments.push(
|
||||
plural(memberCount, {
|
||||
one: `${memberCount} member`,
|
||||
other: `${memberCount} members`,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
if (apiKeyCount > 0) {
|
||||
segments.push(
|
||||
plural(apiKeyCount, {
|
||||
one: `${apiKeyCount} API key`,
|
||||
other: `${apiKeyCount} API keys`,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
if (agentCount > 0) {
|
||||
segments.push(
|
||||
plural(agentCount, {
|
||||
one: `${agentCount} agent`,
|
||||
other: `${agentCount} agents`,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
if (segments.length === 0) {
|
||||
return (
|
||||
<>{t`Confirm deletion of ${roleName} role? This cannot be undone.`}</>
|
||||
);
|
||||
}
|
||||
|
||||
const reassignSubject =
|
||||
segments.length === 1
|
||||
? segments[0]
|
||||
: `${segments.slice(0, -1).join(', ')} ${t`and`} ${segments.at(-1)}`;
|
||||
|
||||
return (
|
||||
<>{t`Confirm deletion of ${roleName} role? This cannot be undone. All members will be reassigned to the default role.`}</>
|
||||
<>{t`Confirm deletion of ${roleName} role? This cannot be undone. ${reassignSubject} will be reassigned to the default role.`}</>
|
||||
);
|
||||
};
|
||||
|
||||
+6
@@ -48,6 +48,8 @@ export enum PermissionsExceptionCode {
|
||||
COMPOSITE_TYPE_NOT_FOUND = 'COMPOSITE_TYPE_NOT_FOUND',
|
||||
ROLE_MUST_HAVE_AT_LEAST_ONE_TARGET = 'ROLE_MUST_HAVE_AT_LEAST_ONE_TARGET',
|
||||
ROLE_CANNOT_BE_ASSIGNED_TO_USERS = 'ROLE_CANNOT_BE_ASSIGNED_TO_USERS',
|
||||
ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS = 'ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS',
|
||||
ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS = 'ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS',
|
||||
APPLICATION_ROLE_NOT_FOUND = 'APPLICATION_ROLE_NOT_FOUND',
|
||||
ROLE_BELONGS_TO_ANOTHER_APPLICATION = 'ROLE_BELONGS_TO_ANOTHER_APPLICATION',
|
||||
}
|
||||
@@ -142,6 +144,10 @@ const getPermissionsExceptionUserFriendlyMessage = (
|
||||
return msg`Role must have at least one target.`;
|
||||
case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_USERS:
|
||||
return msg`This role cannot be assigned to users.`;
|
||||
case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS:
|
||||
return msg`This role cannot be assigned to API keys.`;
|
||||
case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS:
|
||||
return msg`This role cannot be assigned to agents.`;
|
||||
case PermissionsExceptionCode.APPLICATION_ROLE_NOT_FOUND:
|
||||
return msg`No role assigned to the application.`;
|
||||
case PermissionsExceptionCode.ROLE_BELONGS_TO_ANOTHER_APPLICATION:
|
||||
|
||||
+2
@@ -44,6 +44,8 @@ export const permissionGraphqlApiExceptionHandler = (
|
||||
case PermissionsExceptionCode.EMPTY_FIELD_PERMISSION_NOT_ALLOWED:
|
||||
case PermissionsExceptionCode.ROLE_MUST_HAVE_AT_LEAST_ONE_TARGET:
|
||||
case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_USERS:
|
||||
case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS:
|
||||
case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS:
|
||||
throw new UserInputError(error);
|
||||
case PermissionsExceptionCode.ROLE_NOT_FOUND:
|
||||
case PermissionsExceptionCode.OBJECT_METADATA_NOT_FOUND:
|
||||
|
||||
+2
@@ -26,6 +26,8 @@ export const permissionRestApiExceptionCodeToHttpStatus = (
|
||||
case PermissionsExceptionCode.EMPTY_FIELD_PERMISSION_NOT_ALLOWED:
|
||||
case PermissionsExceptionCode.ROLE_MUST_HAVE_AT_LEAST_ONE_TARGET:
|
||||
case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_USERS:
|
||||
case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS:
|
||||
case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS:
|
||||
return 400;
|
||||
case PermissionsExceptionCode.ROLE_NOT_FOUND:
|
||||
case PermissionsExceptionCode.OBJECT_METADATA_NOT_FOUND:
|
||||
|
||||
@@ -5,9 +5,19 @@ import { msg } from '@lingui/core/macro';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { Repository } from 'typeorm';
|
||||
|
||||
import {
|
||||
ApiKeyException,
|
||||
ApiKeyExceptionCode,
|
||||
} from 'src/engine/core-modules/api-key/exceptions/api-key.exception';
|
||||
import { ApiKeyRoleService } from 'src/engine/core-modules/api-key/services/api-key-role.service';
|
||||
import { ApplicationService } from 'src/engine/core-modules/application/application.service';
|
||||
import { type FlatApplication } from 'src/engine/core-modules/application/types/flat-application.type';
|
||||
import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
|
||||
import { AiAgentRoleService } from 'src/engine/metadata-modules/ai/ai-agent-role/ai-agent-role.service';
|
||||
import {
|
||||
AiException,
|
||||
AiExceptionCode,
|
||||
} from 'src/engine/metadata-modules/ai/ai.exception';
|
||||
import { WorkspaceManyOrAllFlatEntityMapsCacheService } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.service';
|
||||
import { findFlatEntityByIdInFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps-or-throw.util';
|
||||
import { findFlatEntityByUniversalIdentifier } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-universal-identifier.util';
|
||||
@@ -40,6 +50,8 @@ export class RoleService {
|
||||
private readonly roleRepository: Repository<RoleEntity>,
|
||||
private readonly userRoleService: UserRoleService,
|
||||
private readonly applicationService: ApplicationService,
|
||||
private readonly apiKeyRoleService: ApiKeyRoleService,
|
||||
private readonly aiAgentRoleService: AiAgentRoleService,
|
||||
) {}
|
||||
|
||||
public async getWorkspaceRoles(workspaceId: string): Promise<RoleEntity[]> {
|
||||
@@ -316,8 +328,9 @@ export class RoleService {
|
||||
);
|
||||
}
|
||||
|
||||
await this.assignDefaultRoleToMembersWithRoleToDelete({
|
||||
await this.rebindTargetsOfRoleToDeleteToDefaultRole({
|
||||
roleId,
|
||||
roleLabel: flatRoleToDelete.label,
|
||||
workspaceId,
|
||||
defaultRoleId,
|
||||
});
|
||||
@@ -407,12 +420,14 @@ export class RoleService {
|
||||
}
|
||||
|
||||
// TODO: Move to migration side effect / To address for rollback of role deletion
|
||||
private async assignDefaultRoleToMembersWithRoleToDelete({
|
||||
private async rebindTargetsOfRoleToDeleteToDefaultRole({
|
||||
roleId,
|
||||
roleLabel,
|
||||
workspaceId,
|
||||
defaultRoleId,
|
||||
}: {
|
||||
roleId: string;
|
||||
roleLabel: string;
|
||||
workspaceId: string;
|
||||
defaultRoleId: string;
|
||||
}): Promise<void> {
|
||||
@@ -427,5 +442,82 @@ export class RoleService {
|
||||
roleId: defaultRoleId,
|
||||
workspaceId,
|
||||
});
|
||||
|
||||
const apiKeysToRebind =
|
||||
await this.apiKeyRoleService.getApiKeysAssignedToRole(
|
||||
roleId,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
for (const apiKey of apiKeysToRebind) {
|
||||
try {
|
||||
await this.apiKeyRoleService.assignRoleToApiKey({
|
||||
apiKeyId: apiKey.id,
|
||||
roleId: defaultRoleId,
|
||||
workspaceId,
|
||||
});
|
||||
} catch (error) {
|
||||
if (
|
||||
error instanceof ApiKeyException &&
|
||||
error.code === ApiKeyExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS
|
||||
) {
|
||||
throw this.toRoleDeleteRebindException({
|
||||
roleLabel,
|
||||
targetKind: 'apiKey',
|
||||
});
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
const agentsToRebind =
|
||||
await this.aiAgentRoleService.getAgentsAssignedToRole(
|
||||
roleId,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
for (const agent of agentsToRebind) {
|
||||
try {
|
||||
await this.aiAgentRoleService.assignRoleToAgent({
|
||||
agentId: agent.id,
|
||||
roleId: defaultRoleId,
|
||||
workspaceId,
|
||||
});
|
||||
} catch (error) {
|
||||
if (
|
||||
error instanceof AiException &&
|
||||
error.code === AiExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS
|
||||
) {
|
||||
throw this.toRoleDeleteRebindException({
|
||||
roleLabel,
|
||||
targetKind: 'agent',
|
||||
});
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private toRoleDeleteRebindException({
|
||||
roleLabel,
|
||||
targetKind,
|
||||
}: {
|
||||
roleLabel: string;
|
||||
targetKind: 'apiKey' | 'agent';
|
||||
}): Error {
|
||||
const targetLabel = targetKind === 'apiKey' ? 'API key' : 'agent';
|
||||
|
||||
return new PermissionsException(
|
||||
`Cannot delete role "${roleLabel}": the workspace default role cannot be assigned to ${targetLabel}s.`,
|
||||
targetKind === 'apiKey'
|
||||
? PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS
|
||||
: PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS,
|
||||
{
|
||||
userFriendlyMessage:
|
||||
targetKind === 'apiKey'
|
||||
? msg`Cannot delete this role: it is still assigned to one or more API keys, and the workspace default role cannot be assigned to API keys. Please reassign these API keys to another role first.`
|
||||
: msg`Cannot delete this role: it is still assigned to one or more agents, and the workspace default role cannot be assigned to agents. Please reassign these agents to another role first.`,
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
+9
-1
@@ -208,7 +208,15 @@ describe('Agent creation should succeed', () => {
|
||||
isCustom: true,
|
||||
});
|
||||
|
||||
// Clean up the role
|
||||
// Delete the agent first so its role_target is removed; otherwise
|
||||
// deleting the role would refuse to orphan the agent (the workspace
|
||||
// default role isn't agent-assignable).
|
||||
await deleteOneAgent({
|
||||
expectToFail: false,
|
||||
input: { id: createdAgentId },
|
||||
});
|
||||
createdAgentId = '';
|
||||
|
||||
await deleteOneRole({
|
||||
expectToFail: false,
|
||||
input: { idToDelete: createdRoleId },
|
||||
|
||||
Reference in New Issue
Block a user