[fga] allow toggling centralizedPermissions on/off (#18444)

Author: svenefftinge

.github/workflows/build.yml

@@ -151,11 +151,16 @@ jobs:
           MYSQL_TCP_PORT: 23306
         ports:
           - 23306:23306
+      redis:
+        image: redis
+        ports:
+          - 6379:6379
     container:
       image: eu.gcr.io/gitpod-core-dev/dev/dev-environment:aledbf-oci-tool-gha.14121
       env:
         DB_HOST: "mysql"
         DB_PORT: "23306"
+        REDIS_HOST: "redis"
     steps:
       - uses: actions/checkout@v3
       - uses: ./.github/actions/setup-environment

components/dashboard/src/data/featureflag-query.ts

@@ -12,7 +12,6 @@ import { useCurrentUser } from "../user-context";
 import { useCurrentOrg } from "./organizations/orgs-query";
 
 const featureFlags = {
-    start_with_options: false,
     showUseLastSuccessfulPrebuild: false,
     publicApiExperimentalWorkspaceService: false,
     personalAccessTokensEnabled: false,

components/server/package.json

@@ -17,9 +17,9 @@
     "clean:node": "rimraf node_modules",
     "purge": "yarn clean && yarn clean:node && yarn run rimraf yarn.lock",
     "test:leeway": "yarn build && yarn test",
-    "test": "cleanup() { echo 'Cleanup started'; yarn stop-services; }; trap cleanup EXIT; yarn test:unit && yarn test:db",
+    "test": "yarn test:unit && yarn test:db",
     "test:unit": "mocha --opts mocha.opts './**/*.spec.js' --exclude './node_modules/**'",
-    "test:db": ". $(leeway run components/gitpod-db:db-test-env) && yarn start-services && mocha --opts mocha.opts './**/*.spec.db.js'  --exclude './node_modules/**'",
+    "test:db": "cleanup() { echo 'Cleanup started'; yarn stop-spicedb; }; trap cleanup EXIT; . $(leeway run components/gitpod-db:db-test-env) && yarn start-spicedb && mocha --opts mocha.opts './**/*.spec.db.js'  --exclude './node_modules/**'",
     "start-services": "yarn start-testdb && yarn start-redis && yarn start-spicedb",
     "stop-services": "yarn stop-redis && yarn stop-spicedb",
     "start-testdb": "if netstat -tuln | grep ':23306 '; then echo 'Mysql is already running.'; else leeway run components/gitpod-db:init-testdb; fi",

components/server/src/authorization/relationship-updater.spec.db.ts

@@ -59,6 +59,27 @@ describe("RelationshipUpdater", async () => {
         await expected(rel.installation.member.user(user.id));
     });
 
+    it("should update a simple user once it was feature-flag disabled", async () => {
+        let user = await userDB.newUser();
+
+        user = await migrator.migrate(user);
+        expect(user.additionalData?.fgaRelationshipsVersion).to.not.be.undefined;
+
+        Experiments.configureTestingClient({
+            centralizedPermissions: false,
+        });
+
+        user = await migrator.migrate(user);
+        expect(user.additionalData?.fgaRelationshipsVersion).to.be.undefined;
+
+        Experiments.configureTestingClient({
+            centralizedPermissions: true,
+        });
+
+        user = await migrator.migrate(user);
+        expect(user.additionalData?.fgaRelationshipsVersion).to.not.be.undefined;
+    });
+
     it("should correctly update a simple user after it moves between org and installation level", async () => {
         let user = await userDB.newUser();
         user = await migrate(user);

components/server/src/authorization/relationship-updater.ts

@@ -13,6 +13,7 @@ import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messag
 import { getExperimentsClientForBackend } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
 import { v1 } from "@authzed/authzed-node";
 import { fgaRelationsUpdateClientLatency } from "../prometheus-metrics";
+import { RedisMutex } from "../redis/mutex";
 
 @injectable()
 export class RelationshipUpdater {
@@ -23,6 +24,7 @@ export class RelationshipUpdater {
         @inject(TeamDB) private readonly orgDB: TeamDB,
         @inject(ProjectDB) private readonly projectDB: ProjectDB,
         @inject(Authorizer) private readonly authorizer: Authorizer,
+        @inject(RedisMutex) private readonly mutex: RedisMutex,
     ) {}
 
     /**
@@ -42,6 +44,12 @@ export class RelationshipUpdater {
             },
         });
         if (!fgaEnabled) {
+            if (user.additionalData?.fgaRelationshipsVersion !== undefined) {
+                log.info({ userId: user.id }, `User has been removed from FGA.`);
+                // reset the fgaRelationshipsVersion to undefined, so the migration is triggered again when the feature is enabled
+                AdditionalUserData.set(user, { fgaRelationshipsVersion: undefined });
+                return await this.userDB.storeUser(user);
+            }
             return user;
         }
         if (user.additionalData?.fgaRelationshipsVersion === this.version) {
@@ -53,41 +61,40 @@ export class RelationshipUpdater {
                 fromVersion: user?.additionalData?.fgaRelationshipsVersion,
                 toVersion: this.version,
             });
-            // TODO run in distributed lock
-            // return this.mutex.using([`fga-migration-${user.id}`], 2000, async () => {
-            const before = new Date().getTime();
+            return this.mutex.using([`fga-migration-${user.id}`], 2000, async () => {
+                const before = new Date().getTime();
 
-            const updatedUser = await this.userDB.findUserById(user.id);
-            if (!updatedUser) {
-                throw new ApplicationError(ErrorCodes.NOT_FOUND, "User not found");
-            }
-            user = updatedUser;
-            if (user.additionalData?.fgaRelationshipsVersion === this.version) {
-                return user;
-            }
-            log.info({ userId: user.id }, `Updating FGA relationships for user.`, {
-                fromVersion: user?.additionalData?.fgaRelationshipsVersion,
-                toVersion: this.version,
-            });
-            const orgs = await this.findAffectedOrganizations(user.id);
+                const updatedUser = await this.userDB.findUserById(user.id);
+                if (!updatedUser) {
+                    throw new ApplicationError(ErrorCodes.NOT_FOUND, "User not found");
+                }
+                user = updatedUser;
+                if (user.additionalData?.fgaRelationshipsVersion === this.version) {
+                    return user;
+                }
+                log.info({ userId: user.id }, `Updating FGA relationships for user.`, {
+                    fromVersion: user?.additionalData?.fgaRelationshipsVersion,
+                    toVersion: this.version,
+                });
+                const orgs = await this.findAffectedOrganizations(user.id);
 
-            // Add relationships
-            await this.updateUser(user);
-            for (const org of orgs) {
-                await this.updateOrganization(user.id, org);
-            }
-            AdditionalUserData.set(user, {
-                fgaRelationshipsVersion: this.version,
-            });
-            await this.userDB.updateUserPartial({
-                id: user.id,
-                additionalData: user.additionalData,
-            });
-            log.info({ userId: user.id }, `Finished updating relationships.`, {
-                duration: new Date().getTime() - before,
+                // Add relationships
+                await this.updateUser(user);
+                for (const org of orgs) {
+                    await this.updateOrganization(user.id, org);
+                }
+                AdditionalUserData.set(user, {
+                    fgaRelationshipsVersion: this.version,
+                });
+                await this.userDB.updateUserPartial({
+                    id: user.id,
+                    additionalData: user.additionalData,
+                });
+                log.info({ userId: user.id }, `Finished updating relationships.`, {
+                    duration: new Date().getTime() - before,
+                });
+                return user;
             });
-            return user;
-            // });
         } catch (error) {
             log.error({ userId: user.id }, `Error updating relationships.`, error);
             return user;

components/server/src/test/service-testing-container-module.ts

@@ -21,6 +21,7 @@ import { testContainer } from "@gitpod/gitpod-db/lib";
 import { productionContainerModule } from "../container-module";
 import { createMock } from "./mocks/mock";
 import { UsageServiceClientMock } from "./mocks/usage-service-client-mock";
+import { env } from "process";
 
 /**
  * Expects a fully configured production container and
@@ -40,7 +41,7 @@ const mockApplyingContainerModule = new ContainerModule((bind, unbound, isbound,
             passlist: [],
         },
         redis: {
-            address: "localhost:6379",
+            address: (env.REDIS_HOST || "127.0.0.1") + ":" + (env.REDIS_PORT || "6379"),
         },
     });
     rebind(IAnalyticsWriter).toConstantValue(NullAnalyticsWriter);