Skip to content

Commit

Permalink
bug fixes / don't keep any machine instantiators
Browse files Browse the repository at this point in the history
  • Loading branch information
CiaranMn committed Feb 13, 2025
1 parent e51a2a0 commit 33cffe0
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,10 @@ const migrate: MigrationFunction = async ({
},
webShortname: "hash",
migrationState,
instantiator: anyUserInstantiator,
instantiator: {
kind: "account",
subjectId: systemAccountId,
},
},
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ const migrate: MigrationFunction = async ({
if (err instanceof NotFoundError) {
const orgAdminAccountId = org.metadata.provenance.edition.createdById;

await createWebMachineActor(
const webActorId = await createWebMachineActor(
context,
// We have to use an org admin's authority to add the machine to their web
{ actorId: orgAdminAccountId },
Expand All @@ -137,7 +137,9 @@ const migrate: MigrationFunction = async ({
machineEntityTypeId: currentMachineEntityTypeId,
},
);
logger.info(`Created web machine actor for org ${orgAccountGroupId}`);
logger.info(
`Created web machine actor for org ${orgAccountGroupId} with accountId ${webActorId} (by actor ${orgAdminAccountId})`,
);
} else {
throw new Error(
`Unexpected error attempting to retrieve machine web actor for organization ${org.metadata.recordId.entityId}`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ export const upgradeWebEntities = async ({

let updateAuthentication = webBotAuthentication;

const temporaryEntityTypePermissionsGranted: VersionedUrl[] = [];
const temporaryEntityTypePermissionsGranted: Record<
VersionedUrl,
AccountId
> = {};

/**
* Determine the actor that should update the entity.
Expand Down Expand Up @@ -189,45 +192,51 @@ export const upgradeWebEntities = async ({
*/
actorId: entity.metadata.provenance.createdById,
};
}

for (const entityTypeId of [currentEntityTypeId, newEntityTypeId]) {
/**
* We may need to temporarily grant the machine account ID the ability
* to instantiate entities of both the old and new entityTypeId,
* because an actor cannot update or remove an entity type without being able to instantiate it.
*/
const relationships = await context.graphApi
.getEntityTypeAuthorizationRelationships(
systemAccountId,
entityTypeId,
)
.then(({ data }) => data);
/**
* We may need to temporarily grant the actor the ability to instantiate entities of both the old and new entityTypeId,
* because an actor cannot remove or add an entity type without being able to instantiate it.
* Some entity types have their instantiator restricted, e.g. User, Org, so that they can only be created in special circumstances.
* If the actor being used here doesn't already have permission we'll grant it and then remove it after the update.
*/
for (const entityTypeId of [currentEntityTypeId, newEntityTypeId]) {
const relationships = await context.graphApi
.getEntityTypeAuthorizationRelationships(
systemAccountId,
entityTypeId,
)
.then(({ data }) => data);

const relationAndSubject = {
subject: {
kind: "account",
subjectId: entity.metadata.provenance.createdById,
},
relation: "instantiator",
} as const;
const relationAndSubject = {
subject: {
kind: "account",
subjectId: updateAuthentication.actorId,
},
relation: "instantiator",
} as const;

if (
!relationships.find((relationship) =>
isEqual(relationship, relationAndSubject),
)
) {
await context.graphApi.modifyEntityTypeAuthorizationRelationships(
systemAccountId,
[
{
operation: "create",
resource: entityTypeId,
relationAndSubject,
},
],
);
temporaryEntityTypePermissionsGranted.push(entityTypeId);
}
if (
!relationships.find(
({ subject, relation }) =>
relation === "instantiator" &&
((subject.kind === "account" &&
subject.subjectId === updateAuthentication.actorId) ||
subject.kind === "public"),
)
) {
await context.graphApi.modifyEntityTypeAuthorizationRelationships(
systemAccountId,
[
{
operation: "create",
resource: entityTypeId,
relationAndSubject,
},
],
);
temporaryEntityTypePermissionsGranted[entityTypeId] =
updateAuthentication.actorId;
}
}

Expand Down Expand Up @@ -256,7 +265,9 @@ export const upgradeWebEntities = async ({
: undefined,
});
} finally {
for (const entityTypeId of temporaryEntityTypePermissionsGranted) {
for (const [entityTypeId, actorId] of Object.entries(
temporaryEntityTypePermissionsGranted,
)) {
/**
* If we updated a machine entity and granted its actor ID a
* new permission, we need to remove the temporary permission.
Expand All @@ -270,7 +281,7 @@ export const upgradeWebEntities = async ({
relationAndSubject: {
subject: {
kind: "account",
subjectId: entity.metadata.provenance.createdById,
subjectId: actorId,
},
relation: "instantiator",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ export const ensureSystemWebEntitiesExist = async ({
identifier: webShortname,
ownedById: accountGroupId as OwnedById,
displayName,
shouldBeAbleToCreateMoreMachineEntities: false,
systemAccountId,
machineEntityTypeId,
});
Expand Down Expand Up @@ -320,7 +319,6 @@ export const ensureSystemEntitiesExist = async (params: {
machineAccountId: aiAssistantAccountId,
ownedById: hashAccountGroupId as OwnedById,
displayName: "HASH AI",
shouldBeAbleToCreateMoreMachineEntities: false,
systemAccountId,
});

Expand Down
57 changes: 30 additions & 27 deletions apps/hash-api/src/graph/knowledge/system-types/org.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,14 @@ export const createOrg: ImpureGraphFunction<
},
};

const entityTypeId =
typeof entityTypeVersion === "undefined"
? systemEntityTypes.organization.entityTypeId
: versionedUrlFromComponents(
systemEntityTypes.organization.entityTypeBaseUrl,
entityTypeVersion,
);

try {
await modifyEntityTypeAuthorizationRelationships(
ctx,
Expand All @@ -186,7 +194,7 @@ export const createOrg: ImpureGraphFunction<
},
resource: {
kind: "entityType",
resourceId: systemEntityTypes.organization.entityTypeId,
resourceId: entityTypeId,
},
},
},
Expand All @@ -196,14 +204,7 @@ export const createOrg: ImpureGraphFunction<
const entity = await createEntity(ctx, authentication, {
ownedById: orgAccountGroupId as OwnedById,
properties,
entityTypeIds: [
typeof entityTypeVersion === "undefined"
? systemEntityTypes.organization.entityTypeId
: versionedUrlFromComponents(
systemEntityTypes.organization.entityTypeBaseUrl,
entityTypeVersion,
),
],
entityTypeIds: [entityTypeId],
entityUuid: orgAccountGroupId as string as EntityUuid,
relationships: [
{
Expand All @@ -227,26 +228,28 @@ export const createOrg: ImpureGraphFunction<
permitOlderVersions: entityTypeVersion !== undefined,
});
} finally {
await modifyEntityTypeAuthorizationRelationships(
ctx,
{ actorId: systemAccountId },
[
{
operation: "delete",
relationship: {
relation: "instantiator",
subject: {
kind: "account",
subjectId: authentication.actorId,
},
resource: {
kind: "entityType",
resourceId: systemEntityTypes.organization.entityTypeId,
if (authentication.actorId !== systemAccountId) {
await modifyEntityTypeAuthorizationRelationships(
ctx,
{ actorId: systemAccountId },
[
{
operation: "delete",
relationship: {
relation: "instantiator",
subject: {
kind: "account",
subjectId: authentication.actorId,
},
resource: {
kind: "entityType",
resourceId: entityTypeId,
},
},
},
},
],
);
],
);
}
}
};

Expand Down
35 changes: 14 additions & 21 deletions libs/@local/hash-backend-utils/src/machine-actors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ export const createMachineActorEntity = async (
machineAccountId,
ownedById,
displayName,
shouldBeAbleToCreateMoreMachineEntities,
systemAccountId,
machineEntityTypeId,
}: {
Expand All @@ -114,8 +113,6 @@ export const createMachineActorEntity = async (
ownedById: OwnedById;
// A display name for the machine actor, to display to users
displayName: string;
// Whether or not this machine should be able to create more machine entities after creating itself
shouldBeAbleToCreateMoreMachineEntities: boolean;
// The accountId of the system account, used to grant the machine actor permissions to instantiate system types
systemAccountId: AccountId;
machineEntityTypeId?: VersionedUrl;
Expand Down Expand Up @@ -194,25 +191,22 @@ export const createMachineActorEntity = async (
},
);

if (!shouldBeAbleToCreateMoreMachineEntities) {
await context.graphApi.modifyEntityTypeAuthorizationRelationships(
systemAccountId,
[
{
operation: "delete",
resource:
machineEntityTypeId ?? systemEntityTypes.machine.entityTypeId,
relationAndSubject: {
subject: {
kind: "account",
subjectId: machineAccountId,
},
relation: "instantiator",
await context.graphApi.modifyEntityTypeAuthorizationRelationships(
systemAccountId,
[
{
operation: "delete",
resource: machineEntityTypeId ?? systemEntityTypes.machine.entityTypeId,
relationAndSubject: {
subject: {
kind: "account",
subjectId: machineAccountId,
},
relation: "instantiator",
},
],
);
}
},
],
);
};

/**
Expand Down Expand Up @@ -260,7 +254,6 @@ export const createWebMachineActor = async (
machineAccountId: machineAccountId as AccountId,
ownedById,
displayName: "HASH",
shouldBeAbleToCreateMoreMachineEntities: true,
systemAccountId,
machineEntityTypeId,
});
Expand Down

0 comments on commit 33cffe0

Please sign in to comment.