Skip to content

Commit

Permalink
Removed unsafe-inline JS from CSP and other fixes
Browse files Browse the repository at this point in the history
- Removed `unsafe-inline` for javascript from CSP.
  The admin interface now uses files instead of inline javascript.
- Modified javascript to work not being inline.
- Run eslint over javascript and fixed some items.
- Added a `to_json` Handlebars helper.
  Used at the diagnostics page.
- Changed `AdminTemplateData` struct to be smaller.
  The `config` was always added, but only used at one page.
  Same goes for `can_backup` and `version`.
- Also inlined CSS.
  We can't remove the `unsafe-inline` from css, because that seems to
  break the web-vault currently. That might need some further checks.
  But for now the 404 page and all the admin pages are clear of inline scripts and styles.
  • Loading branch information
BlackDex authored and dani-garcia committed Jan 9, 2023
1 parent ab2dd0f commit 2020a30
Show file tree
Hide file tree
Showing 18 changed files with 946 additions and 718 deletions.
40 changes: 14 additions & 26 deletions src/api/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ fn render_admin_login(msg: Option<&str>, redirect: Option<String>) -> ApiResult<
let msg = msg.map(|msg| format!("Error: {msg}"));
let json = json!({
"page_content": "admin/login",
"version": VERSION,
"error": msg,
"redirect": redirect,
"urlpath": CONFIG.domain_path()
Expand Down Expand Up @@ -208,34 +207,16 @@ fn _validate_token(token: &str) -> bool {
#[derive(Serialize)]
struct AdminTemplateData {
page_content: String,
version: Option<&'static str>,
page_data: Option<Value>,
config: Value,
can_backup: bool,
logged_in: bool,
urlpath: String,
}

impl AdminTemplateData {
fn new() -> Self {
Self {
page_content: String::from("admin/settings"),
version: VERSION,
config: CONFIG.prepare_json(),
can_backup: *CAN_BACKUP,
logged_in: true,
urlpath: CONFIG.domain_path(),
page_data: None,
}
}

fn with_data(page_content: &str, page_data: Value) -> Self {
fn new(page_content: &str, page_data: Value) -> Self {
Self {
page_content: String::from(page_content),
version: VERSION,
page_data: Some(page_data),
config: CONFIG.prepare_json(),
can_backup: *CAN_BACKUP,
logged_in: true,
urlpath: CONFIG.domain_path(),
}
Expand All @@ -247,7 +228,11 @@ impl AdminTemplateData {
}

fn render_admin_page() -> ApiResult<Html<String>> {
let text = AdminTemplateData::new().render()?;
let settings_json = json!({
"config": CONFIG.prepare_json(),
"can_backup": *CAN_BACKUP,
});
let text = AdminTemplateData::new("admin/settings", settings_json).render()?;
Ok(Html(text))
}

Expand Down Expand Up @@ -342,7 +327,7 @@ async fn users_overview(_token: AdminToken, mut conn: DbConn) -> ApiResult<Html<
users_json.push(usr);
}

let text = AdminTemplateData::with_data("admin/users", json!(users_json)).render()?;
let text = AdminTemplateData::new("admin/users", json!(users_json)).render()?;
Ok(Html(text))
}

Expand Down Expand Up @@ -442,7 +427,7 @@ async fn update_user_org_type(
};

if user_to_edit.atype == UserOrgType::Owner && new_type != UserOrgType::Owner {
// Removing owner permmission, check that there is at least one other confirmed owner
// Removing owner permission, check that there is at least one other confirmed owner
if UserOrganization::count_confirmed_by_org_and_type(&data.org_uuid, UserOrgType::Owner, &mut conn).await <= 1 {
err!("Can't change the type of the last owner")
}
Expand Down Expand Up @@ -494,7 +479,7 @@ async fn organizations_overview(_token: AdminToken, mut conn: DbConn) -> ApiResu
organizations_json.push(org);
}

let text = AdminTemplateData::with_data("admin/organizations", json!(organizations_json)).render()?;
let text = AdminTemplateData::new("admin/organizations", json!(organizations_json)).render()?;
Ok(Html(text))
}

Expand Down Expand Up @@ -617,13 +602,14 @@ async fn diagnostics(_token: AdminToken, ip_header: IpHeader, mut conn: DbConn)

let diagnostics_json = json!({
"dns_resolved": dns_resolved,
"current_release": VERSION,
"latest_release": latest_release,
"latest_commit": latest_commit,
"web_vault_enabled": &CONFIG.web_vault_enabled(),
"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(),
"docker_base_image": if running_within_docker { docker_base_image() } else { "Not applicable" },
"has_http_access": has_http_access,
"ip_header_exists": &ip_header.0.is_some(),
"ip_header_match": ip_header_name == CONFIG.ip_header(),
Expand All @@ -634,11 +620,13 @@ async fn diagnostics(_token: AdminToken, ip_header: IpHeader, mut conn: DbConn)
"db_version": get_sql_server_version(&mut conn).await,
"admin_url": format!("{}/diagnostics", admin_url()),
"overrides": &CONFIG.get_overrides().join(", "),
"host_arch": std::env::consts::ARCH,
"host_os": std::env::consts::OS,
"server_time_local": Local::now().format("%Y-%m-%d %H:%M:%S %Z").to_string(),
"server_time": Utc::now().format("%Y-%m-%d %H:%M:%S UTC").to_string(), // Run the date/time check as the last item to minimize the difference
});

let text = AdminTemplateData::with_data("admin/diagnostics", diagnostics_json).render()?;
let text = AdminTemplateData::new("admin/diagnostics", diagnostics_json).render()?;
Ok(Html(text))
}

Expand Down
11 changes: 11 additions & 0 deletions src/api/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,17 @@ pub fn static_files(filename: String) -> Result<(ContentType, &'static [u8]), Er
"hibp.png" => Ok((ContentType::PNG, include_bytes!("../static/images/hibp.png"))),
"vaultwarden-icon.png" => Ok((ContentType::PNG, include_bytes!("../static/images/vaultwarden-icon.png"))),
"vaultwarden-favicon.png" => Ok((ContentType::PNG, include_bytes!("../static/images/vaultwarden-favicon.png"))),
"404.css" => Ok((ContentType::CSS, include_bytes!("../static/scripts/404.css"))),
"admin.css" => Ok((ContentType::CSS, include_bytes!("../static/scripts/admin.css"))),
"admin.js" => Ok((ContentType::JavaScript, include_bytes!("../static/scripts/admin.js"))),
"admin_settings.js" => Ok((ContentType::JavaScript, include_bytes!("../static/scripts/admin_settings.js"))),
"admin_users.js" => Ok((ContentType::JavaScript, include_bytes!("../static/scripts/admin_users.js"))),
"admin_organizations.js" => {
Ok((ContentType::JavaScript, include_bytes!("../static/scripts/admin_organizations.js")))
}
"admin_diagnostics.js" => {
Ok((ContentType::JavaScript, include_bytes!("../static/scripts/admin_diagnostics.js")))
}
"bootstrap.css" => Ok((ContentType::CSS, include_bytes!("../static/scripts/bootstrap.css"))),
"bootstrap-native.js" => Ok((ContentType::JavaScript, include_bytes!("../static/scripts/bootstrap-native.js"))),
"jdenticon.js" => Ok((ContentType::JavaScript, include_bytes!("../static/scripts/jdenticon.js"))),
Expand Down
15 changes: 15 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,7 @@ where
// 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 {
($name:expr) => {{
Expand Down Expand Up @@ -1183,3 +1184,17 @@ fn js_escape_helper<'reg, 'rc>(
out.write(&escaped_value)?;
Ok(())
}

fn to_json<'reg, 'rc>(
h: &Helper<'reg, '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(|| RenderError::new("Expected 1 parameter for \"to_json\""))?.value();
let json = serde_json::to_string(param)
.map_err(|e| RenderError::new(format!("Can't serialize parameter to JSON: {}", e)))?;
out.write(&json)?;
Ok(())
}
26 changes: 26 additions & 0 deletions src/static/scripts/404.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
body {
padding-top: 75px;
}
.vaultwarden-icon {
width: 48px;
height: 48px;
height: 32px;
width: auto;
margin: -5px 0 0 0;
}
.footer {
padding: 40px 0 40px 0;
border-top: 1px solid #dee2e6;
}
.container {
max-width: 980px;
}
.content {
padding-top: 20px;
padding-bottom: 20px;
padding-left: 15px;
padding-right: 15px;
}
.vw-404 {
max-width: 500px; width: 100%;
}
45 changes: 45 additions & 0 deletions src/static/scripts/admin.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
body {
padding-top: 75px;
}
img {
width: 48px;
height: 48px;
}
.vaultwarden-icon {
height: 32px;
width: auto;
margin: -5px 0 0 0;
}
/* Special alert-row class to use Bootstrap v5.2+ variable colors */
.alert-row {
--bs-alert-border: 1px solid var(--bs-alert-border-color);
color: var(--bs-alert-color);
background-color: var(--bs-alert-bg);
border: var(--bs-alert-border);
}

#users-table .vw-created-at, #users-table .vw-last-active {
width: 85px;
min-width: 70px;
}
#users-table .vw-items {
width: 35px;
min-width: 35px;
}
#users-table .vw-organizations {
min-width: 120px;
}
#users-table .vw-actions, #orgs-table .vw-actions {
width: 130px;
min-width: 130px;
}
#users-table .vw-org-cell {
max-height: 120px;
}

#support-string {
height: 16rem;
}
.vw-copy-toast {
width: 15rem;
}
65 changes: 65 additions & 0 deletions src/static/scripts/admin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
"use strict";

function getBaseUrl() {
// If the base URL is `https://vaultwarden.example.com/base/path/`,
// `window.location.href` should have one of the following forms:
//
// - `https://vaultwarden.example.com/base/path/`
// - `https://vaultwarden.example.com/base/path/#/some/route[?queryParam=...]`
//
// We want to get to just `https://vaultwarden.example.com/base/path`.
const baseUrl = window.location.href;
const adminPos = baseUrl.indexOf("/admin");
return baseUrl.substring(0, adminPos != -1 ? adminPos : baseUrl.length);
}
const BASE_URL = getBaseUrl();

function reload() {
// Reload the page by setting the exact same href
// Using window.location.reload() could cause a repost.
window.location = window.location.href;
}

function msg(text, reload_page = true) {
text && alert(text);
reload_page && reload();
}

function _post(url, successMsg, errMsg, body, reload_page = true) {
fetch(url, {
method: "POST",
body: body,
mode: "same-origin",
credentials: "same-origin",
headers: { "Content-Type": "application/json" }
}).then( resp => {
if (resp.ok) { msg(successMsg, reload_page); return Promise.reject({error: false}); }
const respStatus = resp.status;
const respStatusText = resp.statusText;
return resp.text();
}).then( respText => {
try {
const respJson = JSON.parse(respText);
return respJson ? respJson.ErrorModel.Message : "Unknown error";
} catch (e) {
return Promise.reject({body:respStatus + " - " + respStatusText, error: true});
}
}).then( apiMsg => {
msg(errMsg + "\n" + apiMsg, reload_page);
}).catch( e => {
if (e.error === false) { return true; }
else { msg(errMsg + "\n" + e.body, reload_page); }
});
}

// onLoad events
document.addEventListener("DOMContentLoaded", (/*event*/) => {
// get current URL path and assign "active" class to the correct nav-item
const pathname = window.location.pathname;
if (pathname === "") return;
const 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");
}
});

0 comments on commit 2020a30

Please sign in to comment.