creating distinct fns for org provider calls

Author: selfcontained

components/gitpod-protocol/src/gitpod-service.ts

@@ -467,10 +467,10 @@ export namespace GitpodServer {
         readonly id: string;
     }
     export interface CreateOrgAuthProviderParams {
-        readonly entry: Omit<AuthProviderEntry.NewEntry, "ownerId">;
+        readonly entry: AuthProviderEntry.NewOrgEntry;
     }
     export interface UpdateOrgAuthProviderParams {
-        readonly entry: Omit<AuthProviderEntry.UpdateEntry, "ownerId">;
+        readonly entry: AuthProviderEntry.UpdateOrgEntry;
     }
     export interface GetOrgAuthProviderParams {
         readonly organizationId: string;

components/gitpod-protocol/src/protocol.ts

@@ -1445,12 +1445,20 @@ export interface OAuth2Config {
 export namespace AuthProviderEntry {
     export type Type = "GitHub" | "GitLab" | string;
     export type Status = "pending" | "verified";
-    export type NewEntry = Pick<AuthProviderEntry, "ownerId" | "host" | "type" | "organizationId"> & {
+    export type NewEntry = Pick<AuthProviderEntry, "ownerId" | "host" | "type"> & {
         clientId?: string;
         clientSecret?: string;
     };
-    export type UpdateEntry = Pick<AuthProviderEntry, "id" | "ownerId" | "organizationId"> &
+    export type UpdateEntry = Pick<AuthProviderEntry, "id" | "ownerId"> &
         Pick<OAuth2Config, "clientId" | "clientSecret">;
+    export type NewOrgEntry = NewEntry & {
+        organizationId: string;
+    };
+    export type UpdateOrgEntry = Pick<AuthProviderEntry, "id"> & {
+        clientId: string;
+        clientSecret: string;
+        organizationId: string;
+    };
     export function redact(entry: AuthProviderEntry): AuthProviderEntry {
         return {
             ...entry,

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

@@ -117,6 +117,51 @@ export class AuthProviderService {
         }
         return await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry, true);
     }
+
+    async createOrgAuthProvider(entry: AuthProviderEntry.NewOrgEntry): Promise<AuthProviderEntry> {
+        // TODO: only restrict existing provider w/ same host if it's the same organization
+        const existing = await this.authProviderDB.findByHost(entry.host);
+        if (existing) {
+            throw new Error("Provider for this host already exists.");
+        }
+
+        const authProvider = this.initializeNewProvider(entry);
+
+        return await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry, true);
+    }
+
+    async updateOrgAuthProvider(entry: AuthProviderEntry.UpdateOrgEntry): Promise<AuthProviderEntry> {
+        let authProvider: AuthProviderEntry;
+
+        const { id, organizationId } = entry;
+        // TODO can we change this to query for the provider by id and org instead of loading all from org?
+        const existing = (await this.authProviderDB.findByOrgId(organizationId)).find((p) => p.id === id);
+        if (!existing) {
+            throw new Error("Provider does not exist.");
+        }
+        const changed =
+            entry.clientId !== existing.oauth.clientId ||
+            (entry.clientSecret && entry.clientSecret !== existing.oauth.clientSecret);
+
+        if (!changed) {
+            return existing;
+        }
+
+        // update config on demand
+        const oauth = {
+            ...existing.oauth,
+            clientId: entry.clientId,
+            clientSecret: entry.clientSecret || existing.oauth.clientSecret, // FE may send empty ("") if not changed
+        };
+        authProvider = {
+            ...existing,
+            oauth,
+            status: "pending",
+        };
+
+        return await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry, true);
+    }
+
     protected initializeNewProvider(newEntry: AuthProviderEntry.NewEntry): AuthProviderEntry {
         const { host, type, clientId, clientSecret } = newEntry;
         let urls;

components/server/src/workspace/gitpod-server-impl.ts

@@ -2925,7 +2925,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         let user = this.checkAndBlockUser("createOrgAuthProvider");
 
         // map params to a new provider
-        const newProvider = <AuthProviderEntry.NewEntry>{
+        const newProvider = <AuthProviderEntry.NewOrgEntry>{
             host: entry.host,
             type: entry.type,
             clientId: entry.clientId,
@@ -2966,7 +2966,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
                 throw new Error("Provider for this host already exists.");
             }
 
-            const result = await this.authProviderService.updateAuthProvider(newProvider);
+            const result = await this.authProviderService.createOrgAuthProvider(newProvider);
             return AuthProviderEntry.redact(result);
         } catch (error) {
             const message = error && error.message ? error.message : "Failed to create the provider.";
@@ -2980,12 +2980,11 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
     ): Promise<AuthProviderEntry> {
         traceAPIParams(ctx, {}); // entry contains PII
 
-        const user = this.checkAndBlockUser("updateOrgAuthProvider");
+        this.checkAndBlockUser("updateOrgAuthProvider");
 
         // map params to a provider update
-        const providerUpdate: AuthProviderEntry.UpdateEntry = {
+        const providerUpdate: AuthProviderEntry.UpdateOrgEntry = {
             id: entry.id,
-            ownerId: user.id,
             clientId: entry.clientId,
             clientSecret: entry.clientSecret,
             organizationId: entry.organizationId,
@@ -3000,7 +2999,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         await this.guardTeamOperation(providerUpdate.organizationId, "update");
 
         try {
-            const result = await this.authProviderService.updateAuthProvider(providerUpdate);
+            const result = await this.authProviderService.updateOrgAuthProvider(providerUpdate);
             return AuthProviderEntry.redact(result);
         } catch (error) {
             const message = error && error.message ? error.message : "Failed to update the provider.";