Misc fixes and updates (#7558)

* Update GHA and pre-commit

Signed-off-by: BlackDex <black.dex@gmail.com>

* Update admin diagnostics

Added a check if the templates are overridden and return which specific folder, `admin`, `email` or `scss`.
This way we could more quickly point users to possible outdated templates which they are using.

Also updated the Support String to use some emojis so we should be able to quicker see if there is something wrong.
Just checking `true` or `false` could be difficult sometimes, and sometimes what we had as `false` wasn't bad either.

Also adjusted the eslint comments so it will work with the latest version of eslint.

Signed-off-by: BlackDex <black.dex@gmail.com>

* Fix updating collections for a cipher

The newer clients expect a `cipherDetails` response on the `collections-admin` endpoints.
Without it, the client will cause an error and stops handling the update correctly.

This will fix this by returning the cipher json.

Fixes #7545
Fixes #7546

Signed-off-by: BlackDex <black.dex@gmail.com>

* Cache CSS file in a different way

Currently we set a cache ttl of 24 hours, and users need to do a force refresh if there is anything changed to the CSS file.
In the past we have had several issue reported which were related to a still cached CSS file.

This commit will change the caching and also cache the generated CSS file in memory.
Instead of letting the browser cache it for 24 hours we generate an ETag, this is just a hash of the contents.
This ETag is returned by the browser during a request, and we can match this, and if so, just return a `304` `Not Modified`.
If the ETag is not known, we return the new content.

This should make simple refreshes by clients get updated settings or a new version of Vaultwarden which has other CSS entries get updated instantly.
If a user does a hard refresh, we will not receive the ETag and the content will be served.

The same goes if someone has the `reload_templates` feature enabled, since then we should not cache anyway.
If someone adjust settings via the `/admin` interface, the cache will be invalidated and a new CSS will be generated.

Signed-off-by: BlackDex <black.dex@gmail.com>

* Fix showing events for a specific user

Signed-off-by: BlackDex <black.dex@gmail.com>

* Update crates and adjust code.

- Updated opendal and adjusted code where needed.
- Updated yubico_ng and adjusted code where needed.
  This version now supports using an own HttpClient and it pulls in no reqwest dependency anymore.
  Now it will use our own client which uses custom hickory DNS and other features.

Signed-off-by: BlackDex <black.dex@gmail.com>

* Update web-vault to v2026.7.0

Signed-off-by: BlackDex <black.dex@gmail.com>

* Fix hadolint warnings

Signed-off-by: BlackDex <black.dex@gmail.com>

---------

Signed-off-by: BlackDex <black.dex@gmail.com>

src/api/admin.rs

@@ -716,6 +716,36 @@ fn web_vault_compare(active: &str, latest: &str) -> i8 {
     }
 }
 
+fn check_template_overrides() -> Vec<&'static str> {
+    let template_folder = std::path::PathBuf::from(CONFIG.templates_folder());
+    let mut overrides = Vec::new();
+    for folder in ["admin", "email", "scss"] {
+        if folder_has_hbs_files(&template_folder.join(folder)) {
+            overrides.push(folder);
+        }
+    }
+
+    if folder_has_hbs_files(&template_folder) {
+        overrides.push("other");
+    }
+
+    overrides
+}
+
+fn folder_has_hbs_files(dir: &std::path::Path) -> bool {
+    let Ok(files) = std::fs::read_dir(dir) else {
+        // No files in this directory at all, so we can return false
+        return false;
+    };
+
+    files.flatten().any(|f| {
+        // Validate if it is a file and if it has the `.hbs` extension and starts with a-z or 0-9
+        f.file_type().is_ok_and(|t| t.is_file())
+            && f.path().extension().is_some_and(|e| e.eq_ignore_ascii_case("hbs"))
+            && f.file_name().to_str().is_some_and(|n| n.starts_with(|c: char| c.is_ascii_alphanumeric()))
+    })
+}
+
 #[get("/diagnostics")]
 async fn diagnostics(_token: AdminToken, ip_header: IpHeader, conn: DbConn) -> ApiResult<Html<String>> {
     use chrono::prelude::*;
@@ -770,6 +800,7 @@ async fn diagnostics(_token: AdminToken, ip_header: IpHeader, conn: DbConn) -> A
         "db_version": get_sql_server_version(&conn).await,
         "admin_url": format!("{}/diagnostics", admin_url()),
         "overrides": &CONFIG.get_overrides().join(", "),
+        "template_overrides": check_template_overrides().join(", "),
         "invalid_feature_flags": invalid_feature_flags,
         "host_arch": env::consts::ARCH,
         "host_os":  env::consts::OS,

src/api/core/ciphers.rs

@@ -870,7 +870,7 @@ async fn put_collections_admin(
     headers: Headers,
     conn: DbConn,
     nt: Notify<'_>,
-) -> EmptyResult {
+) -> JsonResult {
     post_collections_admin(cipher_id, data, headers, conn, nt).await
 }
 
@@ -881,7 +881,7 @@ async fn post_collections_admin(
     headers: Headers,
     conn: DbConn,
     nt: Notify<'_>,
-) -> EmptyResult {
+) -> JsonResult {
     let data: CollectionsAdminData = data.into_inner();
 
     let Some(cipher) = Cipher::find_by_uuid(&cipher_id, &conn).await else {
@@ -940,7 +940,7 @@ async fn post_collections_admin(
     )
     .await;
 
-    Ok(())
+    Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, None, CipherSyncType::Organization, &conn).await?))
 }
 
 #[derive(Deserialize)]

src/api/core/two_factor/yubikey.rs

@@ -1,6 +1,10 @@
 use rocket::{Route, serde::json::Json};
 use serde_json::Value;
-use yubico::{config::Config, verify_async};
+use yubico_ng::{
+    Verifier, YubicoError,
+    config::Config,
+    transport::{AsyncTransport, Response},
+};
 
 use crate::{
     CONFIG,
@@ -14,12 +18,39 @@ use crate::{
         models::{EventType, TwoFactor, TwoFactorType},
     },
     error::{Error, MapResult},
+    http_client,
 };
 
 pub fn routes() -> Vec<Route> {
     routes![generate_yubikey, activate_yubikey, activate_yubikey_put,]
 }
 
+struct HttpClientTransport {
+    client: reqwest::Client,
+}
+
+impl HttpClientTransport {
+    fn new() -> Result<Self, reqwest::Error> {
+        http_client::get_reqwest_client_builder(false).redirect(reqwest::redirect::Policy::none()).build().map(
+            |client| Self {
+                client,
+            },
+        )
+    }
+}
+
+impl AsyncTransport for HttpClientTransport {
+    type Error = YubicoError;
+
+    async fn yubico_get(&self, url: &str) -> Result<Response, Self::Error> {
+        let response = self.client.get(url).send().await.map_err(YubicoError::transport)?;
+        Ok(Response {
+            status: response.status().as_u16(),
+            body: response.text().await.map_err(YubicoError::transport)?,
+        })
+    }
+}
+
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct EnableYubikeyData {
@@ -44,8 +75,7 @@ pub struct YubikeyMetadata {
 
 fn parse_yubikeys(data: &EnableYubikeyData) -> Vec<String> {
     let data_keys = [&data.key1, &data.key2, &data.key3, &data.key4, &data.key5];
-
-    data_keys.into_iter().flatten().cloned().collect()
+    data_keys.into_iter().flatten().filter(|e| !e.is_empty()).cloned().collect()
 }
 
 fn jsonify_yubikeys(yubikeys: Vec<String>) -> Value {
@@ -73,13 +103,15 @@ fn get_yubico_credentials() -> Result<(String, String), Error> {
 async fn verify_yubikey_otp(otp: String) -> EmptyResult {
     let (yubico_id, yubico_secret) = get_yubico_credentials()?;
 
-    let config = Config::default().set_client_id(yubico_id).set_key(yubico_secret);
-
-    match CONFIG.yubico_server() {
-        Some(server) => verify_async(otp, config.set_api_hosts(vec![server])).await,
-        None => verify_async(otp, config).await,
+    let mut config = Config::default().set_client_id(yubico_id).set_key(yubico_secret)?;
+    if let Some(yubico_server) = CONFIG.yubico_server() {
+        config = config.set_api_host(yubico_server);
     }
-    .map_res("Failed to verify OTP")
+
+    let client = HttpClientTransport::new()?;
+    let verifier = Verifier::with_client(config, client)?;
+
+    verifier.verify(otp).await.map_res("Failed to verify OTP")
 }
 
 #[post("/two-factor/get-yubikey", data = "<data>")]
@@ -137,10 +169,9 @@ async fn activate_yubikey(data: Json<EnableYubikeyData>, headers: Headers, conn:
     let yubikeys = parse_yubikeys(&data);
 
     if yubikeys.is_empty() {
-        return Ok(Json(json!({
-            "enabled": false,
-            "object": "twoFactorU2f",
-        })));
+        // Return an error to prevent saving empty keys which would cause users not being able to login anymore.
+        // To remove all keys users should click the `Deactivate all keys` button
+        err!("A key is required.");
     }
 
     // Ensure they are valid OTPs

src/api/mod.rs

@@ -30,7 +30,7 @@ pub use crate::api::{
     },
     web::catchers as web_catchers,
     web::routes as web_routes,
-    web::static_files,
+    web::{invalidate_css_cache, static_files},
 };
 use crate::{
     CONFIG,

src/api/web.rs

@@ -1,4 +1,7 @@
-use std::path::{Path, PathBuf};
+use std::{
+    path::{Path, PathBuf},
+    sync::{Arc, RwLock},
+};
 
 use rocket::{
     Catcher, Route,
@@ -13,12 +16,13 @@ use crate::{
     CONFIG,
     api::{ApiResult, EmptyResult, core::now},
     auth::decode_file_download,
+    crypto::sha256_hex,
     db::{
         DbConn,
         models::{AttachmentId, CipherId},
     },
     error::Error,
-    util::Cached,
+    util::{Cached, EtagCached},
 };
 
 pub fn routes() -> Vec<Route> {
@@ -63,8 +67,27 @@ fn not_found() -> ApiResult<Html<String>> {
     Ok(Html(text))
 }
 
+struct CssCache {
+    css: String,
+    etag: String,
+}
+
+static CSS_CACHE: RwLock<Option<Arc<CssCache>>> = RwLock::new(None);
+
+pub fn invalidate_css_cache() {
+    *CSS_CACHE.write().unwrap() = None;
+}
+
 #[get("/css/vaultwarden.css")]
-fn vaultwarden_css() -> Cached<Css<String>> {
+fn vaultwarden_css() -> EtagCached<Css<String>> {
+    // If reload_templates is false, and we already have the CSS Cached, return this
+    if !CONFIG.reload_templates()
+        && let Some(cached) = CSS_CACHE.read().unwrap().as_ref()
+    {
+        return EtagCached::new(Css(cached.css.clone()), &cached.etag);
+    }
+
+    // Else, there is either no cache, or reload_templates is true and we need to rebuild the CSS
     let css_options = json!({
         "emergency_access_allowed": CONFIG.emergency_access_allowed(),
         "load_user_scss": true,
@@ -112,8 +135,18 @@ fn vaultwarden_css() -> Cached<Css<String>> {
         }
     };
 
-    // Cache for one day should be enough and not too much
-    Cached::ttl(Css(css), 86_400, false)
+    let etag = sha256_hex(css.as_bytes());
+    let cached = Arc::new(CssCache {
+        css,
+        etag,
+    });
+
+    if !CONFIG.reload_templates() {
+        *CSS_CACHE.write().unwrap() = Some(Arc::clone(&cached));
+    }
+
+    // Etag Caching will let the browser send us an etag to verify and send new content if needed
+    EtagCached::new(Css(cached.css.clone()), &cached.etag)
 }
 
 #[get("/")]

src/config.rs

@@ -1506,6 +1506,9 @@ impl Config {
         let operator = storage::operator_for_path(&CONFIG_FILE_PARENT_DIR)?;
         operator.write(&CONFIG_FILENAME, config_str).await?;
 
+        // Invalidate CSS Cache because several config items might have impact on the rendered CSS
+        crate::api::invalidate_css_cache();
+
         Ok(())
     }
 
@@ -1588,6 +1591,9 @@ impl Config {
             writer._overrides = Vec::new();
         }
 
+        // Invalidate CSS Cache because several config items might have impact on the rendered CSS
+        crate::api::invalidate_css_cache();
+
         Ok(())
     }
 

src/db/models/event.rs

@@ -298,12 +298,16 @@ impl Event {
     ) -> Vec<Self> {
         conn.run(move |conn| {
             event::table
-                .inner_join(users_organizations::table.on(users_organizations::uuid.eq(member_uuid)))
+                .inner_join(
+                    users_organizations::table
+                        .on(users_organizations::uuid.eq(member_uuid).and(users_organizations::org_uuid.eq(org_uuid))),
+                )
                 .filter(event::org_uuid.eq(org_uuid))
                 .filter(event::event_date.between(start, end))
                 .filter(
-                    event::user_uuid
-                        .eq(users_organizations::user_uuid.nullable())
+                    event::org_user_uuid
+                        .eq(member_uuid)
+                        .or(event::user_uuid.eq(users_organizations::user_uuid.nullable()))
                         .or(event::act_user_uuid.eq(users_organizations::user_uuid.nullable())),
                 )
                 .select(event::all_columns)

src/error.rs

@@ -58,7 +58,7 @@ use serde_json::{Error as SerdeErr, Value};
 use std::io::Error as IoErr;
 use std::time::SystemTimeError as TimeErr;
 use webauthn_rs::prelude::WebauthnError as WebauthnErr;
-use yubico::yubicoerror::YubicoError as YubiErr;
+use yubico_ng::error::YubicoError as YubiErr;
 
 #[derive(Serialize)]
 pub struct Empty {}

src/static/scripts/admin.js

@@ -1,6 +1,5 @@
 "use strict";
-/* eslint-env es2017, browser */
-/* exported BASE_URL, _post _delete */
+/* exported BASE_URL, _post, _delete */
 
 function getBaseUrl() {
     // If the base URL is `https://vaultwarden.example.com/base/path/admin/`,

src/static/scripts/admin_diagnostics.js

@@ -1,5 +1,4 @@
 "use strict";
-/* eslint-env es2017, browser */
 /* global BASE_URL:readable, bootstrap:readable */
 
 var dnsCheck = false;
@@ -80,37 +79,44 @@ async function generateSupportString(event, dj) {
     event.preventDefault();
     event.stopPropagation();
 
+    // Health check Markdown emoji, if something is a failure or not
+    const chk = v => v ? "true :white_check_mark:" : "false :x:";
+    // Yes/No Markdown emoji, if something is not a failure, but just yes or no
+    const yn = v => v ? "yes :heavy_plus_sign:" : "no :heavy_minus_sign:";
+
+    const template_overrides = dj.template_overrides !== "" ? ` (${dj.template_overrides})` : "";
     let supportString = "### Your environment (Generated via diagnostics page)\n\n";
 
     supportString += `* Vaultwarden version: v${dj.current_release}\n`;
     supportString += `* Web-vault version: v${dj.active_web_release}\n`;
     supportString += `* OS/Arch: ${dj.host_os}/${dj.host_arch}\n`;
-    supportString += `* Running within a container: ${dj.running_within_container} (Base: ${dj.container_base_image})\n`;
+    supportString += `* Running within a container: ${yn(dj.running_within_container)} (Base: ${dj.container_base_image})\n`;
     supportString += `* Database type: ${dj.db_type}\n`;
     supportString += `* Database version: ${dj.db_version}\n`;
-    supportString += `* Uses config.json: ${dj.overrides !== ""}\n`;
-    supportString += `* Uses a reverse proxy: ${dj.ip_header_exists}\n`;
+    supportString += `* Uses config.json: ${yn(dj.overrides !== "")}\n`;
+    supportString += `* Uses custom templates: ${yn(dj.template_overrides !== "")}${template_overrides}\n`;
+    supportString += `* Uses a reverse proxy: ${yn(dj.ip_header_exists)}\n`;
     if (dj.ip_header_exists) {
-        supportString += `* IP Header check: ${dj.ip_header_match} (${dj.ip_header_name})\n`;
+        supportString += `* IP Header check: ${chk(dj.ip_header_match)} (${dj.ip_header_name})\n`;
     }
-    supportString += `* Internet access: ${dj.has_http_access}\n`;
-    supportString += `* Internet access via a proxy: ${dj.uses_proxy}\n`;
-    supportString += `* DNS Check: ${dnsCheck}\n`;
+    supportString += `* Internet access: ${chk(dj.has_http_access)}\n`;
+    supportString += `* Internet access via a proxy: ${yn(dj.uses_proxy)}\n`;
+    supportString += `* DNS Check: ${chk(dnsCheck)}\n`;
     if (dj.tz_env !== "") {
         supportString += `* TZ environment: ${dj.tz_env}\n`;
     }
-    supportString += `* Browser/Server Time Check: ${timeCheck}\n`;
-    supportString += `* Server/NTP Time Check: ${ntpTimeCheck}\n`;
-    supportString += `* Domain Configuration Check: ${domainCheck}\n`;
-    supportString += `* HTTPS Check: ${httpsCheck}\n`;
+    supportString += `* Browser/Server Time Check: ${chk(timeCheck)}\n`;
+    supportString += `* Server/NTP Time Check: ${chk(ntpTimeCheck)}\n`;
+    supportString += `* Domain Configuration Check: ${chk(domainCheck)}\n`;
+    supportString += `* HTTPS Check: ${chk(httpsCheck)}\n`;
     if (dj.enable_websocket) {
-        supportString += `* Websocket Check: ${websocketCheck}\n`;
+        supportString += `* Websocket Check: ${chk(websocketCheck)}\n`;
     } else {
         supportString += "* Websocket Check: disabled\n";
     }
-    supportString += `* HTTP Response Checks: ${httpResponseCheck}\n`;
+    supportString += `* HTTP Response Checks: ${chk(httpResponseCheck)}\n`;
     if (dj.invalid_feature_flags != "") {
-        supportString += `* Invalid feature flags: true\n`;
+        supportString += "* Invalid feature flags: true\n";
     }
 
     const jsonResponse = await fetch(`${BASE_URL}/admin/diagnostics/config`, {

src/static/scripts/admin_organizations.js

@@ -1,6 +1,5 @@
 "use strict";
-/* eslint-env es2017, browser, jquery */
-/* global _post:readable, BASE_URL:readable, reload:readable, jdenticon:readable */
+/* global jQuery, _post:readable, BASE_URL:readable, reload:readable, jdenticon:readable */
 
 function deleteOrganization(event) {
     event.preventDefault();

src/static/scripts/admin_settings.js

@@ -1,5 +1,4 @@
 "use strict";
-/* eslint-env es2017, browser */
 /* global _post:readable, BASE_URL:readable */
 
 function smtpTest(event) {

src/static/scripts/admin_users.js

@@ -1,6 +1,5 @@
 "use strict";
-/* eslint-env es2017, browser, jquery */
-/* global _post:readable, _delete:readable BASE_URL:readable, reload:readable, jdenticon:readable */
+/* global jQuery, _post:readable, _delete:readable, BASE_URL:readable, reload:readable, jdenticon:readable */
 
 function deleteUser(event) {
     event.preventDefault();

src/static/templates/admin/diagnostics.hbs

@@ -77,6 +77,16 @@
                         <span class="d-block"><b>No</b></span>
                     {{/unless}}
                     </dd>
+                    <dt class="col-sm-5">Uses custom templates</dt>
+                    <dd class="col-sm-7">
+                    {{#if page_data.template_overrides}}
+                        <span class="d-inline"><b>Yes</b></span>
+                        <span class="badge bg-info text-dark abbr-badge" title="Custom template files are used.&#013;&#010;{{page_data.template_overrides}}">Details</span>
+                    {{/if}}
+                    {{#unless page_data.template_overrides}}
+                        <span class="d-block"><b>No</b></span>
+                    {{/unless}}
+                    </dd>
                     <dt class="col-sm-5">Uses a reverse proxy</dt>
                     <dd class="col-sm-7">
                     {{#if page_data.ip_header_exists}}

src/storage.rs

@@ -67,7 +67,7 @@ pub(crate) fn operator_for_path(path: &str) -> Result<opendal::Operator, crate::
         s3::operator_for_path(path)?
     } else {
         let builder = opendal::services::Fs::default().root(path);
-        opendal::Operator::new(builder)?.finish()
+        opendal::Operator::new(builder)?
     };
 
     OPERATORS_BY_PATH.insert(path.to_owned(), operator.clone());
@@ -236,7 +236,7 @@ mod s3 {
                 builder.credential_provider_chain(ProvideCredentialChain::new().push(OpenDALS3CredentialProvider));
         }
 
-        Ok(opendal::Operator::new(builder)?.finish())
+        Ok(opendal::Operator::new(builder)?)
     }
 
     fn uri_has_option(uri: &opendal::OperatorUri, names: &[&str]) -> bool {

src/util.rs

@@ -257,6 +257,44 @@ impl<'r, R: 'r + Responder<'r, 'static> + Send> Responder<'r, 'static> for Cache
     }
 }
 
+pub struct EtagCached<R> {
+    response: R,
+    etag: String,
+}
+
+impl<R> EtagCached<R> {
+    /// An `etag` response should always be quoted
+    pub fn new(response: R, etag: &str) -> Self {
+        Self {
+            response,
+            etag: format!("\"{etag}\""),
+        }
+    }
+}
+
+impl<'r, R: 'r + Responder<'r, 'static> + Send> Responder<'r, 'static> for EtagCached<R> {
+    fn respond_to(self, request: &'r Request<'_>) -> response::Result<'static> {
+        // Check and validate a `If-None-Match` ETag header
+        // Multiple tags could be returned for the same URI if the browser has multiple versions cached
+        // Also, weak tags are prefixed with `W/`, but ETags are always weak, so just strip it too before comparing
+        let etag_matches = request
+            .headers()
+            .get_one("If-None-Match")
+            .is_some_and(|v| v.split(',').any(|t| t.trim().trim_start_matches("W/") == self.etag));
+
+        let mut res = if etag_matches {
+            Response::build().status(Status::NotModified).ok()?
+        } else {
+            self.response.respond_to(request)?
+        };
+
+        // Both 200 (OK) and 304 (Not Modified) need to return the etag and cache-control
+        res.set_raw_header("Etag", self.etag);
+        res.set_raw_header("Cache-Control", "public, no-cache");
+        Ok(res)
+    }
+}
+
 // Log all the routes from the main paths list, and the attachments endpoint
 // Effectively ignores, any static file route, and the alive endpoint
 const LOGGED_ROUTES: [&str; 7] = ["/api", "/admin", "/identity", "/icons", "/attachments", "/events", "/notifications"];