diff --git a/README.md b/README.md index a35c136..b6d8c41 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ guacd (C, from guacamole-server) ### Connectivity - **Multi-hop SSH tunnels**: chain jump hosts/bastions to reach isolated networks (all session types, including the Proxmox API and console hops) -- **Session sharing**: share tokens for read-only or collaborative access +- **Session sharing**: share links for view-only or collaborative access, plus admin shadowing - **Headless API integration**: create a session over the REST API and hand a browser a ready-to-open URL via a single-use WebSocket ticket, with no OIDC login and no API key in the browser (see [Connecting to a session](docs/api.md#connecting-to-a-session)) - **Encrypted file transfer**: LUKS-encrypted per-session drive storage (RDP), SFTP (SSH) - **Credential variables**: shared credentials across connections entries diff --git a/docs/api.md b/docs/api.md index 74350fc..d3dd3eb 100644 --- a/docs/api.md +++ b/docs/api.md @@ -229,6 +229,7 @@ The `jump_hosts` array defines an ordered chain of SSH bastion hops. Each hop co | `height` | integer | All | Display height in pixels | | `dpi` | integer | All | Display DPI | | `banner` | string | All | Banner message shown before session starts | +| `owner` | string | All | Create the session on behalf of this user (their login email). Admin callers only, see [Creating a session for someone else](#creating-a-session-for-someone-else) | **SPICE fields** (`session_type: spice`, direct connection to a SPICE server): @@ -276,13 +277,33 @@ The token needs `VM.Console` (and `VM.Audit` for node auto-detect) on the target "status": "pending", "client_url": "/client/550e8400-e29b-41d4-a716-446655440000", "ws_url": "/ws/550e8400-e29b-41d4-a716-446655440000", - "share_url": "/client/550e8400-e29b-41d4-a716-446655440000?token=abc123" + "share_url": "/client/550e8400-e29b-41d4-a716-446655440000?token=abc123", + "share_url_readonly": "/client/550e8400-e29b-41d4-a716-446655440000?token=def456" } ``` - `client_url` opens the session in the built-in client (see [Connecting to a session](#connecting-to-a-session)). - `ws_url` is the raw WebSocket endpoint for a custom client. -- `share_url` is present only when sharing is allowed; its `token` lets a second viewer **join** an active session (it is not owner access). +- `share_url` is present only when sharing is allowed; its `token` lets a second viewer **join** an active session with keyboard, mouse and clipboard control (it is not owner access). +- `share_url_readonly` is present whenever `share_url` is. It joins the same session view-only: guacd ignores that participant's keyboard, mouse and clipboard input. It is a separate token, so the recipient cannot turn it into the control link. +- Both share URLs are shown only to the session's creator and to admins. + +### Creating a session for someone else + +A backend that creates sessions with an API key and then opens them in a user's own browser (where the user is logged in through OIDC) must name that user as the session's `owner`: + +```json +{ + "session_type": "ssh", + "hostname": "10.0.0.5", + "owner": "engineer@example.com" +} +``` + +- The session is recorded as created by `owner`, so session history and reports name the person rather than the API key. +- Only `owner` can make the first connection to the session (see [Owner vs. join](#owner-vs-join)). The match is on the login email, ignoring case. +- `owner` is accepted from admin callers only (API keys are always admin). Anyone else gets `403`. +- Without `owner`, the session belongs to the caller, as before. ### `GET /api/sessions` @@ -296,6 +317,27 @@ Get session details. Requires **operator** role or higher. Terminate a session. Requires **operator** role or higher. Non-admins can only delete their own sessions. +### `POST /api/sessions/:id/shadow` + +Mint a short-lived token that lets an admin join another user's **active** session. Requires **admin** role. The body is optional: + +```json +{ "read_only": true } +``` + +`read_only` defaults to `false` (keyboard and mouse control). With `true` the admin watches without being able to type or click. + +```json +{ + "url": "/client/550e8400-e29b-41d4-a716-446655440000?token=...", + "expires_at": "2026-10-01T04:10:00+00:00", + "ttl_seconds": 600, + "read_only": true +} +``` + +The token is valid for 10 minutes and every use is written to the audit log. A session whose owner has not connected yet returns `409`: the owner must connect first. + ### `GET /api/sessions/:id/banner` Get session banner text. Authenticates via share token (not credentials). Used for the ephemeral keypair banner display. @@ -306,10 +348,10 @@ Creating a session (`POST /api/sessions`) only opens the connection to the targe ### Owner vs. join -- The **first** connection to a freshly created session is the **owner** connection. It requires an authenticated identity with the **operator** role or higher. -- A **share token** (`share_url`) only lets a second viewer **join** a session that is already active. It is not an identity and cannot open the owner connection. +- The **first** connection to a freshly created session is the **owner** connection. It requires an authenticated identity with the **operator** role or higher that is the session's creator: the caller that created it, or the `owner` it was created for. No other user can make it, admins included. +- A **share token** (`share_url` or `share_url_readonly`) or an admin **shadow token** only lets another viewer **join** a session that is already active. It is not an identity and cannot open the owner connection. -If the owner connection is not authenticated, rustguac rejects the WebSocket with `403`, no browser attaches, and guacd eventually reports `User is not responding` (its timeout for a session whose client never arrived, roughly 15 seconds after creation). If you see `User is not responding`, the browser did not connect as an authenticated owner. +If the owner connection is not authenticated, or is authenticated as someone other than the creator, rustguac rejects the WebSocket with `403`, no browser attaches, and guacd eventually reports `User is not responding` (its timeout for a session whose client never arrived, roughly 15 seconds after creation). If you see `User is not responding`, the browser did not connect as the session's authenticated owner. A backend that creates sessions with an API key for a user who then connects with their own login must set `owner` (see [Creating a session for someone else](#creating-a-session-for-someone-else)). ### Authenticating the owner connection @@ -339,7 +381,7 @@ The ticket is valid for 30 seconds, may be used once, and inherits the caller's When the browser has no rustguac login of its own (no OIDC cookie), a backend that holds an API key can still hand off a ready-to-open session without exposing that key to the browser: 1. `POST /api/sessions` (Bearer API key) to create the session. -2. `POST /api/ws-ticket` (Bearer API key) to mint a ticket. +2. `POST /api/ws-ticket` (Bearer API key) to mint a ticket. The ticket carries the API key's identity, which is the session's creator, so the owner connection is accepted. Do not set `owner` in this flow: the ticket would then belong to someone other than the owner and be refused. 3. Send the browser to `client_url?ticket=` (i.e. `/client/{id}?ticket=wst_...`). The single-use, 30-second ticket is safe to place in a URL; the durable API key never leaves the backend. Because guacd drops a session whose client has not attached within ~15 seconds, mint the ticket and open the browser promptly after creating the session (on a reload, mint a fresh ticket). diff --git a/docs/overview.md b/docs/overview.md index c8375cf..ba6e501 100644 --- a/docs/overview.md +++ b/docs/overview.md @@ -43,7 +43,7 @@ rustguac and Apache Guacamole share the same foundation: | **Network allowlists** | Not supported | CIDR allowlists per protocol | | **Rate limiting** | Not built-in | Per-IP, per-endpoint (tower_governor) | | **Reverse proxy integration** | Generic | HAProxy + Knocknoc examples | -| **Session sharing** | Connection sharing | Share tokens (read-only or collaborative) | +| **Session sharing** | Connection sharing | Share links (view-only or collaborative) | | **Clipboard control** | Not per-connection | Per-entry disable copy/paste | | **Web session autofill** | Not supported | Native Chromium autofill from Vault credentials | | **Web domain allowlist** | Not supported | Per-entry domain restriction via --host-rules | diff --git a/docs/security.md b/docs/security.md index 2d26ffc..f3db60a 100644 --- a/docs/security.md +++ b/docs/security.md @@ -172,8 +172,8 @@ Additionally, user API token operations are logged to a persistent `token_audit_ - **Pending timeout** — sessions that don't receive a WebSocket connection within 60 seconds (configurable) are automatically cleaned up - **Maximum duration** — active sessions are terminated after 8 hours (configurable) to prevent abandoned sessions -- **Session ownership** — non-admin users can only terminate their own sessions -- **Share tokens** — read-only or collaborative access via time-limited share URLs +- **Session ownership** — non-admin users can only terminate their own sessions. Only the user who created a session can make its first (owner) connection; admins cannot claim another user's pending session, and watch live sessions through shadow tokens instead +- **Share tokens** — each shareable session has two share URLs, one view-only and one with keyboard and mouse control, valid until the session ends. View-only is enforced by guacd, which ignores input from a view-only participant ## Clipboard control diff --git a/src/api.rs b/src/api.rs index 4a369d7..8024a95 100644 --- a/src/api.rs +++ b/src/api.rs @@ -30,7 +30,7 @@ pub async fn create_session( Json(req): Json, ) -> impl IntoResponse { let identity = identity.map(|Extension(id)| id); - let admin_name = identity + let caller_name = identity .as_ref() .map(|id| id.display_name().to_string()) .unwrap_or_else(|| "unknown".into()); @@ -46,6 +46,29 @@ pub async fn create_session( } } + // `owner` creates the session on someone else's behalf: they, not the + // caller, are its creator and the only identity that may connect to it + // first. Admin-only, since it hands that user a logged-in session. + let admin_name = match req.owner.as_deref() { + None => caller_name.clone(), + Some(owner) => { + if !identity.as_ref().is_some_and(|id| id.has_role("admin")) { + return ( + StatusCode::FORBIDDEN, + Json(json!({"error": "admin role required to create a session for another owner"})), + ) + .into_response(); + } + match validate_session_owner(owner) { + Ok(o) => o, + Err(msg) => { + return (StatusCode::BAD_REQUEST, Json(json!({ "error": msg }))) + .into_response(); + } + } + } + }; + let proxies = trusted.map(|Extension(t)| t.0).unwrap_or_default(); let client_ip = client_ip(&headers, addr.ip(), &proxies); @@ -95,6 +118,7 @@ pub async fn create_session( tracing::info!( admin = %admin_name, + caller = %caller_name, client_ip = %client_ip, session_type = ?req.session_type, target = %target, @@ -131,6 +155,23 @@ pub async fn create_session( } } +/// Normalise and check the `owner` of a session created on someone's +/// behalf. It is compared against the connecting user's email, so it must +/// look like an identity rather than arbitrary text. +fn validate_session_owner(owner: &str) -> Result { + let owner = owner.trim(); + if owner.is_empty() { + return Err("owner must not be empty"); + } + if owner.len() > 320 { + return Err("owner is too long"); + } + if owner.chars().any(|c| c.is_control() || c.is_whitespace()) { + return Err("owner must not contain whitespace or control characters"); + } + Ok(owner.to_string()) +} + /// Strip share_url from session info unless the caller is the session creator or an admin. fn redact_share_url( mut info: crate::session::SessionInfo, @@ -142,6 +183,7 @@ fn redact_share_url( }; if !is_owner_or_admin { info.share_url = None; + info.share_url_readonly = None; } info } @@ -368,10 +410,20 @@ pub async fn get_session_thumbnail( } } +/// Optional body for `POST /api/sessions/:id/shadow`. +#[derive(Deserialize, Default)] +pub struct ShadowRequest { + /// Join as a view-only viewer. Default false (keyboard and mouse + /// control), which is what shadow has always granted. + #[serde(default)] + pub read_only: bool, +} + /// POST /api/sessions/:id/shadow — Mint a short-lived viewer token /// that lets the caller join another user's active session without that /// user's cooperation. Admin-only. The raw token is returned once and a /// `token_audit_log` entry is written. Tokens expire after 10 minutes. +#[allow(clippy::too_many_arguments)] pub async fn shadow_session( State(manager): State, Extension(database): Extension, @@ -380,7 +432,9 @@ pub async fn shadow_session( identity: Option>, trusted: Option>, Path(id): Path, + body: Option>, ) -> impl IntoResponse { + let read_only = body.map(|Json(b)| b.read_only).unwrap_or(false); let id_inner = match identity { Some(Extension(ref id)) if id.has_role("admin") => id.clone(), _ => { @@ -404,9 +458,16 @@ pub async fn shadow_session( }; let admin_email = id_inner.display_name().to_string(); - let (raw, expires_at) = match manager.mint_shadow_token(id, &admin_email).await { - Some(t) => t, - None => { + let (raw, expires_at) = match manager.mint_shadow_token(id, &admin_email, read_only).await { + Ok(t) => t, + Err(crate::session::SessionError::NotActive) => { + return ( + StatusCode::CONFLICT, + Json(json!({"error": "session hasn't started yet; its owner must connect before it can be shadowed"})), + ) + .into_response(); + } + Err(_) => { return ( StatusCode::NOT_FOUND, Json(json!({"error": "session not found"})), @@ -419,9 +480,10 @@ pub async fn shadow_session( let proxies = trusted.map(|Extension(t)| t.0).unwrap_or_default(); let ip = client_ip(&headers, addr.ip(), &proxies).to_string(); let details = format!( - "session_id={}, owner={}, expires_at={}", + "session_id={}, owner={}, read_only={}, expires_at={}", id, info.created_by, + read_only, expires_at.to_rfc3339() ); let db_clone = database.clone(); @@ -453,6 +515,7 @@ pub async fn shadow_session( "url": url, "expires_at": expires_at.to_rfc3339(), "ttl_seconds": 600, + "read_only": read_only, })) .into_response() } @@ -2747,6 +2810,7 @@ pub async fn ab_connect_entry( proxmox_token_secret: ab_entry.proxmox_token_secret, proxmox_verify_tls: ab_entry.proxmox_verify_tls, max_monitors: ab_entry.max_monitors, + owner: None, }; let proxies = trusted.map(|Extension(t)| t.0).unwrap_or_default(); @@ -4567,6 +4631,7 @@ pub async fn quick_connect( proxmox_token_secret: None, proxmox_verify_tls: None, max_monitors: None, + owner: None, }; tracing::info!( @@ -4686,6 +4751,7 @@ pub async fn quick_connect( proxmox_token_secret: None, proxmox_verify_tls: None, max_monitors: None, + owner: None, }; match manager.create_session(create_req, admin_name).await { @@ -5128,4 +5194,21 @@ mod tests { assert!(!is_jpeg_magic(&[0xFF, 0xD7, 0xFF])); assert!(!is_jpeg_magic(&[0xFF, 0xD8, 0xFE])); } + + #[test] + fn session_owner_is_trimmed() { + assert_eq!( + validate_session_owner(" alice@example.com "), + Ok("alice@example.com".to_string()) + ); + } + + #[test] + fn session_owner_rejects_empty_and_junk() { + assert!(validate_session_owner("").is_err()); + assert!(validate_session_owner(" ").is_err()); + assert!(validate_session_owner("alice @example.com").is_err()); + assert!(validate_session_owner("alice\u{7}@example.com").is_err()); + assert!(validate_session_owner(&"a".repeat(321)).is_err()); + } } diff --git a/src/guacd.rs b/src/guacd.rs index 279efb6..24a360d 100644 --- a/src/guacd.rs +++ b/src/guacd.rs @@ -451,12 +451,17 @@ pub async fn connect_and_handshake( /// Opens a new TCP connection to guacd and sends `select` with the connection_id /// instead of a protocol name. guacd routes this to the existing session process, /// allowing multiple users to share the same session. +/// +/// `read_only` is enforced by guacd itself: settings are parsed per joining +/// user, and every protocol skips installing that user's key, mouse and +/// clipboard handlers when its `read-only` arg is true. pub async fn join_connection( guacd_addr: &str, connection_id: &str, width: u32, height: u32, dpi: u32, + read_only: bool, tls: Option<&tokio_rustls::TlsConnector>, ) -> Result { let tcp = tokio::time::timeout(Duration::from_secs(10), TcpStream::connect(guacd_addr)) @@ -497,15 +502,7 @@ pub async fn join_connection( tracing::debug!("Join args: {:?}", args_instruction.args); - // For joining, send empty values for all args (the connection is already configured) - let arg_values: Vec = args_instruction - .args - .iter() - .map(|name| match name.as_str() { - "read-only" => "false".into(), - _ => String::new(), - }) - .collect(); + let arg_values = join_arg_values(&args_instruction.args, read_only); // Send handshake instructions (joining user — h264 inherited from session) send_handshake(&mut stream, width, height, dpi, false).await?; @@ -530,6 +527,19 @@ pub async fn join_connection( Ok(stream) } +/// Values for a joining user's `connect`. Everything is left empty because +/// the connection is already configured, except `read-only`, which guacd +/// applies to this user alone. +fn join_arg_values(arg_names: &[String], read_only: bool) -> Vec { + arg_names + .iter() + .map(|name| match name.as_str() { + "read-only" => if read_only { "true" } else { "false" }.into(), + _ => String::new(), + }) + .collect() +} + /// Optionally wrap a TCP stream in TLS. Returns a boxed GuacdStream. /// Derives the TLS server name from `guacd_addr` (host:port format). async fn wrap_tls( @@ -647,3 +657,24 @@ impl std::fmt::Display for GuacdError { } impl std::error::Error for GuacdError {} + +#[cfg(test)] +mod tests { + use super::*; + + fn names(v: &[&str]) -> Vec { + v.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn join_args_read_write_by_default() { + let got = join_arg_values(&names(&["VERSION_1_5_0", "hostname", "read-only"]), false); + assert_eq!(got, vec!["", "", "false"]); + } + + #[test] + fn join_args_read_only() { + let got = join_arg_values(&names(&["hostname", "read-only", "port"]), true); + assert_eq!(got, vec!["", "true", ""]); + } +} diff --git a/src/session.rs b/src/session.rs index d807e4e..d32b4be 100644 --- a/src/session.rs +++ b/src/session.rs @@ -182,6 +182,10 @@ pub struct CreateSessionRequest { /// is told `secondary-monitors = max_monitors - 1`, which it advertises to /// the client. Default 1 (single monitor). pub max_monitors: Option, + /// Create the session on behalf of this user (their email). They become + /// its creator and the only identity that may connect to it first. + /// Admin callers only; handled by the API handler, never set internally. + pub owner: Option, } /// Session status in the lifecycle. @@ -210,6 +214,11 @@ pub struct SessionInfo { pub client_url: String, #[serde(skip_serializing_if = "Option::is_none")] pub share_url: Option, + /// View-only counterpart of `share_url`: a separate token, so a + /// recipient cannot upgrade it to control by editing the URL. Present + /// exactly when `share_url` is. + #[serde(skip_serializing_if = "Option::is_none")] + pub share_url_readonly: Option, pub ws_url: String, pub hostname: String, pub username: String, @@ -250,6 +259,9 @@ pub struct Session { pub guacd_stream: Option, pub connection_id: String, pub share_token: String, + /// Second share token whose joins are read-only (guacd drops their + /// keyboard, mouse and clipboard input). + pub share_token_ro: String, pub width: u32, pub height: u32, pub active_connections: u32, @@ -310,6 +322,8 @@ pub struct ShadowToken { pub token_hash: String, pub issued_by: String, pub expires_at: DateTime, + /// Joins with this token are view-only. + pub read_only: bool, } /// Result of validating a share-or-shadow token. Callers use this to tell @@ -319,14 +333,29 @@ pub struct ShadowToken { #[derive(Debug, Clone, PartialEq)] pub enum ShareTokenValidation { Invalid, + /// The session's read/write share token. Owner, - Shadow { issued_by: String }, + /// The session's read-only share token. + OwnerReadOnly, + Shadow { + issued_by: String, + read_only: bool, + }, } impl ShareTokenValidation { pub fn is_valid(&self) -> bool { !matches!(self, ShareTokenValidation::Invalid) } + + /// Whether a join made with this token must be view-only. + pub fn is_read_only(&self) -> bool { + match self { + ShareTokenValidation::OwnerReadOnly => true, + ShareTokenValidation::Shadow { read_only, .. } => *read_only, + ShareTokenValidation::Owner | ShareTokenValidation::Invalid => false, + } + } } fn generate_share_token() -> String { @@ -496,6 +525,11 @@ impl Session { } else { None }, + share_url_readonly: if self.share_allowed { + Some(format!("/client/{}?token={}", self.id, self.share_token_ro)) + } else { + None + }, ws_url: format!("/ws/{}", self.id), hostname: self.hostname.clone(), username: self.username.clone(), @@ -1645,6 +1679,7 @@ impl SessionManager { guacd_stream, connection_id, share_token: generate_share_token(), + share_token_ro: generate_share_token(), width, height, active_connections: 0, @@ -1785,9 +1820,11 @@ impl SessionManager { /// Join an active session by opening a new guacd connection. /// Returns a new GuacdStream and the session's cancellation token. + /// A `read_only` join sees the session but guacd ignores its input. pub async fn join_session( &self, id: Uuid, + read_only: bool, ) -> Result<(GuacdStream, CancellationToken), SessionError> { let (connection_id, width, height, cancel) = { let sessions = self.sessions.read().await; @@ -1810,6 +1847,7 @@ impl SessionManager { width, height, 96, + read_only, self.guacd_tls.as_ref(), ) .await @@ -1825,7 +1863,7 @@ impl SessionManager { session.active_connections += 1; } - tracing::info!(session_id = %id, "Viewer joined session"); + tracing::info!(session_id = %id, read_only, "Viewer joined session"); Ok((stream, cancel)) } @@ -1841,6 +1879,7 @@ impl SessionManager { let session = session.lock().await; check_share_token_match( &session.share_token, + &session.share_token_ro, &session.shadow_tokens, token, Utc::now(), @@ -1850,15 +1889,24 @@ impl SessionManager { /// Mint a new short-lived (10 min) shadow token for a session. /// Returns the raw token (hand to admin once) and its expiry. /// Expired tokens on the session are pruned on mint. + /// + /// Refuses a session whose owner has not connected yet. The WebSocket + /// handler hands a pending session's stream to whoever connects first, + /// before any token is looked at, so a shadow link to a pending session + /// would make the admin its owner and lock the real owner out. pub async fn mint_shadow_token( &self, id: Uuid, issued_by: &str, - ) -> Option<(String, DateTime)> { + read_only: bool, + ) -> Result<(String, DateTime), SessionError> { use sha2::{Digest, Sha256}; let sessions = self.sessions.read().await; - let session = sessions.get(&id)?; + let session = sessions.get(&id).ok_or(SessionError::NotFound)?; let mut session = session.lock().await; + if session.status != SessionStatus::Active { + return Err(SessionError::NotActive); + } let now = Utc::now(); session.shadow_tokens.retain(|t| t.expires_at > now); @@ -1872,8 +1920,9 @@ impl SessionManager { token_hash: hash, issued_by: issued_by.to_string(), expires_at, + read_only, }); - Some((raw, expires_at)) + Ok((raw, expires_at)) } /// Decrement active connection count when a WebSocket disconnects. @@ -2318,11 +2367,13 @@ pub enum SessionError { } /// Pure token-matching helper — constant-time comparison of a provided -/// token against the owner's long-lived `share_token` and the in-memory -/// set of short-lived admin shadow tokens. Factored out so the logic is -/// unit-testable without spinning up a full `SessionManager`. +/// token against the session's long-lived share tokens (read/write and +/// read-only) and the in-memory set of short-lived admin shadow tokens. +/// Factored out so the logic is unit-testable without spinning up a full +/// `SessionManager`. pub(crate) fn check_share_token_match( share_token: &str, + share_token_ro: &str, shadow_tokens: &[ShadowToken], provided: &str, now: DateTime, @@ -2337,6 +2388,10 @@ pub(crate) fn check_share_token_match( if bool::from(expected.ct_eq(&provided_digest)) { return ShareTokenValidation::Owner; } + let expected_ro = Sha256::digest(share_token_ro.as_bytes()); + if bool::from(expected_ro.ct_eq(&provided_digest)) { + return ShareTokenValidation::OwnerReadOnly; + } // 2. Short-lived admin shadow tokens (sha256 compared to the // pre-hashed hex stored on the session). @@ -2353,6 +2408,7 @@ pub(crate) fn check_share_token_match( { return ShareTokenValidation::Shadow { issued_by: t.issued_by.clone(), + read_only: t.read_only, }; } } @@ -2438,27 +2494,28 @@ mod tests { token_hash: hex::encode(Sha256::digest(raw.as_bytes())), issued_by: issued_by.to_string(), expires_at, + read_only: false, } } #[test] fn share_token_owner_match() { let now = Utc::now(); - let v = check_share_token_match("owner-secret", &[], "owner-secret", now); + let v = check_share_token_match("owner-secret", "ro-secret", &[], "owner-secret", now); assert_eq!(v, ShareTokenValidation::Owner); } #[test] fn share_token_wrong_returns_invalid() { let now = Utc::now(); - let v = check_share_token_match("owner-secret", &[], "wrong", now); + let v = check_share_token_match("owner-secret", "ro-secret", &[], "wrong", now); assert_eq!(v, ShareTokenValidation::Invalid); } #[test] fn share_token_empty_provided_invalid() { let now = Utc::now(); - let v = check_share_token_match("owner-secret", &[], "", now); + let v = check_share_token_match("owner-secret", "ro-secret", &[], "", now); assert_eq!(v, ShareTokenValidation::Invalid); } @@ -2470,11 +2527,12 @@ mod tests { "admin@example.com", now + chrono::Duration::minutes(5), ); - let v = check_share_token_match("owner-secret", &[shadow], "shadow-raw", now); + let v = check_share_token_match("owner-secret", "ro-secret", &[shadow], "shadow-raw", now); assert_eq!( v, ShareTokenValidation::Shadow { - issued_by: "admin@example.com".into() + issued_by: "admin@example.com".into(), + read_only: false, } ); } @@ -2483,7 +2541,7 @@ mod tests { fn share_token_expired_shadow_rejected() { let now = Utc::now(); let expired = make_shadow("shadow-raw", "admin", now - chrono::Duration::minutes(1)); - let v = check_share_token_match("owner-secret", &[expired], "shadow-raw", now); + let v = check_share_token_match("owner-secret", "ro-secret", &[expired], "shadow-raw", now); assert_eq!(v, ShareTokenValidation::Invalid); } @@ -2492,7 +2550,13 @@ mod tests { // Boundary: expires_at <= now must reject. let now = Utc::now(); let at_boundary = make_shadow("shadow-raw", "admin", now); - let v = check_share_token_match("owner-secret", &[at_boundary], "shadow-raw", now); + let v = check_share_token_match( + "owner-secret", + "ro-secret", + &[at_boundary], + "shadow-raw", + now, + ); assert_eq!(v, ShareTokenValidation::Invalid); } @@ -2504,11 +2568,12 @@ mod tests { let b = make_shadow("bbb", "admin2", ttl); let c = make_shadow("ccc", "admin3", ttl); let shadows = vec![a, b, c]; - let v = check_share_token_match("owner", &shadows, "bbb", now); + let v = check_share_token_match("owner", "ro-secret", &shadows, "bbb", now); assert_eq!( v, ShareTokenValidation::Shadow { - issued_by: "admin2".into() + issued_by: "admin2".into(), + read_only: false, } ); } @@ -2519,20 +2584,53 @@ mod tests { // the owner path takes precedence (owner is checked first). let now = Utc::now(); let shadow = make_shadow("collide", "admin", now + chrono::Duration::minutes(5)); - let v = check_share_token_match("collide", &[shadow], "collide", now); + let v = check_share_token_match("collide", "ro-secret", &[shadow], "collide", now); assert_eq!(v, ShareTokenValidation::Owner); } #[test] fn share_token_validation_is_valid_helper() { assert!(ShareTokenValidation::Owner.is_valid()); + assert!(ShareTokenValidation::OwnerReadOnly.is_valid()); assert!(ShareTokenValidation::Shadow { - issued_by: "x".into() + issued_by: "x".into(), + read_only: false, } .is_valid()); assert!(!ShareTokenValidation::Invalid.is_valid()); } + #[test] + fn share_token_readonly_match() { + let now = Utc::now(); + let v = check_share_token_match("owner-secret", "ro-secret", &[], "ro-secret", now); + assert_eq!(v, ShareTokenValidation::OwnerReadOnly); + assert!(v.is_read_only()); + } + + #[test] + fn share_token_read_write_is_not_read_only() { + let now = Utc::now(); + let v = check_share_token_match("owner-secret", "ro-secret", &[], "owner-secret", now); + assert!(!v.is_read_only()); + } + + #[test] + fn share_token_read_only_shadow_carries_flag() { + let now = Utc::now(); + let mut shadow = make_shadow("shadow-raw", "admin", now + chrono::Duration::minutes(5)); + shadow.read_only = true; + let v = check_share_token_match("owner-secret", "ro-secret", &[shadow], "shadow-raw", now); + assert_eq!( + v, + ShareTokenValidation::Shadow { + issued_by: "admin".into(), + read_only: true, + } + ); + assert!(v.is_read_only()); + } + // ── SessionManager async tests (in-memory, no disk/browser/guacd) ── // // These exercise the mint → validate → audit path end-to-end without @@ -2551,6 +2649,7 @@ mod tests { guacd_stream: None, connection_id: "conn-test".into(), share_token: share_token.to_string(), + share_token_ro: format!("{share_token}-ro"), width: 1024, height: 768, active_connections: 0, @@ -2617,7 +2716,7 @@ mod tests { let id = insert_session(&mgr, seed_test_session("owner-secret")).await; let (raw, expires_at) = mgr - .mint_shadow_token(id, "admin@example.com") + .mint_shadow_token(id, "admin@example.com", false) .await .expect("mint"); assert!(expires_at > Utc::now()); @@ -2627,8 +2726,12 @@ mod tests { // Shadow validates and returns the issuer. match mgr.validate_share_token(id, &raw).await { - ShareTokenValidation::Shadow { issued_by } => { - assert_eq!(issued_by, "admin@example.com") + ShareTokenValidation::Shadow { + issued_by, + read_only, + } => { + assert_eq!(issued_by, "admin@example.com"); + assert!(!read_only); } other => panic!("expected Shadow variant, got {:?}", other), } @@ -2641,7 +2744,10 @@ mod tests { let mgr = new_manager_for_tests(); let id_a = insert_session(&mgr, seed_test_session("owner-a")).await; let id_b = insert_session(&mgr, seed_test_session("owner-b")).await; - let (raw, _) = mgr.mint_shadow_token(id_a, "admin").await.expect("mint"); + let (raw, _) = mgr + .mint_shadow_token(id_a, "admin", false) + .await + .expect("mint"); assert_eq!( mgr.validate_share_token(id_b, &raw).await, ShareTokenValidation::Invalid @@ -2658,6 +2764,7 @@ mod tests { token_hash: "deadbeef".into(), issued_by: "stale".into(), expires_at: Utc::now() - chrono::Duration::hours(1), + read_only: false, }); mgr.sessions .write() @@ -2665,7 +2772,7 @@ mod tests { .insert(id, Arc::new(Mutex::new(session))); // Mint pruning is documented on mint_shadow_token. - let _ = mgr.mint_shadow_token(id, "admin").await.unwrap(); + let _ = mgr.mint_shadow_token(id, "admin", false).await.unwrap(); let sessions = mgr.sessions.read().await; let guard = sessions.get(&id).unwrap().lock().await; @@ -2673,6 +2780,68 @@ mod tests { assert_ne!(guard.shadow_tokens[0].issued_by, "stale"); } + #[tokio::test] + async fn manager_readonly_share_token_validates() { + let mgr = new_manager_for_tests(); + let id = insert_session(&mgr, seed_test_session("owner-secret")).await; + let v = mgr.validate_share_token(id, "owner-secret-ro").await; + assert_eq!(v, ShareTokenValidation::OwnerReadOnly); + } + + #[test] + fn session_info_exposes_both_share_urls_only_when_allowed() { + let mut session = seed_test_session("rw"); + let info = session.info(); + assert_eq!( + info.share_url.as_deref(), + Some(format!("/client/{}?token=rw", session.id).as_str()) + ); + assert_eq!( + info.share_url_readonly.as_deref(), + Some(format!("/client/{}?token=rw-ro", session.id).as_str()) + ); + + session.share_allowed = false; + let info = session.info(); + assert!(info.share_url.is_none()); + assert!(info.share_url_readonly.is_none()); + } + + #[tokio::test] + async fn manager_mint_read_only_shadow() { + let mgr = new_manager_for_tests(); + let id = insert_session(&mgr, seed_test_session("owner")).await; + let (raw, _) = mgr.mint_shadow_token(id, "admin", true).await.unwrap(); + assert!(mgr.validate_share_token(id, &raw).await.is_read_only()); + } + + /// H3: shadowing a session nobody has connected to yet would make the + /// admin its owner (the stream goes to the first connection), so mint + /// must refuse it. + #[tokio::test] + async fn manager_mint_refuses_pending_session() { + let mgr = new_manager_for_tests(); + let mut seed = seed_test_session("owner"); + seed.status = SessionStatus::Pending; + let id = insert_session(&mgr, seed).await; + assert!(matches!( + mgr.mint_shadow_token(id, "admin", false).await, + Err(SessionError::NotActive) + )); + let sessions = mgr.sessions.read().await; + let guard = sessions.get(&id).unwrap().lock().await; + assert!(guard.shadow_tokens.is_empty()); + } + + #[tokio::test] + async fn manager_mint_unknown_session_not_found() { + let mgr = new_manager_for_tests(); + assert!(matches!( + mgr.mint_shadow_token(Uuid::new_v4(), "admin", false).await, + Err(SessionError::NotFound) + )); + } + #[tokio::test] async fn manager_validate_rejects_unknown_session() { let mgr = new_manager_for_tests(); diff --git a/src/websocket.rs b/src/websocket.rs index 01addab..a1bbc5e 100644 --- a/src/websocket.rs +++ b/src/websocket.rs @@ -91,16 +91,34 @@ pub async fn ws_handler( } } - // Check if this is an owner connection (session is Pending) + // Check if this is an owner connection (session is Pending). The first + // connection to a pending session takes its guacd stream, already logged + // in to the target, so only the session's creator may make it. There is + // deliberately no admin bypass: admins watch a live session through a + // shadow token instead, and an admin "rescuing" a pending session would + // be indistinguishable from hijacking it. let is_owner = manager.is_session_pending(session_id).await; if is_owner { - // Owner path: require authenticated identity with operator+ role + let creator = manager.get_session_creator(session_id).await; match &identity { - Some(id) if id.has_role("operator") => { - // Authorized — proceed + Some(id) if id.has_role("operator") && is_session_owner(id, creator.as_deref()) => {} + Some(id) => { + tracing::warn!( + session_id = %session_id, + client_ip = %ip, + identity = %id.display_name(), + "Owner connection refused: not the session's creator" + ); + return ( + StatusCode::FORBIDDEN, + axum::Json( + json!({"error": "only the user who created this session can connect to it before it starts"}), + ), + ) + .into_response(); } - _ => { + None => { tracing::warn!(session_id = %session_id, client_ip = %ip, "Unauthorized owner connection attempt"); return ( StatusCode::FORBIDDEN, @@ -121,6 +139,7 @@ pub async fn ws_handler( handle_ws( manager, session_id, + is_owner, query.token, socket, ip, @@ -131,19 +150,26 @@ pub async fn ws_handler( .into_response() } +#[allow(clippy::too_many_arguments)] async fn handle_ws( manager: Arc, session_id: Uuid, + owner_verified: bool, token: Option, ws: WebSocket, client_addr: IpAddr, identity_name: Option, database: Option, ) { - // Try to take the guacd stream (owner/first connection) - let (guacd_stream, cancel) = if let Some((stream, cancel)) = + // Try to take the guacd stream (owner/first connection). Only a + // connection that passed the owner check at upgrade time may, so the + // stream can never go to a caller who was vetted as a joiner. + let owned = if owner_verified { manager.take_guacd_stream(session_id).await - { + } else { + None + }; + let (guacd_stream, cancel) = if let Some((stream, cancel)) = owned { let identity_str = identity_name.as_deref().unwrap_or("unknown"); tracing::info!(session_id = %session_id, client_ip = %client_addr, identity = %identity_str, "Session owner connected"); (stream, cancel) @@ -164,8 +190,11 @@ async fn handle_ws( tracing::warn!(session_id = %session_id, client_ip = %client_addr, "Share token rejected"); return; } - ShareTokenValidation::Owner => {} - ShareTokenValidation::Shadow { issued_by } => { + ShareTokenValidation::Owner | ShareTokenValidation::OwnerReadOnly => {} + ShareTokenValidation::Shadow { + issued_by, + read_only, + } => { // Audit every shadow-token use (not just the mint). A leaked // token remains reusable within its TTL, but each reuse is // now visible in token_audit_log with the connecting IP. @@ -173,7 +202,10 @@ async fn handle_ws( let db_clone = db.clone(); let ip_str = client_addr.to_string(); let issued_by = issued_by.clone(); - let details = format!("session_id={}, issued_by={}", session_id, issued_by); + let details = format!( + "session_id={}, issued_by={}, read_only={}", + session_id, issued_by, read_only + ); let _ = tokio::task::spawn_blocking(move || { if let Err(e) = db::log_token_event( &db_clone, @@ -198,9 +230,10 @@ async fn handle_ws( } } - match manager.join_session(session_id).await { + let read_only = validation.is_read_only(); + match manager.join_session(session_id, read_only).await { Ok((stream, cancel)) => { - tracing::info!(session_id = %session_id, client_ip = %client_addr, "Viewer connected via share token"); + tracing::info!(session_id = %session_id, client_ip = %client_addr, read_only, "Viewer connected via share token"); (stream, cancel) } Err(e) => { @@ -584,6 +617,17 @@ async fn ws_to_guacd( /// If either value is missing or empty, the request is allowed — matches /// the prior `unwrap_or("")` behaviour where axum had no header and no /// CSWSH signal. The caller must still ensure auth is enforced separately. +/// Whether `identity` is the creator of a session. Emails are compared +/// without regard to ASCII case, since an identity provider and an API +/// client sending `owner` on someone's behalf may capitalise the same +/// address differently. +pub(crate) fn is_session_owner(identity: &AuthIdentity, creator: Option<&str>) -> bool { + match creator { + Some(c) => !c.is_empty() && identity.display_name().eq_ignore_ascii_case(c), + None => false, + } +} + pub(crate) fn origin_host_matches(origin: &str, host: &str) -> bool { let origin_host = origin .trim_start_matches("https://") @@ -603,6 +647,64 @@ pub(crate) fn origin_host_matches(origin: &str, host: &str) -> bool { mod tests { use super::*; + fn user(email: &str, role: &str) -> AuthIdentity { + AuthIdentity::User { + email: email.into(), + role: role.into(), + groups: Vec::new(), + } + } + + #[test] + fn owner_is_creator() { + assert!(is_session_owner( + &user("alice@example.com", "operator"), + Some("alice@example.com") + )); + } + + #[test] + fn owner_match_ignores_ascii_case() { + assert!(is_session_owner( + &user("Alice@Example.com", "operator"), + Some("alice@example.com") + )); + } + + /// H3: another user, even an admin, must not claim a pending session. + #[test] + fn non_creator_is_not_owner_even_if_admin() { + assert!(!is_session_owner( + &user("bob@example.com", "operator"), + Some("alice@example.com") + )); + assert!(!is_session_owner( + &user("root@example.com", "admin"), + Some("alice@example.com") + )); + assert!(!is_session_owner( + &AuthIdentity::ApiKey("ops-key".into()), + Some("alice@example.com") + )); + } + + #[test] + fn api_key_owns_its_own_session() { + assert!(is_session_owner( + &AuthIdentity::ApiKey("ops-key".into()), + Some("ops-key") + )); + } + + #[test] + fn unknown_or_empty_creator_has_no_owner() { + assert!(!is_session_owner(&user("a@example.com", "admin"), None)); + assert!(!is_session_owner( + &AuthIdentity::ApiKey(String::new()), + Some("") + )); + } + #[test] fn origin_match_same_hostname_no_ports() { assert!(origin_host_matches( diff --git a/static/connections.html b/static/connections.html index 1e8e436..fb60e81 100644 --- a/static/connections.html +++ b/static/connections.html @@ -849,7 +849,7 @@ Allow session sharing
- When enabled, users can generate a read-only Share URL for active sessions of this entry. + When enabled, users can share active sessions of this entry with a view-only link or a link that also gives control. Off by default — admins opt in per entry.