diff --git a/src/api.rs b/src/api.rs index cfe236da..8edab670 100644 --- a/src/api.rs +++ b/src/api.rs @@ -2565,6 +2565,7 @@ pub async fn ab_list_all( "path": folder.path, "has_children": folder.has_children, "description": folder.description, + "display_name": folder.display_name, "path_only": true, "entries": [], })); @@ -2596,6 +2597,7 @@ pub async fn ab_list_all( "path": folder.path, "has_children": folder.has_children, "description": folder.description, + "display_name": folder.display_name, "allowed_groups": config.as_ref().map(|c| &c.allowed_groups), // allowed_users is deliberately not listed: operators see this, // and a user who can open a folder should not learn who else can. @@ -2674,11 +2676,20 @@ pub async fn ab_search_index( Ok(n) => n, Err(_) => continue, }; + // The folder's human title (e.g. the NetBox tenant name), so the + // search results can show it over the folder key like the folder view. + let folder_display = vault + .get_folder_config(&scope, &path) + .await + .ok() + .map(|c| c.display_name) + .filter(|s| !s.is_empty()); for name in &names { if let Ok(entry) = vault.get_entry(&scope, &path, name).await { emitted.push(json!({ "scope": scope, "folder_path": path, + "folder_display_name": folder_display, "entry": crate::vault::EntryInfo::from((name.as_str(), &entry)), })); } @@ -2738,6 +2749,13 @@ pub struct ConnectRequest { pub width: Option, #[serde(default)] pub height: Option, + /// The screen's size in CSS pixels. An entry that opens fullscreen on + /// connect is created at this size instead of the window's, so nothing + /// has to be resized once the fullscreen transition lands. + #[serde(default)] + pub screen_width: Option, + #[serde(default)] + pub screen_height: Option, #[serde(default)] pub dpi: Option, #[serde(default)] @@ -2829,6 +2847,62 @@ pub struct ProbeHostKeyRequest { pub port: Option, } +/// The size a new session is created at. +/// +/// A bitmap (RDP/VNC) entry that opens fullscreen on connect is created at +/// the screen's size, which is the size it will have once fullscreen lands. +/// Until the client's fullscreen gesture fires it scales that picture to fit +/// the window, so the desktop is painted once at its final size and the +/// fullscreen transition brings no resize, no decoder rebuild and no second +/// repaint (#257). Everything else keeps the window's size: a terminal +/// reflows to fullscreen instantly. +/// +/// Both sizes are CSS pixels, which is what the client's post-connect size +/// send uses today, so creation and that send agree. When #238 lands +/// (native/HiDPI resolution, `display_scale`), that send becomes the +/// device-pixel size, and the mod-16 rounding below must then be applied to +/// the final size after the devicePixelRatio multiply, not to the CSS size. +pub fn initial_session_size( + session_type: &str, + fullscreen_on_connect: bool, + window: (Option, Option), + screen: (Option, Option), +) -> (Option, Option) { + let bitmap = matches!(session_type, "rdp" | "vnc"); + let (w, h) = match (bitmap && fullscreen_on_connect, screen) { + (true, (Some(w), Some(h))) if w > 0 && h > 0 => (Some(w), Some(h)), + _ => window, + }; + // RDP's display channel rounds every requested size down to a multiple + // of 16 (patches/007, which prevents the H.264 green band), so the + // client's first post-connect size send always lands on that grid. + // Create the session there already, fullscreen or windowed, so the two + // match: guacd coalesces them and there is no resize, the H.264 decoder + // is not rebuilt and the picture never flashes on connect (#257). VNC + // has no such rounding, so it keeps the exact size. + // + // #238 (not merged at the time of writing): once the session is created + // at the device-pixel size, round that size, i.e. apply round_down_mod16 + // after multiplying by devicePixelRatio, since rounding the CSS size and + // then scaling it would leave the grid again. + if session_type == "rdp" { + (w.map(round_down_mod16), h.map(round_down_mod16)) + } else { + (w, h) + } +} + +/// Round a dimension down to a multiple of 16, with a 16px floor so a tiny +/// screen never rounds to zero. Mirrors guacd's RDP display-channel rounding. +fn round_down_mod16(v: u32) -> u32 { + let r = v - (v % 16); + if r < 16 { + 16 + } else { + r + } +} + /// POST /api/addressbook/folders/:scope/:folder/entries/:entry/connect — Create session from entry. #[allow(clippy::too_many_arguments)] pub async fn ab_connect_entry( @@ -2931,6 +3005,12 @@ pub async fn ab_connect_entry( // Build CreateSessionRequest from the Vault entry + connect request display params. // ConnectRequest credentials override address book values (for prompted credentials). let ab_entry_key = format!("{}/{}/{}", scope, folder, entry); + let (initial_width, initial_height) = initial_session_size( + &ab_entry.session_type, + ab_entry.fullscreen_on_connect.unwrap_or(false), + (req.width, req.height), + (req.screen_width, req.screen_height), + ); let create_req = CreateSessionRequest { session_type, hostname: ab_entry.hostname, @@ -2954,8 +3034,8 @@ pub async fn ab_connect_entry( jump_username: None, jump_password: None, jump_private_key: None, - width: req.width, - height: req.height, + width: initial_width, + height: initial_height, dpi: req.dpi, banner: req.banner.or(ab_entry.banner), enable_drive: ab_entry.enable_drive, @@ -2964,7 +3044,7 @@ pub async fn ab_connect_entry( remote_app_args: ab_entry.remote_app_args, enable_recording: ab_entry.enable_recording, record_typescript: ab_entry.record_typescript, - address_book_entry: Some(ab_entry_key), + address_book_entry: Some(ab_entry_key.clone()), address_book_folder: Some(folder.to_string()), entry_display_name: ab_entry.display_name.clone(), max_recordings: ab_entry.max_recordings, @@ -3025,6 +3105,28 @@ pub async fn ab_connect_entry( "Address book connect requested" ); + // Reuse a Pending session this user already opened for this entry rather + // than spawning a duplicate (e.g. a repeat Connect click). Matched on the + // fully built request above, credentials included: a retry with a + // different password, key or domain (a mistyped prompt) must not get + // the old Pending session, which still holds the old credentials. The + // fingerprint is a hash, compared only, never logged. + if let Some(existing) = manager + .find_reusable_session( + &admin_name, + &ab_entry_key, + &create_req.credential_fingerprint(), + ) + .await + { + tracing::info!( + user = %admin_name, + session_id = %existing.session_id, + "Reusing existing address book session" + ); + return (StatusCode::CREATED, Json(json!(existing))).into_response(); + } + match manager.create_session(create_req, admin_name.clone()).await { Ok(info) => { tracing::info!( @@ -3057,6 +3159,8 @@ pub struct CreateFolderRequest { pub allowed_users: Vec, #[serde(default)] pub description: String, + #[serde(default)] + pub display_name: String, /// "shared" or "instance" (default: "shared") #[serde(default = "default_scope")] pub scope: String, @@ -3177,6 +3281,7 @@ pub async fn ab_create_folder( allowed_groups: req.allowed_groups, allowed_users, description: req.description, + display_name: req.display_name.trim().to_string(), inherit_from_parent: req.inherit_from_parent, }; @@ -3223,6 +3328,8 @@ pub struct UpdateFolderRequest { #[serde(default)] pub description: String, #[serde(default)] + pub display_name: String, + #[serde(default)] pub inherit_from_parent: bool, } @@ -3267,6 +3374,7 @@ pub async fn ab_update_folder( allowed_groups: req.allowed_groups, allowed_users, description: req.description, + display_name: req.display_name.trim().to_string(), inherit_from_parent: req.inherit_from_parent, }; @@ -3325,6 +3433,7 @@ pub async fn ab_get_folder_config( "allowed_groups": cfg.allowed_groups, "allowed_users": cfg.allowed_users, "description": cfg.description, + "display_name": cfg.display_name, "inherit_from_parent": cfg.inherit_from_parent, })) .into_response(), @@ -5780,6 +5889,53 @@ mod tests { use super::*; use std::path::Path; + #[test] + fn fullscreen_sizing_applies_to_bitmap_sessions_only() { + let win = (Some(1710), Some(954)); + let scr = (Some(1710), Some(1112)); + // RDP opening fullscreen: created at the screen size rounded down to + // a multiple of 16, matching guacd's display-channel rounding so there + // is no post-connect resize (#257). VNC is not mod-16 rounded. + assert_eq!( + initial_session_size("rdp", true, win, scr), + (Some(1696), Some(1104)) + ); + assert_eq!(initial_session_size("vnc", true, win, scr), scr); + // A screen that is already mod-16 is created at its exact size. + assert_eq!( + initial_session_size("rdp", true, win, (Some(1920), Some(1088))), + (Some(1920), Some(1088)) + ); + // a terminal reflows instantly: window size, fullscreen or not + assert_eq!(initial_session_size("ssh", true, win, scr), win); + assert_eq!(initial_session_size("telnet", true, win, scr), win); + assert_eq!(initial_session_size("web", true, win, scr), win); + // not fullscreen, or no screen size offered: window size — still on + // the mod-16 grid for RDP, since the client's first size send is + // rounded to it by guacd either way (no resize on connect). + let win16 = (Some(1696), Some(944)); + assert_eq!(initial_session_size("rdp", false, win, scr), win16); + assert_eq!(initial_session_size("rdp", true, win, (None, None)), win16); + assert_eq!( + initial_session_size("rdp", true, win, (Some(0), Some(0))), + win16 + ); + assert_eq!(initial_session_size("vnc", false, win, scr), win); + assert_eq!( + initial_session_size("rdp", false, (None, None), scr), + (None, None) + ); + } + + #[test] + fn round_down_mod16_floors_at_sixteen() { + assert_eq!(round_down_mod16(1710), 1696); + assert_eq!(round_down_mod16(1696), 1696); + assert_eq!(round_down_mod16(16), 16); + assert_eq!(round_down_mod16(15), 16); + assert_eq!(round_down_mod16(0), 16); + } + #[test] fn report_bounds_read_a_bare_date_as_a_whole_local_day() { // Sydney in September: UTC+10, so getTimezoneOffset() is -600. diff --git a/src/import.rs b/src/import.rs index 47cfc35b..08d22fcc 100644 --- a/src/import.rs +++ b/src/import.rs @@ -309,6 +309,7 @@ pub async fn cmd_import_guacamole( allowed_groups: allowed_groups.to_vec(), allowed_users: Vec::new(), description: "Imported from Guacamole".to_string(), + display_name: String::new(), inherit_from_parent: false, }; if let Err(e) = client.put_folder_config(scope, folder, &root_config).await { @@ -339,6 +340,7 @@ pub async fn cmd_import_guacamole( allowed_groups: vec![], allowed_users: Vec::new(), description: format!("Imported from Guacamole: {}", sub), + display_name: String::new(), inherit_from_parent: true, }; if let Err(e) = client.put_folder_config(scope, &full, &cfg).await { diff --git a/src/main.rs b/src/main.rs index 06f018b9..00dd11ab 100644 --- a/src/main.rs +++ b/src/main.rs @@ -668,7 +668,8 @@ const CONTENT_SECURITY_POLICY: &str = "default-src 'self'; \ script-src 'self' 'unsafe-inline'; \ style-src 'self' 'unsafe-inline'; \ connect-src 'self' wss: ws:; \ - img-src 'self' data: blob:"; + img-src 'self' data: blob:; \ + frame-ancestors 'self'"; async fn security_headers( tls: Extension, @@ -678,7 +679,11 @@ async fn security_headers( let mut response = next.run(request).await; let headers = response.headers_mut(); headers.insert("X-Content-Type-Options", "nosniff".parse().unwrap()); - headers.insert("X-Frame-Options", "DENY".parse().unwrap()); + // SAMEORIGIN (not DENY) so the Connections page can host a session in a + // full-viewport same-origin iframe for zero-click fullscreen-on-connect. + // Cross-origin framing stays blocked (clickjacking); CSP frame-ancestors + // 'self' is the modern equivalent for browsers that honour it. + headers.insert("X-Frame-Options", "SAMEORIGIN".parse().unwrap()); headers.insert( "Referrer-Policy", "strict-origin-when-cross-origin".parse().unwrap(), @@ -1396,7 +1401,13 @@ async fn run_server(config: Config, database: Db) { .route("/reports.html", get(serve_branded_page)) .route("/admin.html", get(serve_branded_page)) .route("/tokens.html", get(serve_branded_page)) - .route("/docs.html", get(serve_branded_page)); + .route("/docs.html", get(serve_branded_page)) + // Optional auth only: an anonymous request still gets the page (the + // page itself sends the user to log in), but a logged-in one gets + // the variant with its navigation already right. + .layer(middleware::from_fn(auth::optional_auth)) + .layer(Extension(ws_ticket_store.clone())) + .layer(Extension(database.clone())); // Build full router (all Router<()> at this point) let mut app: Router<()> = Router::new() @@ -1582,23 +1593,63 @@ fn build_guacd_tls(config: &Config) -> Option { Some(tokio_rustls::TlsConnector::from(Arc::new(tls_config))) } -/// One pre-branded HTML page and the ETag its bytes hash to. +/// One pre-branded HTML page, in one variant per role level, each with the +/// ETag its bytes hash to. /// /// The pages are branded once at startup and cannot change until a restart, -/// so the tag is computed there too -- serving one is a string compare. +/// so the tags are computed there too -- serving one is a string compare. +/// +/// The variants differ only in the `data-role-level` attribute written on +/// ``: the stylesheet gates navigation links and admin-only controls on +/// it, so the header is right on first paint for whoever the page is served +/// to, instead of hiding everything gated and showing it after `/api/me` +/// answers, which flashed and reflowed on every navigation. struct BrandedPage { + /// Indexed by role level, 0 (anonymous) to 4 (admin). + variants: Vec, +} + +struct BrandedVariant { html: String, etag: String, } +/// Highest role level, the index of the last page variant. +const MAX_ROLE_LEVEL: u8 = 4; + impl BrandedPage { fn new(html: String) -> Self { use sha2::{Digest, Sha256}; - // Quoted, because an ETag is a quoted-string on the wire. A bare hash - // tends to be ignored rather than rejected, which would look exactly - // like the stale-cache bug this exists to fix. - let etag = format!("\"{}\"", hex::encode(Sha256::digest(html.as_bytes()))); - Self { html, etag } + let variants = (0..=MAX_ROLE_LEVEL) + .map(|level| { + let html = role_variant_html(&html, level); + // Quoted, because an ETag is a quoted-string on the wire. A + // bare hash tends to be ignored rather than rejected, which + // would look exactly like the stale-cache bug this exists to + // fix. + let etag = format!("\"{}\"", hex::encode(Sha256::digest(html.as_bytes()))); + BrandedVariant { html, etag } + }) + .collect(); + Self { variants } + } + + fn variant(&self, level: u8) -> &BrandedVariant { + &self.variants[level.min(MAX_ROLE_LEVEL) as usize] + } +} + +/// Write the caller's role level onto the page's `` tag. A page that +/// has no bare `` start tag is returned unchanged. Extracted for test. +fn role_variant_html(html: &str, level: u8) -> String { + match html.find("") { + Some(i) => format!( + "{}{}", + &html[..i], + level, + &html[i + "".len()..] + ), + None => html.to_string(), } } @@ -1624,10 +1675,15 @@ fn etag_matches(if_none_match: &str, etag: &str) -> bool { /// costs a 304 and a rebuilt one lands on the next load. Without it these /// responses carry no freshness information at all and Chrome caches them /// heuristically, serving a stale page long after a restart. -fn page_response(page: &BrandedPage, headers: &axum::http::HeaderMap) -> axum::response::Response { +fn page_response( + page: &BrandedPage, + level: u8, + headers: &axum::http::HeaderMap, +) -> axum::response::Response { use axum::http::{header, StatusCode}; use axum::response::IntoResponse; + let page = page.variant(level); let cache_headers = [ (header::CACHE_CONTROL, "no-cache"), (header::ETAG, page.etag.as_str()), @@ -1745,7 +1801,12 @@ fn asset_hash( if !url.starts_with('/') || url.starts_with("//") || url.contains('?') || url.contains('#') { return None; } - if !(url.ends_with(".js") || url.ends_with(".css")) { + if !(url.ends_with(".js") + || url.ends_with(".css") + || url.ends_with(".svg") + || url.ends_with(".png") + || url.ends_with(".ico")) + { return None; } let rel = url.trim_start_matches('/'); @@ -1774,14 +1835,21 @@ fn asset_hash( /// Serve a branded HTML page from the pre-processed in-memory map. async fn serve_branded_page( Extension(pages): Extension, + identity: Option>, request: axum::extract::Request, ) -> axum::response::Response { use axum::http::StatusCode; use axum::response::IntoResponse; let path = request.uri().path().trim_start_matches('/'); let key = if path.is_empty() { "index.html" } else { path }; + // Whoever the page is for decides which navigation it carries. An API-key + // user sends no credential on the page GET and gets the anonymous + // variant; the page reconciles that from /api/me. + let level = identity + .map(|Extension(id)| auth::role_level(id.role())) + .unwrap_or(0); match pages.get(key) { - Some(page) => page_response(page, request.headers()), + Some(page) => page_response(page, level, request.headers()), None => StatusCode::NOT_FOUND.into_response(), } } @@ -1794,7 +1862,7 @@ async fn serve_client_page( ) -> axum::response::Response { use axum::response::IntoResponse; match pages.get("client.html") { - Some(page) => page_response(page, &headers), + Some(page) => page_response(page, 0, &headers), // Unreachable: the map always carries the embedded copy. None => Html(include_str!("../static/client.html")).into_response(), } @@ -1835,6 +1903,44 @@ mod tests { use super::*; + #[test] + fn role_variant_writes_level_on_html_tag_only() { + let page = "\n\nxtext"; + let v = role_variant_html(page, 4); + assert!(v.starts_with("\n\n")); + // only the first bare start tag is touched; the literal later on is content + assert_eq!(v.matches("data-role-level").count(), 1); + assert_eq!( + role_variant_html("

no html tag

", 2), + "

no html tag

" + ); + let branded = BrandedPage::new(page.to_string()); + assert_eq!(branded.variants.len(), 5); + assert_ne!(branded.variant(0).etag, branded.variant(4).etag); + assert_eq!( + branded.variant(9).etag, + branded.variant(4).etag, + "levels clamp to admin" + ); + } + + #[test] + fn test_csp_allows_same_origin_framing_only() { + // Zero-click fullscreen loads the client in a same-origin iframe + // (#257), which needs frame-ancestors 'self'; anything wider would + // reopen clickjacking from other origins. + let directives: Vec<&str> = CONTENT_SECURITY_POLICY.split(';').map(str::trim).collect(); + let fa = directives + .iter() + .find(|d| d.starts_with("frame-ancestors ")) + .expect("CSP must carry a frame-ancestors directive"); + let sources: Vec<&str> = fa.split_whitespace().skip(1).collect(); + assert_eq!( + sources, + vec!["'self'"], + "only the same origin may frame the client" + ); + } #[test] fn test_csp_allows_data_and_blob_images() { // Safari and Firefox draw Guacamole image streams through data: URIs @@ -1870,6 +1976,21 @@ mod tests { assert!(out.ends_with(">")); } + #[test] + fn test_version_assets_versions_the_logo() { + // The logo used to be revalidated on every page load, which blanked + // it for a moment on each navigation in some browsers. It is content + // addressed like the scripts now. + let static_path = std::path::Path::new("static"); + let mut hashes = std::collections::HashMap::new(); + let out = version_assets( + "\"\"", + static_path, + &mut hashes, + ); + assert!(out.contains("src=\"/logo.svg?v="), "{out}"); + } + #[test] fn test_version_assets_leaves_everything_else_alone() { let static_path = std::path::Path::new("static"); @@ -1880,8 +2001,8 @@ mod tests { "", // Already says something about this URL. "", - // Not a kind that goes stale behind a stable name. - "", + // Not a kind we version (a page, a download). + "", // No such file: a hash of nothing is worse than none. "", // Traversal, which these paths have no business doing. @@ -1931,6 +2052,7 @@ mod tests { fn test_branded_page_etag_tracks_content() { let a = BrandedPage::new("one".to_string()); let b = BrandedPage::new("two".to_string()); + let (a, b) = (a.variant(0), b.variant(0)); assert_ne!(a.etag, b.etag); // Quoted, or caches ignore it. assert!(a.etag.starts_with('"') && a.etag.ends_with('"')); diff --git a/src/session.rs b/src/session.rs index ef28dcbb..b1b57592 100644 --- a/src/session.rs +++ b/src/session.rs @@ -205,6 +205,61 @@ pub struct CreateSessionRequest { pub wol_wait_time: Option, } +impl CreateSessionRequest { + /// SHA-256 over the target and every credential this request would + /// connect with, as hex. Two requests with the same fingerprint would + /// open the same connection as the same principal, so a fresh Pending + /// session made from one can stand in for the other + /// (`SessionManager::find_reusable_session`). A retry with a different + /// password, key, domain or jump credential gets a different + /// fingerprint and therefore a new session. The hash is never logged + /// or returned to a client, and a secret cannot be recovered from it. + pub fn credential_fingerprint(&self) -> String { + use sha2::{Digest, Sha256}; + let mut h = Sha256::new(); + // Every field is tagged and length-prefixed, so `("ab", "c")` and + // `("a", "bc")` hash differently and an absent field differs from + // an empty one. + let mut field = |tag: &str, value: Option<&str>| { + h.update(tag.as_bytes()); + match value { + Some(v) => { + h.update((v.len() as u64).to_le_bytes()); + h.update(v.as_bytes()); + } + None => h.update([0xffu8; 8]), + } + }; + let port = self.port.map(|p| p.to_string()); + let jump_hosts = self + .jump_hosts + .as_ref() + .and_then(|j| serde_json::to_string(j).ok()); + let jump_port = self.jump_port.map(|p| p.to_string()); + field("type", Some(&format!("{:?}", self.session_type))); + field("host", self.hostname.as_deref()); + field("port", port.as_deref()); + field("user", self.username.as_deref()); + field("pass", self.password.as_deref()); + field("key", self.private_key.as_deref()); + field("domain", self.domain.as_deref()); + field("auth_pkg", self.auth_pkg.as_deref()); + field("kdc", self.kdc_url.as_deref()); + field("url", self.url.as_deref()); + field("jumps", jump_hosts.as_deref()); + field("jump_host", self.jump_host.as_deref()); + field("jump_port", jump_port.as_deref()); + field("jump_user", self.jump_username.as_deref()); + field("jump_pass", self.jump_password.as_deref()); + field("jump_key", self.jump_private_key.as_deref()); + field("container_user", self.container_username.as_deref()); + field("container_pass", self.container_password.as_deref()); + field("proxmox_token_id", self.proxmox_token_id.as_deref()); + field("proxmox_token_secret", self.proxmox_token_secret.as_deref()); + hex::encode(h.finalize()) + } +} + /// Session status in the lifecycle. #[derive(Debug, Clone, Serialize, PartialEq)] #[serde(rename_all = "lowercase")] @@ -325,6 +380,10 @@ pub struct Session { /// (#154). Surfaced verbatim in `SessionInfo` so client.html can /// trigger fullscreen on first user gesture after CONNECTED. pub fullscreen_on_connect: bool, + /// `CreateSessionRequest::credential_fingerprint()` of the request this + /// session was created from. Compared, never logged or served, when a + /// repeat Connect asks to reuse a Pending session. + pub credential_fingerprint: String, /// Copied from the source entry's `autohide_side_tabs` flag. /// Surfaced in `SessionInfo` so client.html can auto-hide the /// clipboard/files side tabs. @@ -789,6 +848,9 @@ impl SessionManager { req: CreateSessionRequest, created_by: String, ) -> Result { + // Taken before any field of the request is moved out below. + let credential_fingerprint = req.credential_fingerprint(); + // Enforce session limits (only count active/pending sessions) { let sessions = self.sessions.read().await; @@ -918,6 +980,8 @@ impl SessionManager { session_id = %session_id, hostname = %hostname, username = %username, + width, + height, "Creating new SSH session" ); @@ -1945,6 +2009,7 @@ impl SessionManager { shadow_tokens: Vec::new(), share_allowed, fullscreen_on_connect: req.fullscreen_on_connect.unwrap_or(false), + credential_fingerprint, autohide_side_tabs: req.autohide_side_tabs.unwrap_or(false), }; @@ -2009,6 +2074,45 @@ impl SessionManager { result } + /// How long after creation a Pending session is offered to a repeat + /// Connect instead of a fresh one. Short on purpose: an older Pending + /// one, whose guacd may since have gone away, is left to the + /// pending-timeout reaper. + pub const REUSABLE_PENDING_SECS: i64 = 15; + + /// Find a live session this user can be sent back to instead of opening a + /// fresh one for the same address-book entry. Reusing a `Pending` session + /// (guacd connected, owner never attached) stops repeat Connect clicks from + /// piling up orphans; only a Pending session is reusable, since an owner + /// connection consumes the guacd stream and cannot be re-owned. + /// + /// `credential_fingerprint` is `CreateSessionRequest::credential_fingerprint()` + /// of the fully built request, so a session is only reused by a request + /// that would connect to the same target with the same credentials. A + /// retry with a different password, key or domain (a mistyped prompt) + /// never gets the old session, which still holds the old credentials. + pub async fn find_reusable_session( + &self, + created_by: &str, + entry_key: &str, + credential_fingerprint: &str, + ) -> Option { + let sessions = self.sessions.read().await; + for session in sessions.values() { + let s = session.lock().await; + if s.status == SessionStatus::Pending + && s.guacd_stream.is_some() + && s.created_by == created_by + && s.address_book_entry.as_deref() == Some(entry_key) + && s.credential_fingerprint == credential_fingerprint + && (Utc::now() - s.created_at).num_seconds() < Self::REUSABLE_PENDING_SECS + { + return Some(s.info()); + } + } + None + } + /// Get a specific session's info. pub async fn get_session(&self, id: Uuid) -> Option { let sessions = self.sessions.read().await; @@ -2915,6 +3019,7 @@ mod tests { shadow_tokens: Vec::new(), share_allowed: true, fullscreen_on_connect: false, + credential_fingerprint: String::new(), autohide_side_tabs: false, } } @@ -3455,6 +3560,182 @@ mod tests { let cfg = DriveConfig::default(); assert_eq!(drive_cleanup_settings(&Some(cfg)), (true, 0)); } + + // ── Pending-session reuse (repeat Connect on an entry) ── + + fn entry_request(extra: serde_json::Value) -> CreateSessionRequest { + let mut body = serde_json::json!({ + "session_type": "rdp", + "hostname": "rdp.example.test", + "port": 3389, + "username": "alice", + "password": "correct horse", + "domain": "CORP", + "address_book_entry": "shared/ops/rdp-1", + }); + body.as_object_mut() + .unwrap() + .extend(extra.as_object().unwrap().clone()); + serde_json::from_value(body).unwrap() + } + + /// A Pending session as `create_session` leaves it for an entry connect: + /// guacd connected (any stream will do here), owner not yet attached. + fn pending_entry_session(req: &CreateSessionRequest, created_by: &str) -> Session { + let (a, _b) = tokio::io::duplex(64); + let mut s = seed_test_session(&format!("tok-{}", Uuid::new_v4())); + s.status = SessionStatus::Pending; + s.guacd_stream = Some(Box::new(a)); + s.created_by = created_by.into(); + s.address_book_entry = req.address_book_entry.clone(); + s.credential_fingerprint = req.credential_fingerprint(); + s + } + + #[test] + fn credential_fingerprint_tracks_target_and_credentials() { + let base = entry_request(serde_json::json!({})); + let same = entry_request(serde_json::json!({})); + assert_eq!(base.credential_fingerprint(), same.credential_fingerprint()); + assert_eq!(base.credential_fingerprint().len(), 64); + // Does not contain a secret in the clear. + assert!(!base.credential_fingerprint().contains("correct horse")); + + for (key, value) in [ + ("password", serde_json::json!("wrong horse")), + ("username", serde_json::json!("bob")), + ("domain", serde_json::json!("OTHER")), + ("private_key", serde_json::json!("-----BEGIN KEY-----")), + ("hostname", serde_json::json!("rdp2.example.test")), + ("port", serde_json::json!(3390)), + ("jump_password", serde_json::json!("hop")), + ] { + let other = entry_request(serde_json::json!({ key: value })); + assert_ne!( + base.credential_fingerprint(), + other.credential_fingerprint(), + "{key} must change the fingerprint" + ); + } + // Display params are not part of it: a second click from a resized + // window still reuses the session. + let resized = entry_request(serde_json::json!({ "width": 800, "height": 600 })); + assert_eq!( + base.credential_fingerprint(), + resized.credential_fingerprint() + ); + // Absent and empty are different, and so is a shifted boundary. + let empty_pw = entry_request(serde_json::json!({ "password": "" })); + let no_pw = { + let mut r = entry_request(serde_json::json!({})); + r.password = None; + r + }; + assert_ne!( + empty_pw.credential_fingerprint(), + no_pw.credential_fingerprint() + ); + let ab_c = entry_request(serde_json::json!({ "username": "ab", "password": "c" })); + let a_bc = entry_request(serde_json::json!({ "username": "a", "password": "bc" })); + assert_ne!(ab_c.credential_fingerprint(), a_bc.credential_fingerprint()); + } + + #[tokio::test] + async fn fresh_pending_session_reused_for_same_entry_and_credentials() { + let mgr = new_manager_for_tests(); + let req = entry_request(serde_json::json!({})); + let id = insert_session(&mgr, pending_entry_session(&req, "alice@example.com")).await; + + let again = entry_request(serde_json::json!({})); + let found = mgr + .find_reusable_session( + "alice@example.com", + "shared/ops/rdp-1", + &again.credential_fingerprint(), + ) + .await + .expect("same entry, same credentials, within the window: reused"); + assert_eq!(found.session_id, id); + } + + #[tokio::test] + async fn retry_with_different_credentials_gets_a_new_session() { + let mgr = new_manager_for_tests(); + let req = entry_request(serde_json::json!({})); + insert_session(&mgr, pending_entry_session(&req, "alice@example.com")).await; + + // A mistyped password at the prompt, retried within 15 s: the old + // Pending session holds the old password, so it must not be handed + // back. + let retry = entry_request(serde_json::json!({ "password": "wrong horse" })); + assert!(mgr + .find_reusable_session( + "alice@example.com", + "shared/ops/rdp-1", + &retry.credential_fingerprint(), + ) + .await + .is_none()); + let other_user = entry_request(serde_json::json!({ "username": "bob" })); + assert!(mgr + .find_reusable_session( + "alice@example.com", + "shared/ops/rdp-1", + &other_user.credential_fingerprint(), + ) + .await + .is_none()); + } + + #[tokio::test] + async fn reuse_is_scoped_to_user_entry_status_and_age() { + let mgr = new_manager_for_tests(); + let req = entry_request(serde_json::json!({})); + let fp = req.credential_fingerprint(); + + insert_session(&mgr, pending_entry_session(&req, "alice@example.com")).await; + // Another user's identical Pending session is not ours to take. + assert!(mgr + .find_reusable_session("mallory@example.com", "shared/ops/rdp-1", &fp) + .await + .is_none()); + // Nor is the same user's session for a different entry. + assert!(mgr + .find_reusable_session("alice@example.com", "shared/ops/rdp-2", &fp) + .await + .is_none()); + + // An owner already attached: the stream is consumed, not re-ownable. + let mgr = new_manager_for_tests(); + let mut active = pending_entry_session(&req, "alice@example.com"); + active.status = SessionStatus::Active; + insert_session(&mgr, active).await; + assert!(mgr + .find_reusable_session("alice@example.com", "shared/ops/rdp-1", &fp) + .await + .is_none()); + + // Older than the window: left to the pending-timeout reaper. + let mgr = new_manager_for_tests(); + let mut stale = pending_entry_session(&req, "alice@example.com"); + stale.created_at = + Utc::now() - chrono::Duration::seconds(SessionManager::REUSABLE_PENDING_SECS + 1); + insert_session(&mgr, stale).await; + assert!(mgr + .find_reusable_session("alice@example.com", "shared/ops/rdp-1", &fp) + .await + .is_none()); + + // No guacd stream (deferred connect): nothing to hand over yet. + let mgr = new_manager_for_tests(); + let mut deferred = pending_entry_session(&req, "alice@example.com"); + deferred.guacd_stream = None; + insert_session(&mgr, deferred).await; + assert!(mgr + .find_reusable_session("alice@example.com", "shared/ops/rdp-1", &fp) + .await + .is_none()); + } } impl std::fmt::Display for SessionError { diff --git a/src/vault.rs b/src/vault.rs index 1cc97ab8..0a614f01 100644 --- a/src/vault.rs +++ b/src/vault.rs @@ -66,6 +66,11 @@ pub struct FolderConfig { pub allowed_users: Vec, #[serde(default)] pub description: String, + /// Human-readable title shown instead of the folder key (e.g. the NetBox + /// tenant name over its sanitised key). Omitted from the stored config + /// when empty, so existing configs are unchanged. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub display_name: String, /// When true, if the folder's own `allowed_groups` doesn't grant access, /// the access check walks up the parent path and tries each ancestor's /// config. New folders default to `true` in the UI; legacy configs @@ -623,6 +628,9 @@ impl From<(&str, &AddressBookEntry)> for EntryInfo { pub struct FolderInfo { pub name: String, pub description: String, + /// Title to show for the folder; empty when none is set (show the name). + #[serde(default, skip_serializing_if = "String::is_empty")] + pub display_name: String, /// "shared" or "instance" pub scope: String, /// Full path from scope root (e.g. "Clients/Acme"). Same as name for top-level folders. @@ -864,6 +872,7 @@ impl VaultClient { folders.push(FolderInfo { name: name.to_string(), description: String::new(), + display_name: String::new(), scope: scope.to_string(), path: Some(name.to_string()), has_children: None, // enriched below @@ -880,6 +889,7 @@ impl VaultClient { for folder in &mut folders { if let Ok(config) = self.get_folder_config(&folder.scope, &folder.name).await { folder.description = config.description; + folder.display_name = config.display_name; } // Check for subfolders by listing children if let Ok(children) = self.list_children(&folder.scope, &folder.name).await { @@ -919,6 +929,7 @@ impl VaultClient { let mut info = FolderInfo { name: name.to_string(), description: String::new(), + display_name: String::new(), scope: scope.to_string(), path: Some(full_path.clone()), has_children: None, @@ -926,6 +937,7 @@ impl VaultClient { // Enrich with description if let Ok(config) = self.get_folder_config(scope, &full_path).await { info.description = config.description; + info.display_name = config.display_name; } // Check for grandchildren if let Ok(grandchildren) = self.list_children(scope, &full_path).await { @@ -1698,11 +1710,38 @@ pub fn resolve_credential_variables( mod tests { use super::*; + #[test] + fn folder_config_display_name_is_optional_in_storage() { + // A config written before display names existed must still load, and + // a folder without one must be stored exactly as before, so existing + // Vault .config documents are untouched by the feature. + let legacy: FolderConfig = + serde_json::from_str(r#"{"allowed_groups":["ops"],"description":"Acme"}"#).unwrap(); + assert_eq!(legacy.display_name, ""); + let stored = serde_json::to_value(&legacy).unwrap(); + assert!( + stored.get("display_name").is_none(), + "an empty display name is omitted from the stored config" + ); + + let mut named = legacy.clone(); + named.display_name = "Dani & Fiori's Gourmet Pizzeria".to_string(); + let stored = serde_json::to_value(&named).unwrap(); + assert_eq!( + stored["display_name"], + serde_json::json!("Dani & Fiori's Gourmet Pizzeria") + ); + let back: FolderConfig = serde_json::from_value(stored).unwrap(); + assert_eq!(back.display_name, named.display_name); + assert_eq!(back.description, "Acme"); + } + fn cfg(groups: &[&str], users: &[&str]) -> FolderConfig { FolderConfig { allowed_groups: groups.iter().map(|s| s.to_string()).collect(), allowed_users: users.iter().map(|s| s.to_string()).collect(), description: String::new(), + display_name: String::new(), inherit_from_parent: false, } } diff --git a/static/admin.html b/static/admin.html index 5dea3b79..f173c835 100644 --- a/static/admin.html +++ b/static/admin.html @@ -16,12 +16,13 @@

rustguac

 @@ -155,6 +156,7 @@

Connections Audit Log

return res.json(); }) .then(function(data) { + if (data && data.role) document.documentElement.setAttribute('data-role-level', String({admin:4,poweruser:3,operator:2,viewer:1}[data.role] || 0)); if (!data || data.role !== 'admin') { window.location.href = '/connections.html'; return; @@ -296,10 +298,12 @@

Connections Audit Log

delBtn.textContent = 'delete'; delBtn.addEventListener('click', (function(email) { return function() { - if (!confirm('Delete user ' + email + '?')) return; - fetch('/api/users/' + encodeURIComponent(email), { - method: 'DELETE', headers: apiHeaders(), credentials: 'same-origin' - }).then(function() { loadUsers(); }); + rgConfirm('Delete user ' + email + '?', { danger: true, confirmText: 'Delete' }).then(function(ok) { + if (!ok) return; + fetch('/api/users/' + encodeURIComponent(email), { + method: 'DELETE', headers: apiHeaders(), credentials: 'same-origin' + }).then(function() { loadUsers(); }); + }); }; })(u.email)); actTd.appendChild(delBtn); @@ -454,16 +458,18 @@

Connections Audit Log

revokeBtn.textContent = 'revoke'; revokeBtn.addEventListener('click', (function(id, name, email) { return function() { - if (!confirm('Revoke token "' + name + '" for ' + email + '?')) return; - fetch('/api/admin/user-tokens/' + id, { - method: 'DELETE', - headers: apiHeaders(), - credentials: 'same-origin' - }) - .then(function(res) { - if (!res.ok) return res.json().then(function(d) { showError(d.error); }); - loadAllTokens(); - loadAuditLog(); + rgConfirm('Revoke token "' + name + '" for ' + email + '?', { danger: true, confirmText: 'Revoke' }).then(function(ok) { + if (!ok) return; + fetch('/api/admin/user-tokens/' + id, { + method: 'DELETE', + headers: apiHeaders(), + credentials: 'same-origin' + }) + .then(function(res) { + if (!res.ok) return res.json().then(function(d) { showError(d.error); }); + loadAllTokens(); + loadAuditLog(); + }); }); }; })(t.id, t.name, t.email || '')); @@ -651,5 +657,7 @@

Connections Audit Log

// Auto-refresh setInterval(function() { loadUsers(); loadMappings(); loadAllTokens(); }, 10000); + + diff --git a/static/client.html b/static/client.html index 055d1961..3a262b98 100644 --- a/static/client.html +++ b/static/client.html @@ -68,6 +68,38 @@ cursor: pointer; } #banner-continue:hover { background: var(--primary-hover); } + /* Fullscreen-on-connect gate: covers the session until the browser + is fullscreen, so the first picture the user sees is the one that + fits (the fullscreen desktop), not a scaled-down windowed fit. */ + #fs-gate { + display: none; + position: fixed; + top: 0; left: 0; right: 0; bottom: 0; + background: var(--bg); + z-index: 3100; /* above the side tabs (3001); hidden before the disconnect overlay (3000) is shown */ + justify-content: center; + align-items: center; + cursor: pointer; + } + #fs-gate.visible { display: flex; } + #fs-gate-box { + text-align: center; + font-family: monospace; + color: var(--text); + max-width: 600px; + padding: 2em 3em; + } + #fs-gate-spinner { + width: 30px; height: 30px; margin: 0 auto 1.2em; + border: 3px solid var(--border); + border-top-color: var(--accent); + border-radius: 50%; + animation: fs-gate-spin 0.8s linear infinite; + } + @keyframes fs-gate-spin { to { transform: rotate(360deg); } } + #fs-gate-title { font-size: 1.3em; font-weight: bold; margin-bottom: 0.5em; } + #fs-gate-status { color: var(--text-dim); font-size: 0.95em; min-height: 1.2em; letter-spacing: 0.02em; } + #fs-gate-status.connected { color: var(--accent); } #disconnected-overlay { display: none; position: fixed; @@ -239,6 +271,13 @@

Session Ended

+
+
+
+
+
+
+
+ diff --git a/static/connections.html b/static/connections.html index 4b0f0b24..a40890c3 100644 --- a/static/connections.html +++ b/static/connections.html @@ -17,7 +17,7 @@ border-right: 1px solid var(--border); padding-right: var(--s-4); } - .main { flex: 1; min-width: 0; } + .main { flex: 1; min-width: 0; padding: 0 var(--s-5); } /* Compact "open in new window" companion to the Connect button — a small icon, not a full-size button */ @@ -160,17 +160,32 @@ /* Folder action bar */ .folder-actions { - display: flex; align-items: center; gap: var(--s-2); - margin-bottom: var(--s-4); padding-bottom: var(--s-3); + display: flex; align-items: flex-start; gap: var(--s-3); + padding-bottom: var(--s-2); border-bottom: 1px solid var(--border); flex-wrap: wrap; } .folder-actions strong { font-size: var(--fz-xl); color: var(--text); - font-weight: bold; letter-spacing: 0.02em; line-height: 1.3; + font-weight: bold; letter-spacing: 0.02em; line-height: 1.2; + } + .folder-actions-btns { + display: flex; align-items: center; gap: var(--s-2); + flex-wrap: wrap; flex-shrink: 0; margin-left: auto; + } + .entries-heading { + flex: 1 1 auto; min-width: 0; + display: flex; flex-direction: column; gap: 2px; } + .entries-title-row { + display: flex; align-items: baseline; gap: var(--s-3); + flex-wrap: wrap; min-width: 0; + } + .entries-key { color: var(--text-dim); font-size: var(--fz-xs); letter-spacing: 0.04em; } + .entries-key:empty { display: none; } + .folder-desc.folder-key { letter-spacing: 0.04em; } .folder-actions .desc { - color: var(--text-muted); font-size: var(--fz-md); font-style: italic; + color: var(--text-muted); font-size: var(--fz-sm); font-style: italic; min-width: 0; } /* Narrow desktops (a 1710px-wide MacBook display, 1600px monitors): @@ -216,8 +231,6 @@ display: flex; gap: var(--s-1); align-items: center; } .session-card-info .card-meta .type-badge { font-size: 10px; min-width: 36px; } - /* Proxmox VE brand orange badge */ - .type-badge.type-proxmox { background: #e57000; color: #000; padding: 1px 6px; border-radius: 3px; font-weight: 600; } .session-card.dormant { opacity: 0.75; } .session-card.dormant .session-card-thumb { filter: brightness(0.7); } .card-actions { @@ -535,6 +548,84 @@ .folder-list li.filtered-out { display: none; } .folder-row:focus { outline: none; } .folder-row:focus-visible { outline: 2px solid var(--accent); outline-offset: -2px; } + + /* UX (#254) app shell: the header stays put, the sidebar and the + entries area are two independent scroll panes that always fit the + viewport. The earlier sticky sidebar was sized from the viewport, + not from where it starts below the header, so at the top of the + page its bottom hung off screen and the page had to be scrolled + before the list could be scrolled to its end. */ + html, body { height: 100%; } + /* overflow: clip, not hidden: hidden still lets scripts scroll the + body (scrollIntoView on anything inside), which moved the header + out of view; clip makes the body unscrollable for good. The border + box is the full height so the page padding is inside the viewport. */ + body { display: flex; flex-direction: column; overflow: clip; min-height: 0; box-sizing: border-box; } + #main-content { flex: 1 1 auto; min-height: 0; display: flex; flex-direction: column; } + #main-content > :not(.layout) { flex: none; } + .layout { flex: 1 1 auto; min-height: 0; align-items: stretch; } + .sidebar { + position: static; top: auto; align-self: auto; + max-height: none; height: auto; min-height: 0; + display: flex; flex-direction: column; + overflow-y: auto; overflow-x: hidden; + padding: 0; margin: 0; + border: 1px solid var(--border); border-radius: var(--radius); + background: rgba(255,255,255,0.015); + } + .main { min-height: 0; overflow-y: auto; overflow-x: hidden; padding-left: var(--s-5); padding-right: var(--s-5); } + /* The folder header (name, description, search, actions, filters) + pins to the top of the entries pane, so it stays under the nav and + the active-sessions strip while a long table scrolls beneath it. */ + #entries-header { + position: sticky; top: 0; z-index: 30; + background: var(--bg); + padding: var(--s-3) var(--s-4) var(--s-2); + margin-bottom: var(--s-3); + box-shadow: 0 6px 10px -8px rgba(0, 0, 0, 0.6); + } + #entries-header .folder-actions { margin: 0; } + .entries-toolbar { + display: flex; align-items: center; gap: var(--s-3); + flex-wrap: wrap; margin-top: var(--s-2); + } + .entries-toolbar #entries-filters { margin: 0; } + .entries-toolbar .folder-actions-btns { margin-left: auto; } + #entries-header .list-tools { margin-bottom: 0; } + + /* Sidebar panel: one pinned header band (label, new-folder button, + filter) on a surface tone, then the list with hairline dividers. + Every band shares the same 12px horizontal padding so edges line up. */ + .sidebar-pin { + background: var(--surface); + border-bottom: 1px solid var(--border); + padding: var(--s-2) var(--s-3); + gap: var(--s-2); + } + .sidebar-pin .sidebar-header { + margin: 0; padding: 0; border: 0; + min-height: 30px; + justify-content: space-between; + } + .sidebar-pin .sidebar-header strong { font-size: var(--fz-xs); letter-spacing: 0.14em; } + #btn-new-folder { height: 30px; padding: 0 var(--s-2); font-size: var(--fz-xs); } + #sidebar-tools { margin: 0; } + #sidebar-tools:not(:empty) { padding-bottom: 0; } + #folder-filter { + height: 34px; font-size: var(--fz-sm); + background: var(--bg); border: 1px solid var(--border); + } + .folder-list { padding: var(--s-1) 0 var(--s-2); } + .folder-list > li { margin: 0; } + .folder-list > li + li > .folder-row { border-top: 1px solid rgba(255,255,255,0.045); } + .folder-row { + padding: var(--s-2) var(--s-3) var(--s-2) calc(var(--s-3) - 3px); + min-height: 46px; + } + .folder-list li.selected > .folder-row { border-top-color: transparent; } + .folder-row-main { gap: 2px; } + .folder-desc { font-size: 12px; } + .tree-children { margin-left: var(--s-3) !important; } @@ -545,13 +636,13 @@

rustguac