Skip to content

Add post-auth connection binder hook#1411

Closed
Rui Carmo (rcarmo) wants to merge 1 commit into
Devolutions:masterfrom
rcarmo:wrdp/post-auth-connection-binder
Closed

Add post-auth connection binder hook#1411
Rui Carmo (rcarmo) wants to merge 1 commit into
Devolutions:masterfrom
rcarmo:wrdp/post-auth-connection-binder

Conversation

@rcarmo

Copy link
Copy Markdown

This is another portion of auth binding glue following #1409 and #1410

Copilot AI review requested due to automatic review settings July 5, 2026 08:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 BoundConnection struct and the async ConnectionBinder trait, and invoke the binder in client_accepted after credential validation to swap the shared display/input handlers.
  • Wire the binder through RdpServer (field + set_connection_binder) and RdpServerBuilder (with_connection_binder), and re-export the new types from lib.rs.
  • Add docs/wrdp/auth-delegation.md documenting 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() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@piclaw-bot

Copy link
Copy Markdown

Superseded by #1413: #1413

The old split PR head uses a wrdp/* branch name. The fork now uses the canonical wrdp branch, and Git cannot keep both refs/heads/wrdp and refs/heads/wrdp/* at the same time. #1413 carries this PR's changes plus the Copilot review fixes together.

@rcarmo

Copy link
Copy Markdown
Author

Closing in favor of #1413, which my agent put together based on Copilot feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants