[server] move FGA calls into AuthProviderService

AlexTugarev · AlexTugarev · commit 1010fde0553e · 2023-11-06T15:54:37.000Z
diff --git a/components/gitpod-protocol/src/gitpod-service.ts b/components/gitpod-protocol/src/gitpod-service.ts
@@ -81,16 +81,31 @@ export interface GitpodServer extends JsonRpcServer<GitpodClient>, AdminServer,
     updateLoggedInUser(user: Partial<User>): Promise<User>;
     sendPhoneNumberVerificationToken(phoneNumber: string): Promise<{ verificationId: string }>;
     verifyPhoneNumberVerificationToken(phoneNumber: string, token: string, verificationId: string): Promise<boolean>;
-    getAuthProviders(): Promise<AuthProviderInfo[]>;
-    getOwnAuthProviders(): Promise<AuthProviderEntry[]>;
-    updateOwnAuthProvider(params: GitpodServer.UpdateOwnAuthProviderParams): Promise<AuthProviderEntry>;
-    deleteOwnAuthProvider(params: GitpodServer.DeleteOwnAuthProviderParams): Promise<void>;
     getConfiguration(): Promise<Configuration>;
     getToken(query: GitpodServer.GetTokenSearchOptions): Promise<Token | undefined>;
     getGitpodTokenScopes(tokenHash: string): Promise<string[]>;
     deleteAccount(): Promise<void>;
     getClientRegion(): Promise<string | undefined>;
 
+    // Auth Provider API
+    getAuthProviders(): Promise<AuthProviderInfo[]>;
+    // user-level
+    getOwnAuthProviders(): Promise<AuthProviderEntry[]>;
+    updateOwnAuthProvider(params: GitpodServer.UpdateOwnAuthProviderParams): Promise<AuthProviderEntry>;
+    deleteOwnAuthProvider(params: GitpodServer.DeleteOwnAuthProviderParams): Promise<void>;
+    // org-level
+    createOrgAuthProvider(params: GitpodServer.CreateOrgAuthProviderParams): Promise<AuthProviderEntry>;
+    updateOrgAuthProvider(params: GitpodServer.UpdateOrgAuthProviderParams): Promise<AuthProviderEntry>;
+    getOrgAuthProviders(params: GitpodServer.GetOrgAuthProviderParams): Promise<AuthProviderEntry[]>;
+    deleteOrgAuthProvider(params: GitpodServer.DeleteOrgAuthProviderParams): Promise<void>;
+    // public-api compatibility
+    /** @deprecated used for public-api compatibility only */
+    getAuthProvider(id: string): Promise<AuthProviderEntry>;
+    /** @deprecated used for public-api compatibility only */
+    deleteAuthProvider(id: string): Promise<void>;
+    /** @deprecated used for public-api compatibility only */
+    updateAuthProvider(id: string, update: AuthProviderEntry.UpdateEntry): Promise<AuthProviderEntry>;
+
     // Query/retrieve workspaces
     getWorkspaces(options: GitpodServer.GetWorkspacesOptions): Promise<WorkspaceInfo[]>;
     getWorkspaceOwner(workspaceId: string): Promise<UserInfo | undefined>;
@@ -167,10 +182,6 @@ export interface GitpodServer extends JsonRpcServer<GitpodClient>, AdminServer,
     deleteTeam(teamId: string): Promise<void>;
     getOrgSettings(orgId: string): Promise<OrganizationSettings>;
     updateOrgSettings(teamId: string, settings: Partial<OrganizationSettings>): Promise<OrganizationSettings>;
-    createOrgAuthProvider(params: GitpodServer.CreateOrgAuthProviderParams): Promise<AuthProviderEntry>;
-    updateOrgAuthProvider(params: GitpodServer.UpdateOrgAuthProviderParams): Promise<AuthProviderEntry>;
-    getOrgAuthProviders(params: GitpodServer.GetOrgAuthProviderParams): Promise<AuthProviderEntry[]>;
-    deleteOrgAuthProvider(params: GitpodServer.DeleteOrgAuthProviderParams): Promise<void>;
 
     getDefaultWorkspaceImage(params: GetDefaultWorkspaceImageParams): Promise<GetDefaultWorkspaceImageResult>;
 
diff --git a/components/server/src/auth/auth-provider-service.ts b/components/server/src/auth/auth-provider-service.ts
@@ -16,17 +16,19 @@ import { oauthUrls as bbsUrls } from "../bitbucket-server/bitbucket-server-urls"
 import { oauthUrls as bbUrls } from "../bitbucket/bitbucket-urls";
 import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
 import fetch from "node-fetch";
+import { Authorizer } from "../authorization/authorizer";
+import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
+import { HostContextProvider } from "./host-context-provider";
 
 @injectable()
 export class AuthProviderService {
-    @inject(AuthProviderEntryDB)
-    protected authProviderDB: AuthProviderEntryDB;
-
-    @inject(TeamDB)
-    protected teamDB: TeamDB;
-
-    @inject(Config)
-    protected readonly config: Config;
+    constructor(
+        @inject(AuthProviderEntryDB) private readonly authProviderDB: AuthProviderEntryDB,
+        @inject(TeamDB) private readonly teamDB: TeamDB,
+        @inject(Config) protected readonly config: Config,
+        @inject(HostContextProvider) private readonly hostContexts: HostContextProvider,
+        @inject(Authorizer) private readonly auth: Authorizer,
+    ) {}
 
     /**
      * Returns all auth providers.
@@ -57,19 +59,59 @@ export class AuthProviderService {
         };
 
     async getAuthProvidersOfUser(user: User | string): Promise<AuthProviderEntry[]> {
-        const result = await this.authProviderDB.findByUserId(User.is(user) ? user.id : user);
+        const userId = User.is(user) ? user.id : user;
+        await this.auth.checkPermissionOnUser(userId, "read_info", userId);
+
+        const result = await this.authProviderDB.findByUserId(userId);
         return result;
     }
 
-    async getAuthProvidersOfOrg(organizationId: string): Promise<AuthProviderEntry[]> {
+    async getAuthProvidersOfOrg(userId: string, organizationId: string): Promise<AuthProviderEntry[]> {
+        await this.auth.checkPermissionOnOrganization(userId, "read_git_provider", organizationId);
         const result = await this.authProviderDB.findByOrgId(organizationId);
         return result;
     }
 
-    async deleteAuthProvider(authProvider: AuthProviderEntry): Promise<void> {
+    async deleteAuthProviderOfUser(userId: string, authProviderId: string): Promise<void> {
+        await this.auth.checkPermissionOnUser(userId, "write_info", userId);
+
+        const ownProviders = await this.getAuthProvidersOfUser(userId);
+        const authProvider = ownProviders.find((p) => p.id === authProviderId);
+        if (!authProvider) {
+            throw new ApplicationError(ErrorCodes.NOT_FOUND, "User resource not found.");
+        }
+
         await this.authProviderDB.delete(authProvider);
     }
 
+    async deleteAuthProviderOfOrg(userId: string, organizationId: string, authProviderId: string): Promise<void> {
+        await this.auth.checkPermissionOnOrganization(userId, "write_git_provider", organizationId);
+
+        // Find the matching auth provider we're attempting to delete
+        const orgProviders = await this.getAuthProvidersOfOrg(userId, organizationId);
+        const authProvider = orgProviders.find((p) => p.id === authProviderId && p.organizationId === organizationId);
+        if (!authProvider) {
+            throw new ApplicationError(ErrorCodes.NOT_FOUND, "Provider resource not found.");
+        }
+
+        await this.authProviderDB.delete(authProvider);
+    }
+
+    async getAuthProvider(userId: string, id: string): Promise<AuthProviderEntry | undefined> {
+        const result = await this.authProviderDB.findById(id);
+        if (!result) {
+            return undefined;
+        }
+
+        if (result.organizationId) {
+            await this.auth.checkPermissionOnOrganization(userId, "write_git_provider", result.organizationId);
+        } else {
+            await this.auth.checkPermissionOnUser(userId, "write_info", userId);
+        }
+
+        return result;
+    }
+
     async updateAuthProvider(
         entry: AuthProviderEntry.UpdateEntry | AuthProviderEntry.NewEntry,
     ): Promise<AuthProviderEntry> {
@@ -109,20 +151,39 @@ export class AuthProviderService {
         return await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry, true);
     }
 
-    async createOrgAuthProvider(entry: AuthProviderEntry.NewOrgEntry): Promise<AuthProviderEntry> {
-        const orgProviders = await this.authProviderDB.findByOrgId(entry.organizationId);
-        const existing = orgProviders.find((p) => p.host === entry.host);
-        if (existing) {
-            throw new Error("Provider for this host already exists.");
+    async createOrgAuthProvider(userId: string, newEntry: AuthProviderEntry.NewOrgEntry): Promise<AuthProviderEntry> {
+        await this.auth.checkPermissionOnOrganization(userId, "write_git_provider", newEntry.organizationId);
+
+        // on creating we're are checking for already existing runtime providers
+        const host = newEntry.host && newEntry.host.toLowerCase();
+
+        if (!(await this.isHostReachable(host))) {
+            log.info(`Host could not be reached.`, { newEntry });
+            throw new ApplicationError(ErrorCodes.BAD_REQUEST, `Host could not be reached.`);
         }
 
-        const authProvider = this.initializeNewProvider(entry);
+        const hostContext = this.hostContexts.get(host);
+        if (hostContext) {
+            const builtInExists = hostContext.authProvider.params.ownerId === undefined;
+            log.info(`Attempt to override an existing provider.`, { newEntry, builtInExists });
+            throw new ApplicationError(ErrorCodes.CONFLICT, `Attempt to override an existing provider.`);
+        }
 
-        return await this.authProviderDB.storeAuthProvider(authProvider as AuthProviderEntry, true);
+        const orgProviders = await this.authProviderDB.findByOrgId(newEntry.organizationId);
+        const existing = orgProviders.find((p) => p.host === host);
+        if (existing) {
+            log.info(`Attempt to override an existing provider.`, { newEntry });
+            throw new ApplicationError(ErrorCodes.CONFLICT, `Provider for this host already exists.`);
+        }
+
+        const authProvider = this.initializeNewProvider(newEntry);
+        return await this.authProviderDB.storeAuthProvider(authProvider, true);
     }
 
-    async updateOrgAuthProvider(entry: AuthProviderEntry.UpdateOrgEntry): Promise<AuthProviderEntry> {
+    async updateOrgAuthProvider(userId: string, entry: AuthProviderEntry.UpdateOrgEntry): Promise<AuthProviderEntry> {
         const { id, organizationId } = entry;
+        await this.auth.checkPermissionOnOrganization(userId, "write_git_provider", organizationId);
+
         // 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) {
diff --git a/components/server/src/auth/rate-limiter.ts b/components/server/src/auth/rate-limiter.ts
@@ -195,6 +195,9 @@ const defaultFunctions: FunctionsConfig = {
     getIDToken: { group: "default", points: 1 },
     reportErrorBoundary: { group: "default", points: 1 },
     getOnboardingState: { group: "default", points: 1 },
+    getAuthProvider: { group: "default", points: 1 },
+    deleteAuthProvider: { group: "default", points: 1 },
+    updateAuthProvider: { group: "default", points: 1 },
 };
 
 function getConfig(config: RateLimiterConfig): RateLimiterConfig {
diff --git a/components/server/src/user/user-deletion-service.ts b/components/server/src/user/user-deletion-service.ts
@@ -45,7 +45,7 @@ export class UserDeletionService {
         const authProviders = await this.authProviderService.getAuthProvidersOfUser(user);
         for (const provider of authProviders) {
             try {
-                await this.authProviderService.deleteAuthProvider(provider);
+                await this.authProviderService.deleteAuthProviderOfUser(user.id, provider.id);
             } catch (error) {
                 log.error({ userId: user.id }, "Failed to delete user's auth provider.", error);
             }
diff --git a/components/server/src/workspace/gitpod-server-impl.ts b/components/server/src/workspace/gitpod-server-impl.ts
@@ -2989,12 +2989,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         traceAPIParams(ctx, { params });
 
         const user = await this.checkAndBlockUser("deleteOwnAuthProvider");
-        const ownProviders = await this.authProviderService.getAuthProvidersOfUser(user.id);
-        const authProvider = ownProviders.find((p) => p.id === params.id);
-        if (!authProvider) {
-            throw new ApplicationError(ErrorCodes.NOT_FOUND, "User resource not found.");
-        }
-        await this.authProviderService.deleteAuthProvider(authProvider);
+
+        await this.authProviderService.deleteAuthProviderOfUser(user.id, params.id);
     }
 
     async createOrgAuthProvider(
@@ -3018,36 +3014,19 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         if (!newProvider.organizationId || !uuidValidate(newProvider.organizationId)) {
             throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Invalid organizationId");
         }
-
-        await this.guardWithFeatureFlag("orgGitAuthProviders", user, newProvider.organizationId);
-
-        await this.guardTeamOperation(newProvider.organizationId, "update");
-        await this.auth.checkPermissionOnOrganization(user.id, "write_git_provider", newProvider.organizationId);
-
         if (!newProvider.host) {
             throw new ApplicationError(
                 ErrorCodes.BAD_REQUEST,
                 "Must provider a host value when creating a new auth provider.",
             );
         }
 
-        try {
-            // on creating we're are checking for already existing runtime providers
-            const host = newProvider.host && newProvider.host.toLowerCase();
-
-            if (!(await this.authProviderService.isHostReachable(host))) {
-                log.debug(`Host could not be reached.`, { entry, newProvider });
-                throw new Error("Host could not be reached.");
-            }
+        await this.guardWithFeatureFlag("orgGitAuthProviders", user, newProvider.organizationId);
 
-            const hostContext = this.hostContextProvider.get(host);
-            if (hostContext) {
-                const builtInExists = hostContext.authProvider.params.ownerId === undefined;
-                log.debug(`Attempt to override existing auth provider.`, { entry, newProvider, builtInExists });
-                throw new Error("Provider for this host already exists.");
-            }
+        await this.guardTeamOperation(newProvider.organizationId, "update");
 
-            const result = await this.authProviderService.createOrgAuthProvider(newProvider);
+        try {
+            const result = await this.authProviderService.createOrgAuthProvider(user.id, newProvider);
             return AuthProviderEntry.redact(result);
         } catch (error) {
             if (ApplicationError.hasErrorCode(error)) {
@@ -3081,10 +3060,9 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         await this.guardWithFeatureFlag("orgGitAuthProviders", user, providerUpdate.organizationId);
 
         await this.guardTeamOperation(providerUpdate.organizationId, "update");
-        await this.auth.checkPermissionOnOrganization(user.id, "write_git_provider", providerUpdate.organizationId);
 
         try {
-            const result = await this.authProviderService.updateOrgAuthProvider(providerUpdate);
+            const result = await this.authProviderService.updateOrgAuthProvider(user.id, providerUpdate);
             return AuthProviderEntry.redact(result);
         } catch (error) {
             if (ApplicationError.hasErrorCode(error)) {
@@ -3106,10 +3084,9 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         await this.guardWithFeatureFlag("orgGitAuthProviders", user, params.organizationId);
 
         await this.guardTeamOperation(params.organizationId, "get");
-        await this.auth.checkPermissionOnOrganization(user.id, "read_git_provider", params.organizationId);
 
         try {
-            const result = await this.authProviderService.getAuthProvidersOfOrg(params.organizationId);
+            const result = await this.authProviderService.getAuthProvidersOfOrg(user.id, params.organizationId);
             return result.map(AuthProviderEntry.redact.bind(AuthProviderEntry));
         } catch (error) {
             if (ApplicationError.hasErrorCode(error)) {
@@ -3125,24 +3102,76 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
 
         const user = await this.checkAndBlockUser("deleteOrgAuthProvider");
 
+        // check for "orgGitAuthProviders" feature flag
         const team = await this.getTeam(ctx, params.organizationId);
         if (!team) {
             throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Invalid organizationId");
         }
-
         await this.guardWithFeatureFlag("orgGitAuthProviders", user, team.id);
 
         await this.guardTeamOperation(params.organizationId || "", "update");
-        await this.auth.checkPermissionOnOrganization(user.id, "write_git_provider", params.organizationId);
 
-        // Find the matching auth provider we're attempting to delete
-        const orgProviders = await this.authProviderService.getAuthProvidersOfOrg(team.id);
-        const authProvider = orgProviders.find((p) => p.id === params.id && p.organizationId === params.organizationId);
+        await this.authProviderService.deleteAuthProviderOfOrg(user.id, params.organizationId, params.id);
+    }
+
+    async getAuthProvider(ctx: TraceContextWithSpan, id: string): Promise<AuthProviderEntry> {
+        traceAPIParams(ctx, { id });
+
+        const user = await this.checkAndBlockUser("getAuthProvider");
+
+        const result = await this.authProviderService.getAuthProvider(user.id, id);
+        if (!result) {
+            throw new ApplicationError(ErrorCodes.NOT_FOUND, "Provider resource not found.");
+        }
+        return result;
+    }
+
+    /**
+     * Delegates to `deleteOrgAuthProvider` or `deleteOwnAuthProvider` depending on the ownership
+     * of the specified auth provider.
+     */
+    async deleteAuthProvider(ctx: TraceContextWithSpan, id: string): Promise<void> {
+        traceAPIParams(ctx, { id });
+
+        const user = await this.checkAndBlockUser("deleteAuthProvider");
+
+        const authProvider = await this.authProviderService.getAuthProvider(user.id, id);
         if (!authProvider) {
             throw new ApplicationError(ErrorCodes.NOT_FOUND, "Provider resource not found.");
         }
 
-        await this.authProviderService.deleteAuthProvider(authProvider);
+        if (authProvider.organizationId) {
+            return this.deleteOrgAuthProvider(ctx, { id, organizationId: authProvider.organizationId });
+        } else {
+            return this.deleteOwnAuthProvider(ctx, { id });
+        }
+    }
+
+    /**
+     * Delegates to `updateOrgAuthProvider` or `updateOwnAuthProvider` depending on the ownership
+     * of the specified auth provider.
+     */
+    async updateAuthProvider(
+        ctx: TraceContextWithSpan,
+        id: string,
+        entry: AuthProviderEntry.UpdateEntry,
+    ): Promise<AuthProviderEntry> {
+        traceAPIParams(ctx, { id });
+
+        const user = await this.checkAndBlockUser("updateAuthProvider");
+
+        const authProvider = await this.authProviderService.getAuthProvider(user.id, id);
+        if (!authProvider) {
+            throw new ApplicationError(ErrorCodes.NOT_FOUND, "Provider resource not found.");
+        }
+
+        if (authProvider.organizationId) {
+            return this.updateOrgAuthProvider(ctx, {
+                entry: { ...entry, organizationId: authProvider.organizationId },
+            });
+        } else {
+            return this.updateOwnAuthProvider(ctx, { entry });
+        }
     }
 
     async getOnboardingState(ctx: TraceContext): Promise<GitpodServer.OnboardingState> {

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ export class UserDeletionService {`
`45`	`45`	`const authProviders = await this.authProviderService.getAuthProvidersOfUser(user);`
`46`	`46`	`for (const provider of authProviders) {`
`47`	`47`	`try {`
`48`		`- await this.authProviderService.deleteAuthProvider(provider);`
	`48`	`+ await this.authProviderService.deleteAuthProviderOfUser(user.id, provider.id);`
`49`	`49`	`} catch (error) {`
`50`	`50`	`log.error({ userId: user.id }, "Failed to delete user's auth provider.", error);`
`51`	`51`	`}`