From 338756550a27d1084ae947a1a108482abb8f473b Mon Sep 17 00:00:00 2001 From: BlackDex Date: Fri, 8 Oct 2021 00:01:24 +0200 Subject: [PATCH] Fix error reporting in admin and some small fixes - Fixed a bug in JavaScript which caused no messages to be shown to the user in-case of an error send by the server. - Changed mail error handling for better error messages - Changed user/org actions from a to buttons, this should prevent strange issues in-case of javascript issues and the page does re-load. - Added Alpine and Debian info for the running docker image During the mail error testing i encountered a bug which caused lettre to panic. This panic only happens on debug builds and not release builds, so no need to update anything on that part. This bug is also already fixed. See https://github.com/lettre/lettre/issues/678 and https://github.com/lettre/lettre/pull/679 Resolves #2021 Could also fix the issue reported here #2022, or at least no hash `#` in the url. --- src/api/admin.rs | 6 ++++- src/mail.rs | 23 +++++++++++++++----- src/static/templates/admin/base.hbs | 8 +++---- src/static/templates/admin/diagnostics.hbs | 4 ++-- src/static/templates/admin/organizations.hbs | 4 ++-- src/static/templates/admin/settings.hbs | 7 +++--- src/static/templates/admin/users.hbs | 12 +++++----- src/util.rs | 12 ++++++++++ 8 files changed, 52 insertions(+), 24 deletions(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index 3fe4f947..5176ee3c 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -18,7 +18,9 @@ use crate::{ db::{backup_database, get_sql_server_version, models::*, DbConn, DbConnType}, error::{Error, MapResult}, mail, - util::{format_naive_datetime_local, get_display_size, get_reqwest_client, is_running_in_docker}, + util::{ + docker_base_image, format_naive_datetime_local, get_display_size, get_reqwest_client, is_running_in_docker, + }, CONFIG, }; @@ -492,6 +494,7 @@ fn diagnostics(_token: AdminToken, ip_header: IpHeader, conn: DbConn) -> ApiResu // Execute some environment checks let running_within_docker = is_running_in_docker(); + let docker_base_image = docker_base_image(); let has_http_access = has_http_access(); let uses_proxy = env::var_os("HTTP_PROXY").is_some() || env::var_os("http_proxy").is_some() @@ -549,6 +552,7 @@ fn diagnostics(_token: AdminToken, ip_header: IpHeader, conn: DbConn) -> ApiResu "web_vault_version": web_vault_version.version, "latest_web_build": latest_web_build, "running_within_docker": running_within_docker, + "docker_base_image": docker_base_image, "has_http_access": has_http_access, "ip_header_exists": &ip_header.0.is_some(), "ip_header_match": ip_header_name == CONFIG.ip_header(), diff --git a/src/mail.rs b/src/mail.rs index f4278bef..55d944d4 100644 --- a/src/mail.rs +++ b/src/mail.rs @@ -473,15 +473,28 @@ fn send_email(address: &str, subject: &str, body_html: String, body_text: String // Match some common errors and make them more user friendly Err(e) => { if e.is_client() { - err!(format!("SMTP Client error: {}", e)); + debug!("SMTP Client error: {:#?}", e); + err!(format!("SMTP Client error: {}", e.to_string())); } else if e.is_transient() { - err!(format!("SMTP 4xx error: {:?}", e)); + debug!("SMTP 4xx error: {:#?}", e); + err!(format!("SMTP 4xx error: {}", e.to_string())); } else if e.is_permanent() { - err!(format!("SMTP 5xx error: {:?}", e)); + debug!("SMTP 5xx error: {:#?}", e); + let mut msg = e.to_string(); + // Add a special check for 535 to add a more descriptive message + if msg.contains("(535)") { + msg = format!("{} - Authentication credentials invalid", msg); + } + err!(format!("SMTP 5xx error: {}", msg)); } else if e.is_timeout() { - err!(format!("SMTP timeout error: {:?}", e)); + debug!("SMTP timeout error: {:#?}", e); + err!(format!("SMTP timeout error: {}", e.to_string())); + } else if e.is_tls() { + debug!("SMTP Encryption error: {:#?}", e); + err!(format!("SMTP Encryption error: {}", e.to_string())); } else { - Err(e.into()) + debug!("SMTP {:#?}", e); + err!(format!("SMTP {}", e.to_string())); } } } diff --git a/src/static/templates/admin/base.hbs b/src/static/templates/admin/base.hbs index 94684ede..9c876723 100644 --- a/src/static/templates/admin/base.hbs +++ b/src/static/templates/admin/base.hbs @@ -62,8 +62,8 @@ headers: { "Content-Type": "application/json" } }).then( resp => { if (resp.ok) { msg(successMsg, reload_page); return Promise.reject({error: false}); } - respStatus = resp.status; - respStatusText = resp.statusText; + const respStatus = resp.status; + const respStatusText = resp.statusText; return resp.text(); }).then( respText => { try { @@ -126,9 +126,9 @@ // get current URL path and assign 'active' class to the correct nav-item (() => { - var pathname = window.location.pathname; + const pathname = window.location.pathname; if (pathname === "") return; - var navItem = document.querySelectorAll('.navbar-nav .nav-item a[href="'+pathname+'"]'); + let navItem = document.querySelectorAll('.navbar-nav .nav-item a[href="'+pathname+'"]'); if (navItem.length === 1) { navItem[0].className = navItem[0].className + ' active'; navItem[0].setAttribute('aria-current', 'page'); diff --git a/src/static/templates/admin/diagnostics.hbs b/src/static/templates/admin/diagnostics.hbs index 4c4575ad..315acc97 100644 --- a/src/static/templates/admin/diagnostics.hbs +++ b/src/static/templates/admin/diagnostics.hbs @@ -58,7 +58,7 @@
Running within Docker
{{#if page_data.running_within_docker}} - Yes + Yes (Base: {{ page_data.docker_base_image }}) {{/if}} {{#unless page_data.running_within_docker}} No @@ -329,7 +329,7 @@ supportString += "* Vaultwarden version: v{{ version }}\n"; supportString += "* Web-vault version: v{{ page_data.web_vault_version }}\n"; - supportString += "* Running within Docker: {{ page_data.running_within_docker }}\n"; + supportString += "* Running within Docker: {{ page_data.running_within_docker }} (Base: {{ page_data.docker_base_image }})\n"; supportString += "* Environment settings overridden: "; {{#if page_data.overrides}} supportString += "true\n" diff --git a/src/static/templates/admin/organizations.hbs b/src/static/templates/admin/organizations.hbs index 35026a50..05509659 100644 --- a/src/static/templates/admin/organizations.hbs +++ b/src/static/templates/admin/organizations.hbs @@ -37,8 +37,8 @@ Size: {{attachment_size}} {{/if}} - - Delete Organization + + {{/each}} diff --git a/src/static/templates/admin/settings.hbs b/src/static/templates/admin/settings.hbs index b257326b..454891ce 100644 --- a/src/static/templates/admin/settings.hbs +++ b/src/static/templates/admin/settings.hbs @@ -219,11 +219,10 @@ onChange(); // Trigger the event initially checkbox.addEventListener("change", onChange); } - // These are formatted because otherwise the - // VSCode formatter breaks But they still work - // {{#each config}} {{#if grouptoggle}} + + {{#each config}} {{#if grouptoggle}} masterCheck("input_{{grouptoggle}}", "#g_{{group}} input"); - // {{/if}} {{/each}} + {{/if}} {{/each}} // Two functions to help check if there were changes to the form fields // Useful for example during the smtp test to prevent people from clicking save before testing there new settings diff --git a/src/static/templates/admin/users.hbs b/src/static/templates/admin/users.hbs index 2f6eb005..be644553 100644 --- a/src/static/templates/admin/users.hbs +++ b/src/static/templates/admin/users.hbs @@ -61,16 +61,16 @@ {{/each}} - + {{#if TwoFactorEnabled}} - Remove all 2FA + {{/if}} - Deauthorize sessions - Delete User + + {{#if user_enabled}} - Disable User + {{else}} - Enable User + {{/if}} diff --git a/src/util.rs b/src/util.rs index f483b1dc..ebf7e208 100644 --- a/src/util.rs +++ b/src/util.rs @@ -419,6 +419,18 @@ pub fn is_running_in_docker() -> bool { Path::new("/.dockerenv").exists() || Path::new("/run/.containerenv").exists() } +/// Simple check to determine on which docker base image vaultwarden is running. +/// We build images based upon Debian or Alpine, so these we check here. +pub fn docker_base_image() -> String { + if Path::new("/etc/debian_version").exists() { + "Debian".to_string() + } else if Path::new("/etc/alpine-release").exists() { + "Alpine".to_string() + } else { + "Unknown".to_string() + } +} + // // Deserialization methods //