address reviews

backend/src/ee/services/pam-account/pam-account-service.ts

@@ -26,12 +26,10 @@ import {
 } from "@app/lib/errors";
 import { logger } from "@app/lib/logger";
 import { OrgServiceActor } from "@app/lib/types";
-import {
+import { TApprovalPolicyDALFactory } from "@app/services/approval-policy/approval-policy-dal";
-  TApprovalPolicyDALFactory,
-  TApprovalRequestGrantsDALFactory
-} from "@app/services/approval-policy/approval-policy-dal";
 import { ApprovalPolicyType } from "@app/services/approval-policy/approval-policy-enums";
 import { APPROVAL_POLICY_FACTORY_MAP } from "@app/services/approval-policy/approval-policy-factory";
+import { TApprovalRequestGrantsDALFactory } from "@app/services/approval-policy/approval-request-dal";
 import { ActorType } from "@app/services/auth/auth-type";
 import { TKmsServiceFactory } from "@app/services/kms/kms-service";
 import { KmsDataKey } from "@app/services/kms/kms-types";
@@ -579,9 +577,8 @@ export const pamAccountServiceFactory = ({
 
     const canAccess = await fac.canAccess(approvalRequestGrantsDAL, resource.projectId, actor.id, inputs);
 
-    if (canAccess) {
+    // Grant does not exist, check policy and fallback to permission check
-      // Grant exists, allow access without checking permission
+    if (!canAccess) {
-    } else {
       const policy = await fac.matchPolicy(approvalPolicyDAL, resource.projectId, inputs);
 
       if (policy) {

backend/src/server/routes/index.ts

@@ -162,14 +162,16 @@ import { appConnectionServiceFactory } from "@app/services/app-connection/app-co
 import {
   approvalPolicyDALFactory,
   approvalPolicyStepApproversDALFactory,
-  approvalPolicyStepsDALFactory,
+  approvalPolicyStepsDALFactory
+} from "@app/services/approval-policy/approval-policy-dal";
+import { approvalPolicyServiceFactory } from "@app/services/approval-policy/approval-policy-service";
+import {
   approvalRequestApprovalsDALFactory,
   approvalRequestDALFactory,
   approvalRequestGrantsDALFactory,
   approvalRequestStepEligibleApproversDALFactory,
   approvalRequestStepsDALFactory
-} from "@app/services/approval-policy/approval-policy-dal";
+} from "@app/services/approval-policy/approval-request-dal";
-import { approvalPolicyServiceFactory } from "@app/services/approval-policy/approval-policy-service";
 import { authDALFactory } from "@app/services/auth/auth-dal";
 import { authLoginServiceFactory } from "@app/services/auth/auth-login-service";
 import { authPaswordServiceFactory } from "@app/services/auth/auth-password-service";

backend/src/services/approval-policy/approval-policy-dal.ts

@@ -1,14 +1,9 @@
 import { TDbClient } from "@app/db";
-import { TableName, TApprovalRequestApprovals } from "@app/db/schemas";
+import { TableName } from "@app/db/schemas";
 import { DatabaseError } from "@app/lib/errors";
 import { ormify } from "@app/lib/knex";
 
-import {
+import { ApprovalPolicyType, ApproverType } from "./approval-policy-enums";
-  ApprovalPolicyType,
-  ApprovalRequestGrantStatus,
-  ApprovalRequestStatus,
-  ApproverType
-} from "./approval-policy-enums";
 import { ApprovalPolicyStep } from "./approval-policy-types";
 
 // Approval Policy
@@ -153,190 +148,3 @@ export const approvalPolicyStepApproversDALFactory = (db: TDbClient) => {
   const orm = ormify(db, TableName.ApprovalPolicyStepApprovers);
   return orm;
 };
-
-// Approval Request
-export type TApprovalRequestDALFactory = ReturnType<typeof approvalRequestDALFactory>;
-export const approvalRequestDALFactory = (db: TDbClient) => {
-  const orm = ormify(db, TableName.ApprovalRequests);
-
-  const findStepsByRequestId = async (requestId: string) => {
-    try {
-      const dbInstance = db.replicaNode();
-      const steps = await dbInstance(TableName.ApprovalRequestSteps).where({ requestId }).orderBy("stepNumber", "asc");
-
-      if (!steps.length) {
-        return [];
-      }
-
-      const stepIds = steps.map((step) => step.id);
-
-      const [approvers, approvals] = await Promise.all([
-        dbInstance(TableName.ApprovalRequestStepEligibleApprovers)
-          .whereIn("stepId", stepIds)
-          .select("stepId", "userId", "groupId"),
-        dbInstance(TableName.ApprovalRequestApprovals).whereIn("stepId", stepIds)
-      ]);
-
-      const approversByStepId = approvers.reduce<Record<string, { type: ApproverType; id: string }[]>>(
-        (acc, approver) => {
-          const stepApprovers = acc[approver.stepId] || [];
-          stepApprovers.push({
-            type: approver.userId ? ApproverType.User : ApproverType.Group,
-            id: (approver.userId || approver.groupId) as string
-          });
-          acc[approver.stepId] = stepApprovers;
-          return acc;
-        },
-        {}
-      );
-
-      const approvalsByStepId = approvals.reduce<Record<string, TApprovalRequestApprovals[]>>((acc, approval) => {
-        const stepApprovals = acc[approval.stepId] || [];
-        stepApprovals.push(approval);
-        acc[approval.stepId] = stepApprovals;
-        return acc;
-      }, {});
-
-      return steps.map((step) => {
-        return {
-          ...step,
-          approvers: approversByStepId[step.id] || [],
-          approvals: approvalsByStepId[step.id] || []
-        };
-      });
-    } catch (error) {
-      throw new DatabaseError({ error, name: "Find approval request steps" });
-    }
-  };
-
-  const findByProjectId = async (policyType: ApprovalPolicyType, projectId: string) => {
-    try {
-      const dbInstance = db.replicaNode();
-      const requests = await dbInstance(TableName.ApprovalRequests).where({ type: policyType, projectId });
-
-      if (!requests.length) {
-        return [];
-      }
-
-      const requestIds = requests.map((req) => req.id);
-
-      const steps = await dbInstance(TableName.ApprovalRequestSteps)
-        .whereIn("requestId", requestIds)
-        .orderBy("stepNumber", "asc");
-
-      const stepsByRequestId: Record<string, ApprovalPolicyStep[]> = {};
-
-      if (steps.length) {
-        const stepIds = steps.map((step) => step.id);
-
-        const [approvers, approvals] = await Promise.all([
-          dbInstance(TableName.ApprovalRequestStepEligibleApprovers)
-            .whereIn("stepId", stepIds)
-            .select("stepId", "userId", "groupId"),
-          dbInstance(TableName.ApprovalRequestApprovals).whereIn("stepId", stepIds)
-        ]);
-
-        const approversByStepId = approvers.reduce<Record<string, { type: ApproverType; id: string }[]>>(
-          (acc, approver) => {
-            const stepApprovers = acc[approver.stepId] || [];
-            stepApprovers.push({
-              type: approver.userId ? ApproverType.User : ApproverType.Group,
-              id: (approver.userId || approver.groupId) as string
-            });
-            acc[approver.stepId] = stepApprovers;
-            return acc;
-          },
-          {}
-        );
-
-        const approvalsByStepId = approvals.reduce<Record<string, TApprovalRequestApprovals[]>>((acc, approval) => {
-          const stepApprovals = acc[approval.stepId] || [];
-          stepApprovals.push(approval);
-          acc[approval.stepId] = stepApprovals;
-          return acc;
-        }, {});
-
-        steps.forEach((step) => {
-          const formattedStep = {
-            ...step,
-            approvers: approversByStepId[step.id] || [],
-            approvals: approvalsByStepId[step.id] || []
-          };
-
-          if (!stepsByRequestId[step.requestId]) {
-            stepsByRequestId[step.requestId] = [];
-          }
-          stepsByRequestId[step.requestId].push(formattedStep);
-        });
-      }
-
-      return requests.map((req) => ({
-        ...req,
-        steps: stepsByRequestId[req.id] || []
-      }));
-    } catch (error) {
-      throw new DatabaseError({ error, name: "Find approval requests by project id" });
-    }
-  };
-
-  const markExpiredRequests = async () => {
-    try {
-      const result = await db(TableName.ApprovalRequests)
-        .where("status", ApprovalRequestStatus.Pending)
-        .whereNotNull("expiresAt")
-        .where("expiresAt", "<", new Date())
-        .update({ status: ApprovalRequestStatus.Expired });
-
-      return result;
-    } catch (error) {
-      throw new DatabaseError({ error, name: "Mark expired approval requests" });
-    }
-  };
-
-  return { ...orm, findStepsByRequestId, findByProjectId, markExpiredRequests };
-};
-
-// Approval Request Steps
-export type TApprovalRequestStepsDALFactory = ReturnType<typeof approvalRequestStepsDALFactory>;
-export const approvalRequestStepsDALFactory = (db: TDbClient) => {
-  const orm = ormify(db, TableName.ApprovalRequestSteps);
-  return orm;
-};
-
-// Approval Request Step Eligible Approvers
-export type TApprovalRequestStepEligibleApproversDALFactory = ReturnType<
-  typeof approvalRequestStepEligibleApproversDALFactory
->;
-export const approvalRequestStepEligibleApproversDALFactory = (db: TDbClient) => {
-  const orm = ormify(db, TableName.ApprovalRequestStepEligibleApprovers);
-  return orm;
-};
-
-// Approval Request Grants
-export type TApprovalRequestGrantsDALFactory = ReturnType<typeof approvalRequestGrantsDALFactory>;
-export const approvalRequestGrantsDALFactory = (db: TDbClient) => {
-  const orm = ormify(db, TableName.ApprovalRequestGrants);
-
-  const markExpiredGrants = async () => {
-    try {
-      const result = await db(TableName.ApprovalRequestGrants)
-        .where("status", ApprovalRequestGrantStatus.Active)
-        .whereNotNull("expiresAt")
-        .where("expiresAt", "<", new Date())
-        .update({ status: ApprovalRequestGrantStatus.Expired });
-
-      return result;
-    } catch (error) {
-      throw new DatabaseError({ error, name: "Mark expired approval grants" });
-    }
-  };
-
-  return { ...orm, markExpiredGrants };
-};
-
-// Approval Request Approvals
-export type TApprovalRequestApprovalsDALFactory = ReturnType<typeof approvalRequestApprovalsDALFactory>;
-export const approvalRequestApprovalsDALFactory = (db: TDbClient) => {
-  const orm = ormify(db, TableName.ApprovalRequestApprovals);
-  return orm;
-};

backend/src/services/approval-policy/approval-policy-service.ts

@@ -18,12 +18,7 @@ import { TProjectMembershipDALFactory } from "../project-membership/project-memb
 import {
   TApprovalPolicyDALFactory,
   TApprovalPolicyStepApproversDALFactory,
-  TApprovalPolicyStepsDALFactory,
+  TApprovalPolicyStepsDALFactory
-  TApprovalRequestApprovalsDALFactory,
-  TApprovalRequestDALFactory,
-  TApprovalRequestGrantsDALFactory,
-  TApprovalRequestStepEligibleApproversDALFactory,
-  TApprovalRequestStepsDALFactory
 } from "./approval-policy-dal";
 import {
   ApprovalPolicyType,
@@ -41,6 +36,13 @@ import {
   TCreateRequestDTO,
   TUpdatePolicyDTO
 } from "./approval-policy-types";
+import {
+  TApprovalRequestApprovalsDALFactory,
+  TApprovalRequestDALFactory,
+  TApprovalRequestGrantsDALFactory,
+  TApprovalRequestStepEligibleApproversDALFactory,
+  TApprovalRequestStepsDALFactory
+} from "./approval-request-dal";
 
 type TApprovalPolicyServiceFactoryDep = {
   approvalPolicyDAL: TApprovalPolicyDALFactory;
@@ -394,11 +396,17 @@ export const approvalPolicyServiceFactory = ({
     const policy = await fac.matchPolicy(approvalPolicyDAL, projectId, requestData);
 
     if (!policy) {
-      throw new ForbiddenRequestError({ message: "Policy not found" });
+      throw new ForbiddenRequestError({
+        message: "No policies match the requested resource, you can access it without a request"
+      });
     }
 
-    if (!fac.validateConstraints(policy, requestData)) {
+    const constraintValidation = fac.validateConstraints(policy, requestData);
-      throw new ForbiddenRequestError({ message: "Policy constraints not met" });
+    if (!constraintValidation.valid) {
+      const errorMessage = constraintValidation.errors
+        ? `Policy constraints not met: ${constraintValidation.errors.join("; ")}`
+        : "Policy constraints not met";
+      throw new ForbiddenRequestError({ message: errorMessage });
     }
 
     let expiresAt: Date | undefined;

backend/src/services/approval-policy/approval-policy-types.ts

@@ -1,7 +1,5 @@
-import {
+import { TApprovalPolicyDALFactory } from "@app/services/approval-policy/approval-policy-dal";
-  TApprovalPolicyDALFactory,
+import { TApprovalRequestGrantsDALFactory } from "@app/services/approval-policy/approval-request-dal";
-  TApprovalRequestGrantsDALFactory
-} from "@app/services/approval-policy/approval-policy-dal";
 
 import { ApprovalPolicyType, ApproverType } from "./approval-policy-enums";
 import {
@@ -72,7 +70,7 @@ export type TApprovalRequestFactoryCanAccess<I extends TApprovalPolicyInputs> =
 export type TApprovalRequestFactoryValidateConstraints<P extends TApprovalPolicy, R extends TApprovalRequestData> = (
   policy: P,
   inputs: R
-) => boolean;
+) => { valid: boolean; errors?: string[] };
 export type TApprovalRequestFactoryPostApprovalRoutine = (
   approvalRequestGrantsDAL: TApprovalRequestGrantsDALFactory,
   request: TApprovalRequest

backend/src/services/approval-policy/approval-request-dal.ts

@@ -0,0 +1,199 @@
+import { TDbClient } from "@app/db";
+import { TableName, TApprovalRequestApprovals } from "@app/db/schemas";
+import { DatabaseError } from "@app/lib/errors";
+import { ormify } from "@app/lib/knex";
+
+import {
+  ApprovalPolicyType,
+  ApprovalRequestGrantStatus,
+  ApprovalRequestStatus,
+  ApproverType
+} from "./approval-policy-enums";
+import { ApprovalPolicyStep } from "./approval-policy-types";
+
+// Approval Request
+export type TApprovalRequestDALFactory = ReturnType<typeof approvalRequestDALFactory>;
+export const approvalRequestDALFactory = (db: TDbClient) => {
+  const orm = ormify(db, TableName.ApprovalRequests);
+
+  const findStepsByRequestId = async (requestId: string) => {
+    try {
+      const dbInstance = db.replicaNode();
+      const steps = await dbInstance(TableName.ApprovalRequestSteps).where({ requestId }).orderBy("stepNumber", "asc");
+
+      if (!steps.length) {
+        return [];
+      }
+
+      const stepIds = steps.map((step) => step.id);
+
+      const [approvers, approvals] = await Promise.all([
+        dbInstance(TableName.ApprovalRequestStepEligibleApprovers)
+          .whereIn("stepId", stepIds)
+          .select("stepId", "userId", "groupId"),
+        dbInstance(TableName.ApprovalRequestApprovals).whereIn("stepId", stepIds)
+      ]);
+
+      const approversByStepId = approvers.reduce<Record<string, { type: ApproverType; id: string }[]>>(
+        (acc, approver) => {
+          const stepApprovers = acc[approver.stepId] || [];
+          stepApprovers.push({
+            type: approver.userId ? ApproverType.User : ApproverType.Group,
+            id: (approver.userId || approver.groupId) as string
+          });
+          acc[approver.stepId] = stepApprovers;
+          return acc;
+        },
+        {}
+      );
+
+      const approvalsByStepId = approvals.reduce<Record<string, TApprovalRequestApprovals[]>>((acc, approval) => {
+        const stepApprovals = acc[approval.stepId] || [];
+        stepApprovals.push(approval);
+        acc[approval.stepId] = stepApprovals;
+        return acc;
+      }, {});
+
+      return steps.map((step) => {
+        return {
+          ...step,
+          approvers: approversByStepId[step.id] || [],
+          approvals: approvalsByStepId[step.id] || []
+        };
+      });
+    } catch (error) {
+      throw new DatabaseError({ error, name: "Find approval request steps" });
+    }
+  };
+
+  const findByProjectId = async (policyType: ApprovalPolicyType, projectId: string) => {
+    try {
+      const dbInstance = db.replicaNode();
+      const requests = await dbInstance(TableName.ApprovalRequests).where({ type: policyType, projectId });
+
+      if (!requests.length) {
+        return [];
+      }
+
+      const requestIds = requests.map((req) => req.id);
+
+      const steps = await dbInstance(TableName.ApprovalRequestSteps)
+        .whereIn("requestId", requestIds)
+        .orderBy("stepNumber", "asc");
+
+      const stepsByRequestId: Record<string, ApprovalPolicyStep[]> = {};
+
+      if (steps.length) {
+        const stepIds = steps.map((step) => step.id);
+
+        const [approvers, approvals] = await Promise.all([
+          dbInstance(TableName.ApprovalRequestStepEligibleApprovers)
+            .whereIn("stepId", stepIds)
+            .select("stepId", "userId", "groupId"),
+          dbInstance(TableName.ApprovalRequestApprovals).whereIn("stepId", stepIds)
+        ]);
+
+        const approversByStepId = approvers.reduce<Record<string, { type: ApproverType; id: string }[]>>(
+          (acc, approver) => {
+            const stepApprovers = acc[approver.stepId] || [];
+            stepApprovers.push({
+              type: approver.userId ? ApproverType.User : ApproverType.Group,
+              id: (approver.userId || approver.groupId) as string
+            });
+            acc[approver.stepId] = stepApprovers;
+            return acc;
+          },
+          {}
+        );
+
+        const approvalsByStepId = approvals.reduce<Record<string, TApprovalRequestApprovals[]>>((acc, approval) => {
+          const stepApprovals = acc[approval.stepId] || [];
+          stepApprovals.push(approval);
+          acc[approval.stepId] = stepApprovals;
+          return acc;
+        }, {});
+
+        steps.forEach((step) => {
+          const formattedStep = {
+            ...step,
+            approvers: approversByStepId[step.id] || [],
+            approvals: approvalsByStepId[step.id] || []
+          };
+
+          if (!stepsByRequestId[step.requestId]) {
+            stepsByRequestId[step.requestId] = [];
+          }
+          stepsByRequestId[step.requestId].push(formattedStep);
+        });
+      }
+
+      return requests.map((req) => ({
+        ...req,
+        steps: stepsByRequestId[req.id] || []
+      }));
+    } catch (error) {
+      throw new DatabaseError({ error, name: "Find approval requests by project id" });
+    }
+  };
+
+  const markExpiredRequests = async () => {
+    try {
+      const result = await db(TableName.ApprovalRequests)
+        .where("status", ApprovalRequestStatus.Pending)
+        .whereNotNull("expiresAt")
+        .where("expiresAt", "<", new Date())
+        .update({ status: ApprovalRequestStatus.Expired });
+
+      return result;
+    } catch (error) {
+      throw new DatabaseError({ error, name: "Mark expired approval requests" });
+    }
+  };
+
+  return { ...orm, findStepsByRequestId, findByProjectId, markExpiredRequests };
+};
+
+// Approval Request Steps
+export type TApprovalRequestStepsDALFactory = ReturnType<typeof approvalRequestStepsDALFactory>;
+export const approvalRequestStepsDALFactory = (db: TDbClient) => {
+  const orm = ormify(db, TableName.ApprovalRequestSteps);
+  return orm;
+};
+
+// Approval Request Step Eligible Approvers
+export type TApprovalRequestStepEligibleApproversDALFactory = ReturnType<
+  typeof approvalRequestStepEligibleApproversDALFactory
+>;
+export const approvalRequestStepEligibleApproversDALFactory = (db: TDbClient) => {
+  const orm = ormify(db, TableName.ApprovalRequestStepEligibleApprovers);
+  return orm;
+};
+
+// Approval Request Grants
+export type TApprovalRequestGrantsDALFactory = ReturnType<typeof approvalRequestGrantsDALFactory>;
+export const approvalRequestGrantsDALFactory = (db: TDbClient) => {
+  const orm = ormify(db, TableName.ApprovalRequestGrants);
+
+  const markExpiredGrants = async () => {
+    try {
+      const result = await db(TableName.ApprovalRequestGrants)
+        .where("status", ApprovalRequestGrantStatus.Active)
+        .whereNotNull("expiresAt")
+        .where("expiresAt", "<", new Date())
+        .update({ status: ApprovalRequestGrantStatus.Expired });
+
+      return result;
+    } catch (error) {
+      throw new DatabaseError({ error, name: "Mark expired approval grants" });
+    }
+  };
+
+  return { ...orm, markExpiredGrants };
+};
+
+// Approval Request Approvals
+export type TApprovalRequestApprovalsDALFactory = ReturnType<typeof approvalRequestApprovalsDALFactory>;
+export const approvalRequestApprovalsDALFactory = (db: TDbClient) => {
+  const orm = ormify(db, TableName.ApprovalRequestApprovals);
+  return orm;
+};

backend/src/services/approval-policy/pam-access/pam-access-policy-factory.ts

@@ -79,8 +79,27 @@ export const pamAccessPolicyFactory: TApprovalResourceFactory<
   ) => {
     const reqDuration = ms(inputs.accessDuration);
     const durationConstraint = policy.constraints.constraints.accessDuration;
+    const minDuration = ms(durationConstraint.min);
+    const maxDuration = ms(durationConstraint.max);
 
-    return reqDuration >= ms(durationConstraint.min) && reqDuration <= ms(durationConstraint.max);
+    const errors: string[] = [];
+
+    if (reqDuration < minDuration) {
+      errors.push(
+        `Access duration ${inputs.accessDuration} is below the minimum allowed duration of ${durationConstraint.min}`
+      );
+    }
+
+    if (reqDuration > maxDuration) {
+      errors.push(
+        `Access duration ${inputs.accessDuration} exceeds the maximum allowed duration of ${durationConstraint.max}`
+      );
+    }
+
+    return {
+      valid: errors.length === 0,
+      errors: errors.length > 0 ? errors : undefined
+    };
   };
 
   const postApprovalRoutine: TApprovalRequestFactoryPostApprovalRoutine = async (approvalRequestGrantsDAL, request) => {

backend/src/services/approval-policy/pam-access/pam-access-policy-schemas.ts

@@ -1,3 +1,4 @@
+import picomatch from "picomatch";
 import { z } from "zod";
 
 import { ms } from "@app/lib/ms";
@@ -19,7 +20,20 @@ export const PamAccessPolicyInputsSchema = z.object({
 // Conditions
 export const PamAccessPolicyConditionsSchema = z
   .object({
-    accountPaths: z.string().array() // TODO(andrey): Add path & wildcard validation
+    accountPaths: z
+      .string()
+      .refine(
+        (el) => {
+          try {
+            picomatch.parse([el]);
+            return true;
+          } catch {
+            return false;
+          }
+        },
+        { message: "Invalid glob pattern" }
+      )
+      .array()
   })
   .array();
 

backend/src/services/resource-cleanup/resource-cleanup-queue.ts

@@ -7,7 +7,7 @@ import { logger } from "@app/lib/logger";
 import { QueueJobs, QueueName, TQueueServiceFactory } from "@app/queue";
 import { TUserNotificationDALFactory } from "@app/services/notification/user-notification-dal";
 
-import { TApprovalRequestDALFactory, TApprovalRequestGrantsDALFactory } from "../approval-policy/approval-policy-dal";
+import { TApprovalRequestDALFactory, TApprovalRequestGrantsDALFactory } from "../approval-policy/approval-request-dal";
 import { TIdentityAccessTokenDALFactory } from "../identity-access-token/identity-access-token-dal";
 import { TIdentityUaClientSecretDALFactory } from "../identity-ua/identity-ua-client-secret-dal";
 import { TOrgServiceFactory } from "../org/org-service";