From ea5e8e29e68e306f44c4d40016856c9a4da9240c Mon Sep 17 00:00:00 2001 From: Daniel Hougaard Date: Wed, 11 Sep 2024 12:45:14 +0400 Subject: [PATCH] Requested changes --- .../services/audit-log/audit-log-service.ts | 13 ++- .../ee/services/audit-log/audit-log-types.ts | 2 +- backend/src/server/routes/index.ts | 1 + .../server/routes/v1/organization-router.ts | 3 +- backend/src/server/routes/v1/user-router.ts | 32 ++----- .../group-project/group-project-dal.ts | 96 +------------------ backend/src/services/user/user-service.ts | 24 ++++- backend/src/services/user/user-types.ts | 5 + frontend/src/hooks/api/groups/types.ts | 20 ---- .../UserProjectsSection/UserGroupsRow.tsx | 38 ++++---- .../UserProjectsSection/UserGroupsSection.tsx | 2 +- 11 files changed, 74 insertions(+), 162 deletions(-) diff --git a/backend/src/ee/services/audit-log/audit-log-service.ts b/backend/src/ee/services/audit-log/audit-log-service.ts index 2fc7e0de3..b730f5ee7 100644 --- a/backend/src/ee/services/audit-log/audit-log-service.ts +++ b/backend/src/ee/services/audit-log/audit-log-service.ts @@ -3,6 +3,7 @@ import { ForbiddenError } from "@casl/ability"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError } from "@app/lib/errors"; +import { OrgPermissionActions, OrgPermissionSubjects } from "../permission/org-permission"; import { TPermissionServiceFactory } from "../permission/permission-service"; import { ProjectPermissionActions, ProjectPermissionSub } from "../permission/project-permission"; import { TAuditLogDALFactory } from "./audit-log-dal"; @@ -11,7 +12,7 @@ import { EventType, TCreateAuditLogDTO, TListProjectAuditLogDTO } from "./audit- type TAuditLogServiceFactoryDep = { auditLogDAL: TAuditLogDALFactory; - permissionService: Pick; + permissionService: Pick; auditLogQueue: TAuditLogQueueServiceFactory; }; @@ -45,6 +46,16 @@ export const auditLogServiceFactory = ({ actorOrgId ); ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.AuditLogs); + } else { + const { permission } = await permissionService.getOrgPermission( + actor, + actorId, + actorOrgId, + actorAuthMethod, + actorOrgId + ); + + ForbiddenError.from(permission).throwUnlessCan(OrgPermissionActions.Read, OrgPermissionSubjects.Member); } // If project ID is not provided, then we need to return all the audit logs for the organization itself. diff --git a/backend/src/ee/services/audit-log/audit-log-types.ts b/backend/src/ee/services/audit-log/audit-log-types.ts index 6202d30f2..c9d04602d 100644 --- a/backend/src/ee/services/audit-log/audit-log-types.ts +++ b/backend/src/ee/services/audit-log/audit-log-types.ts @@ -6,7 +6,7 @@ import { PkiItemType } from "@app/services/pki-collection/pki-collection-types"; export type TListProjectAuditLogDTO = { auditLogActor?: string; - projectId: string | null; + projectId?: string; eventType?: string; startDate?: string; endDate?: string; diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index 792bc8787..70e7639b2 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -464,6 +464,7 @@ export const registerRoutes = async ( userAliasDAL, orgMembershipDAL, tokenService, + permissionService, groupProjectDAL, smtpService, projectMembershipDAL diff --git a/backend/src/server/routes/v1/organization-router.ts b/backend/src/server/routes/v1/organization-router.ts index 2e49d5b8e..68a1dba45 100644 --- a/backend/src/server/routes/v1/organization-router.ts +++ b/backend/src/server/routes/v1/organization-router.ts @@ -121,8 +121,7 @@ export const registerOrgRouter = async (server: FastifyZodProvider) => { endDate: req.query.endDate, startDate: req.query.startDate || getLastMidnightDateISO(), auditLogActor: req.query.actor, - actor: req.permission.type, - projectId: null + actor: req.permission.type }); return { auditLogs }; } diff --git a/backend/src/server/routes/v1/user-router.ts b/backend/src/server/routes/v1/user-router.ts index 944925a95..4e4583196 100644 --- a/backend/src/server/routes/v1/user-router.ts +++ b/backend/src/server/routes/v1/user-router.ts @@ -1,6 +1,6 @@ import { z } from "zod"; -import { ProjectsSchema, UserEncryptionKeysSchema, UsersSchema } from "@app/db/schemas"; +import { UserEncryptionKeysSchema, UsersSchema } from "@app/db/schemas"; import { getConfig } from "@app/lib/config/env"; import { logger } from "@app/lib/logger"; import { authRateLimit, readLimit, writeLimit } from "@app/server/config/rateLimiter"; @@ -151,34 +151,20 @@ export const registerUserRouter = async (server: FastifyZodProvider) => { id: z.string(), name: z.string(), slug: z.string(), - orgId: z.string(), - projectMemberships: z.array( - z.object({ - id: z.string(), - project: ProjectsSchema.pick({ id: true, name: true, slug: true }), - roles: z.array( - z.object({ - id: z.string(), - role: z.string(), - customRoleId: z.string().nullable(), - customRoleName: z.string().nullable(), - customRoleSlug: z.string().nullable(), - temporaryRange: z.string().nullable(), - temporaryMode: z.string().nullable(), - temporaryAccessEndTime: z.string().nullable(), - temporaryAccessStartTime: z.string().nullable(), - isTemporary: z.boolean() - }) - ) - }) - ) + orgId: z.string() }) .array() } }, onRequest: verifyAuth([AuthMode.JWT]), handler: async (req) => { - const groupMemberships = await server.services.user.listUserGroups(req.params.username, req.permission.orgId); + const groupMemberships = await server.services.user.listUserGroups({ + username: req.params.username, + actorOrgId: req.permission.orgId, + actorId: req.permission.id, + actorAuthMethod: req.permission.authMethod, + actor: req.permission.type + }); return groupMemberships; } diff --git a/backend/src/services/group-project/group-project-dal.ts b/backend/src/services/group-project/group-project-dal.ts index b17aaee5b..c74a6b1b2 100644 --- a/backend/src/services/group-project/group-project-dal.ts +++ b/backend/src/services/group-project/group-project-dal.ts @@ -106,100 +106,14 @@ export const groupProjectDALFactory = (db: TDbClient) => { db.raw("?", [orgId]) ); }) - .leftJoin( - TableName.GroupProjectMembership, - `${TableName.GroupProjectMembership}.groupId`, - `${TableName.Groups}.id` - ) - .leftJoin( - TableName.GroupProjectMembershipRole, - `${TableName.GroupProjectMembershipRole}.projectMembershipId`, - `${TableName.GroupProjectMembership}.id` - ) - .leftJoin( - TableName.ProjectRoles, - `${TableName.GroupProjectMembershipRole}.customRoleId`, - `${TableName.ProjectRoles}.id` - ) - .leftJoin(TableName.Project, `${TableName.GroupProjectMembership}.projectId`, `${TableName.Project}.id`) .select( - db.ref("id").withSchema(TableName.Groups).as("groupId"), - db.ref("name").withSchema(TableName.Groups).as("groupName"), - db.ref("slug").withSchema(TableName.Groups).as("groupSlug"), - db.ref("orgId").withSchema(TableName.Groups), - db.ref("id").withSchema(TableName.GroupProjectMembership).as("projectMembershipId"), - db.ref("id").withSchema(TableName.Project).as("projectId"), - db.ref("name").withSchema(TableName.Project).as("projectName"), - db.ref("slug").withSchema(TableName.Project).as("projectSlug"), - db.ref("role").withSchema(TableName.GroupProjectMembershipRole), - db.ref("id").withSchema(TableName.GroupProjectMembershipRole).as("membershipRoleId"), - db.ref("customRoleId").withSchema(TableName.GroupProjectMembershipRole), - db.ref("name").withSchema(TableName.ProjectRoles).as("customRoleName"), - db.ref("slug").withSchema(TableName.ProjectRoles).as("customRoleSlug"), - db.ref("temporaryMode").withSchema(TableName.GroupProjectMembershipRole), - db.ref("isTemporary").withSchema(TableName.GroupProjectMembershipRole), - db.ref("temporaryRange").withSchema(TableName.GroupProjectMembershipRole), - db.ref("temporaryAccessStartTime").withSchema(TableName.GroupProjectMembershipRole), - db.ref("temporaryAccessEndTime").withSchema(TableName.GroupProjectMembershipRole) + db.ref("id").withSchema(TableName.Groups), + db.ref("name").withSchema(TableName.Groups), + db.ref("slug").withSchema(TableName.Groups), + db.ref("orgId").withSchema(TableName.Groups) ); - const groupsWithProjects = sqlNestRelationships({ - data: docs, - parentMapper: ({ groupId, groupName, groupSlug, orgId: organizationId }) => ({ - id: groupId, - name: groupName, - slug: groupSlug, - orgId: organizationId, - projectMemberships: [] - }), - key: "groupId", - childrenMapper: [ - { - label: "projectMemberships" as const, - key: "projectMembershipId", - mapper: ({ projectId, projectName, projectSlug, projectMembershipId }) => ({ - id: projectMembershipId, - project: { - id: projectId, - name: projectName, - slug: projectSlug - }, - roles: [] - }), - childrenMapper: [ - { - label: "roles" as const, - key: "membershipRoleId", - mapper: ({ - role, - customRoleId, - customRoleName, - customRoleSlug, - membershipRoleId, - temporaryRange, - temporaryMode, - temporaryAccessEndTime, - temporaryAccessStartTime, - isTemporary - }) => ({ - id: membershipRoleId, - role, - customRoleId, - customRoleName, - customRoleSlug, - temporaryRange, - temporaryMode, - temporaryAccessEndTime, - temporaryAccessStartTime, - isTemporary - }) - } - ] - } - ] - }); - - return groupsWithProjects; + return docs; } catch (error) { throw new DatabaseError({ error, name: "FindByUserId" }); } diff --git a/backend/src/services/user/user-service.ts b/backend/src/services/user/user-service.ts index 97bbaecf0..8d7d1ffbe 100644 --- a/backend/src/services/user/user-service.ts +++ b/backend/src/services/user/user-service.ts @@ -1,4 +1,8 @@ +import { ForbiddenError } from "@casl/ability"; + import { SecretKeyEncoding } from "@app/db/schemas"; +import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; +import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { infisicalSymmetricDecrypt } from "@app/lib/crypto/encryption"; import { BadRequestError } from "@app/lib/errors"; import { TAuthTokenServiceFactory } from "@app/services/auth-token/auth-token-service"; @@ -11,6 +15,7 @@ import { AuthMethod } from "../auth/auth-type"; import { TGroupProjectDALFactory } from "../group-project/group-project-dal"; import { TProjectMembershipDALFactory } from "../project-membership/project-membership-dal"; import { TUserDALFactory } from "./user-dal"; +import { TListUserGroupsDTO } from "./user-types"; type TUserServiceFactoryDep = { userDAL: Pick< @@ -33,6 +38,7 @@ type TUserServiceFactoryDep = { tokenService: Pick; projectMembershipDAL: Pick; smtpService: Pick; + permissionService: TPermissionServiceFactory; }; export type TUserServiceFactory = ReturnType; @@ -44,7 +50,8 @@ export const userServiceFactory = ({ projectMembershipDAL, groupProjectDAL, tokenService, - smtpService + smtpService, + permissionService }: TUserServiceFactoryDep) => { const sendEmailVerificationCode = async (username: string) => { const user = await userDAL.findOne({ username }); @@ -298,13 +305,24 @@ export const userServiceFactory = ({ return updatedOrgMembership.projectFavorites; }; - const listUserGroups = async (username: string, orgId: string) => { + const listUserGroups = async ({ username, actorOrgId, actor, actorId, actorAuthMethod }: TListUserGroupsDTO) => { const user = await userDAL.findOne({ username }); - const memberships = await groupProjectDAL.findByUserId(user.id, orgId); + // This makes it so the user can always read information about themselves, but no one else if they don't have the Members Read permission. + if (user.id !== actorId) { + const { permission } = await permissionService.getOrgPermission( + actor, + actorId, + actorOrgId, + actorAuthMethod, + actorOrgId + ); + ForbiddenError.from(permission).throwUnlessCan(OrgPermissionActions.Read, OrgPermissionSubjects.Member); + } + const memberships = await groupProjectDAL.findByUserId(user.id, actorOrgId); return memberships; }; diff --git a/backend/src/services/user/user-types.ts b/backend/src/services/user/user-types.ts index e69de29bb..e91b23910 100644 --- a/backend/src/services/user/user-types.ts +++ b/backend/src/services/user/user-types.ts @@ -0,0 +1,5 @@ +import { TOrgPermission } from "@app/lib/types"; + +export type TListUserGroupsDTO = { + username: string; +} & Omit; diff --git a/frontend/src/hooks/api/groups/types.ts b/frontend/src/hooks/api/groups/types.ts index 126ca56d4..3f69b9a0e 100644 --- a/frontend/src/hooks/api/groups/types.ts +++ b/frontend/src/hooks/api/groups/types.ts @@ -40,24 +40,4 @@ export type TGroupWithProjectMemberships = { name: string; slug: string; orgId: string; - projectMemberships: { - id: string; - project: { - id: string; - name: string; - slug: string; - }; - roles: { - id: string; - role: string; - customRoleId: string | null; - customRoleName: string | null; - customRoleSlug: string | null; - temporaryRange: string | null; - temporaryMode: string | null; - temporaryAccessEndTime: string | null; - temporaryAccessStartTime: string | null; - isTemporary: boolean; - }[]; - }[]; }; diff --git a/frontend/src/views/Org/UserPage/components/UserProjectsSection/UserGroupsRow.tsx b/frontend/src/views/Org/UserPage/components/UserProjectsSection/UserGroupsRow.tsx index 712f8f0be..620fe5b7e 100644 --- a/frontend/src/views/Org/UserPage/components/UserProjectsSection/UserGroupsRow.tsx +++ b/frontend/src/views/Org/UserPage/components/UserProjectsSection/UserGroupsRow.tsx @@ -20,26 +20,24 @@ export const UserGroupsRow = ({ group, handlePopUpOpen }: Props) => { > {group.name} - {true && ( -
- - { - e.stopPropagation(); - handlePopUpOpen("removeUserFromGroup", { - groupSlug: group.slug - }); - }} - > - - - -
- )} +
+ + { + e.stopPropagation(); + handlePopUpOpen("removeUserFromGroup", { + groupSlug: group.slug + }); + }} + > + + + +
diff --git a/frontend/src/views/Org/UserPage/components/UserProjectsSection/UserGroupsSection.tsx b/frontend/src/views/Org/UserPage/components/UserProjectsSection/UserGroupsSection.tsx index 41340aec4..2967348a2 100644 --- a/frontend/src/views/Org/UserPage/components/UserProjectsSection/UserGroupsSection.tsx +++ b/frontend/src/views/Org/UserPage/components/UserProjectsSection/UserGroupsSection.tsx @@ -52,7 +52,7 @@ export const UserGroupsSection = ({ orgMembership }: Props) => { handlePopUpToggle("removeUserFromGroup", isOpen)} deleteKey="confirm" onDeleteApproved={() => {