From 2cbf33ac140327c4e03d35263d3189e91ea60e2d Mon Sep 17 00:00:00 2001 From: = Date: Fri, 24 Jan 2025 18:32:44 +0530 Subject: [PATCH 01/16] feat: added new permission check --- .../services/permission/permission-types.ts | 16 - backend/src/lib/casl/index.ts | 281 ++++++++++++++++-- 2 files changed, 252 insertions(+), 45 deletions(-) diff --git a/backend/src/ee/services/permission/permission-types.ts b/backend/src/ee/services/permission/permission-types.ts index 1ad0b205b..1708404f6 100644 --- a/backend/src/ee/services/permission/permission-types.ts +++ b/backend/src/ee/services/permission/permission-types.ts @@ -5,22 +5,6 @@ import { PermissionConditionOperators } from "@app/lib/casl"; export const PermissionConditionSchema = { [PermissionConditionOperators.$IN]: z.string().trim().min(1).array(), - [PermissionConditionOperators.$ALL]: z.string().trim().min(1).array(), - [PermissionConditionOperators.$REGEX]: z - .string() - .min(1) - .refine( - (el) => { - try { - // eslint-disable-next-line no-new - new RegExp(el); - return true; - } catch { - return false; - } - }, - { message: "Invalid regex pattern" } - ), [PermissionConditionOperators.$EQ]: z.string().min(1), [PermissionConditionOperators.$NEQ]: z.string().min(1), [PermissionConditionOperators.$GLOB]: z diff --git a/backend/src/lib/casl/index.ts b/backend/src/lib/casl/index.ts index ad4bf028f..1bc3f4f59 100644 --- a/backend/src/lib/casl/index.ts +++ b/backend/src/lib/casl/index.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ -import { buildMongoQueryMatcher, MongoAbility } from "@casl/ability"; +import { buildMongoQueryMatcher, createMongoAbility, MongoAbility } from "@casl/ability"; import { FieldCondition, FieldInstruction, JsInterpreter } from "@ucast/mongo2js"; import picomatch from "picomatch"; @@ -20,21 +20,193 @@ const glob: JsInterpreter> = (node, object, context) => { export const conditionsMatcher = buildMongoQueryMatcher({ $glob }, { glob }); -/** - * Extracts and formats permissions from a CASL Ability object or a raw permission set. - */ -const extractPermissions = (ability: MongoAbility) => { - const permissions: string[] = []; - ability.rules.forEach((permission) => { - if (typeof permission.action === "string") { - permissions.push(`${permission.action}_${permission.subject as string}`); - } else { - permission.action.forEach((permissionAction) => { - permissions.push(`${permissionAction}_${permission.subject as string}`); - }); +export enum PermissionConditionOperators { + $IN = "$in", + $EQ = "$eq", + $NEQ = "$ne", + $GLOB = "$glob" +} + +type TPermissionConditionShape = { + [PermissionConditionOperators.$EQ]: string; + [PermissionConditionOperators.$NEQ]: string; + [PermissionConditionOperators.$GLOB]: string; + [PermissionConditionOperators.$IN]: string[]; +}; + +const getPermissionSetContainerID = (action: string, subject: string) => `${action}:${subject}`; +const invertTheOperation = (shouldInvert: boolean, operation: boolean) => (shouldInvert ? !operation : operation); +const formatConditionOperator = (condition: TPermissionConditionShape | string) => { + return ( + typeof condition === "string" ? { [PermissionConditionOperators.$EQ]: condition } : condition + ) as TPermissionConditionShape; +}; + +const isOperatorsASubset = (parentSet: TPermissionConditionShape, subset: TPermissionConditionShape) => { + if (subset[PermissionConditionOperators.$EQ] || subset[PermissionConditionOperators.$NEQ]) { + const subsetOperatorValue = subset[PermissionConditionOperators.$EQ] || subset[PermissionConditionOperators.$NEQ]; + const isInverted = Boolean(subset[PermissionConditionOperators.$NEQ]); + if ( + parentSet[PermissionConditionOperators.$EQ] && + invertTheOperation(isInverted, parentSet[PermissionConditionOperators.$EQ] !== subsetOperatorValue) + ) { + return false; } + if ( + parentSet[PermissionConditionOperators.$NEQ] && + invertTheOperation(isInverted, parentSet[PermissionConditionOperators.$NEQ] === subsetOperatorValue) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$IN] && + invertTheOperation(isInverted, !parentSet[PermissionConditionOperators.$IN].includes(subsetOperatorValue)) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$GLOB] && + invertTheOperation( + isInverted, + !picomatch.isMatch(subsetOperatorValue, parentSet[PermissionConditionOperators.$GLOB], { strictSlashes: false }) + ) + ) { + return false; + } + } + if (subset[PermissionConditionOperators.$IN]) { + const subsetOperatorValue = subset[PermissionConditionOperators.$IN]; + if ( + parentSet[PermissionConditionOperators.$EQ] && + (subsetOperatorValue.length !== 1 || subsetOperatorValue[0] !== parentSet[PermissionConditionOperators.$EQ]) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$NEQ] && + !subsetOperatorValue.includes(parentSet[PermissionConditionOperators.$NEQ]) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$IN] && + !subsetOperatorValue.every((el) => parentSet[PermissionConditionOperators.$IN].includes(el)) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$GLOB] && + !subsetOperatorValue.every((el) => + picomatch.isMatch(el, parentSet[PermissionConditionOperators.$GLOB], { + strictSlashes: false + }) + ) + ) { + return false; + } + } + if (subset[PermissionConditionOperators.$GLOB]) { + const subsetOperatorValue = subset[PermissionConditionOperators.$GLOB]; + const { isGlob } = picomatch.scan(subsetOperatorValue); + // if it's glob, all other fixed operators would make this superset because glob is powerful. like eq + // example: $in [dev, prod] => glob: dev** could mean anything starting with dev: thus is bigger + if ( + isGlob && + Object.keys(parentSet).some( + (el) => el !== PermissionConditionOperators.$GLOB && el !== PermissionConditionOperators.$NEQ + ) + ) { + return false; + } + + if ( + parentSet[PermissionConditionOperators.$EQ] && + parentSet[PermissionConditionOperators.$EQ] !== subsetOperatorValue + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$NEQ] && + picomatch.isMatch(parentSet[PermissionConditionOperators.$NEQ], subsetOperatorValue, { + strictSlashes: false + }) + ) { + return false; + } + // if parent set is IN, glob cannot be used for children - It's a bigger scope + if ( + parentSet[PermissionConditionOperators.$IN] && + !parentSet[PermissionConditionOperators.$IN].includes(subsetOperatorValue) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$GLOB] && + !picomatch.isMatch(subsetOperatorValue, parentSet[PermissionConditionOperators.$GLOB], { + strictSlashes: false + }) + ) { + return false; + } + } + return true; +}; + +const isSubsetForSamePermissionSubjectAction = ( + // [{action, subject,conditions{env: dev}}] + parentSetRules: ReturnType, + // [{action, subject,conditions{env: prod}}, {action, subject,conditions{env: dev,secretPath: "/"}] + subsetRules: ReturnType +) => { + const isMissingConditionInParent = parentSetRules.every((el) => !el.conditions); + if (isMissingConditionInParent) return true; + + // all subset rules must pass in comparison to parent rul + return subsetRules.every((subsetRule) => { + const subsetRuleConditions = subsetRule.conditions as Record; + + // compare subset rule with all parent rules + const isSubsetOfNonInvertedParentSet = parentSetRules + .filter((el) => !el.inverted) + .some((parentSetRule) => { + // get conditions and iterate + const parentSetRuleConditions = parentSetRule?.conditions as Record; + if (!parentSetRuleConditions) return true; + return Object.keys(parentSetRuleConditions).every((parentConditionField) => { + // if parent condition is missing then it's never a subset + if (!subsetRuleConditions?.[parentConditionField]) return false; + + // standardize the conditions plain string operator => $eq function + const parentRuleConditionOperators = formatConditionOperator(parentSetRuleConditions[parentConditionField]); + const selectedSubsetRuleCondition = subsetRuleConditions?.[parentConditionField]; + const subsetRuleConditionOperators = formatConditionOperator(selectedSubsetRuleCondition); + return isOperatorsASubset(parentRuleConditionOperators, subsetRuleConditionOperators); + }); + }); + + const invertedParentSetRules = parentSetRules.filter((el) => el.inverted); + const isNotSubsetOfInvertedParentSet = invertedParentSetRules.length + ? !invertedParentSetRules.some((parentSetRule) => { + // get conditions and iterate + const parentSetRuleConditions = parentSetRule?.conditions as Record< + string, + TPermissionConditionShape | string + >; + if (!parentSetRuleConditions) return true; + return Object.keys(parentSetRuleConditions).every((parentConditionField) => { + // if parent condition is missing then it's never a subset + if (!subsetRuleConditions?.[parentConditionField]) return false; + + // standardize the conditions plain string operator => $eq function + const parentRuleConditionOperators = formatConditionOperator(parentSetRuleConditions[parentConditionField]); + const selectedSubsetRuleCondition = subsetRuleConditions?.[parentConditionField]; + const subsetRuleConditionOperators = formatConditionOperator(selectedSubsetRuleCondition); + return isOperatorsASubset(parentRuleConditionOperators, subsetRuleConditionOperators); + }); + }) + : true; + return isSubsetOfNonInvertedParentSet && isNotSubsetOfInvertedParentSet; }); - return permissions; }; /** @@ -42,24 +214,75 @@ const extractPermissions = (ability: MongoAbility) => { * The function checks if all permissions in the second set are contained within the first set and if the first set has equal or more permissions. * */ -export const isAtLeastAsPrivileged = (permissions1: MongoAbility, permissions2: MongoAbility) => { - const set1 = new Set(extractPermissions(permissions1)); - const set2 = new Set(extractPermissions(permissions2)); +export const isAtLeastAsPrivileged = (parentSetPermissions: MongoAbility, subsetPermissions: MongoAbility) => { + const checkedPermissionRules = new Set(); + for (const subsetPermissionRules of subsetPermissions.rules) { + const subsetPermissionSubject = subsetPermissionRules.subject.toString(); + let subsetPermissionActions: string[] = []; - for (const perm of set2) { - if (!set1.has(perm)) { - return false; + if (typeof subsetPermissionRules.action === "string") { + subsetPermissionActions.push(subsetPermissionRules.action); + } else { + subsetPermissionRules.action.forEach((subsetPermissionAction) => { + subsetPermissionActions.push(subsetPermissionAction); + }); } + subsetPermissionActions = subsetPermissionActions.filter( + (el) => !checkedPermissionRules.has(getPermissionSetContainerID(el, subsetPermissionSubject)) + ); + + // eslint-disable-next-line no-continue + if (!subsetPermissionActions.length) continue; + // eslint-disable-next-line no-unreachable-loop + for (const subsetPermissionAction of subsetPermissionActions) { + const parentSetRulesOfSubset = parentSetPermissions.possibleRulesFor( + subsetPermissionAction, + subsetPermissionSubject + ); + const nonInveretedOnes = parentSetRulesOfSubset.filter((el) => !el.inverted); + if (!nonInveretedOnes.length) return false; + + const subsetRules = subsetPermissions.possibleRulesFor(subsetPermissionAction, subsetPermissionSubject); + const isSubset = isSubsetForSamePermissionSubjectAction(parentSetRulesOfSubset, subsetRules); + if (!isSubset) return false; + } + + subsetPermissionActions.forEach((el) => + checkedPermissionRules.add(getPermissionSetContainerID(el, subsetPermissionSubject)) + ); } - return set1.size >= set2.size; + return true; }; -export enum PermissionConditionOperators { - $IN = "$in", - $ALL = "$all", - $REGEX = "$regex", - $EQ = "$eq", - $NEQ = "$ne", - $GLOB = "$glob" -} +const superset = createMongoAbility([ + { + action: ["create", "edit", "delete", "read"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" } + } + }, + { + action: "read", + subject: "secrets", + inverted: true, + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" }, + secretPath: { [PermissionConditionOperators.$GLOB]: "/hello" } + } + } +]); + +const subset = createMongoAbility([ + { + action: "edit", + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" }, + secretPath: { [PermissionConditionOperators.$EQ]: "/hello" } + } + } +]); + +console.log(isAtLeastAsPrivileged(superset, subset)); From c993b1bbe3c2b7e8549f86f7e0c7443abbcc5c49 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 28 Jan 2025 19:50:55 +0530 Subject: [PATCH 02/16] feat: completed new permission boundary check --- backend/package.json | 1 + .../src/ee/services/group/group-service.ts | 43 +- ...project-additional-privilege-v2-service.ts | 32 +- ...ty-project-additional-privilege-service.ts | 32 +- ...oject-user-additional-privilege-service.ts | 22 +- backend/src/lib/casl/boundary.test.ts | 665 ++++++++++++++++++ backend/src/lib/casl/boundary.ts | 249 +++++++ backend/src/lib/casl/index.ts | 262 +------ backend/src/lib/errors/index.ts | 10 +- backend/src/server/plugins/error-handler.ts | 3 +- .../group-project/group-project-service.ts | 27 +- .../identity-aws-auth-service.ts | 9 +- .../identity-azure-auth-service.ts | 9 +- .../identity-gcp-auth-service.ts | 9 +- .../identity-jwt-auth-service.ts | 10 +- .../identity-kubernetes-auth-service.ts | 9 +- .../identity-oidc-auth-service.ts | 10 +- .../identity-project-service.ts | 33 +- .../identity-token-auth-service.ts | 26 +- .../identity-ua/identity-ua-service.ts | 39 +- .../src/services/identity/identity-service.ts | 42 +- .../project-membership-service.ts | 12 +- backend/vitest.unit.config.ts | 17 + 23 files changed, 1183 insertions(+), 388 deletions(-) create mode 100644 backend/src/lib/casl/boundary.test.ts create mode 100644 backend/src/lib/casl/boundary.ts create mode 100644 backend/vitest.unit.config.ts diff --git a/backend/package.json b/backend/package.json index 43409e619..c0dc33bf0 100644 --- a/backend/package.json +++ b/backend/package.json @@ -40,6 +40,7 @@ "type:check": "tsc --noEmit", "lint:fix": "eslint --fix --ext js,ts ./src", "lint": "eslint 'src/**/*.ts'", + "test:unit": "vitest run -c vitest.unit.config.ts", "test:e2e": "vitest run -c vitest.e2e.config.ts --bail=1", "test:e2e-watch": "vitest -c vitest.e2e.config.ts --bail=1", "test:e2e-coverage": "vitest run --coverage -c vitest.e2e.config.ts", diff --git a/backend/src/ee/services/group/group-service.ts b/backend/src/ee/services/group/group-service.ts index 7de3f8f92..7f1bca5d8 100644 --- a/backend/src/ee/services/group/group-service.ts +++ b/backend/src/ee/services/group/group-service.ts @@ -3,7 +3,7 @@ import slugify from "@sindresorhus/slugify"; import { OrgMembershipRole, TOrgRoles } from "@app/db/schemas"; import { TOidcConfigDALFactory } from "@app/ee/services/oidc/oidc-config-dal"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { BadRequestError, ForbiddenRequestError, NotFoundError, UnauthorizedError } from "@app/lib/errors"; import { alphaNumericNanoId } from "@app/lib/nanoid"; import { TGroupProjectDALFactory } from "@app/services/group-project/group-project-dal"; @@ -87,9 +87,14 @@ export const groupServiceFactory = ({ actorOrgId ); const isCustomRole = Boolean(customRole); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, rolePermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to create a more privileged group" }); + + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to create a more privileged group", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const group = await groupDAL.transaction(async (tx) => { const existingGroup = await groupDAL.findOne({ orgId: actorOrgId, name }, tx); @@ -156,9 +161,13 @@ export const groupServiceFactory = ({ ); const isCustomRole = Boolean(customOrgRole); - const hasRequiredNewRolePermission = isAtLeastAsPrivileged(permission, rolePermission); - if (!hasRequiredNewRolePermission) - throw new ForbiddenRequestError({ message: "Failed to create a more privileged group" }); + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to create a more privileged group", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); if (isCustomRole) customRole = customOrgRole; } @@ -329,9 +338,13 @@ export const groupServiceFactory = ({ const { permission: groupRolePermission } = await permissionService.getOrgPermissionByRole(group.role, actorOrgId); // check if user has broader or equal to privileges than group - const hasRequiredPrivileges = isAtLeastAsPrivileged(permission, groupRolePermission); - if (!hasRequiredPrivileges) - throw new ForbiddenRequestError({ message: "Failed to add user to more privileged group" }); + const permissionBoundary = validatePermissionBoundary(permission, groupRolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to add user to more privileged group", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const user = await userDAL.findOne({ username }); if (!user) throw new NotFoundError({ message: `Failed to find user with username ${username}` }); @@ -396,9 +409,13 @@ export const groupServiceFactory = ({ const { permission: groupRolePermission } = await permissionService.getOrgPermissionByRole(group.role, actorOrgId); // check if user has broader or equal to privileges than group - const hasRequiredPrivileges = isAtLeastAsPrivileged(permission, groupRolePermission); - if (!hasRequiredPrivileges) - throw new ForbiddenRequestError({ message: "Failed to delete user from more privileged group" }); + const permissionBoundary = validatePermissionBoundary(permission, groupRolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to delete user from more privileged group", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const user = await userDAL.findOne({ username }); if (!user) throw new NotFoundError({ message: `Failed to find user with username ${username}` }); diff --git a/backend/src/ee/services/identity-project-additional-privilege-v2/identity-project-additional-privilege-v2-service.ts b/backend/src/ee/services/identity-project-additional-privilege-v2/identity-project-additional-privilege-v2-service.ts index 3a38c0d65..08e3a7727 100644 --- a/backend/src/ee/services/identity-project-additional-privilege-v2/identity-project-additional-privilege-v2-service.ts +++ b/backend/src/ee/services/identity-project-additional-privilege-v2/identity-project-additional-privilege-v2-service.ts @@ -3,7 +3,7 @@ import { packRules } from "@casl/ability/extra"; import ms from "ms"; import { ActionProjectType, TableName } from "@app/db/schemas"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; import { unpackPermissions } from "@app/server/routes/santizedSchemas/permission"; import { ActorType } from "@app/services/auth/auth-type"; @@ -79,9 +79,13 @@ export const identityProjectAdditionalPrivilegeV2ServiceFactory = ({ // we need to validate that the privilege given is not higher than the assigning users permission // @ts-expect-error this is expected error because of one being really accurate rule definition other being a bit more broader. Both are valid casl rules targetIdentityPermission.update(targetIdentityPermission.rules.concat(customPermission)); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, targetIdentityPermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to update more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, targetIdentityPermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to update more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const existingSlug = await identityProjectAdditionalPrivilegeDAL.findOne({ slug, @@ -161,9 +165,13 @@ export const identityProjectAdditionalPrivilegeV2ServiceFactory = ({ // we need to validate that the privilege given is not higher than the assigning users permission // @ts-expect-error this is expected error because of one being really accurate rule definition other being a bit more broader. Both are valid casl rules targetIdentityPermission.update(targetIdentityPermission.rules.concat(data.permissions || [])); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, targetIdentityPermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to update more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, targetIdentityPermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to update more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); if (data?.slug) { const existingSlug = await identityProjectAdditionalPrivilegeDAL.findOne({ @@ -239,9 +247,13 @@ export const identityProjectAdditionalPrivilegeV2ServiceFactory = ({ actorOrgId, actionProjectType: ActionProjectType.Any }); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, identityRolePermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to update more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, identityRolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to update more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const deletedPrivilege = await identityProjectAdditionalPrivilegeDAL.deleteById(identityPrivilege.id); return { diff --git a/backend/src/ee/services/identity-project-additional-privilege/identity-project-additional-privilege-service.ts b/backend/src/ee/services/identity-project-additional-privilege/identity-project-additional-privilege-service.ts index 16c0cc212..4ac60a659 100644 --- a/backend/src/ee/services/identity-project-additional-privilege/identity-project-additional-privilege-service.ts +++ b/backend/src/ee/services/identity-project-additional-privilege/identity-project-additional-privilege-service.ts @@ -3,7 +3,7 @@ import { PackRule, packRules, unpackRules } from "@casl/ability/extra"; import ms from "ms"; import { ActionProjectType } from "@app/db/schemas"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; import { UnpackedPermissionSchema } from "@app/server/routes/santizedSchemas/permission"; import { ActorType } from "@app/services/auth/auth-type"; @@ -88,9 +88,13 @@ export const identityProjectAdditionalPrivilegeServiceFactory = ({ // we need to validate that the privilege given is not higher than the assigning users permission // @ts-expect-error this is expected error because of one being really accurate rule definition other being a bit more broader. Both are valid casl rules targetIdentityPermission.update(targetIdentityPermission.rules.concat(customPermission)); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, targetIdentityPermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to update more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, targetIdentityPermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to update more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const existingSlug = await identityProjectAdditionalPrivilegeDAL.findOne({ slug, @@ -172,9 +176,13 @@ export const identityProjectAdditionalPrivilegeServiceFactory = ({ // we need to validate that the privilege given is not higher than the assigning users permission // @ts-expect-error this is expected error because of one being really accurate rule definition other being a bit more broader. Both are valid casl rules targetIdentityPermission.update(targetIdentityPermission.rules.concat(data.permissions || [])); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, targetIdentityPermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to update more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, targetIdentityPermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to update more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const identityPrivilege = await identityProjectAdditionalPrivilegeDAL.findOne({ slug, @@ -268,9 +276,13 @@ export const identityProjectAdditionalPrivilegeServiceFactory = ({ actorOrgId, actionProjectType: ActionProjectType.Any }); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, identityRolePermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to edit more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, identityRolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to edit more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const identityPrivilege = await identityProjectAdditionalPrivilegeDAL.findOne({ slug, diff --git a/backend/src/ee/services/project-user-additional-privilege/project-user-additional-privilege-service.ts b/backend/src/ee/services/project-user-additional-privilege/project-user-additional-privilege-service.ts index 14586d5e2..7cf255bcc 100644 --- a/backend/src/ee/services/project-user-additional-privilege/project-user-additional-privilege-service.ts +++ b/backend/src/ee/services/project-user-additional-privilege/project-user-additional-privilege-service.ts @@ -3,7 +3,7 @@ import { PackRule, packRules, unpackRules } from "@casl/ability/extra"; import ms from "ms"; import { ActionProjectType, TableName } from "@app/db/schemas"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; import { UnpackedPermissionSchema } from "@app/server/routes/santizedSchemas/permission"; import { ActorType } from "@app/services/auth/auth-type"; @@ -76,9 +76,13 @@ export const projectUserAdditionalPrivilegeServiceFactory = ({ // we need to validate that the privilege given is not higher than the assigning users permission // @ts-expect-error this is expected error because of one being really accurate rule definition other being a bit more broader. Both are valid casl rules targetUserPermission.update(targetUserPermission.rules.concat(customPermission)); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, targetUserPermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to update more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, targetUserPermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to update more privileged user", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const existingSlug = await projectUserAdditionalPrivilegeDAL.findOne({ slug, @@ -163,9 +167,13 @@ export const projectUserAdditionalPrivilegeServiceFactory = ({ // we need to validate that the privilege given is not higher than the assigning users permission // @ts-expect-error this is expected error because of one being really accurate rule definition other being a bit more broader. Both are valid casl rules targetUserPermission.update(targetUserPermission.rules.concat(dto.permissions || [])); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, targetUserPermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to update more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, targetUserPermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to update more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); if (dto?.slug) { const existingSlug = await projectUserAdditionalPrivilegeDAL.findOne({ diff --git a/backend/src/lib/casl/boundary.test.ts b/backend/src/lib/casl/boundary.test.ts new file mode 100644 index 000000000..c5e284a2e --- /dev/null +++ b/backend/src/lib/casl/boundary.test.ts @@ -0,0 +1,665 @@ +import { createMongoAbility } from "@casl/ability"; + +import { PermissionConditionOperators } from "."; +import { validatePermissionBoundary } from "./boundary"; + +describe("Validate Permission Boundary Function", () => { + test.each([ + { + title: "child with equal privilege", + parentPermission: createMongoAbility([ + { + action: ["create", "edit", "delete", "read"], + subject: "secrets" + } + ]), + childPermission: createMongoAbility([ + { + action: ["create", "edit", "delete", "read"], + subject: "secrets" + } + ]), + expectValid: true, + missingPermissions: [] + }, + { + title: "child with less privilege", + parentPermission: createMongoAbility([ + { + action: ["create", "edit", "delete", "read"], + subject: "secrets" + } + ]), + childPermission: createMongoAbility([ + { + action: ["create", "edit"], + subject: "secrets" + } + ]), + expectValid: true, + missingPermissions: [] + }, + { + title: "child with more privilege", + parentPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets" + } + ]), + childPermission: createMongoAbility([ + { + action: ["create", "edit"], + subject: "secrets" + } + ]), + expectValid: false, + missingPermissions: [{ action: "edit", subject: "secrets" }] + }, + { + title: "parent with multiple and child with multiple", + parentPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets" + }, + { + action: ["create", "edit"], + subject: "members" + } + ]), + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "members" + } + ]), + expectValid: true, + missingPermissions: [] + }, + { + title: "Child with no access", + parentPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets" + }, + { + action: ["create", "edit"], + subject: "members" + } + ]), + childPermission: createMongoAbility([]), + expectValid: true, + missingPermissions: [] + }, + { + title: "Parent and child disjoint set", + parentPermission: createMongoAbility([ + { + action: ["create", "edit", "delete", "read"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" } + } + } + ]), + childPermission: createMongoAbility([ + { + action: ["create", "edit", "delete", "read"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$EQ]: "dev" } + } + } + ]), + expectValid: false, + missingPermissions: ["create", "edit", "delete", "read"].map((el) => ({ + action: el, + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$EQ]: "dev" } + } + })) + }, + { + title: "Parent with inverted rules", + parentPermission: createMongoAbility([ + { + action: ["create", "edit", "delete", "read"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" } + } + }, + { + action: "read", + subject: "secrets", + inverted: true, + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" }, + secretPath: { [PermissionConditionOperators.$GLOB]: "/hello/**" } + } + } + ]), + childPermission: createMongoAbility([ + { + action: "read", + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" }, + secretPath: { [PermissionConditionOperators.$EQ]: "/" } + } + } + ]), + expectValid: true, + missingPermissions: [] + }, + { + title: "Parent with inverted rules - child accessing invalid one", + parentPermission: createMongoAbility([ + { + action: ["create", "edit", "delete", "read"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" } + } + }, + { + action: "read", + subject: "secrets", + inverted: true, + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" }, + secretPath: { [PermissionConditionOperators.$GLOB]: "/hello/**" } + } + } + ]), + childPermission: createMongoAbility([ + { + action: "read", + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" }, + secretPath: { [PermissionConditionOperators.$EQ]: "/hello/world" } + } + } + ]), + expectValid: false, + missingPermissions: [ + { + action: "read", + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" }, + secretPath: { [PermissionConditionOperators.$EQ]: "/hello/world" } + } + } + ] + } + ])("Check permission: $title", ({ parentPermission, childPermission, expectValid, missingPermissions }) => { + const permissionBoundary = validatePermissionBoundary(parentPermission, childPermission); + if (expectValid) { + expect(permissionBoundary.isValid).toBeTruthy(); + } else { + expect(permissionBoundary.isValid).toBeFalsy(); + expect(permissionBoundary.missingPermissions).toEqual(expect.arrayContaining(missingPermissions)); + } + }); +}); + +describe("Validate Permission Boundary: Checking Parent $eq operator", () => { + const parentPermission = createMongoAbility([ + { + action: ["create", "read"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" } + } + } + ]); + + test.each([ + { + operator: PermissionConditionOperators.$EQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$IN, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$IN]: ["dev"] } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$GLOB, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$GLOB]: "dev" } + } + } + ]) + } + ])("Child $operator truthy cases", ({ childPermission }) => { + const permissionBoundary = validatePermissionBoundary(parentPermission, childPermission); + expect(permissionBoundary.isValid).toBeTruthy(); + }); + + test.each([ + { + operator: PermissionConditionOperators.$EQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "prod" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$IN, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$IN]: ["dev", "prod"] } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$GLOB, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$GLOB]: "dev**" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$NEQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$GLOB]: "staging" } + } + } + ]) + } + ])("Child $operator falsy cases", ({ childPermission }) => { + const permissionBoundary = validatePermissionBoundary(parentPermission, childPermission); + expect(permissionBoundary.isValid).toBeFalsy(); + }); +}); + +describe("Validate Permission Boundary: Checking Parent $neq operator", () => { + const parentPermission = createMongoAbility([ + { + action: ["create", "read"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$NEQ]: "/hello" } + } + } + ]); + + test.each([ + { + operator: PermissionConditionOperators.$EQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$EQ]: "/" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$NEQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$NEQ]: "/hello" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$IN, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$IN]: ["/", "/staging"] } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$GLOB, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$GLOB]: "/dev**" } + } + } + ]) + } + ])("Child $operator truthy cases", ({ childPermission }) => { + const permissionBoundary = validatePermissionBoundary(parentPermission, childPermission); + expect(permissionBoundary.isValid).toBeTruthy(); + }); + + test.each([ + { + operator: PermissionConditionOperators.$EQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$EQ]: "/hello" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$NEQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$NEQ]: "/" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$IN, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$IN]: ["/", "/hello"] } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$GLOB, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$GLOB]: "/hello**" } + } + } + ]) + } + ])("Child $operator falsy cases", ({ childPermission }) => { + const permissionBoundary = validatePermissionBoundary(parentPermission, childPermission); + expect(permissionBoundary.isValid).toBeFalsy(); + }); +}); + +describe("Validate Permission Boundary: Checking Parent $IN operator", () => { + const parentPermission = createMongoAbility([ + { + action: ["edit"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$IN]: ["dev", "staging"] } + } + } + ]); + + test.each([ + { + operator: PermissionConditionOperators.$EQ, + childPermission: createMongoAbility([ + { + action: ["edit"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "dev" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$IN, + childPermission: createMongoAbility([ + { + action: ["edit"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$IN]: ["dev"] } + } + } + ]) + }, + { + operator: `${PermissionConditionOperators.$IN} - 2`, + childPermission: createMongoAbility([ + { + action: ["edit"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$IN]: ["dev", "staging"] } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$GLOB, + childPermission: createMongoAbility([ + { + action: ["edit"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$GLOB]: "dev" } + } + } + ]) + } + ])("Child $operator truthy cases", ({ childPermission }) => { + const permissionBoundary = validatePermissionBoundary(parentPermission, childPermission); + expect(permissionBoundary.isValid).toBeTruthy(); + }); + + test.each([ + { + operator: PermissionConditionOperators.$EQ, + childPermission: createMongoAbility([ + { + action: ["edit"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$EQ]: "prod" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$NEQ, + childPermission: createMongoAbility([ + { + action: ["edit"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$NEQ]: "dev" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$IN, + childPermission: createMongoAbility([ + { + action: ["edit"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$IN]: ["dev", "prod"] } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$GLOB, + childPermission: createMongoAbility([ + { + action: ["edit"], + subject: "secrets", + conditions: { + environment: { [PermissionConditionOperators.$GLOB]: "dev**" } + } + } + ]) + } + ])("Child $operator falsy cases", ({ childPermission }) => { + const permissionBoundary = validatePermissionBoundary(parentPermission, childPermission); + expect(permissionBoundary.isValid).toBeFalsy(); + }); +}); + +describe("Validate Permission Boundary: Checking Parent $GLOB operator", () => { + const parentPermission = createMongoAbility([ + { + action: ["create", "read"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$GLOB]: "/hello/**" } + } + } + ]); + + test.each([ + { + operator: PermissionConditionOperators.$EQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$EQ]: "/hello/world" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$IN, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$IN]: ["/hello/world", "/hello/world2"] } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$GLOB, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$GLOB]: "/hello/**/world" } + } + } + ]) + } + ])("Child $operator truthy cases", ({ childPermission }) => { + const permissionBoundary = validatePermissionBoundary(parentPermission, childPermission); + expect(permissionBoundary.isValid).toBeTruthy(); + }); + + test.each([ + { + operator: PermissionConditionOperators.$EQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$EQ]: "/print" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$NEQ, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$NEQ]: "/hello/world" } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$IN, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$IN]: ["/", "/hello"] } + } + } + ]) + }, + { + operator: PermissionConditionOperators.$GLOB, + childPermission: createMongoAbility([ + { + action: ["create"], + subject: "secrets", + conditions: { + secretPath: { [PermissionConditionOperators.$GLOB]: "/hello**" } + } + } + ]) + } + ])("Child $operator falsy cases", ({ childPermission }) => { + const permissionBoundary = validatePermissionBoundary(parentPermission, childPermission); + expect(permissionBoundary.isValid).toBeFalsy(); + }); +}); diff --git a/backend/src/lib/casl/boundary.ts b/backend/src/lib/casl/boundary.ts new file mode 100644 index 000000000..dee006f25 --- /dev/null +++ b/backend/src/lib/casl/boundary.ts @@ -0,0 +1,249 @@ +import { MongoAbility } from "@casl/ability"; +import { MongoQuery } from "@ucast/mongo2js"; +import picomatch from "picomatch"; + +import { PermissionConditionOperators } from "./index"; + +type TMissingPermission = { + action: string; + subject: string; + conditions?: MongoQuery; +}; + +type TPermissionConditionShape = { + [PermissionConditionOperators.$EQ]: string; + [PermissionConditionOperators.$NEQ]: string; + [PermissionConditionOperators.$GLOB]: string; + [PermissionConditionOperators.$IN]: string[]; +}; + +const getPermissionSetID = (action: string, subject: string) => `${action}:${subject}`; +const invertTheOperation = (shouldInvert: boolean, operation: boolean) => (shouldInvert ? !operation : operation); +const formatConditionOperator = (condition: TPermissionConditionShape | string) => { + return ( + typeof condition === "string" ? { [PermissionConditionOperators.$EQ]: condition } : condition + ) as TPermissionConditionShape; +}; + +const isOperatorsASubset = (parentSet: TPermissionConditionShape, subset: TPermissionConditionShape) => { + // we compute each operator against each other in left hand side and right hand side + if (subset[PermissionConditionOperators.$EQ] || subset[PermissionConditionOperators.$NEQ]) { + const subsetOperatorValue = subset[PermissionConditionOperators.$EQ] || subset[PermissionConditionOperators.$NEQ]; + const isInverted = Boolean(subset[PermissionConditionOperators.$NEQ]); + if ( + parentSet[PermissionConditionOperators.$EQ] && + invertTheOperation(isInverted, parentSet[PermissionConditionOperators.$EQ] !== subsetOperatorValue) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$NEQ] && + invertTheOperation(isInverted, parentSet[PermissionConditionOperators.$NEQ] === subsetOperatorValue) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$IN] && + invertTheOperation(isInverted, !parentSet[PermissionConditionOperators.$IN].includes(subsetOperatorValue)) + ) { + return false; + } + // ne and glob cannot match each other + if (parentSet[PermissionConditionOperators.$GLOB] && isInverted) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$GLOB] && + !picomatch.isMatch(subsetOperatorValue, parentSet[PermissionConditionOperators.$GLOB], { strictSlashes: false }) + ) { + return false; + } + } + if (subset[PermissionConditionOperators.$IN]) { + const subsetOperatorValue = subset[PermissionConditionOperators.$IN]; + if ( + parentSet[PermissionConditionOperators.$EQ] && + (subsetOperatorValue.length !== 1 || subsetOperatorValue[0] !== parentSet[PermissionConditionOperators.$EQ]) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$NEQ] && + subsetOperatorValue.includes(parentSet[PermissionConditionOperators.$NEQ]) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$IN] && + !subsetOperatorValue.every((el) => parentSet[PermissionConditionOperators.$IN].includes(el)) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$GLOB] && + !subsetOperatorValue.every((el) => + picomatch.isMatch(el, parentSet[PermissionConditionOperators.$GLOB], { + strictSlashes: false + }) + ) + ) { + return false; + } + } + if (subset[PermissionConditionOperators.$GLOB]) { + const subsetOperatorValue = subset[PermissionConditionOperators.$GLOB]; + const { isGlob } = picomatch.scan(subsetOperatorValue); + // if it's glob, all other fixed operators would make this superset because glob is powerful. like eq + // example: $in [dev, prod] => glob: dev** could mean anything starting with dev: thus is bigger + if ( + isGlob && + Object.keys(parentSet).some( + (el) => el !== PermissionConditionOperators.$GLOB && el !== PermissionConditionOperators.$NEQ + ) + ) { + return false; + } + + if ( + parentSet[PermissionConditionOperators.$EQ] && + parentSet[PermissionConditionOperators.$EQ] !== subsetOperatorValue + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$NEQ] && + picomatch.isMatch(parentSet[PermissionConditionOperators.$NEQ], subsetOperatorValue, { + strictSlashes: false + }) + ) { + return false; + } + // if parent set is IN, glob cannot be used for children - It's a bigger scope + if ( + parentSet[PermissionConditionOperators.$IN] && + !parentSet[PermissionConditionOperators.$IN].includes(subsetOperatorValue) + ) { + return false; + } + if ( + parentSet[PermissionConditionOperators.$GLOB] && + !picomatch.isMatch(subsetOperatorValue, parentSet[PermissionConditionOperators.$GLOB], { + strictSlashes: false + }) + ) { + return false; + } + } + return true; +}; + +const isSubsetForSamePermissionSubjectAction = ( + parentSetRules: ReturnType, + subsetRules: ReturnType, + appendToMissingPermission: (condition?: MongoQuery) => void +) => { + const isMissingConditionInParent = parentSetRules.every((el) => !el.conditions); + if (isMissingConditionInParent) return true; + + // all subset rules must pass in comparison to parent rul + return subsetRules.every((subsetRule) => { + const subsetRuleConditions = subsetRule.conditions as Record; + // compare subset rule with all parent rules + const isSubsetOfNonInvertedParentSet = parentSetRules + .filter((el) => !el.inverted) + .some((parentSetRule) => { + // get conditions and iterate + const parentSetRuleConditions = parentSetRule?.conditions as Record; + if (!parentSetRuleConditions) return true; + return Object.keys(parentSetRuleConditions).every((parentConditionField) => { + // if parent condition is missing then it's never a subset + if (!subsetRuleConditions?.[parentConditionField]) return false; + + // standardize the conditions plain string operator => $eq function + const parentRuleConditionOperators = formatConditionOperator(parentSetRuleConditions[parentConditionField]); + const selectedSubsetRuleCondition = subsetRuleConditions?.[parentConditionField]; + const subsetRuleConditionOperators = formatConditionOperator(selectedSubsetRuleCondition); + return isOperatorsASubset(parentRuleConditionOperators, subsetRuleConditionOperators); + }); + }); + + const invertedParentSetRules = parentSetRules.filter((el) => el.inverted); + const isNotSubsetOfInvertedParentSet = invertedParentSetRules.length + ? !invertedParentSetRules.some((parentSetRule) => { + // get conditions and iterate + const parentSetRuleConditions = parentSetRule?.conditions as Record< + string, + TPermissionConditionShape | string + >; + if (!parentSetRuleConditions) return true; + return Object.keys(parentSetRuleConditions).every((parentConditionField) => { + // if parent condition is missing then it's never a subset + if (!subsetRuleConditions?.[parentConditionField]) return false; + + // standardize the conditions plain string operator => $eq function + const parentRuleConditionOperators = formatConditionOperator(parentSetRuleConditions[parentConditionField]); + const selectedSubsetRuleCondition = subsetRuleConditions?.[parentConditionField]; + const subsetRuleConditionOperators = formatConditionOperator(selectedSubsetRuleCondition); + return isOperatorsASubset(parentRuleConditionOperators, subsetRuleConditionOperators); + }); + }) + : true; + const isSubset = isSubsetOfNonInvertedParentSet && isNotSubsetOfInvertedParentSet; + if (!isSubset) { + appendToMissingPermission(subsetRule.conditions); + } + return isSubset; + }); +}; + +export const validatePermissionBoundary = (parentSetPermissions: MongoAbility, subsetPermissions: MongoAbility) => { + const checkedPermissionRules = new Set(); + const missingPermissions: TMissingPermission[] = []; + + subsetPermissions.rules.forEach((subsetPermissionRules) => { + const subsetPermissionSubject = subsetPermissionRules.subject.toString(); + let subsetPermissionActions: string[] = []; + + // actions can be string or string[] + if (typeof subsetPermissionRules.action === "string") { + subsetPermissionActions.push(subsetPermissionRules.action); + } else { + subsetPermissionRules.action.forEach((subsetPermissionAction) => { + subsetPermissionActions.push(subsetPermissionAction); + }); + } + + // if action is already processed ignore + subsetPermissionActions = subsetPermissionActions.filter( + (el) => !checkedPermissionRules.has(getPermissionSetID(el, subsetPermissionSubject)) + ); + + if (!subsetPermissionActions.length) return; + subsetPermissionActions.forEach((subsetPermissionAction) => { + const parentSetRulesOfSubset = parentSetPermissions.possibleRulesFor( + subsetPermissionAction, + subsetPermissionSubject + ); + const nonInveretedOnes = parentSetRulesOfSubset.filter((el) => !el.inverted); + if (!nonInveretedOnes.length) { + missingPermissions.push({ action: subsetPermissionAction, subject: subsetPermissionSubject }); + return; + } + + const subsetRules = subsetPermissions.possibleRulesFor(subsetPermissionAction, subsetPermissionSubject); + isSubsetForSamePermissionSubjectAction(parentSetRulesOfSubset, subsetRules, (conditions) => { + missingPermissions.push({ action: subsetPermissionAction, subject: subsetPermissionSubject, conditions }); + }); + }); + + subsetPermissionActions.forEach((el) => + checkedPermissionRules.add(getPermissionSetID(el, subsetPermissionSubject)) + ); + }); + + if (missingPermissions.length) { + return { isValid: false as const, missingPermissions }; + } + + return { isValid: true }; +}; diff --git a/backend/src/lib/casl/index.ts b/backend/src/lib/casl/index.ts index 1bc3f4f59..147d12ef7 100644 --- a/backend/src/lib/casl/index.ts +++ b/backend/src/lib/casl/index.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ -import { buildMongoQueryMatcher, createMongoAbility, MongoAbility } from "@casl/ability"; +import { buildMongoQueryMatcher } from "@casl/ability"; import { FieldCondition, FieldInstruction, JsInterpreter } from "@ucast/mongo2js"; import picomatch from "picomatch"; @@ -26,263 +26,3 @@ export enum PermissionConditionOperators { $NEQ = "$ne", $GLOB = "$glob" } - -type TPermissionConditionShape = { - [PermissionConditionOperators.$EQ]: string; - [PermissionConditionOperators.$NEQ]: string; - [PermissionConditionOperators.$GLOB]: string; - [PermissionConditionOperators.$IN]: string[]; -}; - -const getPermissionSetContainerID = (action: string, subject: string) => `${action}:${subject}`; -const invertTheOperation = (shouldInvert: boolean, operation: boolean) => (shouldInvert ? !operation : operation); -const formatConditionOperator = (condition: TPermissionConditionShape | string) => { - return ( - typeof condition === "string" ? { [PermissionConditionOperators.$EQ]: condition } : condition - ) as TPermissionConditionShape; -}; - -const isOperatorsASubset = (parentSet: TPermissionConditionShape, subset: TPermissionConditionShape) => { - if (subset[PermissionConditionOperators.$EQ] || subset[PermissionConditionOperators.$NEQ]) { - const subsetOperatorValue = subset[PermissionConditionOperators.$EQ] || subset[PermissionConditionOperators.$NEQ]; - const isInverted = Boolean(subset[PermissionConditionOperators.$NEQ]); - if ( - parentSet[PermissionConditionOperators.$EQ] && - invertTheOperation(isInverted, parentSet[PermissionConditionOperators.$EQ] !== subsetOperatorValue) - ) { - return false; - } - if ( - parentSet[PermissionConditionOperators.$NEQ] && - invertTheOperation(isInverted, parentSet[PermissionConditionOperators.$NEQ] === subsetOperatorValue) - ) { - return false; - } - if ( - parentSet[PermissionConditionOperators.$IN] && - invertTheOperation(isInverted, !parentSet[PermissionConditionOperators.$IN].includes(subsetOperatorValue)) - ) { - return false; - } - if ( - parentSet[PermissionConditionOperators.$GLOB] && - invertTheOperation( - isInverted, - !picomatch.isMatch(subsetOperatorValue, parentSet[PermissionConditionOperators.$GLOB], { strictSlashes: false }) - ) - ) { - return false; - } - } - if (subset[PermissionConditionOperators.$IN]) { - const subsetOperatorValue = subset[PermissionConditionOperators.$IN]; - if ( - parentSet[PermissionConditionOperators.$EQ] && - (subsetOperatorValue.length !== 1 || subsetOperatorValue[0] !== parentSet[PermissionConditionOperators.$EQ]) - ) { - return false; - } - if ( - parentSet[PermissionConditionOperators.$NEQ] && - !subsetOperatorValue.includes(parentSet[PermissionConditionOperators.$NEQ]) - ) { - return false; - } - if ( - parentSet[PermissionConditionOperators.$IN] && - !subsetOperatorValue.every((el) => parentSet[PermissionConditionOperators.$IN].includes(el)) - ) { - return false; - } - if ( - parentSet[PermissionConditionOperators.$GLOB] && - !subsetOperatorValue.every((el) => - picomatch.isMatch(el, parentSet[PermissionConditionOperators.$GLOB], { - strictSlashes: false - }) - ) - ) { - return false; - } - } - if (subset[PermissionConditionOperators.$GLOB]) { - const subsetOperatorValue = subset[PermissionConditionOperators.$GLOB]; - const { isGlob } = picomatch.scan(subsetOperatorValue); - // if it's glob, all other fixed operators would make this superset because glob is powerful. like eq - // example: $in [dev, prod] => glob: dev** could mean anything starting with dev: thus is bigger - if ( - isGlob && - Object.keys(parentSet).some( - (el) => el !== PermissionConditionOperators.$GLOB && el !== PermissionConditionOperators.$NEQ - ) - ) { - return false; - } - - if ( - parentSet[PermissionConditionOperators.$EQ] && - parentSet[PermissionConditionOperators.$EQ] !== subsetOperatorValue - ) { - return false; - } - if ( - parentSet[PermissionConditionOperators.$NEQ] && - picomatch.isMatch(parentSet[PermissionConditionOperators.$NEQ], subsetOperatorValue, { - strictSlashes: false - }) - ) { - return false; - } - // if parent set is IN, glob cannot be used for children - It's a bigger scope - if ( - parentSet[PermissionConditionOperators.$IN] && - !parentSet[PermissionConditionOperators.$IN].includes(subsetOperatorValue) - ) { - return false; - } - if ( - parentSet[PermissionConditionOperators.$GLOB] && - !picomatch.isMatch(subsetOperatorValue, parentSet[PermissionConditionOperators.$GLOB], { - strictSlashes: false - }) - ) { - return false; - } - } - return true; -}; - -const isSubsetForSamePermissionSubjectAction = ( - // [{action, subject,conditions{env: dev}}] - parentSetRules: ReturnType, - // [{action, subject,conditions{env: prod}}, {action, subject,conditions{env: dev,secretPath: "/"}] - subsetRules: ReturnType -) => { - const isMissingConditionInParent = parentSetRules.every((el) => !el.conditions); - if (isMissingConditionInParent) return true; - - // all subset rules must pass in comparison to parent rul - return subsetRules.every((subsetRule) => { - const subsetRuleConditions = subsetRule.conditions as Record; - - // compare subset rule with all parent rules - const isSubsetOfNonInvertedParentSet = parentSetRules - .filter((el) => !el.inverted) - .some((parentSetRule) => { - // get conditions and iterate - const parentSetRuleConditions = parentSetRule?.conditions as Record; - if (!parentSetRuleConditions) return true; - return Object.keys(parentSetRuleConditions).every((parentConditionField) => { - // if parent condition is missing then it's never a subset - if (!subsetRuleConditions?.[parentConditionField]) return false; - - // standardize the conditions plain string operator => $eq function - const parentRuleConditionOperators = formatConditionOperator(parentSetRuleConditions[parentConditionField]); - const selectedSubsetRuleCondition = subsetRuleConditions?.[parentConditionField]; - const subsetRuleConditionOperators = formatConditionOperator(selectedSubsetRuleCondition); - return isOperatorsASubset(parentRuleConditionOperators, subsetRuleConditionOperators); - }); - }); - - const invertedParentSetRules = parentSetRules.filter((el) => el.inverted); - const isNotSubsetOfInvertedParentSet = invertedParentSetRules.length - ? !invertedParentSetRules.some((parentSetRule) => { - // get conditions and iterate - const parentSetRuleConditions = parentSetRule?.conditions as Record< - string, - TPermissionConditionShape | string - >; - if (!parentSetRuleConditions) return true; - return Object.keys(parentSetRuleConditions).every((parentConditionField) => { - // if parent condition is missing then it's never a subset - if (!subsetRuleConditions?.[parentConditionField]) return false; - - // standardize the conditions plain string operator => $eq function - const parentRuleConditionOperators = formatConditionOperator(parentSetRuleConditions[parentConditionField]); - const selectedSubsetRuleCondition = subsetRuleConditions?.[parentConditionField]; - const subsetRuleConditionOperators = formatConditionOperator(selectedSubsetRuleCondition); - return isOperatorsASubset(parentRuleConditionOperators, subsetRuleConditionOperators); - }); - }) - : true; - return isSubsetOfNonInvertedParentSet && isNotSubsetOfInvertedParentSet; - }); -}; - -/** - * Compares two sets of permissions to determine if the first set is at least as privileged as the second set. - * The function checks if all permissions in the second set are contained within the first set and if the first set has equal or more permissions. - * - */ -export const isAtLeastAsPrivileged = (parentSetPermissions: MongoAbility, subsetPermissions: MongoAbility) => { - const checkedPermissionRules = new Set(); - for (const subsetPermissionRules of subsetPermissions.rules) { - const subsetPermissionSubject = subsetPermissionRules.subject.toString(); - let subsetPermissionActions: string[] = []; - - if (typeof subsetPermissionRules.action === "string") { - subsetPermissionActions.push(subsetPermissionRules.action); - } else { - subsetPermissionRules.action.forEach((subsetPermissionAction) => { - subsetPermissionActions.push(subsetPermissionAction); - }); - } - subsetPermissionActions = subsetPermissionActions.filter( - (el) => !checkedPermissionRules.has(getPermissionSetContainerID(el, subsetPermissionSubject)) - ); - - // eslint-disable-next-line no-continue - if (!subsetPermissionActions.length) continue; - // eslint-disable-next-line no-unreachable-loop - for (const subsetPermissionAction of subsetPermissionActions) { - const parentSetRulesOfSubset = parentSetPermissions.possibleRulesFor( - subsetPermissionAction, - subsetPermissionSubject - ); - const nonInveretedOnes = parentSetRulesOfSubset.filter((el) => !el.inverted); - if (!nonInveretedOnes.length) return false; - - const subsetRules = subsetPermissions.possibleRulesFor(subsetPermissionAction, subsetPermissionSubject); - const isSubset = isSubsetForSamePermissionSubjectAction(parentSetRulesOfSubset, subsetRules); - if (!isSubset) return false; - } - - subsetPermissionActions.forEach((el) => - checkedPermissionRules.add(getPermissionSetContainerID(el, subsetPermissionSubject)) - ); - } - - return true; -}; - -const superset = createMongoAbility([ - { - action: ["create", "edit", "delete", "read"], - subject: "secrets", - conditions: { - environment: { [PermissionConditionOperators.$EQ]: "dev" } - } - }, - { - action: "read", - subject: "secrets", - inverted: true, - conditions: { - environment: { [PermissionConditionOperators.$EQ]: "dev" }, - secretPath: { [PermissionConditionOperators.$GLOB]: "/hello" } - } - } -]); - -const subset = createMongoAbility([ - { - action: "edit", - subject: "secrets", - conditions: { - environment: { [PermissionConditionOperators.$EQ]: "dev" }, - secretPath: { [PermissionConditionOperators.$EQ]: "/hello" } - } - } -]); - -console.log(isAtLeastAsPrivileged(superset, subset)); diff --git a/backend/src/lib/errors/index.ts b/backend/src/lib/errors/index.ts index cc1c1d66b..f3dc83f26 100644 --- a/backend/src/lib/errors/index.ts +++ b/backend/src/lib/errors/index.ts @@ -52,10 +52,18 @@ export class ForbiddenRequestError extends Error { error: unknown; - constructor({ name, error, message }: { message?: string; name?: string; error?: unknown } = {}) { + details?: unknown; + + constructor({ + name, + error, + message, + details + }: { message?: string; name?: string; error?: unknown; details?: unknown } = {}) { super(message ?? "You are not allowed to access this resource"); this.name = name || "ForbiddenError"; this.error = error; + this.details = details; } } diff --git a/backend/src/server/plugins/error-handler.ts b/backend/src/server/plugins/error-handler.ts index 7f9e16197..0fd18bc29 100644 --- a/backend/src/server/plugins/error-handler.ts +++ b/backend/src/server/plugins/error-handler.ts @@ -122,7 +122,8 @@ export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider reqId: req.id, statusCode: HttpStatusCodes.Forbidden, message: error.message, - error: error.name + error: error.name, + details: error?.details }); } else if (error instanceof RateLimitError) { void res.status(HttpStatusCodes.TooManyRequests).send({ diff --git a/backend/src/services/group-project/group-project-service.ts b/backend/src/services/group-project/group-project-service.ts index 067ff17b0..b408e2e95 100644 --- a/backend/src/services/group-project/group-project-service.ts +++ b/backend/src/services/group-project/group-project-service.ts @@ -4,7 +4,7 @@ import ms from "ms"; import { ActionProjectType, ProjectMembershipRole, SecretKeyEncoding, TGroups } from "@app/db/schemas"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { decryptAsymmetric, encryptAsymmetric } from "@app/lib/crypto"; import { infisicalSymmetricDecrypt } from "@app/lib/crypto/encryption"; import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; @@ -102,11 +102,13 @@ export const groupProjectServiceFactory = ({ project.id ); - const hasRequiredPrivileges = isAtLeastAsPrivileged(permission, rolePermission); - - if (!hasRequiredPrivileges) { - throw new ForbiddenRequestError({ message: "Failed to assign group to a more privileged role" }); - } + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to assign group to a more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); } // validate custom roles input @@ -267,12 +269,13 @@ export const groupProjectServiceFactory = ({ requestedRoleChange, project.id ); - - const hasRequiredPrivileges = isAtLeastAsPrivileged(permission, rolePermission); - - if (!hasRequiredPrivileges) { - throw new ForbiddenRequestError({ message: "Failed to assign group to a more privileged role" }); - } + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to assign group to a more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); } // validate custom roles input diff --git a/backend/src/services/identity-aws-auth/identity-aws-auth-service.ts b/backend/src/services/identity-aws-auth/identity-aws-auth-service.ts index ff202f225..ddd9e0278 100644 --- a/backend/src/services/identity-aws-auth/identity-aws-auth-service.ts +++ b/backend/src/services/identity-aws-auth/identity-aws-auth-service.ts @@ -7,7 +7,7 @@ import { IdentityAuthMethod } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, ForbiddenRequestError, NotFoundError, UnauthorizedError } from "@app/lib/errors"; import { extractIPDetails, isValidIpOrCidr } from "@app/lib/ip"; @@ -339,9 +339,12 @@ export const identityAwsAuthServiceFactory = ({ actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to revoke aws auth of identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to revoke aws auth of identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const revokedIdentityAwsAuth = await identityAwsAuthDAL.transaction(async (tx) => { diff --git a/backend/src/services/identity-azure-auth/identity-azure-auth-service.ts b/backend/src/services/identity-azure-auth/identity-azure-auth-service.ts index 01d013734..01878dbb3 100644 --- a/backend/src/services/identity-azure-auth/identity-azure-auth-service.ts +++ b/backend/src/services/identity-azure-auth/identity-azure-auth-service.ts @@ -5,7 +5,7 @@ import { IdentityAuthMethod } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, ForbiddenRequestError, NotFoundError, UnauthorizedError } from "@app/lib/errors"; import { extractIPDetails, isValidIpOrCidr } from "@app/lib/ip"; @@ -312,9 +312,12 @@ export const identityAzureAuthServiceFactory = ({ actorAuthMethod, actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to revoke azure auth of identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to revoke azure auth of identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const revokedIdentityAzureAuth = await identityAzureAuthDAL.transaction(async (tx) => { diff --git a/backend/src/services/identity-gcp-auth/identity-gcp-auth-service.ts b/backend/src/services/identity-gcp-auth/identity-gcp-auth-service.ts index 5e404ca20..7b0dd4390 100644 --- a/backend/src/services/identity-gcp-auth/identity-gcp-auth-service.ts +++ b/backend/src/services/identity-gcp-auth/identity-gcp-auth-service.ts @@ -5,7 +5,7 @@ import { IdentityAuthMethod } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, ForbiddenRequestError, NotFoundError, UnauthorizedError } from "@app/lib/errors"; import { extractIPDetails, isValidIpOrCidr } from "@app/lib/ip"; @@ -358,9 +358,12 @@ export const identityGcpAuthServiceFactory = ({ actorAuthMethod, actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to revoke gcp auth of identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to revoke gcp auth of identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const revokedIdentityGcpAuth = await identityGcpAuthDAL.transaction(async (tx) => { diff --git a/backend/src/services/identity-jwt-auth/identity-jwt-auth-service.ts b/backend/src/services/identity-jwt-auth/identity-jwt-auth-service.ts index 6757b0b84..2c9b6306e 100644 --- a/backend/src/services/identity-jwt-auth/identity-jwt-auth-service.ts +++ b/backend/src/services/identity-jwt-auth/identity-jwt-auth-service.ts @@ -7,7 +7,7 @@ import { IdentityAuthMethod, TIdentityJwtAuthsUpdate } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, ForbiddenRequestError, NotFoundError, UnauthorizedError } from "@app/lib/errors"; import { extractIPDetails, isValidIpOrCidr } from "@app/lib/ip"; @@ -508,11 +508,13 @@ export const identityJwtAuthServiceFactory = ({ actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) { + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to revoke JWT auth of identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to revoke jwt auth of identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); - } const revokedIdentityJwtAuth = await identityJwtAuthDAL.transaction(async (tx) => { const deletedJwtAuth = await identityJwtAuthDAL.delete({ identityId }, tx); diff --git a/backend/src/services/identity-kubernetes-auth/identity-kubernetes-auth-service.ts b/backend/src/services/identity-kubernetes-auth/identity-kubernetes-auth-service.ts index 4508a255d..bd83885dc 100644 --- a/backend/src/services/identity-kubernetes-auth/identity-kubernetes-auth-service.ts +++ b/backend/src/services/identity-kubernetes-auth/identity-kubernetes-auth-service.ts @@ -7,7 +7,7 @@ import { IdentityAuthMethod, SecretKeyEncoding, TIdentityKubernetesAuthsUpdate } import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { getConfig } from "@app/lib/config/env"; import { decryptSymmetric, @@ -616,9 +616,12 @@ export const identityKubernetesAuthServiceFactory = ({ actorAuthMethod, actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to revoke kubernetes auth of identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to revoke kubernetes auth of identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const revokedIdentityKubernetesAuth = await identityKubernetesAuthDAL.transaction(async (tx) => { diff --git a/backend/src/services/identity-oidc-auth/identity-oidc-auth-service.ts b/backend/src/services/identity-oidc-auth/identity-oidc-auth-service.ts index a1dbed46b..08c4e116f 100644 --- a/backend/src/services/identity-oidc-auth/identity-oidc-auth-service.ts +++ b/backend/src/services/identity-oidc-auth/identity-oidc-auth-service.ts @@ -8,7 +8,7 @@ import { IdentityAuthMethod, SecretKeyEncoding, TIdentityOidcAuthsUpdate } from import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { getConfig } from "@app/lib/config/env"; import { generateAsymmetricKeyPair } from "@app/lib/crypto"; import { @@ -531,11 +531,13 @@ export const identityOidcAuthServiceFactory = ({ actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) { + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to revoke OIDC auth of identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to revoke oidc auth of identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); - } const revokedIdentityOidcAuth = await identityOidcAuthDAL.transaction(async (tx) => { const deletedOidcAuth = await identityOidcAuthDAL.delete({ identityId }, tx); diff --git a/backend/src/services/identity-project/identity-project-service.ts b/backend/src/services/identity-project/identity-project-service.ts index 36b9b0562..0b71165e2 100644 --- a/backend/src/services/identity-project/identity-project-service.ts +++ b/backend/src/services/identity-project/identity-project-service.ts @@ -4,7 +4,7 @@ import ms from "ms"; import { ActionProjectType, ProjectMembershipRole } from "@app/db/schemas"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; import { groupBy } from "@app/lib/fn"; @@ -91,11 +91,13 @@ export const identityProjectServiceFactory = ({ projectId ); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, rolePermission); - - if (!hasRequiredPriviledges) { - throw new ForbiddenRequestError({ message: "Failed to change to a more privileged role" }); - } + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to change to a more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); } // validate custom roles input @@ -185,9 +187,13 @@ export const identityProjectServiceFactory = ({ projectId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) { - throw new ForbiddenRequestError({ message: "Failed to change to a more privileged role" }); - } + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to change to a more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); } // validate custom roles input @@ -277,8 +283,13 @@ export const identityProjectServiceFactory = ({ actorOrgId, actionProjectType: ActionProjectType.Any }); - if (!isAtLeastAsPrivileged(permission, identityRolePermission)) - throw new ForbiddenRequestError({ message: "Failed to delete more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, identityRolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to delete more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const [deletedIdentity] = await identityProjectDAL.delete({ identityId, projectId }); return deletedIdentity; diff --git a/backend/src/services/identity-token-auth/identity-token-auth-service.ts b/backend/src/services/identity-token-auth/identity-token-auth-service.ts index bf38c5fa1..d9e2d66fa 100644 --- a/backend/src/services/identity-token-auth/identity-token-auth-service.ts +++ b/backend/src/services/identity-token-auth/identity-token-auth-service.ts @@ -5,7 +5,7 @@ import { IdentityAuthMethod, TableName } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; import { extractIPDetails, isValidIpOrCidr } from "@app/lib/ip"; @@ -245,11 +245,13 @@ export const identityTokenAuthServiceFactory = ({ actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) { + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to revoke Token Auth of identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to revoke token auth of identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); - } const revokedIdentityTokenAuth = await identityTokenAuthDAL.transaction(async (tx) => { const deletedTokenAuth = await identityTokenAuthDAL.delete({ identityId }, tx); @@ -295,10 +297,12 @@ export const identityTokenAuthServiceFactory = ({ actorAuthMethod, actorOrgId ); - const hasPriviledge = isAtLeastAsPrivileged(permission, rolePermission); - if (!hasPriviledge) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to create token for identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to create token for identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const identityTokenAuth = await identityTokenAuthDAL.findOne({ identityId }); @@ -415,10 +419,12 @@ export const identityTokenAuthServiceFactory = ({ actorAuthMethod, actorOrgId ); - const hasPriviledge = isAtLeastAsPrivileged(permission, rolePermission); - if (!hasPriviledge) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to update token for identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to update token for identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const [token] = await identityAccessTokenDAL.update( diff --git a/backend/src/services/identity-ua/identity-ua-service.ts b/backend/src/services/identity-ua/identity-ua-service.ts index b9837265a..650f0511b 100644 --- a/backend/src/services/identity-ua/identity-ua-service.ts +++ b/backend/src/services/identity-ua/identity-ua-service.ts @@ -8,7 +8,7 @@ import { IdentityAuthMethod } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, ForbiddenRequestError, NotFoundError, UnauthorizedError } from "@app/lib/errors"; import { checkIPAgainstBlocklist, extractIPDetails, isValidIpOrCidr, TIp } from "@app/lib/ip"; @@ -367,9 +367,12 @@ export const identityUaServiceFactory = ({ actorAuthMethod, actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to revoke universal auth of identity with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to revoke universal auth of identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const revokedIdentityUniversalAuth = await identityUaDAL.transaction(async (tx) => { @@ -414,10 +417,12 @@ export const identityUaServiceFactory = ({ actorAuthMethod, actorOrgId ); - const hasPriviledge = isAtLeastAsPrivileged(permission, rolePermission); - if (!hasPriviledge) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to add identity to project with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to add identity to project with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const appCfg = getConfig(); @@ -475,9 +480,12 @@ export const identityUaServiceFactory = ({ actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to add identity to project with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to get identity with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const identityUniversalAuth = await identityUaDAL.findOne({ @@ -524,9 +532,12 @@ export const identityUaServiceFactory = ({ actorAuthMethod, actorOrgId ); - if (!isAtLeastAsPrivileged(permission, rolePermission)) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to read identity client secret of project with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to read identity client secret of project with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const clientSecret = await identityUaClientSecretDAL.findById(clientSecretId); @@ -566,10 +577,12 @@ export const identityUaServiceFactory = ({ actorAuthMethod, actorOrgId ); - - if (!isAtLeastAsPrivileged(permission, rolePermission)) + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: "Failed to revoke identity client secret with more privileged role" + name: "PermissionBoundaryError", + message: "Failed to revoke identity client secret with more privileged role", + details: { missingPermissions: permissionBoundary.missingPermissions } }); const clientSecret = await identityUaClientSecretDAL.updateById(clientSecretId, { diff --git a/backend/src/services/identity/identity-service.ts b/backend/src/services/identity/identity-service.ts index fffcbacc2..68ec75287 100644 --- a/backend/src/services/identity/identity-service.ts +++ b/backend/src/services/identity/identity-service.ts @@ -4,7 +4,7 @@ import { OrgMembershipRole, TableName, TOrgRoles } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; import { TIdentityProjectDALFactory } from "@app/services/identity-project/identity-project-dal"; @@ -58,9 +58,13 @@ export const identityServiceFactory = ({ orgId ); const isCustomRole = Boolean(customRole); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, rolePermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to create a more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to create a more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const plan = await licenseService.getPlan(orgId); @@ -129,9 +133,13 @@ export const identityServiceFactory = ({ actorAuthMethod, actorOrgId ); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, identityRolePermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to delete more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, identityRolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to delete a more privileged identity", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); let customRole: TOrgRoles | undefined; if (role) { @@ -141,9 +149,13 @@ export const identityServiceFactory = ({ ); const isCustomRole = Boolean(customOrgRole); - const hasRequiredNewRolePermission = isAtLeastAsPrivileged(permission, rolePermission); - if (!hasRequiredNewRolePermission) - throw new ForbiddenRequestError({ message: "Failed to create a more privileged identity" }); + const appliedRolePermissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!appliedRolePermissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to create a more privileged identity", + details: { missingPermissions: appliedRolePermissionBoundary.missingPermissions } + }); if (isCustomRole) customRole = customOrgRole; } @@ -216,9 +228,13 @@ export const identityServiceFactory = ({ actorAuthMethod, actorOrgId ); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, identityRolePermission); - if (!hasRequiredPriviledges) - throw new ForbiddenRequestError({ message: "Failed to delete more privileged identity" }); + const permissionBoundary = validatePermissionBoundary(permission, identityRolePermission); + if (!permissionBoundary.isValid) + throw new ForbiddenRequestError({ + name: "PermissionBoundaryError", + message: "Failed to delete more privileged user", + details: { missingPermissions: permissionBoundary.missingPermissions } + }); const deletedIdentity = await identityDAL.deleteById(id); diff --git a/backend/src/services/project-membership/project-membership-service.ts b/backend/src/services/project-membership/project-membership-service.ts index fd1382dcf..f47a37222 100644 --- a/backend/src/services/project-membership/project-membership-service.ts +++ b/backend/src/services/project-membership/project-membership-service.ts @@ -7,7 +7,7 @@ import { TLicenseServiceFactory } from "@app/ee/services/license/license-service import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; import { TProjectUserAdditionalPrivilegeDALFactory } from "@app/ee/services/project-user-additional-privilege/project-user-additional-privilege-dal"; -import { isAtLeastAsPrivileged } from "@app/lib/casl"; +import { validatePermissionBoundary } from "@app/lib/casl/boundary"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, ForbiddenRequestError, NotFoundError } from "@app/lib/errors"; import { groupBy } from "@app/lib/fn"; @@ -274,13 +274,13 @@ export const projectMembershipServiceFactory = ({ projectId ); - const hasRequiredPriviledges = isAtLeastAsPrivileged(permission, rolePermission); - - if (!hasRequiredPriviledges) { + const permissionBoundary = validatePermissionBoundary(permission, rolePermission); + if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ - message: `Failed to change to a more privileged role ${requestedRoleChange}` + name: "PermissionBoundaryError", + message: `Failed to change to a more privileged role ${requestedRoleChange}`, + details: { missingPermissions: permissionBoundary.missingPermissions } }); - } } // validate custom roles input diff --git a/backend/vitest.unit.config.ts b/backend/vitest.unit.config.ts new file mode 100644 index 000000000..97862d288 --- /dev/null +++ b/backend/vitest.unit.config.ts @@ -0,0 +1,17 @@ +import path from "path"; +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + test: { + globals: true, + env: { + NODE_ENV: "test" + }, + include: ["./src/**/*.test.ts"] + }, + resolve: { + alias: { + "@app": path.resolve(__dirname, "./src") + } + } +}); From 8061066e2755fa71bdc09edade6d52fc802dfe43 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 28 Jan 2025 19:59:20 +0530 Subject: [PATCH 03/16] feat: added detail description in ui notification --- frontend/src/hooks/api/reactQuery.tsx | 101 ++++++++++++++++++++++++++ frontend/src/hooks/api/types.ts | 14 ++++ 2 files changed, 115 insertions(+) diff --git a/frontend/src/hooks/api/reactQuery.tsx b/frontend/src/hooks/api/reactQuery.tsx index f6ae0ca76..be9dadbec 100644 --- a/frontend/src/hooks/api/reactQuery.tsx +++ b/frontend/src/hooks/api/reactQuery.tsx @@ -74,6 +74,107 @@ export const queryClient = new QueryClient({ ); return; } + if (serverResponse?.error === ApiErrorTypes.PermissionBoundaryError) { + createNotification( + { + title: "Forbidden Access", + type: "error", + text: `${serverResponse.message}.`, + callToAction: serverResponse?.details?.missingPermissions?.length ? ( + + + + + +
+ {serverResponse.details?.missingPermissions?.map((el, index) => { + const hasConditions = Boolean(Object.keys(el.conditions || {}).length); + return ( +
+
+ You are not authorized to perform the {el.action} action on the{" "} + {el.subject} resource.{" "} + {hasConditions && + "Your permission does not allow access to the following conditions:"} +
+ {hasConditions && ( +
    + {Object.keys(el.conditions || {}).flatMap((field, fieldIndex) => { + const operators = ( + el.conditions as Record< + string, + | string + | { [K in PermissionConditionOperators]: string | string[] } + > + )[field]; + + const formattedFieldName = camelCaseToSpaces(field).toLowerCase(); + if (typeof operators === "string") { + return ( +
  • + + {formattedFieldName} + {" "} + equal to{" "} + {operators} +
    • + ); + } + + return Object.keys(operators).map((operator, operatorIndex) => ( +
  • + + {formattedFieldName} + {" "} + + { + formatedConditionsOperatorNames[ + operator as PermissionConditionOperators + ] + } + {" "} + + {operators[ + operator as PermissionConditionOperators + ].toString()} + +
    • + )); + })} +
+ )} +
+ ); + })} +
+ + + ) : undefined, + copyActions: [ + { + value: serverResponse.reqId, + name: "Request ID", + label: `Request ID: ${serverResponse.reqId}` + } + ] + }, + { closeOnClick: false } + ); + return; + } if (serverResponse?.error === ApiErrorTypes.ForbiddenError) { createNotification( { diff --git a/frontend/src/hooks/api/types.ts b/frontend/src/hooks/api/types.ts index c03358b42..cda34ad60 100644 --- a/frontend/src/hooks/api/types.ts +++ b/frontend/src/hooks/api/types.ts @@ -44,6 +44,7 @@ export type { export enum ApiErrorTypes { ValidationError = "ValidationFailure", + PermissionBoundaryError = "PermissionBoundaryError", BadRequestError = "BadRequest", UnauthorizedError = "UnauthorizedError", ForbiddenError = "PermissionDenied" @@ -74,4 +75,17 @@ export type TApiErrors = statusCode: 400; message: string; error: ApiErrorTypes.BadRequestError; + } + | { + reqId: string; + statusCode: 403; + message: string; + error: ApiErrorTypes.PermissionBoundaryError; + details: { + missingPermissions: { + action: string; + subject: string; + conditions: Record>; + }[]; + }; }; From 1d57629036f0371d1e3963c2b732e433053e1a43 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 28 Jan 2025 20:00:19 +0530 Subject: [PATCH 04/16] feat: added unit test in github action --- .github/workflows/run-backend-tests.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/run-backend-tests.yml b/.github/workflows/run-backend-tests.yml index 1fc9deff6..f2ba04e76 100644 --- a/.github/workflows/run-backend-tests.yml +++ b/.github/workflows/run-backend-tests.yml @@ -34,7 +34,10 @@ jobs: working-directory: backend - name: Start postgres and redis run: touch .env && docker compose -f docker-compose.dev.yml up -d db redis - - name: Start integration test + - name: Run unit test + run: npm run test:unit + working-directory: backend + - name: Run integration test run: npm run test:e2e working-directory: backend env: @@ -44,4 +47,5 @@ jobs: ENCRYPTION_KEY: 4bnfe4e407b8921c104518903515b218 - name: cleanup run: | - docker compose -f "docker-compose.dev.yml" down \ No newline at end of file + docker compose -f "docker-compose.dev.yml" down + From 757942aefcc944e709870791813dc73583ab0676 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 31 Jan 2025 15:16:27 +0530 Subject: [PATCH 05/16] feat: resolved nits --- backend/src/ee/services/group/group-service.ts | 2 +- backend/src/lib/casl/boundary.ts | 2 +- .../services/identity-project/identity-project-service.ts | 4 ++-- backend/src/services/identity-ua/identity-ua-service.ts | 6 +++--- backend/src/services/identity/identity-service.ts | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/backend/src/ee/services/group/group-service.ts b/backend/src/ee/services/group/group-service.ts index 7f1bca5d8..27e847896 100644 --- a/backend/src/ee/services/group/group-service.ts +++ b/backend/src/ee/services/group/group-service.ts @@ -165,7 +165,7 @@ export const groupServiceFactory = ({ if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ name: "PermissionBoundaryError", - message: "Failed to create a more privileged group", + message: "Failed to update a more privileged group", details: { missingPermissions: permissionBoundary.missingPermissions } }); if (isCustomRole) customRole = customOrgRole; diff --git a/backend/src/lib/casl/boundary.ts b/backend/src/lib/casl/boundary.ts index dee006f25..15592a7bd 100644 --- a/backend/src/lib/casl/boundary.ts +++ b/backend/src/lib/casl/boundary.ts @@ -29,7 +29,7 @@ const isOperatorsASubset = (parentSet: TPermissionConditionShape, subset: TPermi // we compute each operator against each other in left hand side and right hand side if (subset[PermissionConditionOperators.$EQ] || subset[PermissionConditionOperators.$NEQ]) { const subsetOperatorValue = subset[PermissionConditionOperators.$EQ] || subset[PermissionConditionOperators.$NEQ]; - const isInverted = Boolean(subset[PermissionConditionOperators.$NEQ]); + const isInverted = !subset[PermissionConditionOperators.$EQ]; if ( parentSet[PermissionConditionOperators.$EQ] && invertTheOperation(isInverted, parentSet[PermissionConditionOperators.$EQ] !== subsetOperatorValue) diff --git a/backend/src/services/identity-project/identity-project-service.ts b/backend/src/services/identity-project/identity-project-service.ts index 0b71165e2..e16ffb3d4 100644 --- a/backend/src/services/identity-project/identity-project-service.ts +++ b/backend/src/services/identity-project/identity-project-service.ts @@ -95,7 +95,7 @@ export const identityProjectServiceFactory = ({ if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ name: "PermissionBoundaryError", - message: "Failed to change to a more privileged role", + message: "Failed to assign to a more privileged role", details: { missingPermissions: permissionBoundary.missingPermissions } }); } @@ -287,7 +287,7 @@ export const identityProjectServiceFactory = ({ if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ name: "PermissionBoundaryError", - message: "Failed to delete more privileged identity", + message: "Failed to remove more privileged identity", details: { missingPermissions: permissionBoundary.missingPermissions } }); diff --git a/backend/src/services/identity-ua/identity-ua-service.ts b/backend/src/services/identity-ua/identity-ua-service.ts index 650f0511b..078b50c08 100644 --- a/backend/src/services/identity-ua/identity-ua-service.ts +++ b/backend/src/services/identity-ua/identity-ua-service.ts @@ -421,7 +421,7 @@ export const identityUaServiceFactory = ({ if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ name: "PermissionBoundaryError", - message: "Failed to add identity to project with more privileged role", + message: "Failed to create client secret for a more privileged identity.", details: { missingPermissions: permissionBoundary.missingPermissions } }); @@ -484,7 +484,7 @@ export const identityUaServiceFactory = ({ if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ name: "PermissionBoundaryError", - message: "Failed to get identity with more privileged role", + message: "Failed to get identity client secret with more privileged role", details: { missingPermissions: permissionBoundary.missingPermissions } }); @@ -536,7 +536,7 @@ export const identityUaServiceFactory = ({ if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ name: "PermissionBoundaryError", - message: "Failed to read identity client secret of project with more privileged role", + message: "Failed to read identity client secret of identity with more privileged role", details: { missingPermissions: permissionBoundary.missingPermissions } }); diff --git a/backend/src/services/identity/identity-service.ts b/backend/src/services/identity/identity-service.ts index 68ec75287..8ada2a5d1 100644 --- a/backend/src/services/identity/identity-service.ts +++ b/backend/src/services/identity/identity-service.ts @@ -137,7 +137,7 @@ export const identityServiceFactory = ({ if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ name: "PermissionBoundaryError", - message: "Failed to delete a more privileged identity", + message: "Failed to update a more privileged identity", details: { missingPermissions: permissionBoundary.missingPermissions } }); @@ -232,7 +232,7 @@ export const identityServiceFactory = ({ if (!permissionBoundary.isValid) throw new ForbiddenRequestError({ name: "PermissionBoundaryError", - message: "Failed to delete more privileged user", + message: "Failed to delete more privileged identity", details: { missingPermissions: permissionBoundary.missingPermissions } }); From c54eafc128e246ade7fb90721092033851f1faf0 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 1 Feb 2025 01:54:56 +0530 Subject: [PATCH 06/16] fix: resolved typo --- backend/src/lib/casl/boundary.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/backend/src/lib/casl/boundary.test.ts b/backend/src/lib/casl/boundary.test.ts index c5e284a2e..05c2b9ecf 100644 --- a/backend/src/lib/casl/boundary.test.ts +++ b/backend/src/lib/casl/boundary.test.ts @@ -72,6 +72,10 @@ describe("Validate Permission Boundary Function", () => { { action: ["create"], subject: "members" + }, + { + action: ["create"], + subject: "secrets" } ]), expectValid: true, From 75345d91c02e50c0dcd5f3e638fc2f3d3f1b508e Mon Sep 17 00:00:00 2001 From: Maidul Islam Date: Thu, 6 Mar 2025 18:49:57 -0500 Subject: [PATCH 07/16] add gateway security docs --- .../platform/gateways/gateway-security.mdx | 110 ++++++++++++++++++ docs/mint.json | 2 +- 2 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 docs/documentation/platform/gateways/gateway-security.mdx diff --git a/docs/documentation/platform/gateways/gateway-security.mdx b/docs/documentation/platform/gateways/gateway-security.mdx new file mode 100644 index 000000000..83490fd4d --- /dev/null +++ b/docs/documentation/platform/gateways/gateway-security.mdx @@ -0,0 +1,110 @@ +--- +title: "Gateway Security Architecture" +sidebarTitle: "Architecture" +description: "Understand the security model and tenant isolation of Infisical's Gateway" +--- + +# Gateway Security Architecture + +The Infisical Gateway enables Infisical Cloud to securely interact with private resources using mutual TLS authentication and private PKI (Public Key Infrastructure) system to ensure secure, isolated communication between multiple tenants. +This document explains the internal security architecture and how tenant isolation is maintained. + +## Security Model Overview + +### Private PKI System +Each organization (tenant) in Infisical has its own private PKI system consisting of: + +1. **Root CA**: The ultimate trust anchor for the organization +2. **Intermediate CAs**: + - Client CA: Issues certificates for cloud components + - Gateway CA: Issues certificates for gateway instances + +This hierarchical structure ensures complete isolation between organizations as each has its own independent certificate chain. + +### Certificate Hierarchy +``` +Root CA (Organization Specific) +├── Client CA +│ └── Client Certificates (Cloud Components) +└── Gateway CA + └── Gateway Certificates (Gateway Instances) +``` + +## Communication Security + +### 1. Gateway Registration +When a gateway is first deployed: + +1. Establishes initial connection using machine identity token +2. Allocates a relay address for communication +3. Exchanges certificates through a secure handshake: + - Gateway receives a unique certificate signed by organization's Gateway CA along with certificate chain for verification + +### 2. Mutual TLS Authentication +All communication between gateway and cloud uses mutual TLS (mTLS): + +- **Gateway Authentication**: + - Presents certificate signed by organization's Gateway CA + - Certificate contains unique identifiers (Organization ID, Gateway ID) + - Cloud validates complete certificate chain + +- **Cloud Authentication**: + - Presents certificate signed by organization's Client CA + - Certificate includes required organizational unit ("gateway-client") + - Gateway validates certificate chain back to organization's root CA + +### 3. Relay Communication +The relay system provides secure tunneling: + +1. **Connection Establishment**: + - Uses QUIC protocol over UDP for efficient, secure communication + - Provides built-in encryption, congestion control, and multiplexing + - Enables faster connection establishment and reduced latency + - Each organization's traffic is isolated using separate relay sessions + +2. **Traffic Isolation**: + - Each gateway gets unique relay credentials + - Traffic is end-to-end encrypted using QUIC's TLS 1.3 + - Organization's private keys never leave their environment + +## Tenant Isolation + +### Certificate-Based Isolation +- Each organization has unique root CA and intermediate CAs +- Certificates contain organization-specific identifiers +- Cross-tenant communication is cryptographically impossible + +### Gateway-Project Mapping +- Gateways are explicitly mapped to specific projects +- Access controls enforce organization boundaries +- Project-level permissions determine resource accessibility + +### Resource Access Control +1. **Project Verification**: + - Gateway verifies project membership + - Validates organization ownership + - Enforces project-level permissions + +2. **Resource Restrictions**: + - Gateways only accept connections to approved resources + - Each connection requires explicit project authorization + - Resources remain private to their assigned organization + +## Security Measures + +### Certificate Lifecycle +- Certificates have limited validity periods +- Automatic certificate rotation +- Immediate certificate revocation capabilities + +### Monitoring and Verification +1. **Continuous Verification**: + - Regular heartbeat checks + - Certificate chain validation + - Connection state monitoring + +2. **Security Controls**: + - Automatic connection termination on verification failure + - Audit logging of all access attempts + - Machine identity based authentication + diff --git a/docs/mint.json b/docs/mint.json index 480337dc5..f604ff310 100644 --- a/docs/mint.json +++ b/docs/mint.json @@ -203,7 +203,7 @@ }, { "group": "Gateway", - "pages": ["documentation/platform/gateways/overview"] + "pages": ["documentation/platform/gateways/overview", "documentation/platform/gateways/gateway-security"] }, "documentation/platform/project-templates", { From ada04ed4fca6de197546ae31fe08df458a18c16e Mon Sep 17 00:00:00 2001 From: akoullick1 Date: Fri, 7 Mar 2025 10:19:54 -0800 Subject: [PATCH 08/16] Update meetings.mdx Added daily standup --- company/handbook/meetings.mdx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/company/handbook/meetings.mdx b/company/handbook/meetings.mdx index af6a3b54e..ff8db24e8 100644 --- a/company/handbook/meetings.mdx +++ b/company/handbook/meetings.mdx @@ -10,6 +10,10 @@ Being a remote-first company, we try to be as async as possible. When an issue a In other words, we have almost no (recurring) meetings and prefer written communication or quick Slack huddles. +## Daily Standup + +Towards the end of each day, everyone on the Engineering and GTM teams should document their progress in the respective Slack standup channels, ensuring the team stays informed of important updates. + ## Weekly All-hands -All-hands is the single recurring meeting that we run every Monday at 8:30am PT. Typically, we would discuss everything important that happened during the previous week and plan out the week ahead. This is also an opportunity to bring up any important topics in front of the whole company (but feel free to post those in Slack too). +All-hands is the single recurring meeting that we run every Monday at 8:00am PT. Typically, we would discuss everything important that happened during the previous week and plan out the week ahead. This is also an opportunity to bring up any important topics in front of the whole company (but feel free to post those in Slack too). From 0f42fcd6881c538b6adc969eec8aa3ef0f653bc9 Mon Sep 17 00:00:00 2001 From: carlosmonastyrski Date: Fri, 7 Mar 2025 16:59:12 -0300 Subject: [PATCH 09/16] Remove addAllMembers option from project creation modal --- .../components/projects/NewProjectModal.tsx | 53 +------------------ 1 file changed, 2 insertions(+), 51 deletions(-) diff --git a/frontend/src/components/projects/NewProjectModal.tsx b/frontend/src/components/projects/NewProjectModal.tsx index 5ef17ec84..dcd3040d8 100644 --- a/frontend/src/components/projects/NewProjectModal.tsx +++ b/frontend/src/components/projects/NewProjectModal.tsx @@ -14,7 +14,6 @@ import { AccordionItem, AccordionTrigger, Button, - Checkbox, FormControl, Input, Modal, @@ -33,13 +32,7 @@ import { useUser } from "@app/context"; import { getProjectHomePage } from "@app/helpers/project"; -import { - fetchOrgUsers, - useAddUserToWsNonE2EE, - useCreateWorkspace, - useGetExternalKmsList, - useGetUserWorkspaces -} from "@app/hooks/api"; +import { useCreateWorkspace, useGetExternalKmsList, useGetUserWorkspaces } from "@app/hooks/api"; import { INTERNAL_KMS_KEY_ID } from "@app/hooks/api/kms/types"; import { InfisicalProjectTemplate, useListProjectTemplates } from "@app/hooks/api/projectTemplates"; import { ProjectType } from "@app/hooks/api/workspace/types"; @@ -51,7 +44,6 @@ const formSchema = z.object({ .trim() .max(256, "Description too long, max length is 256 characters") .optional(), - addMembers: z.boolean(), kmsKeyId: z.string(), template: z.string() }); @@ -73,7 +65,6 @@ const NewProjectForm = ({ onOpenChange, projectType }: NewProjectFormProps) => { const { user } = useUser(); const createWs = useCreateWorkspace(); const { refetch: refetchWorkspaces } = useGetUserWorkspaces(); - const addUsersToProject = useAddUserToWsNonE2EE(); const { subscription } = useSubscription(); const canReadProjectTemplates = permission.can( @@ -111,7 +102,6 @@ const NewProjectForm = ({ onOpenChange, projectType }: NewProjectFormProps) => { const onCreateProject = async ({ name, description, - addMembers, kmsKeyId, template }: TAddProjectFormData) => { @@ -128,21 +118,6 @@ const NewProjectForm = ({ onOpenChange, projectType }: NewProjectFormProps) => { template, type: projectType }); - const { id: newProjectId } = project; - - if (addMembers) { - const orgUsers = await fetchOrgUsers(currentOrg.id); - await addUsersToProject.mutateAsync({ - usernames: orgUsers - .filter( - (member) => member.user.username !== user.username && member.status === "accepted" - ) - .map((member) => member.user.username), - projectId: newProjectId, - orgId: currentOrg.id - }); - } - await refetchWorkspaces(); createNotification({ text: "Project created", type: "success" }); @@ -246,31 +221,7 @@ const NewProjectForm = ({ onOpenChange, projectType }: NewProjectFormProps) => { )} /> -
- ( - - {(isAllowed) => ( -
- - Add all members of my organization to this project - -
- )} - - )} - /> -
-
+
From 99eb8eb8ed1ab3e4ba4e1922af62b8734d406c39 Mon Sep 17 00:00:00 2001 From: carlosmonastyrski Date: Fri, 7 Mar 2025 19:45:10 -0300 Subject: [PATCH 10/16] Use slug to check tag on remove icon click --- .../components/SecretListView/SecretDetailSidebar.tsx | 5 +++-- .../components/SecretListView/SecretListView.tsx | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx index 5e7af286d..f557c53b3 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx @@ -463,7 +463,7 @@ export const SecretDetailSidebar = ({
0 ? "pt-2" : ""}`} > - {tagFields.fields.map(({ tagColor, id: formId, slug, id }) => ( + {tagFields.fields.map(({ tagColor, id: formId, slug }) => ( id === tagId); + + const tag = tags?.find(({ slug: tagSlug }) => slug === tagSlug); if (tag) handleTagSelect(tag); }} > diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretListView.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretListView.tsx index e817aa1f9..65af9f895 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretListView.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretListView.tsx @@ -241,8 +241,8 @@ export const SecretListView = ({ let successMessage; if (isReminderEvent) { - successMessage = reminderRepeatDays - ? "Successfully saved secret reminder" + successMessage = reminderRepeatDays + ? "Successfully saved secret reminder" : "Successfully deleted secret reminder"; } else { successMessage = "Successfully saved secrets"; From d742534f6a4c851069480ba47bd8d1f45a59b38c Mon Sep 17 00:00:00 2001 From: akoullick1 Date: Fri, 7 Mar 2025 14:54:38 -0800 Subject: [PATCH 11/16] Update meetings.mdx ECD detail --- company/handbook/meetings.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/company/handbook/meetings.mdx b/company/handbook/meetings.mdx index ff8db24e8..172c114c8 100644 --- a/company/handbook/meetings.mdx +++ b/company/handbook/meetings.mdx @@ -12,7 +12,7 @@ In other words, we have almost no (recurring) meetings and prefer written commun ## Daily Standup -Towards the end of each day, everyone on the Engineering and GTM teams should document their progress in the respective Slack standup channels, ensuring the team stays informed of important updates. +Towards the end of each day, everyone on the Engineering and GTM teams should document their progress in the respective Slack standup channels, ensuring the team stays informed of important updates. On the engineering side, if you are working on something that takes longer than 1-2 days, please add an estimated completion date (ECD) for that item in standup specifying when it will be pushed to production. ## Weekly All-hands From b055cda64da20d57321f442db7bee3d897008647 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 9 Mar 2025 21:13:28 +0530 Subject: [PATCH 12/16] feat: increased turn cred duration, and fixed gateway crashing --- backend/src/lib/gateway/index.ts | 90 +++++++++++++++++------------ backend/src/lib/turn/credentials.ts | 2 +- 2 files changed, 53 insertions(+), 39 deletions(-) diff --git a/backend/src/lib/gateway/index.ts b/backend/src/lib/gateway/index.ts index 8d25c2af6..118283f64 100644 --- a/backend/src/lib/gateway/index.ts +++ b/backend/src/lib/gateway/index.ts @@ -96,6 +96,7 @@ export const pingGatewayAndVerify = async ({ error: err as Error }); }); + for (let attempt = 1; attempt <= maxRetries; attempt += 1) { try { const stream = quicClient.connection.newStream("bidi"); @@ -108,17 +109,13 @@ export const pingGatewayAndVerify = async ({ const { value, done } = await reader.read(); if (done) { - throw new BadRequestError({ - message: "Gateway closed before receiving PONG" - }); + throw new Error("Gateway closed before receiving PONG"); } const response = Buffer.from(value).toString(); if (response !== "PONG\n" && response !== "PONG") { - throw new BadRequestError({ - message: `Failed to Ping. Unexpected response: ${response}` - }); + throw new Error(`Failed to Ping. Unexpected response: ${response}`); } reader.releaseLock(); @@ -146,6 +143,7 @@ interface TProxyServer { server: net.Server; port: number; cleanup: () => Promise; + getProxyError: () => string; } const setupProxyServer = async ({ @@ -170,6 +168,7 @@ const setupProxyServer = async ({ error: err as Error }); }); + const proxyErrorMsg = [""]; return new Promise((resolve, reject) => { const server = net.createServer(); @@ -185,31 +184,33 @@ const setupProxyServer = async ({ const forwardWriter = stream.writable.getWriter(); await forwardWriter.write(Buffer.from(`FORWARD-TCP ${targetHost}:${targetPort}\n`)); forwardWriter.releaseLock(); - /* eslint-disable @typescript-eslint/no-misused-promises */ + // Set up bidirectional copy - const setupCopy = async () => { + const setupCopy = () => { // Client to QUIC // eslint-disable-next-line (async () => { - try { - const writer = stream.writable.getWriter(); + const writer = stream.writable.getWriter(); - // Create a handler for client data - clientConn.on("data", async (chunk) => { - await writer.write(chunk); + // Create a handler for client data + clientConn.on("data", (chunk) => { + writer.write(chunk).catch((err) => { + proxyErrorMsg.push((err as Error)?.message); }); + }); - // Handle client connection close - clientConn.on("end", async () => { - await writer.close(); + // Handle client connection close + clientConn.on("end", () => { + writer.close().catch((err) => { + logger.error(err); }); + }); - clientConn.on("error", async (err) => { - await writer.abort(err); + clientConn.on("error", (clientConnErr) => { + writer.abort(clientConnErr?.message).catch((err) => { + proxyErrorMsg.push((err as Error)?.message); }); - } catch (err) { - clientConn.destroy(); - } + }); })(); // QUIC to Client @@ -238,15 +239,18 @@ const setupProxyServer = async ({ } } } catch (err) { + proxyErrorMsg.push((err as Error)?.message); clientConn.destroy(); } })(); }; - await setupCopy(); - // + + setupCopy(); // Handle connection closure - clientConn.on("close", async () => { - await stream.destroy(); + clientConn.on("close", () => { + stream.destroy().catch((err) => { + proxyErrorMsg.push((err as Error)?.message); + }); }); const cleanup = async () => { @@ -254,13 +258,18 @@ const setupProxyServer = async ({ await stream.destroy(); }; - clientConn.on("error", (err) => { - logger.error(err, "Client socket error"); - void cleanup(); - reject(err); + clientConn.on("error", (clientConnErr) => { + logger.error(clientConnErr, "Client socket error"); + cleanup().catch((err) => { + logger.error(err, "Client conn cleanup"); + }); }); - clientConn.on("end", cleanup); + clientConn.on("end", () => { + cleanup().catch((err) => { + logger.error(err, "Client conn end"); + }); + }); } catch (err) { logger.error(err, "Failed to establish target connection:"); clientConn.end(); @@ -272,12 +281,12 @@ const setupProxyServer = async ({ reject(err); }); - server.on("close", async () => { - await quicClient?.destroy(); + server.on("close", () => { + quicClient?.destroy().catch((err) => { + logger.error(err, "Failed to destroy quic client"); + }); }); - /* eslint-enable */ - server.listen(0, () => { const address = server.address(); if (!address || typeof address === "string") { @@ -293,7 +302,8 @@ const setupProxyServer = async ({ cleanup: async () => { server.close(); await quicClient?.destroy(); - } + }, + getProxyError: () => proxyErrorMsg.join(",") }); }); }); @@ -316,7 +326,7 @@ export const withGatewayProxy = async ( const { relayHost, relayPort, targetHost, targetPort, tlsOptions, identityId, orgId } = options; // Setup the proxy server - const { port, cleanup } = await setupProxyServer({ + const { port, cleanup, getProxyError } = await setupProxyServer({ targetHost, targetPort, relayPort, @@ -330,8 +340,12 @@ export const withGatewayProxy = async ( // Execute the callback with the allocated port await callback(port); } catch (err) { - logger.error(err, "Failed to proxy"); - throw new BadRequestError({ message: (err as Error)?.message }); + const proxyErrorMessage = getProxyError(); + if (proxyErrorMessage) { + logger.error(new Error(proxyErrorMessage), "Failed to proxy"); + } + logger.error(err, "Failed to do gateway"); + throw new BadRequestError({ message: proxyErrorMessage || (err as Error)?.message }); } finally { // Ensure cleanup happens regardless of success or failure await cleanup(); diff --git a/backend/src/lib/turn/credentials.ts b/backend/src/lib/turn/credentials.ts index 817148c30..37dcaa78b 100644 --- a/backend/src/lib/turn/credentials.ts +++ b/backend/src/lib/turn/credentials.ts @@ -1,6 +1,6 @@ import crypto from "node:crypto"; -const TURN_TOKEN_TTL = 60 * 60 * 1000; // 24 hours in milliseconds +const TURN_TOKEN_TTL = 24 * 60 * 60 * 1000; // 24 hours in milliseconds export const getTurnCredentials = (id: string, authSecret: string, ttl = TURN_TOKEN_TTL) => { const timestamp = Math.floor((Date.now() + ttl) / 1000); const username = `${timestamp}:${id}`; From dcb05a309399bf60166b2338b4273f6235c0e6e2 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 9 Mar 2025 21:13:47 +0530 Subject: [PATCH 13/16] feat: resolved not able to edit sql form due to gateway change --- .../EditDynamicSecretSqlProviderForm.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/components/DynamicSecretListView/EditDynamicSecretForm/EditDynamicSecretSqlProviderForm.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/components/DynamicSecretListView/EditDynamicSecretForm/EditDynamicSecretSqlProviderForm.tsx index 493806258..39655e431 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/components/DynamicSecretListView/EditDynamicSecretForm/EditDynamicSecretSqlProviderForm.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/components/DynamicSecretListView/EditDynamicSecretForm/EditDynamicSecretSqlProviderForm.tsx @@ -36,7 +36,7 @@ const formSchema = z.object({ revocationStatement: z.string().min(1), renewStatement: z.string().optional(), ca: z.string().optional(), - projectGatewayId: z.string().optional() + projectGatewayId: z.string().optional().nullable() }) .partial(), defaultTTL: z.string().superRefine((val, ctx) => { @@ -207,7 +207,7 @@ export const EditDynamicSecretSqlProviderForm = ({ helperText="" >