relax param validation on updateAuthProvider

allow to update clientId or clientSecret separately.

Author: AlexTugarev

components/gitpod-protocol/src/protocol.ts

@@ -1577,14 +1577,16 @@ export namespace AuthProviderEntry {
         clientId?: string;
         clientSecret?: string;
     };
-    export type UpdateEntry = Pick<AuthProviderEntry, "id" | "ownerId"> &
-        Pick<OAuth2Config, "clientId" | "clientSecret">;
+    export type UpdateEntry = Pick<AuthProviderEntry, "id" | "ownerId"> & {
+        clientId?: string;
+        clientSecret?: string;
+    };
     export type NewOrgEntry = NewEntry & {
         organizationId: string;
     };
     export type UpdateOrgEntry = Pick<AuthProviderEntry, "id"> & {
-        clientId: string;
-        clientSecret: string;
+        clientId?: string;
+        clientSecret?: string;
         organizationId: string;
     };
     export type UpdateOAuth2Config = Pick<OAuth2Config, "clientId" | "clientSecret">;

components/server/src/api/auth-provider-service-api.ts

@@ -142,7 +142,7 @@ export class AuthProviderServiceAPI implements ServiceImpl<typeof AuthProviderSe
         }
         const clientId = request.clientId;
         const clientSecret = request.clientSecret;
-        if (!clientId || typeof clientSecret === "undefined") {
+        if (!clientId && !clientSecret) {
             throw new ConnectError("clientId or clientSecret are required", Code.InvalidArgument);
         }
 

components/server/src/auth/auth-provider-service.spec.db.ts

@@ -309,7 +309,7 @@ describe("AuthProviderService", async () => {
             const oauthProperty: keyof AuthProviderEntry = "oauth";
             expect(providers[0]).to.not.haveOwnProperty(oauthProperty);
         });
-        it.only("as regular member, should find org-level providers if no built-in providers present", async () => {
+        it("as regular member, should find org-level providers if no built-in providers present", async () => {
             const member = await userService.createUser({
                 identity: {
                     authId: "gh-user-2",
@@ -337,7 +337,7 @@ describe("AuthProviderService", async () => {
             const oauthProperty: keyof AuthProviderEntry = "oauth";
             expect(providers[0]).to.not.haveOwnProperty(oauthProperty);
         });
-        it.only("as regular member, should find only built-in providers if present", async () => {
+        it("as regular member, should find only built-in providers if present", async () => {
             addBuiltInProvider("localhost");
 
             const member = await userService.createUser({
@@ -359,4 +359,65 @@ describe("AuthProviderService", async () => {
             expect(providers[0].host).to.be.equal("localhost");
         });
     });
+
+    describe("updateAuthProvider", async () => {
+        it("should update user-level provider", async () => {
+            const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
+            const someRandomString = String(Date.now());
+            const updatedClientId = await service.updateAuthProviderOfUser(currentUser.id, {
+                id: created.id,
+                ownerId: currentUser.id,
+                clientId: someRandomString,
+            });
+            expect(updatedClientId.oauth?.clientId).to.be.equal(someRandomString);
+            expect(updatedClientId.oauthRevision).to.be.not.equal(created.oauthRevision);
+
+            const updatedClientSecret = await service.updateAuthProviderOfUser(currentUser.id, {
+                id: created.id,
+                ownerId: currentUser.id,
+                clientSecret: String(Date.now()),
+            });
+            expect(updatedClientSecret.oauthRevision).to.be.not.equal(updatedClientId.oauthRevision);
+        });
+        it("should fail if permissions do not permit", async () => {
+            const created = await service.createAuthProviderOfUser(currentUser.id, newEntry());
+            await expectError(
+                ErrorCodes.NOT_FOUND,
+                service.updateAuthProviderOfUser("some-stranger", {
+                    id: created.id,
+                    ownerId: currentUser.id,
+                    clientId: "any",
+                }),
+            );
+        });
+        it("should update org-level provider", async () => {
+            const created = await service.createOrgAuthProvider(currentUser.id, newOrgEntry());
+            const someRandomString = String(Date.now());
+            const updatedClientId = await service.updateOrgAuthProvider(currentUser.id, {
+                id: created.id,
+                organizationId: org.id,
+                clientId: someRandomString,
+            });
+            expect(updatedClientId.oauth?.clientId).to.be.equal(someRandomString);
+            expect(updatedClientId.oauthRevision).to.be.not.equal(created.oauthRevision);
+
+            const updatedClientSecret = await service.updateOrgAuthProvider(currentUser.id, {
+                id: created.id,
+                organizationId: org.id,
+                clientSecret: String(Date.now()),
+            });
+            expect(updatedClientSecret.oauthRevision).to.be.not.equal(updatedClientId.oauthRevision);
+        });
+        it("should fail if org-permissions do not permit", async () => {
+            const created = await service.createOrgAuthProvider(currentUser.id, newOrgEntry());
+            await expectError(
+                ErrorCodes.NOT_FOUND,
+                service.updateOrgAuthProvider("some-stranger", {
+                    id: created.id,
+                    organizationId: org.id,
+                    clientId: "any",
+                }),
+            );
+        });
+    });
 });

components/server/src/auth/auth-provider-service.ts

@@ -5,7 +5,7 @@
  */
 
 import { injectable, inject } from "inversify";
-import { AuthProviderEntry as AuthProviderEntry, AuthProviderInfo, User } from "@gitpod/gitpod-protocol";
+import { AuthProviderEntry as AuthProviderEntry, AuthProviderInfo, OAuth2Config, User } from "@gitpod/gitpod-protocol";
 import { AuthProviderParams } from "./auth-provider";
 import { AuthProviderEntryDB, TeamDB } from "@gitpod/gitpod-db/lib";
 import { Config } from "../config";
@@ -236,10 +236,10 @@ export class AuthProviderService {
         }
 
         // update config on demand
-        const oauth = {
+        const oauth: OAuth2Config = {
             ...existing.oauth,
-            clientId: entry.clientId,
-            clientSecret: entry.clientSecret || existing.oauth.clientSecret, // FE may send empty ("") if not changed
+            clientId: entry.clientId || existing.oauth?.clientId,
+            clientSecret: entry.clientSecret || existing.oauth?.clientSecret, // FE may send empty ("") if not changed
         };
         const authProvider: AuthProviderEntry = {
             ...existing,
@@ -297,18 +297,18 @@ export class AuthProviderService {
         }
 
         // update config on demand
-        const oauth = {
+        const oauth: OAuth2Config = {
             ...existing.oauth,
-            clientId: entry.clientId,
-            clientSecret: entry.clientSecret || existing.oauth.clientSecret, // FE may send empty ("") if not changed
+            clientId: entry.clientId || existing.oauth?.clientId,
+            clientSecret: entry.clientSecret || existing.oauth?.clientSecret, // FE may send empty ("") if not changed
         };
         const authProvider: AuthProviderEntry = {
             ...existing,
             oauth,
             status: "pending",
         };
 
-        const result = await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry, true);
+        const result = await this.authProviderDB.storeAuthProvider(authProvider, true);
         return AuthProviderEntry.redact(result);
     }