From 3bc5c0873bd8ebdc15832d0e0298b20f67b61d90 Mon Sep 17 00:00:00 2001 From: abnvle Date: Tue, 5 May 2026 21:56:49 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=EF=BB=BFfix(share):=20password-protected?= =?UTF-8?q?=20downloads=20via=20signed=20unlock=20cookie?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After successful POST /api/s/{token}/verify, the server issues a short-lived signed JWT cookie (oxi_share_unlock_; HttpOnly; SameSite=Lax; 1h TTL). Subsequent /api/s/{token} and /api/s/{token}/download requests honour the cookie to bypass the password gate, so password-protected file downloads work end-to-end. - New module src/infrastructure/services/share_unlock_cookie.rs: issue_jwt, verify_jwt, extract_from_cookie_header, build_set_cookie + 10 unit tests. - New ShareService methods issue_unlock_jwt and get_shared_link_with_unlock; trait method get_shared_link_by_token delegates to a private fetch_share_resolved with an allow_password_protected flag. - access_shared_item, verify_shared_item_password, and download_shared_file honour the unlock cookie. Reuses OXICLOUD_JWT_SECRET, no new env var. Auth-token JWTs and unlock-cookie JWTs cannot be confused: auth requires username/email/role/jti claims, unlock has only sub/exp/iat. Cross-share replay rejected via claims.sub == requested_token check. --- src/application/services/share_service.rs | 82 ++++++--- src/infrastructure/services/mod.rs | 1 + .../services/share_unlock_cookie.rs | 168 ++++++++++++++++++ src/interfaces/api/handlers/share_handler.rs | 40 ++++- 4 files changed, 260 insertions(+), 31 deletions(-) create mode 100644 src/infrastructure/services/share_unlock_cookie.rs diff --git a/src/application/services/share_service.rs b/src/application/services/share_service.rs index 14e25800..a86bb1fe 100644 --- a/src/application/services/share_service.rs +++ b/src/application/services/share_service.rs @@ -159,6 +159,60 @@ impl ShareService { Ok(share) } + + /// `allow_password_protected = true` only after the caller's right to + /// bypass has been verified (e.g. via an unlock cookie). + async fn fetch_share_resolved( + &self, + token: &str, + allow_password_protected: bool, + ) -> Result { + let share = self + .share_repository + .find_share_by_token(token) + .await + .map_err(|e| { + ShareServiceError::NotFound(format!("Share with token {} not found: {}", token, e)) + })?; + + if share.is_expired() { + return Err(ShareServiceError::Expired.into()); + } + + if share.has_password() && !allow_password_protected { + return Err(DomainError::new( + ErrorKind::AccessDenied, + "Share", + "This share is password protected", + )); + } + + Ok(ShareDto::from_entity(&share, &self.config.base_url())) + } + + pub fn issue_unlock_jwt(&self, share_token: &str) -> Result { + crate::infrastructure::services::share_unlock_cookie::issue_jwt( + &self.config.auth.jwt_secret, + share_token, + crate::infrastructure::services::share_unlock_cookie::DEFAULT_TTL_SECS, + ) + } + + pub async fn get_shared_link_with_unlock( + &self, + token: &str, + unlock_jwt: Option<&str>, + ) -> Result { + let unlocked = match unlock_jwt { + Some(jwt) => crate::infrastructure::services::share_unlock_cookie::verify_jwt( + &self.config.auth.jwt_secret, + token, + jwt, + ), + None => false, + }; + self.fetch_share_resolved(token, unlocked).await + } } impl ShareUseCase for ShareService { @@ -221,33 +275,7 @@ impl ShareUseCase for ShareService { } async fn get_shared_link_by_token(&self, token: &str) -> Result { - // Find the shared link by its token - let share = self - .share_repository - .find_share_by_token(token) - .await - .map_err(|e| { - ShareServiceError::NotFound(format!("Share with token {} not found: {}", token, e)) - })?; - - // Check if it has expired - if share.is_expired() { - return Err(ShareServiceError::Expired.into()); - } - - // SECURITY: If the share is password-protected, do NOT return - // the full metadata. Force the caller to verify the password - // first via `verify_shared_link_password`. - if share.has_password() { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Share", - "This share is password protected", - )); - } - - // Convert the entity to DTO for the response - Ok(ShareDto::from_entity(&share, &self.config.base_url())) + self.fetch_share_resolved(token, false).await } async fn get_shared_links_for_item( diff --git a/src/infrastructure/services/mod.rs b/src/infrastructure/services/mod.rs index 92fe01bb..1952d231 100644 --- a/src/infrastructure/services/mod.rs +++ b/src/infrastructure/services/mod.rs @@ -21,6 +21,7 @@ pub mod path_resolver_service; pub mod path_service; pub mod retry_blob_backend; pub mod s3_blob_backend; +pub mod share_unlock_cookie; pub mod thumbnail_service; #[cfg(test)] mod thumbnail_service_test; diff --git a/src/infrastructure/services/share_unlock_cookie.rs b/src/infrastructure/services/share_unlock_cookie.rs new file mode 100644 index 00000000..900e4c5b --- /dev/null +++ b/src/infrastructure/services/share_unlock_cookie.rs @@ -0,0 +1,168 @@ +//! Signed cookie issued after `/verify` so subsequent share requests bypass +//! the password gate. JWT carries `sub` (share token), `exp`, `iat`. + +use chrono::Utc; +use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header, Validation, decode, encode}; +use serde::{Deserialize, Serialize}; + +use crate::common::errors::DomainError; + +pub const DEFAULT_TTL_SECS: i64 = 3600; + +#[derive(Debug, Serialize, Deserialize)] +struct UnlockClaims { + sub: String, + exp: i64, + iat: i64, +} + +pub fn issue_jwt(secret: &str, share_token: &str, ttl_secs: i64) -> Result { + if secret.is_empty() { + return Err(DomainError::internal_error( + "ShareUnlockCookie", + "JWT secret is empty", + )); + } + let now = Utc::now().timestamp(); + let claims = UnlockClaims { + sub: share_token.to_string(), + exp: now + ttl_secs, + iat: now, + }; + encode( + &Header::new(Algorithm::HS256), + &claims, + &EncodingKey::from_secret(secret.as_bytes()), + ) + .map_err(|e| DomainError::internal_error("ShareUnlockCookie", format!("sign: {}", e))) +} + +/// `true` iff the JWT is well-formed, signed by `secret`, unexpired, and its +/// `sub` matches `share_token`. Any failure returns `false`. +pub fn verify_jwt(secret: &str, share_token: &str, jwt: &str) -> bool { + if secret.is_empty() || jwt.is_empty() { + return false; + } + let mut validation = Validation::new(Algorithm::HS256); + validation.validate_exp = true; + validation.leeway = 0; + validation.required_spec_claims.clear(); + validation.required_spec_claims.insert("exp".to_string()); + validation.required_spec_claims.insert("sub".to_string()); + + match decode::( + jwt, + &DecodingKey::from_secret(secret.as_bytes()), + &validation, + ) { + Ok(data) => data.claims.sub == share_token, + Err(_) => false, + } +} + +pub fn extract_from_cookie_header(cookie_header: &str, share_token: &str) -> Option { + let target_name = format!("oxi_share_unlock_{}", share_token); + for part in cookie_header.split(';') { + let part = part.trim(); + if let Some(eq_idx) = part.find('=') { + let (name, value) = part.split_at(eq_idx); + if name == target_name { + return Some(value[1..].to_string()); + } + } + } + None +} + +pub fn build_set_cookie(share_token: &str, jwt: &str, ttl_secs: i64) -> String { + format!( + "oxi_share_unlock_{}={}; HttpOnly; SameSite=Lax; Path=/; Max-Age={}", + share_token, jwt, ttl_secs + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + const TEST_SECRET: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + const TOKEN: &str = "49dc31a5-62c8-4ce5-b82c-16092b513805"; + + #[test] + fn issue_then_verify_succeeds() { + let jwt = issue_jwt(TEST_SECRET, TOKEN, 60).expect("issue"); + assert!(verify_jwt(TEST_SECRET, TOKEN, &jwt)); + } + + #[test] + fn verify_rejects_different_token() { + let jwt = issue_jwt(TEST_SECRET, TOKEN, 60).expect("issue"); + assert!(!verify_jwt(TEST_SECRET, "other-token", &jwt)); + } + + #[test] + fn verify_rejects_different_secret() { + let jwt = issue_jwt(TEST_SECRET, TOKEN, 60).expect("issue"); + let other_secret = "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + assert!(!verify_jwt(other_secret, TOKEN, &jwt)); + } + + #[test] + fn verify_rejects_expired_token() { + let jwt = issue_jwt(TEST_SECRET, TOKEN, -10).expect("issue"); + assert!(!verify_jwt(TEST_SECRET, TOKEN, &jwt)); + } + + #[test] + fn verify_rejects_garbage() { + assert!(!verify_jwt(TEST_SECRET, TOKEN, "not.a.jwt")); + assert!(!verify_jwt(TEST_SECRET, TOKEN, "")); + } + + #[test] + fn issue_rejects_empty_secret() { + assert!(issue_jwt("", TOKEN, 60).is_err()); + } + + #[test] + fn verify_rejects_empty_secret_or_jwt() { + let jwt = issue_jwt(TEST_SECRET, TOKEN, 60).expect("issue"); + assert!(!verify_jwt("", TOKEN, &jwt)); + assert!(!verify_jwt(TEST_SECRET, TOKEN, "")); + } + + #[test] + fn extract_from_cookie_header_finds_target() { + let jwt = "abc.def.ghi"; + let header = format!( + "session=xyz; oxi_share_unlock_{}={}; theme=dark", + TOKEN, jwt + ); + assert_eq!( + extract_from_cookie_header(&header, TOKEN), + Some(jwt.to_string()) + ); + } + + #[test] + fn extract_from_cookie_header_returns_none_for_other_token() { + let header = format!("oxi_share_unlock_{}=xxx", TOKEN); + assert_eq!(extract_from_cookie_header(&header, "different-token"), None); + } + + #[test] + fn extract_from_cookie_header_handles_empty() { + assert_eq!(extract_from_cookie_header("", TOKEN), None); + assert_eq!(extract_from_cookie_header("malformed", TOKEN), None); + } + + #[test] + fn build_set_cookie_has_required_attributes() { + let s = build_set_cookie(TOKEN, "jwt.value.here", 3600); + assert!(s.contains(&format!("oxi_share_unlock_{}=jwt.value.here", TOKEN))); + assert!(s.contains("HttpOnly")); + assert!(s.contains("SameSite=Lax")); + assert!(s.contains("Path=/")); + assert!(s.contains("Max-Age=3600")); + } +} diff --git a/src/interfaces/api/handlers/share_handler.rs b/src/interfaces/api/handlers/share_handler.rs index 7df43dbb..34b699ed 100644 --- a/src/interfaces/api/handlers/share_handler.rs +++ b/src/interfaces/api/handlers/share_handler.rs @@ -5,7 +5,7 @@ use axum::{ Json, body::Body, extract::{Path, Query, State}, - http::{StatusCode, header}, + http::{HeaderMap, StatusCode, header}, response::{IntoResponse, Response}, }; use serde::Deserialize; @@ -13,6 +13,7 @@ use serde_json::json; use utoipa::ToSchema; use crate::application::services::share_service::ShareService; +use crate::infrastructure::services::share_unlock_cookie; use crate::{ application::{ dtos::share_dto::{CreateShareDto, UpdateShareDto}, @@ -27,6 +28,15 @@ use crate::{ interfaces::middleware::auth::AuthUser, }; +fn unlock_jwt_from_headers(headers: &HeaderMap, share_token: &str) -> Option { + headers + .get(header::COOKIE) + .and_then(|h| h.to_str().ok()) + .and_then(|cookie_header| { + share_unlock_cookie::extract_from_cookie_header(cookie_header, share_token) + }) +} + #[derive(Debug, Deserialize)] pub struct GetSharesQuery { pub page: Option, @@ -211,12 +221,19 @@ pub async fn delete_shared_link( pub async fn access_shared_item( State(share_use_case): State>, Path(token): Path, + headers: HeaderMap, ) -> impl IntoResponse { // Register the access let _ = share_use_case.register_shared_link_access(&token).await; + // Honour an unlock cookie if one was issued by a prior `/verify` call. + let unlock_jwt = unlock_jwt_from_headers(&headers, &token); + // Get the shared link - match share_use_case.get_shared_link_by_token(&token).await { + match share_use_case + .get_shared_link_with_unlock(&token, unlock_jwt.as_deref()) + .await + { Ok(item) => (StatusCode::OK, Json(item)).into_response(), Err(err) => { // Special handling for share access errors @@ -261,7 +278,17 @@ pub async fn verify_shared_item_password( .verify_shared_link_password(&token, &req.password) .await { - Ok(item) => (StatusCode::OK, Json(item)).into_response(), + Ok(item) => match share_use_case.issue_unlock_jwt(&token) { + Ok(jwt) => { + let cookie = share_unlock_cookie::build_set_cookie( + &token, + &jwt, + share_unlock_cookie::DEFAULT_TTL_SECS, + ); + (StatusCode::OK, [(header::SET_COOKIE, cookie)], Json(item)).into_response() + } + Err(_) => (StatusCode::OK, Json(item)).into_response(), + }, Err(err) => { if err.kind == ErrorKind::AccessDenied { if err.message.contains("expired") { @@ -296,6 +323,7 @@ pub async fn verify_shared_item_password( pub async fn download_shared_file( State(state): State>, Path(token): Path, + headers: HeaderMap, ) -> impl IntoResponse { // 1. Resolve share service let share_service = match &state.share_service { @@ -311,7 +339,11 @@ pub async fn download_shared_file( }; // 2. Validate the share token (handles expiry + password checks) - let share_dto = match share_service.get_shared_link_by_token(&token).await { + let unlock_jwt = unlock_jwt_from_headers(&headers, &token); + let share_dto = match share_service + .get_shared_link_with_unlock(&token, unlock_jwt.as_deref()) + .await + { Ok(dto) => dto, Err(err) => { if err.kind == ErrorKind::AccessDenied { From 8527765bf2f6ea6caf5c9cee2ba5deafc3f4f590 Mon Sep 17 00:00:00 2001 From: abnvle Date: Tue, 5 May 2026 21:57:15 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=EF=BB=BFstyle(share-dialog):=20give=20shar?= =?UTF-8?q?e=20button=20breathing=20room?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Share button moved into .share-options in bdb5a6c sat flush against the section bottom border. Adds 20px bottom padding plus margin-top:16px and margin-left:auto on the button so it aligns right with the Close button below. --- static/css/components/dialogs.css | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/static/css/components/dialogs.css b/static/css/components/dialogs.css index 8790a190..284b2724 100644 --- a/static/css/components/dialogs.css +++ b/static/css/components/dialogs.css @@ -174,7 +174,13 @@ } .share-options { - padding: 20px 24px 0; + padding: 20px 24px 20px; +} + +.share-options > #share-confirm-btn { + display: block; + margin-left: auto; + margin-top: 16px; } .share-options h3,