From 54bfcb8bc3aa1d15cf20821dc5f3747892011272 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Fri, 12 Jul 2024 22:59:48 +0200 Subject: [PATCH] Update admin interface (#4737) - Updated datatables - Set Cookie Secure flag if the connection is https - Prevent possible XSS via Organization Name Converted all `innerHTML` and `innerText` to the Safe Sink version `textContent` - Removed `jsesc` function as handlebars escapes all these chars already and more by default --- src/api/admin.rs | 12 +++-- src/auth.rs | 32 +++++++++++- src/config.rs | 27 ---------- src/static/scripts/admin.js | 4 +- src/static/scripts/admin_diagnostics.js | 10 ++-- src/static/scripts/admin_settings.js | 2 +- src/static/scripts/admin_users.js | 6 ++- src/static/scripts/datatables.css | 4 +- src/static/scripts/datatables.js | 53 +++++++++++++------- src/static/templates/admin/organizations.hbs | 2 +- src/static/templates/admin/users.hbs | 10 ++-- 11 files changed, 95 insertions(+), 67 deletions(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index 1ea9aa59..9a1d3417 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -18,7 +18,7 @@ use crate::{ core::{log_event, two_factor}, unregister_push_device, ApiResult, EmptyResult, JsonResult, Notify, }, - auth::{decode_admin, encode_jwt, generate_admin_claims, ClientIp}, + auth::{decode_admin, encode_jwt, generate_admin_claims, ClientIp, Secure}, config::ConfigBuilder, db::{backup_database, get_sql_server_version, models::*, DbConn, DbConnType}, error::{Error, MapResult}, @@ -169,7 +169,12 @@ struct LoginForm { } #[post("/", data = "")] -fn post_admin_login(data: Form, cookies: &CookieJar<'_>, ip: ClientIp) -> Result { +fn post_admin_login( + data: Form, + cookies: &CookieJar<'_>, + ip: ClientIp, + secure: Secure, +) -> Result { let data = data.into_inner(); let redirect = data.redirect; @@ -193,7 +198,8 @@ fn post_admin_login(data: Form, cookies: &CookieJar<'_>, ip: ClientIp .path(admin_path()) .max_age(rocket::time::Duration::minutes(CONFIG.admin_session_lifetime())) .same_site(SameSite::Strict) - .http_only(true); + .http_only(true) + .secure(secure.https); cookies.add(cookie); if let Some(redirect) = redirect { diff --git a/src/auth.rs b/src/auth.rs index c8060a28..4ee9c188 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -379,8 +379,6 @@ impl<'r> FromRequest<'r> for Host { referer.to_string() } else { // Try to guess from the headers - use std::env; - let protocol = if let Some(proto) = headers.get_one("X-Forwarded-Proto") { proto } else if env::var("ROCKET_TLS").is_ok() { @@ -806,6 +804,7 @@ impl<'r> FromRequest<'r> for OwnerHeaders { // Client IP address detection // use std::{ + env, fs::File, io::{Read, Write}, net::IpAddr, @@ -842,6 +841,35 @@ impl<'r> FromRequest<'r> for ClientIp { } } +pub struct Secure { + pub https: bool, +} + +#[rocket::async_trait] +impl<'r> FromRequest<'r> for Secure { + type Error = (); + + async fn from_request(request: &'r Request<'_>) -> Outcome { + let headers = request.headers(); + + // Try to guess from the headers + let protocol = match headers.get_one("X-Forwarded-Proto") { + Some(proto) => proto, + None => { + if env::var("ROCKET_TLS").is_ok() { + "https" + } else { + "http" + } + } + }; + + Outcome::Success(Secure { + https: protocol == "https", + }) + } +} + pub struct WsAccessTokenHeader { pub access_token: Option, } diff --git a/src/config.rs b/src/config.rs index 7beb86ab..6a3a3f78 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1277,7 +1277,6 @@ where hb.set_strict_mode(true); // Register helpers hb.register_helper("case", Box::new(case_helper)); - hb.register_helper("jsesc", Box::new(js_escape_helper)); hb.register_helper("to_json", Box::new(to_json)); macro_rules! reg { @@ -1365,32 +1364,6 @@ fn case_helper<'reg, 'rc>( } } -fn js_escape_helper<'reg, 'rc>( - h: &Helper<'rc>, - _r: &'reg Handlebars<'_>, - _ctx: &'rc Context, - _rc: &mut RenderContext<'reg, 'rc>, - out: &mut dyn Output, -) -> HelperResult { - let param = - h.param(0).ok_or_else(|| RenderErrorReason::Other(String::from("Param not found for helper \"jsesc\"")))?; - - let no_quote = h.param(1).is_some(); - - let value = param - .value() - .as_str() - .ok_or_else(|| RenderErrorReason::Other(String::from("Param for helper \"jsesc\" is not a String")))?; - - let mut escaped_value = value.replace('\\', "").replace('\'', "\\x22").replace('\"', "\\x27"); - if !no_quote { - escaped_value = format!(""{escaped_value}""); - } - - out.write(&escaped_value)?; - Ok(()) -} - fn to_json<'reg, 'rc>( h: &Helper<'rc>, _r: &'reg Handlebars<'_>, diff --git a/src/static/scripts/admin.js b/src/static/scripts/admin.js index b35f3fb1..bc06c0a3 100644 --- a/src/static/scripts/admin.js +++ b/src/static/scripts/admin.js @@ -98,7 +98,7 @@ const showActiveTheme = (theme, focus = false) => { const themeSwitcherText = document.querySelector("#bd-theme-text"); const activeThemeIcon = document.querySelector(".theme-icon-active use"); const btnToActive = document.querySelector(`[data-bs-theme-value="${theme}"]`); - const svgOfActiveBtn = btnToActive.querySelector("span use").innerText; + const svgOfActiveBtn = btnToActive.querySelector("span use").textContent; document.querySelectorAll("[data-bs-theme-value]").forEach(element => { element.classList.remove("active"); @@ -107,7 +107,7 @@ const showActiveTheme = (theme, focus = false) => { btnToActive.classList.add("active"); btnToActive.setAttribute("aria-pressed", "true"); - activeThemeIcon.innerText = svgOfActiveBtn; + activeThemeIcon.textContent = svgOfActiveBtn; const themeSwitcherLabel = `${themeSwitcherText.textContent} (${btnToActive.dataset.bsThemeValue})`; themeSwitcher.setAttribute("aria-label", themeSwitcherLabel); diff --git a/src/static/scripts/admin_diagnostics.js b/src/static/scripts/admin_diagnostics.js index 9f2aca66..6a178e4b 100644 --- a/src/static/scripts/admin_diagnostics.js +++ b/src/static/scripts/admin_diagnostics.js @@ -117,7 +117,7 @@ async function generateSupportString(event, dj) { supportString += `\n**Environment settings which are overridden:** ${dj.overrides}\n`; supportString += "\n\n```json\n" + JSON.stringify(configJson, undefined, 2) + "\n```\n\n"; - document.getElementById("support-string").innerText = supportString; + document.getElementById("support-string").textContent = supportString; document.getElementById("support-string").classList.remove("d-none"); document.getElementById("copy-support").classList.remove("d-none"); } @@ -126,7 +126,7 @@ function copyToClipboard(event) { event.preventDefault(); event.stopPropagation(); - const supportStr = document.getElementById("support-string").innerText; + const supportStr = document.getElementById("support-string").textContent; const tmpCopyEl = document.createElement("textarea"); tmpCopyEl.setAttribute("id", "copy-support-string"); @@ -201,7 +201,7 @@ function checkDns(dns_resolved) { function init(dj) { // Time check - document.getElementById("time-browser-string").innerText = browserUTC; + document.getElementById("time-browser-string").textContent = browserUTC; // Check if we were able to fetch a valid NTP Time // If so, compare both browser and server with NTP @@ -217,7 +217,7 @@ function init(dj) { // Domain check const browserURL = location.href.toLowerCase(); - document.getElementById("domain-browser-string").innerText = browserURL; + document.getElementById("domain-browser-string").textContent = browserURL; checkDomain(browserURL, dj.admin_url.toLowerCase()); // Version check @@ -229,7 +229,7 @@ function init(dj) { // onLoad events document.addEventListener("DOMContentLoaded", (event) => { - const diag_json = JSON.parse(document.getElementById("diagnostics_json").innerText); + const diag_json = JSON.parse(document.getElementById("diagnostics_json").textContent); init(diag_json); const btnGenSupport = document.getElementById("gen-support"); diff --git a/src/static/scripts/admin_settings.js b/src/static/scripts/admin_settings.js index ffdd778b..3d61a508 100644 --- a/src/static/scripts/admin_settings.js +++ b/src/static/scripts/admin_settings.js @@ -122,7 +122,7 @@ function submitTestEmailOnEnter() { function colorRiskSettings() { const risk_items = document.getElementsByClassName("col-form-label"); Array.from(risk_items).forEach((el) => { - if (el.innerText.toLowerCase().includes("risks") ) { + if (el.textContent.toLowerCase().includes("risks") ) { el.parentElement.className += " alert-danger"; } }); diff --git a/src/static/scripts/admin_users.js b/src/static/scripts/admin_users.js index 8b569296..c2462521 100644 --- a/src/static/scripts/admin_users.js +++ b/src/static/scripts/admin_users.js @@ -198,7 +198,8 @@ userOrgTypeDialog.addEventListener("show.bs.modal", function(event) { const orgName = event.relatedTarget.dataset.vwOrgName; const orgUuid = event.relatedTarget.dataset.vwOrgUuid; - document.getElementById("userOrgTypeDialogTitle").innerHTML = `Update User Type:
Organization: ${orgName}
User: ${userEmail}`; + document.getElementById("userOrgTypeDialogOrgName").textContent = orgName; + document.getElementById("userOrgTypeDialogUserEmail").textContent = userEmail; document.getElementById("userOrgTypeUserUuid").value = userUuid; document.getElementById("userOrgTypeOrgUuid").value = orgUuid; document.getElementById(`userOrgType${userOrgTypeName}`).checked = true; @@ -206,7 +207,8 @@ userOrgTypeDialog.addEventListener("show.bs.modal", function(event) { // Prevent accidental submission of the form with valid elements after the modal has been hidden. userOrgTypeDialog.addEventListener("hide.bs.modal", function() { - document.getElementById("userOrgTypeDialogTitle").innerHTML = ""; + document.getElementById("userOrgTypeDialogOrgName").textContent = ""; + document.getElementById("userOrgTypeDialogUserEmail").textContent = ""; document.getElementById("userOrgTypeUserUuid").value = ""; document.getElementById("userOrgTypeOrgUuid").value = ""; }, false); diff --git a/src/static/scripts/datatables.css b/src/static/scripts/datatables.css index 83e4f44b..878e2347 100644 --- a/src/static/scripts/datatables.css +++ b/src/static/scripts/datatables.css @@ -4,10 +4,10 @@ * * To rebuild or modify this file with the latest versions of the included * software please visit: - * https://datatables.net/download/#bs5/dt-2.0.7 + * https://datatables.net/download/#bs5/dt-2.0.8 * * Included libraries: - * DataTables 2.0.7 + * DataTables 2.0.8 */ @charset "UTF-8"; diff --git a/src/static/scripts/datatables.js b/src/static/scripts/datatables.js index 88d0b627..3d22cbde 100644 --- a/src/static/scripts/datatables.js +++ b/src/static/scripts/datatables.js @@ -4,20 +4,20 @@ * * To rebuild or modify this file with the latest versions of the included * software please visit: - * https://datatables.net/download/#bs5/dt-2.0.7 + * https://datatables.net/download/#bs5/dt-2.0.8 * * Included libraries: - * DataTables 2.0.7 + * DataTables 2.0.8 */ -/*! DataTables 2.0.7 +/*! DataTables 2.0.8 * © SpryMedia Ltd - datatables.net/license */ /** * @summary DataTables * @description Paginate, search and order HTML tables - * @version 2.0.7 + * @version 2.0.8 * @author SpryMedia Ltd * @contact www.datatables.net * @copyright SpryMedia Ltd. @@ -563,7 +563,7 @@ * * @type string */ - builder: "bs5/dt-2.0.7", + builder: "bs5/dt-2.0.8", /** @@ -7572,6 +7572,16 @@ order = opts.order, // applied, current, index (original - compatibility with 1.9) page = opts.page; // all, current + if ( _fnDataSource( settings ) == 'ssp' ) { + // In server-side processing mode, most options are irrelevant since + // rows not shown don't exist and the index order is the applied order + // Removed is a special case - for consistency just return an empty + // array + return search === 'removed' ? + [] : + _range( 0, displayMaster.length ); + } + if ( page == 'current' ) { // Current page implies that order=current and filter=applied, since it is // fairly senseless otherwise, regardless of what order and search actually @@ -8243,7 +8253,7 @@ _api_register( _child_obj+'.isShown()', function () { var ctx = this.context; - if ( ctx.length && this.length ) { + if ( ctx.length && this.length && ctx[0].aoData[ this[0] ] ) { // _detailsShown as false or undefined will fall through to return false return ctx[0].aoData[ this[0] ]._detailsShow || false; } @@ -8266,7 +8276,7 @@ // can be an array of these items, comma separated list, or an array of comma // separated lists - var __re_column_selector = /^([^:]+):(name|title|visIdx|visible)$/; + var __re_column_selector = /^([^:]+)?:(name|title|visIdx|visible)$/; // r1 and r2 are redundant - but it means that the parameters match for the @@ -8338,17 +8348,24 @@ switch( match[2] ) { case 'visIdx': case 'visible': - var idx = parseInt( match[1], 10 ); - // Visible index given, convert to column index - if ( idx < 0 ) { - // Counting from the right - var visColumns = columns.map( function (col,i) { - return col.bVisible ? i : null; - } ); - return [ visColumns[ visColumns.length + idx ] ]; + if (match[1]) { + var idx = parseInt( match[1], 10 ); + // Visible index given, convert to column index + if ( idx < 0 ) { + // Counting from the right + var visColumns = columns.map( function (col,i) { + return col.bVisible ? i : null; + } ); + return [ visColumns[ visColumns.length + idx ] ]; + } + // Counting from the left + return [ _fnVisibleToColumnIndex( settings, idx ) ]; } - // Counting from the left - return [ _fnVisibleToColumnIndex( settings, idx ) ]; + + // `:visible` on its own + return columns.map( function (col, i) { + return col.bVisible ? i : null; + } ); case 'name': // match by name. `names` is column index complete and in order @@ -9623,7 +9640,7 @@ * @type string * @default Version number */ - DataTable.version = "2.0.7"; + DataTable.version = "2.0.8"; /** * Private data store, containing all of the settings objects that are diff --git a/src/static/templates/admin/organizations.hbs b/src/static/templates/admin/organizations.hbs index 46547b28..654f904e 100644 --- a/src/static/templates/admin/organizations.hbs +++ b/src/static/templates/admin/organizations.hbs @@ -44,7 +44,7 @@ Events: {{event_count}} -
+
{{/each}} diff --git a/src/static/templates/admin/users.hbs b/src/static/templates/admin/users.hbs index 1765876a..09efc113 100644 --- a/src/static/templates/admin/users.hbs +++ b/src/static/templates/admin/users.hbs @@ -54,14 +54,14 @@ {{/if}} -
+
{{#each organizations}} - + {{/each}}
- + {{#if twoFactorEnabled}}
{{/if}} @@ -109,7 +109,9 @@
- +
+ Update User Type:
Organization:
User: +