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
dani-garcia · Jul 12, 2024 · 54bfcb8 · 54bfcb8
1 parent 035f694
commit 54bfcb8
Show file tree

Hide file tree

Showing 11 changed files with 95 additions and 67 deletions.
diff --git 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 = "<data>")]
-fn post_admin_login(data: Form<LoginForm>, cookies: &CookieJar<'_>, ip: ClientIp) -> Result<Redirect, AdminResponse> {
+fn post_admin_login(
+    data: Form<LoginForm>,
+    cookies: &CookieJar<'_>,
+    ip: ClientIp,
+    secure: Secure,
+) -> Result<Redirect, AdminResponse> {
     let data = data.into_inner();
     let redirect = data.redirect;
 
@@ -193,7 +198,8 @@ fn post_admin_login(data: Form<LoginForm>, 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
@@ -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<Self, Self::Error> {
+        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<String>,
 }

diff --git 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!("&quot;{escaped_value}&quot;");
-    }
-
-    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
@@ -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
@@ -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</details>\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
@@ -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
@@ -198,15 +198,17 @@ userOrgTypeDialog.addEventListener("show.bs.modal", function(event) {
     const orgName = event.relatedTarget.dataset.vwOrgName;
     const orgUuid = event.relatedTarget.dataset.vwOrgUuid;
 
-    document.getElementById("userOrgTypeDialogTitle").innerHTML = `<b>Update User Type:</b><br><b>Organization:</b> ${orgName}<br><b>User:</b> ${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;
 }, false);
 
 // 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
@@ -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
@@ -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
@@ -44,7 +44,7 @@
                             <span class="d-block"><strong>Events:</strong> {{event_count}}</span>
                         </td>
                         <td class="text-end px-0 small">
-                            <button type="button" class="btn btn-sm btn-link p-0 border-0 float-right" vw-delete-organization data-vw-org-uuid="{{jsesc id no_quote}}" data-vw-org-name="{{jsesc name no_quote}}" data-vw-billing-email="{{jsesc billingEmail no_quote}}">Delete Organization</button><br>
+                            <button type="button" class="btn btn-sm btn-link p-0 border-0 float-right" vw-delete-organization data-vw-org-uuid="{{id}}" data-vw-org-name="{{name}}" data-vw-billing-email="{{billingEmail}}">Delete Organization</button><br>
                         </td>
                     </tr>
                     {{/each}}

diff --git a/src/static/templates/admin/users.hbs b/src/static/templates/admin/users.hbs
@@ -54,14 +54,14 @@
                             {{/if}}
                         </td>
                         <td>
-                            <div class="overflow-auto vw-org-cell" data-vw-user-email="{{jsesc email no_quote}}" data-vw-user-uuid="{{jsesc id no_quote}}">
+                            <div class="overflow-auto vw-org-cell" data-vw-user-email="{{email}}" data-vw-user-uuid="{{id}}">
                             {{#each organizations}}
-                            <button class="badge" data-bs-toggle="modal" data-bs-target="#userOrgTypeDialog" data-vw-org-type="{{type}}" data-vw-org-uuid="{{jsesc id no_quote}}" data-vw-org-name="{{jsesc name no_quote}}">{{name}}</button>
+                            <button class="badge" data-bs-toggle="modal" data-bs-target="#userOrgTypeDialog" data-vw-org-type="{{type}}" data-vw-org-uuid="{{id}}" data-vw-org-name="{{name}}">{{name}}</button>
                             {{/each}}
                             </div>
                         </td>
                         <td class="text-end px-0 small">
-                            <span data-vw-user-uuid="{{jsesc id no_quote}}" data-vw-user-email="{{jsesc email no_quote}}">
+                            <span data-vw-user-uuid="{{id}}" data-vw-user-email="{{email}}">
                                 {{#if twoFactorEnabled}}
                                 <button type="button" class="btn btn-sm btn-link p-0 border-0 float-right" vw-remove2fa>Remove all 2FA</button><br>
                                 {{/if}}
@@ -109,7 +109,9 @@
         <div class="modal-dialog modal-dialog-centered modal-sm">
             <div class="modal-content">
                 <div class="modal-header">
-                    <h6 class="modal-title" id="userOrgTypeDialogTitle"></h6>
+                    <h6 class="modal-title">
+                        <b>Update User Type:</b><br><b>Organization:</b> <span id="userOrgTypeDialogOrgName"></span><br><b>User:</b> <span id="userOrgTypeDialogUserEmail"></span>
+                    </h6>
                     <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
                 </div>
                 <form class="form" id="userOrgTypeForm">