Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions backend/src/db/migrations/20260417245211_pam-session-reason.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Knex } from "knex";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore comment placement.

Image

Can we limit the amount of text to a maximum? How do you think it is necessary to show the full reason?

import { TableName } from "../schemas";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore comment placement.
Can you update the docs as well? Should be a quick one.


export async function up(knex: Knex): Promise<void> {
if (await knex.schema.hasTable(TableName.PamSession)) {
const hasCol = await knex.schema.hasColumn(TableName.PamSession, "reason");
if (!hasCol) {
await knex.schema.alterTable(TableName.PamSession, (t) => {
t.text("reason").nullable();
});
}
}
}

export async function down(knex: Knex): Promise<void> {
if (await knex.schema.hasTable(TableName.PamSession)) {
const hasCol = await knex.schema.hasColumn(TableName.PamSession, "reason");
if (hasCol) {
await knex.schema.alterTable(TableName.PamSession, (t) => {
t.dropColumn("reason");
});
}
}
}
3 changes: 2 additions & 1 deletion backend/src/db/schemas/pam-sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export const PamSessionsSchema = z.object({
resourceId: z.string().uuid().nullable().optional(),
encryptedAiInsights: zodBuffer.nullable().optional(),
aiInsightsStatus: z.string().nullable().optional(),
aiInsightsError: z.string().nullable().optional()
aiInsightsError: z.string().nullable().optional(),
reason: z.string().nullable().optional()
});

export type TPamSessions = z.infer<typeof PamSessionsSchema>;
Expand Down
17 changes: 12 additions & 5 deletions backend/src/ee/routes/v1/pam-account-routers/pam-account-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ export const registerPamAccountRouter = async (server: FastifyZodProvider) => {
accountName: z.string().trim(),
projectId: z.string().uuid(),
mfaSessionId: z.string().optional(),
reason: z.string().trim().max(1000).optional(),
duration: z
.string()
.min(1)
Expand Down Expand Up @@ -567,7 +568,8 @@ export const registerPamAccountRouter = async (server: FastifyZodProvider) => {
accountName: req.body.accountName,
projectId: req.body.projectId,
duration: req.body.duration,
mfaSessionId: req.body.mfaSessionId
mfaSessionId: req.body.mfaSessionId,
reason: req.body.reason
},
req.permission
);
Expand All @@ -582,7 +584,8 @@ export const registerPamAccountRouter = async (server: FastifyZodProvider) => {
accountId: response.account.id,
resourceName: req.body.resourceName,
accountName: response.account.name,
duration: req.body.duration ? new Date(req.body.duration).toISOString() : undefined
duration: req.body.duration ? new Date(req.body.duration).toISOString() : undefined,
reason: req.body.reason
}
}
});
Expand Down Expand Up @@ -618,7 +621,8 @@ export const registerPamAccountRouter = async (server: FastifyZodProvider) => {
}),
body: z.object({
projectId: z.string().uuid(),
mfaSessionId: z.string().optional()
mfaSessionId: z.string().optional(),
reason: z.string().trim().max(1000).optional()
}),
response: {
200: z.object({ ticket: z.string() })
Expand All @@ -639,7 +643,8 @@ export const registerPamAccountRouter = async (server: FastifyZodProvider) => {
actorEmail: req.auth.user.email ?? "",
actorName: `${req.auth.user.firstName ?? ""} ${req.auth.user.lastName ?? ""}`.trim(),
auditLogInfo: req.auditLogInfo,
mfaSessionId: req.body.mfaSessionId
mfaSessionId: req.body.mfaSessionId,
reason: req.body.reason
});

await server.services.telemetry
Expand Down Expand Up @@ -708,6 +713,7 @@ export const registerPamAccountRouter = async (server: FastifyZodProvider) => {
accountName: z.string(),
actorEmail: z.string(),
actorName: z.string(),
reason: z.string().nullable().optional(),
auditLogInfo: z.object({
ipAddress: z.string().optional(),
userAgent: z.string().optional(),
Expand Down Expand Up @@ -737,7 +743,8 @@ export const registerPamAccountRouter = async (server: FastifyZodProvider) => {
auditLogInfo: payload.auditLogInfo as AuditLogInfo,
userId,
actorIp: req.realIp ?? "",
actorUserAgent: req.headers["user-agent"] ?? ""
actorUserAgent: req.headers["user-agent"] ?? "",
reason: payload.reason
});
} catch (err) {
logger.error(err, "WebSocket ticket validation failed");
Expand Down
1 change: 1 addition & 0 deletions backend/src/ee/services/audit-log/audit-log-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4923,6 +4923,7 @@ interface PamAccountAccessEvent {
resourceName: string;
accountName: string;
duration?: string;
reason?: string;
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { PamAccountPolicyRuleType } from "./pam-account-policy-enums";

export const PAM_ACCOUNT_POLICY_RULE_SUPPORTED_RESOURCES: Record<PamAccountPolicyRuleType, PamResource[] | "all"> = {
[PamAccountPolicyRuleType.CommandBlocking]: [PamResource.SSH],
[PamAccountPolicyRuleType.SessionLogMasking]: "all"
[PamAccountPolicyRuleType.SessionLogMasking]: "all",
[PamAccountPolicyRuleType.RequireReason]: "all"
};

export const PAM_ACCOUNT_POLICY_RULE_METADATA: Record<PamAccountPolicyRuleType, { name: string; description: string }> =
Expand All @@ -16,5 +17,9 @@ export const PAM_ACCOUNT_POLICY_RULE_METADATA: Record<PamAccountPolicyRuleType,
[PamAccountPolicyRuleType.SessionLogMasking]: {
name: "Session Log Masking",
description: "Mask sensitive data in session logs matching specified patterns"
},
[PamAccountPolicyRuleType.RequireReason]: {
name: "Require Access Reason",
description: "Require users to provide a reason before they can start a session"
}
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export enum PamAccountPolicyRuleType {
CommandBlocking = "command-blocking",
SessionLogMasking = "session-log-masking"
SessionLogMasking = "session-log-masking",
RequireReason = "require-reason"
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,12 @@ const RuleConfigSchema = z.object({
patterns: z.array(re2PatternSchema).min(1).max(20, "A rule can have at most 20 patterns")
});

const RequireReasonConfigSchema = z.object({});

export const PolicyRulesBaseSchema = z.object({
[PamAccountPolicyRuleType.CommandBlocking]: RuleConfigSchema.optional(),
[PamAccountPolicyRuleType.SessionLogMasking]: RuleConfigSchema.optional()
[PamAccountPolicyRuleType.SessionLogMasking]: RuleConfigSchema.optional(),
[PamAccountPolicyRuleType.RequireReason]: RequireReasonConfigSchema.optional()
});

export const PolicyRulesInputSchema = PolicyRulesBaseSchema.refine(
Expand Down
38 changes: 29 additions & 9 deletions backend/src/ee/services/pam-account/pam-account-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,8 @@
actorName,
actorUserAgent,
duration,
mfaSessionId
mfaSessionId,
reason
}: TAccessAccountDTO,
actor: OrgServiceActor
) => {
Expand All @@ -652,9 +653,22 @@
if (!account) {
throw new NotFoundError({
message: `Account with name '${inputAccountName}' not found for resource '${inputResourceName}'`
});
}

const trimmedReason = reason?.trim() || null;

if (account.policyId) {
const policy = await pamAccountPolicyDAL.findById(account.policyId);
const policyRules = (policy?.rules ?? {}) as TPolicyRules;
if (policy?.isActive && policyRules[PamAccountPolicyRuleType.RequireReason] && !trimmedReason) {
throw new BadRequestError({
message: "A reason is required to access this account",
name: "PAM_REASON_REQUIRED"
Comment thread
carlosmonastyrski marked this conversation as resolved.
Outdated
});

Check failure on line 668 in backend/src/ee/services/pam-account/pam-account-service.ts

View check run for this annotation

Claude / Claude Code Review

RequireReason check runs before authorization, leaking policy config

The RequireReason policy check runs before the authorization check in both pam-account-service.ts (access function) and pam-web-access-service.ts (issueWebSocketTicket), leaking policy configuration to authenticated-but-unauthorized users. An attacker who knows a projectId, resourceName, and accountName can determine whether an account has RequireReason enabled: without a reason they get BadRequestError with name='PAM_REASON_REQUIRED' (revealing the policy exists), but with a reason they get the
Comment thread
carlosmonastyrski marked this conversation as resolved.
Outdated
}
}

const fac = APPROVAL_POLICY_FACTORY_MAP[ApprovalPolicyType.PamAccess](ApprovalPolicyType.PamAccess);

const inputs = {
Expand Down Expand Up @@ -819,7 +833,8 @@
resourceId: resource.id,
userId: actor.id,
expiresAt,
startedAt: new Date()
startedAt: new Date(),
reason: trimmedReason
});

// Schedule session expiration job to run at expiresAt
Expand Down Expand Up @@ -853,7 +868,8 @@
accountId: account.id,
resourceId: resource.id,
userId: actor.id,
expiresAt: new Date(Date.now() + duration)
expiresAt: new Date(Date.now() + duration),
reason: trimmedReason
});

if (!gatewayId) {
Expand Down Expand Up @@ -1045,13 +1061,17 @@
const policy = await pamAccountPolicyDAL.findById(account.policyId);
if (policy && policy.isActive) {
const rules = (policy.rules ?? {}) as TPolicyRules;
for (const ruleType of Object.values(PamAccountPolicyRuleType)) {

const gatewayRuleTypes = [
PamAccountPolicyRuleType.CommandBlocking,
PamAccountPolicyRuleType.SessionLogMasking
] as const;
for (const ruleType of gatewayRuleTypes) {
const ruleConfig = rules[ruleType];
if (ruleConfig) {
const supported = PAM_ACCOUNT_POLICY_RULE_SUPPORTED_RESOURCES[ruleType];
if (supported === "all" || supported.includes(resource.resourceType as PamResource)) {
policyRules[ruleType] = ruleConfig;
}
const supported = PAM_ACCOUNT_POLICY_RULE_SUPPORTED_RESOURCES[ruleType];
const isSupported = supported === "all" || supported.includes(resource.resourceType as PamResource);
if (ruleConfig && isSupported) {
policyRules[ruleType] = ruleConfig;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions backend/src/ee/services/pam-account/pam-account-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export type TAccessAccountDTO = {
actorUserAgent: string;
duration: number;
mfaSessionId?: string;
reason?: string;
};

export type TListAccountsDTO = {
Expand Down
34 changes: 29 additions & 5 deletions backend/src/ee/services/pam-web-access/pam-web-access-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ import { TUserDALFactory } from "@app/services/user/user-dal";

import { TPamAccountDALFactory } from "../pam-account/pam-account-dal";
import { decryptAccountCredentials } from "../pam-account/pam-account-fns";
import { TPamAccountPolicyDALFactory } from "../pam-account-policy/pam-account-policy-dal";
import { PamAccountPolicyRuleType } from "../pam-account-policy/pam-account-policy-enums";
import { TPolicyRules } from "../pam-account-policy/pam-account-policy-types";
import { TPamResourceDALFactory } from "../pam-resource/pam-resource-dal";
import { decryptResourceConnectionDetails } from "../pam-resource/pam-resource-fns";
import {
Expand Down Expand Up @@ -64,6 +67,7 @@ const SUPPORTED_WEB_ACCESS_RESOURCES = [PamResource.Postgres, PamResource.SSH, P

type TPamWebAccessServiceFactoryDep = {
pamAccountDAL: Pick<TPamAccountDALFactory, "findById" | "findMetadataByAccountIds">;
pamAccountPolicyDAL: Pick<TPamAccountPolicyDALFactory, "findById">;
pamResourceDAL: Pick<TPamResourceDALFactory, "findById">;
permissionService: Pick<TPermissionServiceFactory, "getProjectPermission">;
auditLogService: Pick<TAuditLogServiceFactory, "createAuditLog">;
Expand Down Expand Up @@ -98,9 +102,11 @@ type THandleWebSocketConnectionDTO = {
actorName: string;
actorIp: string;
actorUserAgent: string;
reason?: string | null;
};
export const pamWebAccessServiceFactory = ({
pamAccountDAL,
pamAccountPolicyDAL,
pamResourceDAL,
permissionService,
auditLogService,
Expand Down Expand Up @@ -160,7 +166,8 @@ export const pamWebAccessServiceFactory = ({
actorEmail,
actorName,
auditLogInfo,
mfaSessionId
mfaSessionId,
reason
}: TIssueWebSocketTicketDTO) => {
const account = await pamAccountDAL.findById(accountId);

Expand All @@ -172,6 +179,19 @@ export const pamWebAccessServiceFactory = ({
throw new NotFoundError({ message: `Account with ID '${accountId}' not found` });
}

const trimmedReason = reason?.trim() || null;

if (account.policyId) {
const policy = await pamAccountPolicyDAL.findById(account.policyId);
const policyRules = (policy?.rules ?? {}) as TPolicyRules;
if (policy?.isActive && policyRules[PamAccountPolicyRuleType.RequireReason] && !trimmedReason) {
throw new BadRequestError({
message: "A reason is required to access this account",
name: "PAM_REASON_REQUIRED"
});
}
}

const resource = await pamResourceDAL.findById(account.resourceId);

if (!resource) {
Expand Down Expand Up @@ -295,7 +315,8 @@ export const pamWebAccessServiceFactory = ({
accountName: account.name,
actorEmail,
actorName,
auditLogInfo
auditLogInfo,
reason: trimmedReason
})
});

Expand Down Expand Up @@ -328,7 +349,8 @@ export const pamWebAccessServiceFactory = ({
actorEmail,
actorName,
actorIp,
actorUserAgent
actorUserAgent,
reason: accessReason
}: THandleWebSocketConnectionDTO): Promise<void> => {
let session: { id: string } | null = null;
let cleanedUp = false;
Expand Down Expand Up @@ -476,7 +498,8 @@ export const pamWebAccessServiceFactory = ({
resourceType: resource.resourceType,
accountId: account.id,
resourceId: resource.id,
userId
userId,
reason: accessReason?.trim() || null
});

await pamSessionExpirationService.scheduleSessionExpiration(session.id, expiresAt);
Expand Down Expand Up @@ -578,7 +601,8 @@ export const pamWebAccessServiceFactory = ({
accountId,
resourceName,
accountName,
duration: expiresAt.toISOString()
duration: expiresAt.toISOString(),
reason: accessReason ?? undefined
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,5 @@ export type TIssueWebSocketTicketDTO = {
actorName: string;
auditLogInfo: AuditLogInfo;
mfaSessionId?: string;
reason?: string;
};
1 change: 1 addition & 0 deletions backend/src/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2954,6 +2954,7 @@ export const registerRoutes = async (

const pamWebAccessService = pamWebAccessServiceFactory({
pamAccountDAL,
pamAccountPolicyDAL,
pamResourceDAL,
permissionService,
auditLogService,
Expand Down
7 changes: 5 additions & 2 deletions frontend/src/hooks/api/pam/mutations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ export type TAccessPamAccountDTO = {
accountName: string;
projectId: string;
duration: string;
reason?: string;
};

export type TAccessPamAccountResponse = {
Expand All @@ -214,7 +215,8 @@ export const useAccessPamAccount = () => {
resourceName,
accountName,
projectId,
duration
duration,
reason
}: TAccessPamAccountDTO) => {
const { data } = await apiRequest.post<TAccessPamAccountResponse>(
"/api/v1/pam/accounts/access",
Expand All @@ -223,7 +225,8 @@ export const useAccessPamAccount = () => {
resourceName,
accountName,
projectId,
duration
duration,
reason
}
);

Expand Down
Loading
Loading