[auth] fix missing updates to dynamic login providers

this primarily affects self-hosted installation where the `ownerId` is about to change during the initial setup of the login/git provider.
gitpod-io · Apr 11, 2022 · 3330000 · 3330000
1 parent 77a055f
commit 3330000
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 14 deletions.
diff --git a/components/gitpod-db/src/auth-provider-entry.spec.db.ts b/components/gitpod-db/src/auth-provider-entry.spec.db.ts
@@ -88,7 +88,7 @@ export class AuthProviderEntryDBSpec {
         const loadedAp = await this.db.findByHost(ap.host);
         expect(loadedAp, "findByHost()").to.deep.equal(ap);
         expect(loadedAp?.oauthRevision, "findByHost()").to.equal(
-            "e05ea6fab8efcaba4b3246c2b2d3931af897c3bc2c1cf075c31614f0954f9840",
+            "b05eb3256a101f6cbca1d8885c8ee241891582e78c567b7305f097ab3556d5f0",
         );
     }
 }

diff --git a/components/gitpod-db/src/typeorm/auth-provider-entry-db-impl.ts b/components/gitpod-db/src/typeorm/auth-provider-entry-db-impl.ts
@@ -31,7 +31,7 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB {
     async storeAuthProvider(ap: AuthProviderEntry, updateOAuthRevision: boolean): Promise<AuthProviderEntry> {
         const repo = await this.getAuthProviderRepo();
         if (updateOAuthRevision) {
-            (ap.oauthRevision as any) = this.oauthContentHash(ap.oauth);
+            (ap.oauthRevision as any) = this.oauthContentHash(ap);
         }
         return repo.save(ap);
     }
@@ -91,8 +91,10 @@ export class AuthProviderEntryDBImpl implements AuthProviderEntryDB {
         return query.getMany();
     }
 
-    protected oauthContentHash(oauth: AuthProviderEntry["oauth"]): string {
-        const result = createHash("sha256").update(JSON.stringify(oauth)).digest("hex");
+    protected oauthContentHash(entry: AuthProviderEntry): string {
+        const result = createHash("sha256")
+            .update(JSON.stringify({ oauth: entry.oauth, ownerId: entry.ownerId, status: entry.status }))
+            .digest("hex");
         return result;
     }
 }
diff --git a/components/server/src/auth/host-context-provider-impl.ts b/components/server/src/auth/host-context-provider-impl.ts
@@ -84,17 +84,17 @@ export class HostContextProviderImpl implements HostContextProvider {
             const existingContext = this.dynamicHosts.get(host);
             const existingConfig = existingContext && existingContext.authProvider.params;
             if (existingConfig && config.id === existingConfig.id) {
-                if (existingConfig.host !== config.host) {
+                const sameHost = config.host === existingConfig.host;
+                if (!sameHost) {
                     log.warn("Ignoring host update for dynamic Auth Provider: " + host, { config, existingConfig });
                     continue;
                 }
-                if (existingConfig.status === config.status) {
-                    if (!!config.oauthRevision && existingConfig.oauthRevision === config.oauthRevision) {
-                        continue;
-                    }
-                    if (JSON.stringify(existingConfig.oauth) === JSON.stringify(config.oauth)) {
-                        continue;
-                    }
+                const sameOwner = config.ownerId === existingConfig.ownerId;
+                const sameStatus = config.status === existingConfig.status;
+                const sameOAuthRevision =
+                    !!config.oauthRevision && existingConfig.oauthRevision === config.oauthRevision;
+                if (sameOwner && sameStatus && sameOAuthRevision) {
+                    continue;
                 }
                 log.debug("Updating existing dynamic Auth Provider: " + host, { config, existingConfig });
             } else {

diff --git a/components/server/src/auth/login-completion-handler.ts b/components/server/src/auth/login-completion-handler.ts
@@ -105,10 +105,10 @@ export class LoginCompletionHandler {
         const hostCtx = this.hostContextProvider.get(hostname);
         if (hostCtx) {
             const { params: config } = hostCtx.authProvider;
-            const { id, verified, ownerId, builtin } = config;
+            const { id, verified, builtin } = config;
             if (!builtin && !verified) {
                 try {
-                    await this.authProviderService.markAsVerified({ id, ownerId });
+                    await this.authProviderService.markAsVerified({ id, ownerId: user.id });
                 } catch (error) {
                     log.error(LogContext.from({ user }), `Failed to mark AuthProvider as verified!`, { error });
                 }