feat(pam): enable Windows RDP access#6132
Merged
Merged
Conversation
Phase 5 of the native RDP client architecture. Unblocks the Windows resource type that was previously disabled pending RDP client work (now landed in the CLI repo as Phases 1-4). Backend: - Remove the temporary BadRequestError throw that blocked Windows resources from creating access sessions. - Add WindowsSessionCredentialsSchema to the session-credentials route's response union. - Add PamResource.Windows to the access route's discriminated union so GatewayAccessResponseSchema is returned for Windows sessions. - In getSessionCredentials, remap the Windows connection-details hostname field to host before responding. Every other resource type already returns host; this lets the gateway's handler dispatch share one code path instead of special-casing Windows. - Redefine WindowsSessionCredentialsSchema with host to match the wire format (it was previously an unused alias of the resource-facing schema with hostname). Frontend: - Remove the Windows/RDP guard on the Access button in PamResourceAccountsSection and PamAccountByIDPage; domain (AD) accounts remain disabled pending a future AD phase. - Add PamResourceType.Windows case in the access modal's CLI-command switch, producing the infisical pam rdp access command. The browser-access option remains hidden for Windows since there is no browser RDP viewer in the MVP. - Add TWindowsSessionCredentials type mirroring the backend schema. Out of scope for this phase (explicitly not ported from the POC): browser RDP viewer (ironrdp-web, PamWindowsRdpPage), session recording / event batches, Active Directory resource type, any WinRM-specific rotation changes beyond what already exists on main.
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
# Conflicts: # backend/src/ee/routes/v1/pam-account-routers/pam-account-router.ts # frontend/src/pages/pam/PamAccountsPage/components/PamAccessAccountModal.tsx
bernie-g
commented
Apr 24, 2026
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, reopen this pull request to trigger a review.
Contributor
Author
|
@claude review once |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72259a99d5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
💬 Discussion in Slack: #pr-review-infisical-6132-feat-pam-enable-windows-rdp-access Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
Adds accountName to the Windows session credentials response so the gateway's RDP MITM bridge can configure its CredSSP acceptor to expect the PAM account name as the username, separate from the actual Windows username injected to the target.
x032205
requested changes
May 2, 2026
…chema Adds a Windows case to the access endpoint's metadata switch so the CLI can pre-fill the username field in the generated .rdp file (matching the SSH/Postgres pattern). Drops the dedicated accountName field from the session credentials response since the gateway can use the existing username from the credentials. Reuses the connection details schema and trims an over-long comment.
The frontend never calls the gateway-only /credentials endpoint, so this type was dead code.
# Conflicts: # backend/src/ee/services/pam-account/pam-account-service.ts
main added gatewayPool to TFeatureSet; without it in getDefaultOnPremFeatures, type:check fails after the merge.
6 tasks
x032205
requested changes
May 4, 2026
Adds recording: sessionRecordingSecrets to the Windows return so RDP sessions support recording, matching the SSH and default branches. Also drops the loose inline cast on connectionDetails — the resourceType discriminator narrows the union without it.
Reverts the un-disable from this PR. We don't want to ship Windows RDP access until session recording is fully implemented; restoring the hard block keeps that off the release path. The rest of the wiring (schemas, gateway, FFI, bridge) stays in place so the block can be flipped back off cleanly when recording lands.
Removed when the recording-config-required check on the access endpoint was reverted in favor of the temporary Windows disable.
Keeping these blocks in place under the temporary Windows disable so they don't need to be re-added when the disable is lifted. The disable uses a cast to avoid narrowing resourceType, so the recording-config check and switch case below stay typecheck-valid.
x032205
approved these changes
May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Unblocks the Windows PAM resource type so users can start RDP sessions against it from the CLI. Removes the temporary "Windows resources cannot be accessed" throw on the access endpoint, adds the Windows variant to the access / session-credentials response schemas, and remaps the schema's `hostname` to `host` on the session-credentials wire so the gateway's handler dispatch can share one code path with every other resource type.
Frontend: un-disables the Access button on Windows accounts and adds the Windows case to the CLI-command generator in the access modal (`infisical pam rdp access ...`). No browser RDP viewer — native client only for MVP.
Pairs with the RDP implementation PR on the cli repo: Infisical/cli#191
Screenshots
(N/A — only surface change is the Access button becoming clickable for Windows accounts and the access modal now generating the `infisical pam rdp access` command for them.)
Steps to verify the change
Type
Checklist