From 39d2d065cd3ba11db558afaf29383c39acbfe013 Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Mon, 18 Sep 2023 11:06:26 +0300 Subject: [PATCH] feat: private project filtering and store implementation (#4758) --- .../createFeatureToggleService.ts | 4 +- .../createPrivateProjectChecker.ts | 8 +- .../fakePrivateProjectChecker.ts | 8 +- .../private-project/privateProjectChecker.ts | 9 ++ .../privateProjectCheckerType.ts | 1 + .../privateProjectMiddleware.ts | 5 +- .../private-project/privateProjectStore.ts | 89 ++++++++++++++----- .../features/project/createProjectService.ts | 16 ++-- src/lib/routes/admin-api/archive.ts | 4 +- src/lib/routes/controller.ts | 1 - src/lib/services/feature-toggle-service.ts | 11 ++- src/lib/services/index.ts | 6 +- src/lib/services/project-service.ts | 21 +++-- src/lib/types/services.ts | 2 + .../e2e/services/access-service.e2e.test.ts | 1 + .../services/api-token-service.e2e.test.ts | 1 + .../project-health-service.e2e.test.ts | 1 + .../e2e/services/project-service.e2e.test.ts | 1 + 18 files changed, 142 insertions(+), 47 deletions(-) diff --git a/src/lib/features/feature-toggle/createFeatureToggleService.ts b/src/lib/features/feature-toggle/createFeatureToggleService.ts index df70f9707d..7ee45326f1 100644 --- a/src/lib/features/feature-toggle/createFeatureToggleService.ts +++ b/src/lib/features/feature-toggle/createFeatureToggleService.ts @@ -42,7 +42,7 @@ import { import StrategyStore from '../../db/strategy-store'; import FakeStrategiesStore from '../../../test/fixtures/fake-strategies-store'; import { - createFakeprivateProjectChecker, + createFakePrivateProjectChecker, createPrivateProjectChecker, } from '../private-project/createPrivateProjectChecker'; @@ -155,7 +155,7 @@ export const createFakeFeatureToggleService = ( ); const segmentService = createFakeSegmentService(config); const changeRequestAccessReadModel = createFakeChangeRequestAccessService(); - const fakeprivateProjectChecker = createFakeprivateProjectChecker(); + const fakeprivateProjectChecker = createFakePrivateProjectChecker(); const featureToggleService = new FeatureToggleService( { featureStrategiesStore, diff --git a/src/lib/features/private-project/createPrivateProjectChecker.ts b/src/lib/features/private-project/createPrivateProjectChecker.ts index e68f8520b6..4844025ca9 100644 --- a/src/lib/features/private-project/createPrivateProjectChecker.ts +++ b/src/lib/features/private-project/createPrivateProjectChecker.ts @@ -1,7 +1,7 @@ import { Db, IUnleashConfig } from 'lib/server-impl'; import PrivateProjectStore from './privateProjectStore'; import { PrivateProjectChecker } from './privateProjectChecker'; -import { FakeprivateProjectChecker } from './fakePrivateProjectChecker'; +import { FakePrivateProjectChecker } from './fakePrivateProjectChecker'; export const createPrivateProjectChecker = ( db: Db, @@ -15,7 +15,7 @@ export const createPrivateProjectChecker = ( }); }; -export const createFakeprivateProjectChecker = - (): FakeprivateProjectChecker => { - return new FakeprivateProjectChecker(); +export const createFakePrivateProjectChecker = + (): FakePrivateProjectChecker => { + return new FakePrivateProjectChecker(); }; diff --git a/src/lib/features/private-project/fakePrivateProjectChecker.ts b/src/lib/features/private-project/fakePrivateProjectChecker.ts index 4c002e66e1..2dba73db65 100644 --- a/src/lib/features/private-project/fakePrivateProjectChecker.ts +++ b/src/lib/features/private-project/fakePrivateProjectChecker.ts @@ -1,8 +1,14 @@ import { IPrivateProjectChecker } from './privateProjectCheckerType'; +import { Promise } from 'ts-toolbelt/out/Any/Promise'; -export class FakeprivateProjectChecker implements IPrivateProjectChecker { +export class FakePrivateProjectChecker implements IPrivateProjectChecker { // eslint-disable-next-line @typescript-eslint/no-unused-vars async getUserAccessibleProjects(userId: number): Promise { throw new Error('Method not implemented.'); } + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + hasAccessToProject(userId: number, projectId: string): Promise { + throw new Error('Method not implemented.'); + } } diff --git a/src/lib/features/private-project/privateProjectChecker.ts b/src/lib/features/private-project/privateProjectChecker.ts index 837127edfa..daa30a1a51 100644 --- a/src/lib/features/private-project/privateProjectChecker.ts +++ b/src/lib/features/private-project/privateProjectChecker.ts @@ -14,4 +14,13 @@ export class PrivateProjectChecker implements IPrivateProjectChecker { async getUserAccessibleProjects(userId: number): Promise { return this.privateProjectStore.getUserAccessibleProjects(userId); } + + async hasAccessToProject( + userId: number, + projectId: string, + ): Promise { + return (await this.getUserAccessibleProjects(userId)).includes( + projectId, + ); + } } diff --git a/src/lib/features/private-project/privateProjectCheckerType.ts b/src/lib/features/private-project/privateProjectCheckerType.ts index e4cfb53a9c..c8b537a906 100644 --- a/src/lib/features/private-project/privateProjectCheckerType.ts +++ b/src/lib/features/private-project/privateProjectCheckerType.ts @@ -1,3 +1,4 @@ export interface IPrivateProjectChecker { getUserAccessibleProjects(userId: number): Promise; + hasAccessToProject(userId: number, projectId: string): Promise; } diff --git a/src/lib/features/private-project/privateProjectMiddleware.ts b/src/lib/features/private-project/privateProjectMiddleware.ts index 9af2a28f4b..ad99f704d7 100644 --- a/src/lib/features/private-project/privateProjectMiddleware.ts +++ b/src/lib/features/private-project/privateProjectMiddleware.ts @@ -7,7 +7,7 @@ const privateProjectMiddleware = ( getLogger, flagResolver, }: Pick, - { projectService, accessService }: IUnleashServices, + { accessService, privateProjectChecker }: IUnleashServices, ): any => { const logger = getLogger('/middleware/project-middleware.ts'); logger.debug('Enabling private project middleware'); @@ -27,10 +27,9 @@ const privateProjectMiddleware = ( return true; } const permissions = await accessService.getPermissionsForUser(user); - return ( permissions.map((p) => p.permission).includes('ADMIN') || - projectService.isProjectUser(user.id, projectId) + privateProjectChecker.hasAccessToProject(user.id, projectId) ); }; next(); diff --git a/src/lib/features/private-project/privateProjectStore.ts b/src/lib/features/private-project/privateProjectStore.ts index e1d5bcfc43..6c1efc24b3 100644 --- a/src/lib/features/private-project/privateProjectStore.ts +++ b/src/lib/features/private-project/privateProjectStore.ts @@ -15,27 +15,76 @@ class PrivateProjectStore implements IPrivateProjectStore { destroy(): void {} async getUserAccessibleProjects(userId: number): Promise { - const projects = await this.db - .from((db) => { - db.select('project') - .from('role_user') - .leftJoin('roles', 'role_user.role_id', 'roles.id') - .where('user_id', userId) - .union((queryBuilder) => { - queryBuilder - .select('project') - .from('group_role') - .leftJoin( - 'group_user', - 'group_user.group_id', - 'group_role.group_id', - ) - .where('user_id', userId); - }) - .as('query'); + const isNotViewer = await this.db('role_user') + .join('roles', 'role_user.role_id', 'roles.id') + .where('role_user.user_id', userId) + .andWhere((db) => { + db.whereNot({ + 'roles.name': 'Viewer', + 'roles.type': 'root', + }); }) - .pluck('project'); - return projects; + .count('*') + .first(); + + if (isNotViewer && isNotViewer.count > 0) { + const allProjects = await this.db('projects').pluck('id'); + return allProjects; + } + + const accessibleProjects = await this.db + .from((db) => { + db.distinct('accessible_projects.project_id') + .select('projects.id as project_id') + .from('projects') + .leftJoin( + 'project_settings', + 'projects.id', + 'project_settings.project', + ) + .where('project_settings.project_mode', '!=', 'private') + .unionAll((queryBuilder) => { + queryBuilder + .select('projects.id as project_id') + .from('projects') + .join( + 'project_settings', + 'projects.id', + 'project_settings.project', + ) + .where( + 'project_settings.project_mode', + '=', + 'private', + ) + .whereIn('projects.id', (whereBuilder) => { + whereBuilder + .select('role_user.project') + .from('role_user') + .leftJoin( + 'roles', + 'role_user.role_id', + 'roles.id', + ) + .where('role_user.user_id', userId); + }) + .orWhereIn('projects.id', (whereBuilder) => { + whereBuilder + .select('group_role.project') + .from('group_role') + .leftJoin( + 'group_user', + 'group_user.group_id', + 'group_role.group_id', + ) + .where('group_user.user_id', userId); + }); + }) + .as('accessible_projects'); + }) + .select('*'); + + return accessibleProjects; } } diff --git a/src/lib/features/project/createProjectService.ts b/src/lib/features/project/createProjectService.ts index ba641fd04a..2121e4530a 100644 --- a/src/lib/features/project/createProjectService.ts +++ b/src/lib/features/project/createProjectService.ts @@ -15,7 +15,6 @@ import ProjectStore from '../../db/project-store'; import FeatureToggleStore from '../../db/feature-toggle-store'; import FeatureTypeStore from '../../db/feature-type-store'; import { FeatureEnvironmentStore } from '../../db/feature-environment-store'; -import FeatureTagStore from '../../db/feature-tag-store'; import ProjectStatsStore from '../../db/project-stats-store'; import { createAccessService, @@ -32,11 +31,14 @@ import FakeFeatureToggleStore from '../../../test/fixtures/fake-feature-toggle-s 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 FakeFeatureTagStore from '../../../test/fixtures/fake-feature-tag-store'; import FakeProjectStatsStore from '../../../test/fixtures/fake-project-stats-store'; import FakeFavoriteFeaturesStore from '../../../test/fixtures/fake-favorite-features-store'; import FakeFavoriteProjectsStore from '../../../test/fixtures/fake-favorite-projects-store'; import { FakeAccountStore } from '../../../test/fixtures/fake-account-store'; +import { + createFakePrivateProjectChecker, + createPrivateProjectChecker, +} from '../private-project/createPrivateProjectChecker'; export const createProjectService = ( db: Db, @@ -60,7 +62,6 @@ export const createProjectService = ( eventBus, getLogger, ); - const featureTagStore = new FeatureTagStore(db, eventBus, getLogger); const projectStatsStore = new ProjectStatsStore(db, eventBus, getLogger); const accessService: AccessService = createAccessService(db, config); const featureToggleService = createFeatureToggleService(db, config); @@ -87,6 +88,8 @@ export const createProjectService = ( { getLogger }, ); + const privateProjectChecker = createPrivateProjectChecker(db, config); + return new ProjectService( { projectStore, @@ -95,7 +98,6 @@ export const createProjectService = ( featureTypeStore, environmentStore, featureEnvironmentStore, - featureTagStore, accountStore, projectStatsStore, }, @@ -104,6 +106,7 @@ export const createProjectService = ( featureToggleService, groupService, favoriteService, + privateProjectChecker, ); }; @@ -119,7 +122,6 @@ export const createFakeProjectService = ( const accountStore = new FakeAccountStore(); const environmentStore = new FakeEnvironmentStore(); const featureEnvironmentStore = new FakeFeatureEnvironmentStore(); - const featureTagStore = new FakeFeatureTagStore(); const projectStatsStore = new FakeProjectStatsStore(); const accessService = createFakeAccessService(config); const featureToggleService = createFakeFeatureToggleService(config); @@ -138,6 +140,8 @@ export const createFakeProjectService = ( { getLogger }, ); + const privateProjectChecker = createFakePrivateProjectChecker(); + return new ProjectService( { projectStore, @@ -146,7 +150,6 @@ export const createFakeProjectService = ( featureTypeStore, environmentStore, featureEnvironmentStore, - featureTagStore, accountStore, projectStatsStore, }, @@ -155,5 +158,6 @@ export const createFakeProjectService = ( featureToggleService, groupService, favoriteService, + privateProjectChecker, ); }; diff --git a/src/lib/routes/admin-api/archive.ts b/src/lib/routes/admin-api/archive.ts index a886a73ab6..9ca110160d 100644 --- a/src/lib/routes/admin-api/archive.ts +++ b/src/lib/routes/admin-api/archive.ts @@ -122,11 +122,13 @@ export default class ArchiveController extends Controller { } async getArchivedFeatures( - req: Request, + req: IAuthRequest, res: Response, ): Promise { + const { user } = req; const features = await this.featureService.getMetadataForAllFeatures( true, + user.id, ); this.openApiService.respondWithValidation( 200, diff --git a/src/lib/routes/controller.ts b/src/lib/routes/controller.ts index 413756991d..db0c8e56df 100644 --- a/src/lib/routes/controller.ts +++ b/src/lib/routes/controller.ts @@ -61,7 +61,6 @@ const checkPrivateProjectPermissions = () => async (req, res, next) => { ) { return next(); } - return res.status(404).end(); }; diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index b5224cf010..b5d37c344f 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -1854,8 +1854,17 @@ class FeatureToggleService { async getMetadataForAllFeatures( archived: boolean, + userId: number, ): Promise { - return this.featureToggleStore.getAll({ archived }); + const features = await this.featureToggleStore.getAll({ archived }); + if (this.flagResolver.isEnabled('privateProjects')) { + const projects = + await this.privateProjectChecker.getUserAccessibleProjects( + userId, + ); + return features.filter((f) => projects.includes(f.project)); + } + return features; } async getMetadataForAllFeaturesByProjectId( diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index c3a2fe018a..3205009c8c 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -61,7 +61,7 @@ import { createFeatureToggleService } from '../features'; import EventAnnouncerService from './event-announcer-service'; import { createGroupService } from '../features/group/createGroupService'; import { - createFakeprivateProjectChecker, + createFakePrivateProjectChecker, createPrivateProjectChecker, } from '../features/private-project/createPrivateProjectChecker'; @@ -190,7 +190,7 @@ export const createServices = ( ); const privateProjectChecker = db ? createPrivateProjectChecker(db, config) - : createFakeprivateProjectChecker(); + : createFakePrivateProjectChecker(); const featureToggleServiceV2 = new FeatureToggleService( stores, config, @@ -209,6 +209,7 @@ export const createServices = ( featureToggleServiceV2, groupService, favoritesService, + privateProjectChecker, ); const projectHealthService = new ProjectHealthService( stores, @@ -323,6 +324,7 @@ export const createServices = ( configurationRevisionService, transactionalFeatureToggleService, transactionalGroupService, + privateProjectChecker, }; }; diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index 45779aac81..41cf1334a3 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -48,7 +48,6 @@ import { } from '../types/stores/access-store'; import FeatureToggleService from './feature-toggle-service'; import IncompatibleProjectError from '../error/incompatible-project-error'; -import { IFeatureTagStore } from 'lib/types/stores/feature-tag-store'; import ProjectWithoutOwnerError from '../error/project-without-owner-error'; import { arraysHaveSameItems } from '../util'; import { GroupService } from './group-service'; @@ -60,6 +59,7 @@ import { uniqueByKey } from '../util/unique'; import { BadDataError, PermissionError } from '../error'; import { ProjectDoraMetricsSchema } from 'lib/openapi'; import { checkFeatureNamingData } from '../features/feature-naming-pattern/feature-naming-validation'; +import { IPrivateProjectChecker } from '../features/private-project/privateProjectCheckerType'; const getCreatedBy = (user: IUser) => user.email || user.username || 'unknown'; @@ -103,7 +103,7 @@ export default class ProjectService { private featureToggleService: FeatureToggleService; - private tagStore: IFeatureTagStore; + private privateProjectChecker: IPrivateProjectChecker; private accountStore: IAccountStore; @@ -121,7 +121,6 @@ export default class ProjectService { featureTypeStore, environmentStore, featureEnvironmentStore, - featureTagStore, accountStore, projectStatsStore, }: Pick< @@ -132,7 +131,6 @@ export default class ProjectService { | 'featureTypeStore' | 'environmentStore' | 'featureEnvironmentStore' - | 'featureTagStore' | 'accountStore' | 'projectStatsStore' >, @@ -141,6 +139,7 @@ export default class ProjectService { featureToggleService: FeatureToggleService, groupService: GroupService, favoriteService: FavoritesService, + privateProjectChecker: IPrivateProjectChecker, ) { this.store = projectStore; this.environmentStore = environmentStore; @@ -151,7 +150,7 @@ export default class ProjectService { this.featureTypeStore = featureTypeStore; this.featureToggleService = featureToggleService; this.favoritesService = favoriteService; - this.tagStore = featureTagStore; + this.privateProjectChecker = privateProjectChecker; this.accountStore = accountStore; this.groupService = groupService; this.projectStatsStore = projectStatsStore; @@ -163,7 +162,17 @@ export default class ProjectService { query?: IProjectQuery, userId?: number, ): Promise { - return this.store.getProjectsWithCounts(query, userId); + const projects = await this.store.getProjectsWithCounts(query, userId); + if (this.flagResolver.isEnabled('privateProjects') && userId) { + const accessibleProjects = + await this.privateProjectChecker.getUserAccessibleProjects( + userId, + ); + return projects.filter((project) => + accessibleProjects.includes(project.id), + ); + } + return projects; } async getProject(id: string): Promise { diff --git a/src/lib/types/services.ts b/src/lib/types/services.ts index 94503ce1d4..047c1d9938 100644 --- a/src/lib/types/services.ts +++ b/src/lib/types/services.ts @@ -43,6 +43,7 @@ import ExportImportService from '../features/export-import-toggles/export-import import { ISegmentService } from '../segments/segment-service-interface'; import ConfigurationRevisionService from '../features/feature-toggle/configuration-revision-service'; import EventAnnouncerService from 'lib/services/event-announcer-service'; +import { IPrivateProjectChecker } from '../features/private-project/privateProjectCheckerType'; export interface IUnleashServices { accessService: AccessService; @@ -97,4 +98,5 @@ export interface IUnleashServices { db: Knex.Transaction, ) => FeatureToggleService; transactionalGroupService: (db: Knex.Transaction) => GroupService; + privateProjectChecker: IPrivateProjectChecker; } diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index 0696e97e03..93beeac2b0 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -265,6 +265,7 @@ beforeAll(async () => { featureToggleService, groupService, favoritesService, + privateProjectChecker, ); editorUser = await createUser(editorRole.id); diff --git a/src/test/e2e/services/api-token-service.e2e.test.ts b/src/test/e2e/services/api-token-service.e2e.test.ts index 4362d325fd..4f535c21c5 100644 --- a/src/test/e2e/services/api-token-service.e2e.test.ts +++ b/src/test/e2e/services/api-token-service.e2e.test.ts @@ -63,6 +63,7 @@ beforeAll(async () => { featureToggleService, groupService, favoritesService, + privateProjectChecker, ); await projectService.createProject(project, user); diff --git a/src/test/e2e/services/project-health-service.e2e.test.ts b/src/test/e2e/services/project-health-service.e2e.test.ts index 0aaeff0424..ddc21e7ccd 100644 --- a/src/test/e2e/services/project-health-service.e2e.test.ts +++ b/src/test/e2e/services/project-health-service.e2e.test.ts @@ -58,6 +58,7 @@ beforeAll(async () => { featureToggleService, groupService, favoritesService, + privateProjectChecker, ); projectHealthService = new ProjectHealthService( stores, diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index 5bbddeb3b5..2c33e40af8 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -80,6 +80,7 @@ beforeAll(async () => { featureToggleService, groupService, favoritesService, + privateProjectChecker, ); });