[server, db] Cleanup UpdateOrgSettings API handling

Tool: gitpod/catfood.gitpod.cloud

Author: geropl

components/gitpod-db/package.json

@@ -43,6 +43,7 @@
     "prom-client": "^14.2.0",
     "reflect-metadata": "^0.1.13",
     "slugify": "^1.6.5",
+    "ts-deepmerge": "^7.0.2",
     "the-big-username-blacklist": "^1.5.2",
     "typeorm": "0.2.38"
   },

components/gitpod-db/src/typeorm/team-db-impl.ts

@@ -30,6 +30,7 @@ import { TypeORM } from "./typeorm";
 import { EncryptionService } from "@gitpod/gitpod-protocol/lib/encryption/encryption-service";
 import { DBOrgEnvVar } from "./entity/db-org-env-var";
 import { filter } from "../utils";
+import { merge } from "ts-deepmerge";
 
 @injectable()
 export class TeamDBImpl extends TransactionalDBImpl<TeamDB> implements TeamDB {
@@ -394,19 +395,22 @@ export class TeamDBImpl extends TransactionalDBImpl<TeamDB> implements TeamDB {
         });
     }
 
-    public async setOrgSettings(orgId: string, settings: Partial<OrganizationSettings>): Promise<OrganizationSettings> {
+    public async setOrgSettings(
+        orgId: string,
+        partialUpdate: Partial<OrganizationSettings>,
+    ): Promise<OrganizationSettings> {
         const repo = await this.getOrgSettingsRepo();
-        const team = await repo.findOne({ where: { orgId, deleted: false } });
-        if (!team) {
+        const currentSettings = await repo.findOne({ where: { orgId, deleted: false } });
+        if (!currentSettings) {
             return await repo.save({
-                ...settings,
+                ...partialUpdate,
                 orgId,
             });
         }
-        return await repo.save({
-            ...team,
-            ...settings,
-        });
+        // We want to deep-merge columns that are JSON shapes here.
+        // By ignore fields set to undefined, and don't merge arrays to match our API semantics
+        const settings = merge.withOptions({ mergeArrays: false }, currentSettings, partialUpdate);
+        return await repo.save(settings);
     }
 
     public async hasActiveSSO(organizationId: string): Promise<boolean> {

components/gitpod-protocol/src/teams-projects-protocol.ts

@@ -280,11 +280,14 @@ export interface OnboardingSettings {
     /**
      * the welcome message for new members of the organization
      */
-    welcomeMessage?: {
-        featuredMemberId?: string;
-        message?: string;
-        footer?: string;
-    };
+    welcomeMessage?: WelcomeMessage;
+}
+
+export interface WelcomeMessage {
+    enabled?: boolean;
+    featuredMemberId?: string;
+    featuredMemberResolvedAvatarUrl?: string;
+    message?: string;
 }
 
 export type TeamMemberInfo = OrgMemberInfo;

components/public-api/typescript-common/src/public-api-converter.ts

@@ -56,6 +56,10 @@ import {
     Organization as ProtocolOrganization,
     OrgMemberPermission,
     OrgMemberRole,
+    RoleRestrictions,
+    TimeoutSettings as TimeoutSettingsProtocol,
+    OnboardingSettings as OnboardingSettingsProtocol,
+    WelcomeMessage as WelcomeMessageProtocol,
 } from "@gitpod/gitpod-protocol/lib/teams-projects-protocol";
 import type { DeepPartial } from "@gitpod/gitpod-protocol/lib/util/deep-partial";
 import { parseGoDurationToMs } from "@gitpod/gitpod-protocol/lib/util/timeutil";
@@ -113,11 +117,15 @@ import {
     OnboardingState,
 } from "@gitpod/public-api/lib/gitpod/v1/installation_pb";
 import {
+    OnboardingSettings,
+    OnboardingSettings_WelcomeMessage,
     Organization,
     OrganizationMember,
     OrganizationPermission,
     OrganizationRole,
     OrganizationSettings,
+    RoleRestrictionEntry,
+    TimeoutSettings,
 } from "@gitpod/public-api/lib/gitpod/v1/organization_pb";
 import {
     Prebuild,
@@ -1040,6 +1048,61 @@ export class PublicAPIConverter {
         }
     }
 
+    fromOrgMemberRoleString(role: string): OrgMemberRole {
+        switch (role) {
+            case "owner":
+            case "member":
+            case "collaborator":
+                return role;
+            default:
+                throw new Error("invalid org member role");
+        }
+    }
+
+    fromTimeoutSettings(timeoutSettings: TimeoutSettings): TimeoutSettingsProtocol {
+        const result: TimeoutSettingsProtocol = {
+            denyUserTimeouts: timeoutSettings.denyUserTimeouts,
+        };
+        if (timeoutSettings.inactivity) {
+            result.inactivity = this.toDurationString(timeoutSettings.inactivity);
+        }
+        return result;
+    }
+
+    fromRoleRestrictions(roleRestrictions: RoleRestrictionEntry[]): RoleRestrictions {
+        const result: RoleRestrictions = {};
+        for (const roleRestriction of roleRestrictions) {
+            const role = this.fromOrgMemberRole(roleRestriction.role);
+            const permissions = roleRestriction.permissions.map((p) => this.fromOrganizationPermission(p));
+            result[role] = permissions;
+        }
+        return result;
+    }
+
+    fromOnboardingSettings(onboardingSettings: OnboardingSettings): OnboardingSettingsProtocol {
+        const result: OnboardingSettingsProtocol = {
+            internalLink: onboardingSettings.internalLink,
+        };
+
+        if (onboardingSettings.welcomeMessage) {
+            result.welcomeMessage = this.fromWelcomeMessage(onboardingSettings.welcomeMessage);
+        }
+
+        if (onboardingSettings.updateRecommendedRepositories) {
+            result.recommendedRepositories = onboardingSettings.recommendedRepositories;
+        }
+
+        return result;
+    }
+
+    fromWelcomeMessage(welcomeMessage: OnboardingSettings_WelcomeMessage): WelcomeMessageProtocol {
+        return {
+            enabled: welcomeMessage.enabled,
+            message: welcomeMessage.message,
+            featuredMemberId: welcomeMessage.featuredMemberId,
+        };
+    }
+
     fromWorkspaceSettings(settings?: DeepPartial<WorkspaceSettings>) {
         const result: Partial<
             Pick<ProjectSettings, "workspaceClasses" | "restrictedWorkspaceClasses" | "restrictedEditorNames">
@@ -1134,26 +1197,52 @@ export class PublicAPIConverter {
             pinnedEditorVersions: settings.pinnedEditorVersions || {},
             restrictedEditorNames: settings.restrictedEditorNames || [],
             defaultRole: settings.defaultRole || undefined,
-            timeoutSettings: {
-                inactivity: settings.timeoutSettings?.inactivity
-                    ? this.toDuration(settings.timeoutSettings?.inactivity)
-                    : undefined,
-                denyUserTimeouts: settings.timeoutSettings?.denyUserTimeouts,
-            },
-            roleRestrictions: Object.entries(settings.roleRestrictions ?? {}).map(([role, permissions]) => ({
-                role: this.toOrgMemberRole(role as OrgMemberRole),
-                permissions: permissions.map((permission) => this.toOrganizationPermission(permission)),
-            })),
+            timeoutSettings: settings.timeoutSettings ? this.toTimeoutSettings(settings.timeoutSettings) : undefined,
+            roleRestrictions: settings.roleRestrictions
+                ? this.toRoleRestrictions(settings.roleRestrictions)
+                : undefined,
             maxParallelRunningWorkspaces: settings.maxParallelRunningWorkspaces ?? 0,
-            onboardingSettings: {
-                internalLink: settings?.onboardingSettings?.internalLink ?? undefined,
-                welcomeMessage: settings?.onboardingSettings?.welcomeMessage ?? undefined,
-                recommendedRepositories: settings?.onboardingSettings?.recommendedRepositories ?? [],
-            },
+            onboardingSettings: settings?.onboardingSettings
+                ? this.toOnboardingSettings(settings.onboardingSettings)
+                : undefined,
             annotateGitCommits: settings.annotateGitCommits ?? false,
         });
     }
 
+    toTimeoutSettings(settings: TimeoutSettingsProtocol): TimeoutSettings {
+        return new TimeoutSettings({
+            inactivity: settings.inactivity ? this.toDuration(settings.inactivity) : undefined,
+            denyUserTimeouts: settings.denyUserTimeouts,
+        });
+    }
+
+    toRoleRestrictions(roleRestrictions: RoleRestrictions): RoleRestrictionEntry[] {
+        return Object.entries(roleRestrictions ?? {}).map(
+            ([role, permissions]) =>
+                new RoleRestrictionEntry({
+                    role: this.toOrgMemberRole(role as OrgMemberRole),
+                    permissions: permissions.map((permission) => this.toOrganizationPermission(permission)),
+                }),
+        );
+    }
+
+    toOnboardingSettings(settings: OnboardingSettingsProtocol): OnboardingSettings {
+        return new OnboardingSettings({
+            internalLink: settings.internalLink,
+            welcomeMessage: settings.welcomeMessage ? this.toWelcomeMessage(settings.welcomeMessage) : undefined,
+            recommendedRepositories: settings.recommendedRepositories,
+        });
+    }
+
+    toWelcomeMessage(settings: WelcomeMessageProtocol): OnboardingSettings_WelcomeMessage {
+        return new OnboardingSettings_WelcomeMessage({
+            enabled: settings.enabled,
+            message: settings.message,
+            featuredMemberId: settings.featuredMemberId,
+            featuredMemberResolvedAvatarUrl: settings.featuredMemberResolvedAvatarUrl,
+        });
+    }
+
     toConfiguration(project: Project): Configuration {
         const result = new Configuration();
         result.id = project.id;

components/server/src/api/organization-service-api.ts

@@ -46,22 +46,16 @@ import { validate as uuidValidate } from "uuid";
 import { ctxUserId } from "../util/request-context";
 import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
 import { EntitlementService } from "../billing/entitlement-service";
-import { Config } from "../config";
-import { ProjectsService } from "../projects/projects-service";
 
 @injectable()
 export class OrganizationServiceAPI implements ServiceImpl<typeof OrganizationServiceInterface> {
     constructor(
-        @inject(Config)
-        private readonly config: Config,
         @inject(OrganizationService)
         private readonly orgService: OrganizationService,
         @inject(PublicAPIConverter)
         private readonly apiConverter: PublicAPIConverter,
         @inject(EntitlementService)
         private readonly entitlementService: EntitlementService,
-        @inject(ProjectsService)
-        private readonly projectService: ProjectsService,
     ) {}
 
     async listOrganizationWorkspaceClasses(
@@ -188,7 +182,7 @@ export class OrganizationServiceAPI implements ServiceImpl<typeof OrganizationSe
             throw new ApplicationError(ErrorCodes.BAD_REQUEST, "organizationId is required");
         }
 
-        const members = await this.orgService.listMembers(ctxUserId(), req.organizationId, true);
+        const members = await this.orgService.listMembers(ctxUserId(), req.organizationId);
         //TODO pagination
         const response = new ListOrganizationMembersResponse();
         response.members = members.map((member) => this.apiConverter.toOrganizationMember(member));
@@ -218,7 +212,7 @@ export class OrganizationServiceAPI implements ServiceImpl<typeof OrganizationSe
             this.apiConverter.fromOrgMemberRole(req.role),
         );
         const member = await this.orgService
-            .listMembers(ctxUserId(), req.organizationId, true)
+            .listMembers(ctxUserId(), req.organizationId)
             .then((members) => members.find((member) => member.userId === req.userId));
         return new UpdateOrganizationMemberResponse({
             member: member && this.apiConverter.toOrganizationMember(member),
@@ -252,19 +246,6 @@ export class OrganizationServiceAPI implements ServiceImpl<typeof OrganizationSe
         const response = new GetOrganizationSettingsResponse();
         response.settings = this.apiConverter.toOrganizationSettings(settings);
 
-        // resolve the avatar URL for the featured member in the welcome message
-        const onboardingWelcomeMessageFeaturedMemberId =
-            response.settings.onboardingSettings?.welcomeMessage?.featuredMemberId;
-        if (onboardingWelcomeMessageFeaturedMemberId) {
-            const member = await this.orgService
-                .getOrganizationMember(ctxUserId(), req.organizationId, onboardingWelcomeMessageFeaturedMemberId, true)
-                .catch(() => undefined);
-            if (member?.user.avatarUrl && response?.settings?.onboardingSettings?.welcomeMessage) {
-                response.settings.onboardingSettings.welcomeMessage.featuredMemberResolvedAvatarUrl =
-                    member.user.avatarUrl;
-            }
-        }
-
         return response;
     }
 
@@ -354,30 +335,8 @@ export class OrganizationServiceAPI implements ServiceImpl<typeof OrganizationSe
             update.maxParallelRunningWorkspaces = req.maxParallelRunningWorkspaces;
         }
 
-        if (req.onboardingSettings && Object.keys(req.onboardingSettings).length > 0) {
-            update.onboardingSettings = {};
-
-            if (!this.config.isDedicatedInstallation) {
-                throw new ApplicationError(
-                    ErrorCodes.BAD_REQUEST,
-                    "onboardingSettings can only be set on enterprise installations",
-                );
-            }
-
-            if (req.onboardingSettings.welcomeMessage?.featuredMemberResolvedAvatarUrl) {
-                throw new ApplicationError(
-                    ErrorCodes.BAD_REQUEST,
-                    "featuredMemberResolvedAvatarUrl is not allowed to be set",
-                );
-            }
-
-            if (req.onboardingSettings.internalLink) {
-                if (req.onboardingSettings.internalLink.length > 255) {
-                    throw new ApplicationError(ErrorCodes.BAD_REQUEST, "internalLink must be <= 255 characters long");
-                }
-
-                update.onboardingSettings.internalLink = req.onboardingSettings.internalLink;
-            }
+        if (req.onboardingSettings) {
+            update.onboardingSettings = this.apiConverter.fromOnboardingSettings(req.onboardingSettings);
 
             if (
                 !req.onboardingSettings.updateRecommendedRepositories &&
@@ -388,65 +347,8 @@ export class OrganizationServiceAPI implements ServiceImpl<typeof OrganizationSe
                     "recommendedRepositories can only be set when updateRecommendedRepositories is true",
                 );
             }
-
-            if (
-                req.onboardingSettings.updateRecommendedRepositories &&
-                req.onboardingSettings.recommendedRepositories
-            ) {
-                if (req.onboardingSettings.recommendedRepositories.length > 3) {
-                    throw new ApplicationError(
-                        ErrorCodes.BAD_REQUEST,
-                        "there can't be more than 3 recommendedRepositories",
-                    );
-                }
-                for (const configurationId of req.onboardingSettings.recommendedRepositories) {
-                    if (!uuidValidate(configurationId)) {
-                        throw new ApplicationError(ErrorCodes.BAD_REQUEST, "recommendedRepositories must be UUIDs");
-                    }
-
-                    const project = await this.projectService.getProject(ctxUserId(), configurationId);
-                    if (!project) {
-                        throw new ApplicationError(ErrorCodes.BAD_REQUEST, `repository ${configurationId} not found`);
-                    }
-                }
-
-                update.onboardingSettings.recommendedRepositories = req.onboardingSettings.recommendedRepositories;
-            }
-
-            if (
-                req.onboardingSettings.welcomeMessage &&
-                Object.keys(req.onboardingSettings.welcomeMessage).length > 0
-            ) {
-                if (req.onboardingSettings.welcomeMessage.featuredMemberId) {
-                    const member = await this.orgService
-                        .getOrganizationMember(
-                            ctxUserId(),
-                            req.organizationId,
-                            req.onboardingSettings.welcomeMessage.featuredMemberId,
-                            true,
-                        )
-                        .catch(() => undefined);
-                    if (!member) {
-                        throw new ApplicationError(ErrorCodes.BAD_REQUEST, "featuredMemberId not found");
-                    }
-                }
-
-                if (
-                    req.onboardingSettings.welcomeMessage.enabled &&
-                    req.onboardingSettings.welcomeMessage.message?.length === 0
-                ) {
-                    throw new ApplicationError(ErrorCodes.BAD_REQUEST, "welcomeMessage must not be empty when enabled");
-                }
-
-                update.onboardingSettings.welcomeMessage = req.onboardingSettings.welcomeMessage;
-            }
-
-            const existingOnboardingSettings = await this.orgService.getSettings(ctxUserId(), req.organizationId);
-            update.onboardingSettings = {
-                ...existingOnboardingSettings.onboardingSettings,
-                ...update.onboardingSettings,
-            };
         }
+
         if (req.annotateGitCommits !== undefined) {
             update.annotateGitCommits = req.annotateGitCommits;
         }