Repository navigation
Add post-auth connection binder hook - #1411
Rui Carmo (rcarmo) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a post-auth ConnectionBinder hook to ironrdp-server, continuing the credential/session hand-off work from #1409 and #1410. The hook is intended for multi-user servers (following the xrdp-sesman model) that authenticate a client with a single public listener and then bind the connection's display/input handlers to a per-user session after credentials are accepted, but before the RDP client loop begins. IronRDP retains protocol ownership while downstream servers keep session lifecycle logic outside the RDP state machine.
Changes:
- Introduce
BoundConnectionstruct and the asyncConnectionBindertrait, and invoke the binder inclient_acceptedafter credential validation to swap the shared display/input handlers. - Wire the binder through
RdpServer(field +set_connection_binder) andRdpServerBuilder(with_connection_binder), and re-export the new types fromlib.rs. - Add
docs/wrdp/auth-delegation.mddocumenting the delegation model.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/ironrdp-server/src/server.rs | Adds BoundConnection/ConnectionBinder, stores the binder on RdpServer, captures authenticated credentials, and swaps display/input handlers post-auth. |
| crates/ironrdp-server/src/builder.rs | Threads the connection binder through the builder state and adds with_connection_binder. |
| crates/ironrdp-server/src/lib.rs | Re-exports BoundConnection and ConnectionBinder. |
| docs/wrdp/auth-delegation.md | New doc describing the post-auth connection binding model. |
The core issue: the binder runs on every client_accepted call, including deactivation-reactivation. Since reactivation AcceptorResults carry no credentials, a configured binder will send access-denied and bail on the first client resize, tearing down an already-authenticated session. It should be guarded to run only on the initial connection.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result.credentials.clone() | ||
| }; | ||
|
|
||
| if let Some(binder) = self.connection_binder.clone() { |
There was a problem hiding this comment.
Fixed on the canonical rcarmo/IronRDP:wrdp branch in 3b05f6fc70d91c2529df8454ce73a292c05195e6: the connection binder is now guarded with !result.reactivation, so it runs only for the initial accept on a TCP connection. Reactivation now skips rebinding and preserves the installed display/input handlers.
|
Closing in favor of #1413, which my agent put together based on Copilot feedback |
…servers (#1413) ## Summary This replaces and consolidates #1409, #1410, #1411, and #1412 into a single branch from `rcarmo:wrdp`. The split PR heads use `wrdp/*` branch names, but the fork now uses the canonical `wrdp` branch. Because Git cannot keep both `refs/heads/wrdp` and `refs/heads/wrdp/*` at the same time, this PR carries the updated commits and review fixes together. ## Why `wrdp` follows the `xrdp-sesman` multi-user architecture model, so it needs a way to hand authenticated credentials from the protocol handshake into the per-user session binder. ## Changes - Adds an async post-auth connection binder hook in `ironrdp-server`. - Scopes reactivation credential reuse to the current TCP connection. - Exposes CredSSP delegated credentials to server consumers via the same `Credentials` shape used by ClientInfo. - Restores the client-facing denial PDU on credential reject/backend-error paths before returning the error. - Clarifies binder docs so they refer to authenticated credentials being available, not only to configured validator acceptance. - Skips binder re-execution during deactivation/reactivation so resize/reactivation does not restart per-user binding or replace handlers mid-session. ## Copilot review follow-up This includes fixes for the Copilot comments from the superseded PRs: - #1409: denial PDU regression and binder doc wording. - #1411: binder should not run again during reactivation. - #1412: denial PDU regression and reactivation rebinding. - #1410 had no inline Copilot review comments. ## Validation - `cargo check -p ironrdp-server -p ironrdp-acceptor` - `git diff --check` --------- Co-authored-by: Rui Carmo <rcarmo@users.noreply.github.com> Co-authored-by: Rui Carmo <rcarmo@rcarmo.com> Co-authored-by: Rui Carmo <rui.carmo@gmail.com> Co-authored-by: Marc-André Moreau <mamoreau@devolutions.net> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This is another portion of auth binding glue following #1409 and #1410