feat: ensure at least one owner on remove user/group access (#5085)

## About the changes This makes sure that projects have at least one owner, either a group or a user. This is to prevent accidentally losing access to a project. We check this when removing a user/group or when changing the role of a user/group **Note**: We can still leave a group empty as the only owner of the project, but that's okay because we can still add more users to the group
Unleash · Oct 19, 2023 · 3d9f31f · 3d9f31f
1 parent 6760fc0
commit 3d9f31f
Show file tree

Hide file tree

Showing 4 changed files with 269 additions and 94 deletions.
diff --git a/src/lib/features/project/createProjectService.ts b/src/lib/features/project/createProjectService.ts
@@ -14,7 +14,6 @@ import FakeGroupStore from '../../../test/fixtures/fake-group-store';
 import FakeEventStore from '../../../test/fixtures/fake-event-store';
 import ProjectStore from '../../db/project-store';
 import FeatureToggleStore from '../feature-toggle/feature-toggle-store';
-import FeatureTypeStore from '../../db/feature-type-store';
 import { FeatureEnvironmentStore } from '../../db/feature-environment-store';
 import ProjectStatsStore from '../../db/project-stats-store';
 import {
@@ -29,7 +28,6 @@ import { FavoriteFeaturesStore } from '../../db/favorite-features-store';
 import { FavoriteProjectsStore } from '../../db/favorite-projects-store';
 import FakeProjectStore from '../../../test/fixtures/fake-project-store';
 import FakeFeatureToggleStore from '../feature-toggle/fakes/fake-feature-toggle-store';
-import FakeFeatureTypeStore from '../../../test/fixtures/fake-feature-type-store';
 import FakeEnvironmentStore from '../../../test/fixtures/fake-environment-store';
 import FakeFeatureEnvironmentStore from '../../../test/fixtures/fake-feature-environment-store';
 import FakeProjectStatsStore from '../../../test/fixtures/fake-project-stats-store';
@@ -41,8 +39,6 @@ import {
     createPrivateProjectChecker,
 } from '../private-project/createPrivateProjectChecker';
 import FakeFeatureTagStore from '../../../test/fixtures/fake-feature-tag-store';
-import { LastSeenAtReadModel } from '../../services/client-metrics/last-seen/last-seen-read-model';
-import { FakeLastSeenReadModel } from '../../services/client-metrics/last-seen/fake-last-seen-read-model';
 
 export const createProjectService = (
     db: Db,
@@ -63,7 +59,6 @@ export const createProjectService = (
         getLogger,
         flagResolver,
     );
-    const featureTypeStore = new FeatureTypeStore(db, getLogger);
     const accountStore = new AccountStore(db, getLogger);
     const environmentStore = new EnvironmentStore(db, eventBus, getLogger);
     const featureEnvironmentStore = new FeatureEnvironmentStore(
@@ -106,14 +101,12 @@ export const createProjectService = (
     );
 
     const privateProjectChecker = createPrivateProjectChecker(db, config);
-    const lastSeenReadModel = new LastSeenAtReadModel(db);
 
     return new ProjectService(
         {
             projectStore,
             eventStore,
             featureToggleStore,
-            featureTypeStore,
             environmentStore,
             featureEnvironmentStore,
             accountStore,
@@ -126,7 +119,6 @@ export const createProjectService = (
         favoriteService,
         eventService,
         privateProjectChecker,
-        lastSeenReadModel,
     );
 };
 
@@ -138,7 +130,6 @@ export const createFakeProjectService = (
     const projectStore = new FakeProjectStore();
     const groupStore = new FakeGroupStore();
     const featureToggleStore = new FakeFeatureToggleStore();
-    const featureTypeStore = new FakeFeatureTypeStore();
     const accountStore = new FakeAccountStore();
     const environmentStore = new FakeEnvironmentStore();
     const featureEnvironmentStore = new FakeFeatureEnvironmentStore();
@@ -169,14 +160,12 @@ export const createFakeProjectService = (
     );
 
     const privateProjectChecker = createFakePrivateProjectChecker();
-    const fakeLastSeenReadModel = new FakeLastSeenReadModel();
 
     return new ProjectService(
         {
             projectStore,
             eventStore,
             featureToggleStore,
-            featureTypeStore,
             environmentStore,
             featureEnvironmentStore,
             accountStore,
@@ -189,6 +178,5 @@ export const createFakeProjectService = (
         favoriteService,
         eventService,
         privateProjectChecker,
-        fakeLastSeenReadModel,
     );
 };
diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts
@@ -568,7 +568,7 @@ export class AccessService {
     }
 
     async removeDefaultProjectRoles(
-        owner: User,
+        owner: IUser,
         projectId: string,
     ): Promise<void> {
         this.logger.info(`Removing project roles for ${projectId}`);

diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts
@@ -1,6 +1,6 @@
 import { subDays } from 'date-fns';
 import { ValidationError } from 'joi';
-import User, { IUser } from '../types/user';
+import { IUser } from '../types/user';
 import { AccessService, AccessWithRoles } from './access-service';
 import NameExistsError from '../error/name-exists-error';
 import InvalidOperationError from '../error/invalid-operation-error';
@@ -15,7 +15,6 @@ import {
     IEventStore,
     IFeatureEnvironmentStore,
     IFeatureToggleStore,
-    IFeatureTypeStore,
     IProject,
     IProjectOverview,
     IProjectWithCount,
@@ -65,8 +64,6 @@ import { ProjectDoraMetricsSchema } from 'lib/openapi';
 import { checkFeatureNamingData } from '../features/feature-naming-pattern/feature-naming-validation';
 import { IPrivateProjectChecker } from '../features/private-project/privateProjectCheckerType';
 import EventService from './event-service';
-import { ILastSeenReadModel } from './client-metrics/last-seen/types/last-seen-read-model-type';
-import { LastSeenMapper } from './client-metrics/last-seen/last-seen-mapper';
 
 const getCreatedBy = (user: IUser) => user.email || user.username || 'unknown';
 
@@ -89,6 +86,10 @@ interface ICalculateStatus {
     updates: IProjectStats;
 }
 
+function includes(list: number[], { id }: { id: number }): boolean {
+    return list.some((l) => l === id);
+}
+
 export default class ProjectService {
     private projectStore: IProjectStore;
 
@@ -98,8 +99,6 @@ export default class ProjectService {
 
     private featureToggleStore: IFeatureToggleStore;
 
-    private featureTypeStore: IFeatureTypeStore;
-
     private featureEnvironmentStore: IFeatureEnvironmentStore;
 
     private environmentStore: IEnvironmentStore;
@@ -120,8 +119,6 @@ export default class ProjectService {
 
     private projectStatsStore: IProjectStatsStore;
 
-    private lastSeenReadModel: ILastSeenReadModel;
-
     private flagResolver: IFlagResolver;
 
     private isEnterprise: boolean;
@@ -131,7 +128,6 @@ export default class ProjectService {
             projectStore,
             eventStore,
             featureToggleStore,
-            featureTypeStore,
             environmentStore,
             featureEnvironmentStore,
             accountStore,
@@ -141,7 +137,6 @@ export default class ProjectService {
             | 'projectStore'
             | 'eventStore'
             | 'featureToggleStore'
-            | 'featureTypeStore'
             | 'environmentStore'
             | 'featureEnvironmentStore'
             | 'accountStore'
@@ -154,23 +149,20 @@ export default class ProjectService {
         favoriteService: FavoritesService,
         eventService: EventService,
         privateProjectChecker: IPrivateProjectChecker,
-        lastSeenReadModel: ILastSeenReadModel,
     ) {
         this.projectStore = projectStore;
         this.environmentStore = environmentStore;
         this.featureEnvironmentStore = featureEnvironmentStore;
         this.accessService = accessService;
         this.eventStore = eventStore;
         this.featureToggleStore = featureToggleStore;
-        this.featureTypeStore = featureTypeStore;
         this.featureToggleService = featureToggleService;
         this.favoritesService = favoriteService;
         this.privateProjectChecker = privateProjectChecker;
         this.accountStore = accountStore;
         this.groupService = groupService;
         this.eventService = eventService;
         this.projectStatsStore = projectStatsStore;
-        this.lastSeenReadModel = lastSeenReadModel;
         this.logger = config.getLogger('services/project-service.js');
         this.flagResolver = config.flagResolver;
         this.isEnterprise = config.isEnterprise;
@@ -267,7 +259,7 @@ export default class ProjectService {
         return data;
     }
 
-    async updateProject(updatedProject: IProject, user: User): Promise<void> {
+    async updateProject(updatedProject: IProject, user: IUser): Promise<void> {
         const preData = await this.projectStore.get(updatedProject.id);
 
         await this.projectStore.update(updatedProject);
@@ -283,7 +275,7 @@ export default class ProjectService {
 
     async updateProjectEnterpriseSettings(
         updatedProject: IProjectEnterpriseSettingsUpdate,
-        user: User,
+        user: IUser,
     ): Promise<void> {
         const preData = await this.projectStore.get(updatedProject.id);
 
@@ -330,7 +322,7 @@ export default class ProjectService {
     async changeProject(
         newProjectId: string,
         featureName: string,
-        user: User,
+        user: IUser,
         currentProjectId: string,
     ): Promise<any> {
         const feature = await this.featureToggleStore.get(featureName);
@@ -372,7 +364,7 @@ export default class ProjectService {
         return updatedFeature;
     }
 
-    async deleteProject(id: string, user: User): Promise<void> {
+    async deleteProject(id: string, user: IUser): Promise<void> {
         if (id === DEFAULT_PROJECT) {
             throw new InvalidOperationError(
                 'You can not delete the default project!',
@@ -508,6 +500,11 @@ export default class ProjectService {
             userId,
         );
 
+        const ownerRole = await this.accessService.getRoleByName(
+            RoleName.OWNER,
+        );
+        await this.validateAtLeastOneOwner(projectId, ownerRole);
+
         await this.accessService.removeUserAccess(projectId, userId);
 
         await this.eventService.storeEvent(
@@ -532,6 +529,11 @@ export default class ProjectService {
             groupId,
         );
 
+        const ownerRole = await this.accessService.getRoleByName(
+            RoleName.OWNER,
+        );
+        await this.validateAtLeastOneOwner(projectId, ownerRole);
+
         await this.accessService.removeGroupAccess(projectId, groupId);
 
         await this.eventService.storeEvent(
@@ -598,6 +600,8 @@ export default class ProjectService {
                 undefined,
             );
 
+        await this.validateAtLeastOneOwner(projectId, role);
+
         await this.accessService.removeGroupFromRole(
             group.id,
             role.id,
@@ -675,28 +679,39 @@ export default class ProjectService {
     async setRolesForUser(
         projectId: string,
         userId: number,
-        roles: number[],
+        newRoles: number[],
         createdByUserName: string,
     ): Promise<void> {
-        const existingRoles = await this.accessService.getProjectRolesForUser(
+        const currentRoles = await this.accessService.getProjectRolesForUser(
             projectId,
             userId,
         );
+
+        const ownerRole = await this.accessService.getRoleByName(
+            RoleName.OWNER,
+        );
+
+        const hasOwnerRole = includes(currentRoles, ownerRole);
+        const isRemovingOwnerRole = !includes(newRoles, ownerRole);
+        if (hasOwnerRole && isRemovingOwnerRole) {
+            await this.validateAtLeastOneOwner(projectId, ownerRole);
+        }
+
         await this.accessService.setProjectRolesForUser(
             projectId,
             userId,
-            roles,
+            newRoles,
         );
         await this.eventService.storeEvent(
             new ProjectAccessUserRolesUpdated({
                 project: projectId,
                 createdBy: createdByUserName,
                 data: {
-                    roles,
+                    roles: newRoles,
                     userId,
                 },
                 preData: {
-                    roles: existingRoles,
+                    roles: currentRoles,
                     userId,
                 },
             }),
@@ -706,29 +721,40 @@ export default class ProjectService {
     async setRolesForGroup(
         projectId: string,
         groupId: number,
-        roles: number[],
+        newRoles: number[],
         createdBy: string,
     ): Promise<void> {
-        const existingRoles = await this.accessService.getProjectRolesForGroup(
+        const currentRoles = await this.accessService.getProjectRolesForGroup(
             projectId,
             groupId,
         );
+
+        const ownerRole = await this.accessService.getRoleByName(
+            RoleName.OWNER,
+        );
+        const hasOwnerRole = includes(currentRoles, ownerRole);
+        const isRemovingOwnerRole = !includes(newRoles, ownerRole);
+        if (hasOwnerRole && isRemovingOwnerRole) {
+            await this.validateAtLeastOneOwner(projectId, ownerRole);
+        }
+        await this.validateAtLeastOneOwner(projectId, ownerRole);
+
         await this.accessService.setProjectRolesForGroup(
             projectId,
             groupId,
-            roles,
+            newRoles,
             createdBy,
         );
         await this.eventService.storeEvent(
             new ProjectAccessGroupRolesUpdated({
                 project: projectId,
                 createdBy,
                 data: {
-                    roles,
+                    roles: newRoles,
                     groupId,
                 },
                 preData: {
-                    roles: existingRoles,
+                    roles: currentRoles,
                     groupId,
                 },
             }),
@@ -1091,7 +1117,7 @@ export default class ProjectService {
         return {
             stats: projectStats,
             name: project.name,
-            description: project.description,
+            description: project.description!,
             mode: project.mode,
             featureLimit: project.featureLimit,
             featureNaming: project.featureNaming,