From 2ae927a4548a34ee70f424d4100df29b17cf3c4c Mon Sep 17 00:00:00 2001 From: Sven Efftinge Date: Mon, 11 Sep 2023 11:00:09 +0200 Subject: [PATCH] [fga] fix and log missing relationships (#18692) --- .../src/authorization/relationship-updater.ts | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/components/server/src/authorization/relationship-updater.ts b/components/server/src/authorization/relationship-updater.ts index 074dc598a67ff7..e1537dc90a70e7 100644 --- a/components/server/src/authorization/relationship-updater.ts +++ b/components/server/src/authorization/relationship-updater.ts @@ -13,6 +13,7 @@ import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messag import { v1 } from "@authzed/authzed-node"; import { fgaRelationsUpdateClientLatency } from "../prometheus-metrics"; import { RedisMutex } from "../redis/mutex"; +import { rel } from "./definitions"; @injectable() export class RelationshipUpdater { @@ -48,7 +49,7 @@ export class RelationshipUpdater { } return user; } - if (this.isMigrated(user)) { + if (await this.isMigrated(user)) { return user; } const stopTimer = fgaRelationsUpdateClientLatency.startTimer(); @@ -61,7 +62,7 @@ export class RelationshipUpdater { throw new ApplicationError(ErrorCodes.NOT_FOUND, "User not found"); } user = updatedUser; - if (this.isMigrated(user)) { + if (await this.isMigrated(user)) { return user; } log.info({ userId: user.id }, `Updating FGA relationships for user.`, { @@ -93,8 +94,23 @@ export class RelationshipUpdater { } } - private isMigrated(user: User) { - return user.additionalData?.fgaRelationshipsVersion === RelationshipUpdater.version; + private async isMigrated(user: User) { + const isMigrated = user.additionalData?.fgaRelationshipsVersion === RelationshipUpdater.version; + if (isMigrated) { + const hasSelfRelationship = await this.authorizer.find(rel.user(user.id).self.user(user.id)); + if (!hasSelfRelationship) { + log.warn({ userId: user.id }, `User is marked as migrated but doesn't have self relationship.`); + //TODO(se) this is an extra safety net to detect + // reset the fgaRelationshipsVersion to undefined, so the migration is triggered again when the feature is enabled + AdditionalUserData.set(user, { fgaRelationshipsVersion: undefined }); + await this.userDB.updateUserPartial({ + id: user.id, + additionalData: user.additionalData, + }); + return false; + } + } + return isMigrated; } private async findAffectedOrganizations(userId: string): Promise {