Skip to content

feat(pam): view sensitive credentials/password for PAM accounts#5925

Merged
saifsmailbox98 merged 24 commits intomainfrom
saif/pam-153-add-password-view-feature-for-accounts
Apr 7, 2026
Merged

feat(pam): view sensitive credentials/password for PAM accounts#5925
saifsmailbox98 merged 24 commits intomainfrom
saif/pam-153-add-password-view-feature-for-accounts

Conversation

@saifsmailbox98
Copy link
Copy Markdown
Contributor

Context

Provides users with a UI to view account credentials/password with proper audit logging.

Useful for:

  • Post rotation value fetching
  • Using Infisical PAM as the source of truth/inventory for account passwords
  • Legacy/non gateway target resources what require user connecting directly and entering the password

Screenshots

Steps to verify the change

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Updated CLAUDE.md files (if needed)
  • Read the contributing guide

Add a new endpoint and UI for viewing full (unsanitized) PAM account
credentials and resource connection details behind a dedicated
ReadCredentials permission, with optional MFA enforcement and audit logging.
Only show account credentials in the view credentials feature,
not resource connection details, to avoid coupling the two entities.
…s gate

Replace the modal dialog approach with an inline card-based UI. Public
fields (username, host) are always visible, while sensitive fields
(password, private key) are behind a gated area that handles MFA
verification and permission checks in-place.
Move project and user DB lookups inside the MFA-required block so they
are skipped when MFA is not needed. Simplify credentials gate UI by
consolidating loading/MFA-verifying states into the button, adding
descriptive text for non-MFA mode, and removing unnecessary fragments
and section comments.
Replace the dashed-border gate box with a simple full-width button.
Use v3 Button isPending for loading/MFA states with inline MFA status
text. Remove MFA detail leaking to users without permission.
… field defs

Use readLimit instead of writeLimit for the GET credentials endpoint.
Simplify RESOURCE_FIELD_DEFS to only contain sensitive fields since
non-sensitive fields are already rendered by CredentialsContent.
Accounts like SSH certificate auth and AWS IAM have no sensitive
credentials beyond the sanitized view. The API now returns a 400
early (before MFA/audit log), and the frontend uses an account-aware
function to decide whether to show the button.
…ubernetes sensitive check

- Remove error state from RevealState and SensitiveCredentialsGate; all errors
  now show a toast notification and reset to the initial button view
- MFA timeout notification includes popup blocker hint
- Remove redundant "Waiting for MFA..." button text (keep only the p tag)
- Add Kubernetes to hasSensitiveCredentials exclusion list to match frontend
- Remove unused onRetry prop from SensitiveCredentialsGate
- Replace Lottie isPending spinner with LoaderCircleIcon for all loading states
- Loading state shows spinner only, MFA state shows spinner + "Waiting for MFA..."
- Extract ButtonContent component to avoid nested ternary lint error
- Change children prop to React.ReactElement to avoid useless fragment
@linear
Copy link
Copy Markdown

linear bot commented Apr 2, 2026

@maidul98
Copy link
Copy Markdown
Collaborator

maidul98 commented Apr 2, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 2, 2026

Greptile Summary

This PR adds a "View Credentials" feature for PAM accounts, allowing authorized users to reveal sensitive fields (passwords, private keys) that are intentionally omitted from the sanitized account response. The implementation includes a new GET /:accountId/credentials backend endpoint with CASL ABAC permission enforcement, optional MFA gate (one-time session validated and consumed server-side), full audit logging, and a polished frontend flow with MFA popup polling, per-field visibility toggle, and copy-to-clipboard. A new ReadCredentials permission action is wired through the permission system, default roles, and the role management UI.

Key highlights:

  • Permission check runs before credential decryption — authorization is correctly ordered
  • MFA sessions are validated (user ownership, account binding, status) and consumed atomically, preventing replay
  • The isPollingRef guard in useCredentialsReveal prevents concurrent poll iterations from double-fetching — prior concern fully addressed
  • SensitiveCredentialsGate now correctly passes resourceName, accountName, and metadata to ProjectPermissionCan — prior concern fully addressed
  • Minor: mfaSessionId is passed as a URL query parameter on a security-sensitive GET endpoint; low practical risk given one-time-use tokens, but it will appear in server access logs and browser history
  • No user-facing documentation was added for this feature

Confidence Score: 5/5

Safe to merge; all previously flagged P1 concerns are resolved and remaining findings are P2 style/hardening suggestions

Auth, permission, and MFA logic are correctly implemented. Both remaining comments are P2: one is a hardening suggestion about mfaSessionId in a URL query param (low practical risk given one-time-use tokens), and one is a forward-looking note about hasSensitiveCredentials opt-out pattern. Neither blocks merge.

backend/src/ee/routes/v1/pam-account-routers/pam-account-router.ts (mfaSessionId as query param), backend/src/ee/services/pam-account/pam-account-fns.ts (hasSensitiveCredentials default-true)

Important Files Changed

Filename Overview
backend/src/ee/routes/v1/pam-account-routers/pam-account-router.ts Adds GET /:accountId/credentials endpoint with JWT auth, permission checks, and audit logging; mfaSessionId as a URL query parameter is a minor security concern
backend/src/ee/services/pam-account/pam-account-service.ts Adds viewCredentials with CASL permission check, MFA enforcement, one-time session validation, and credential decryption — logic is solid
backend/src/ee/services/pam-account/pam-account-fns.ts Adds hasSensitiveCredentials helper with default-true (opt-out) return; correct for existing types but fragile when adding future resource types
frontend/src/pages/pam/PamAccountByIDPage/components/useCredentialsReveal.ts Well-structured hook with isPollingRef guard against concurrent iterations, proper cleanup on unmount, and clean MFA popup/polling flow
frontend/src/pages/pam/PamAccountByIDPage/components/SensitiveCredentialsGate.tsx Correctly passes accountName, resourceName, and metadata to ProjectPermissionCan for proper ABAC subject matching
frontend/src/pages/pam/PamAccountByIDPage/components/PamAccountCredentialsSection.tsx Adds SensitiveField and MultilineSensitiveField with toggle visibility and copy-to-clipboard; getSensitiveFieldDefs correctly enumerates resource types with hidden fields
backend/src/ee/services/audit-log/audit-log-types.ts Adds PamAccountReadCredentialsEvent with accountId, accountName, resourceId, resourceType — consistent with other PAM account events
backend/src/ee/services/permission/default-roles.ts ReadCredentials added to Admin role only — appropriate for a privileged credential-reveal action
frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx Adds ReadCredentials to PamAccountPolicyActionSchema and PROJECT_PERMISSION_OBJECT with clear label and description

Reviews (2): Last reviewed commit: "fix: add re-entrancy guard to MFA pollin..." | Re-trigger Greptile

Comment thread backend/src/ee/services/audit-log/audit-log-types.ts
Comment thread backend/src/ee/services/pam-account/pam-account-service.ts
…ct conditions to credentials gate

- Change ReadCredentials audit event metadata from resourceName to resourceId
  to match Delete/Rotation/RotationFailed event patterns
- Pass accountName, resourceName, and metadata to SensitiveCredentialsGate
  so ProjectPermissionCan evaluates resource-scoped ReadCredentials policies
@saifsmailbox98
Copy link
Copy Markdown
Contributor Author

@greptile re-review this pr

@saifsmailbox98 saifsmailbox98 requested a review from x032205 April 3, 2026 00:51
@x032205
Copy link
Copy Markdown
Member

x032205 commented Apr 6, 2026

@codex review this PR

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

Comment thread backend/src/ee/services/audit-log/audit-log-types.ts
@saifsmailbox98 saifsmailbox98 requested a review from x032205 April 7, 2026 00:18
Copy link
Copy Markdown
Member

@x032205 x032205 left a comment

Choose a reason for hiding this comment

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

Discovered account without a password throw a 500 error when clicking view password. Zod error I think where strings must be non-empty. Let's just show a front-end error that password does not exist for this account.

Image

@saifsmailbox98 saifsmailbox98 requested a review from x032205 April 7, 2026 16:44
Comment thread backend/src/ee/routes/v1/pam-account-routers/pam-account-router.ts
x032205
x032205 previously approved these changes Apr 7, 2026
Comment thread backend/src/ee/routes/v1/pam-account-routers/pam-account-router.ts
@saifsmailbox98 saifsmailbox98 merged commit 7f84ff4 into main Apr 7, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants