feat(web): cross-account tenant sharing with eviction persistence
Tier-1 online collaboration: an owner grants accounts into their tenant's ACL (grant/revoke/list under /api/share/), and a visitor addresses it per request with ?tenant=<ownerId> — query rather than header because EventSource cannot set headers, and share routes always administer the caller's own tenant so a grant is not re-shareable. The wasm shell threads the parameter through its four XHR helpers and the event stream in one place. A shared tenant has no collaboration session, so the 409 auto-resolve now also accepts when the daemon advertises serveMode online (the daemon's counter is the total order and SSE is already delivering the newer document; the old latch would freeze a visitor permanently). Evicted tenants persist document + ACL under sha256(user_id) directories via atomic writes — written before the registry remove so no instant lacks both copies; unwritable tenants stay resident and unloadable files are set aside as .corrupt rather than overwritten. Also restores the Dockerfile entrypoint literal the M2 CMD restructure dropped (its CI test only ran in the workspace suite) and splits live_sync_glue under the file cap.
This commit is contained in:
Kayshen-X
parent
79beee711f
commit
5536018c73
|
|
@ -166,6 +166,5 @@ STOPSIGNAL SIGTERM
|
|||
# reverse proxy for anything beyond a trusted network. The port is taken from
|
||||
# the build-time SERVE_PORT (baked into OPENPENCIL_SERVE_PORT); `sh -c` lets the
|
||||
# env var expand at container start.
|
||||
CMD ["sh", "-c", "set -- --serve-web \"${OPENPENCIL_SERVE_PORT}\" --host 0.0.0.0; \
|
||||
if [ \"${OPENPENCIL_SERVE_MODE}\" = online ]; then set -- \"$@\" --online; fi; \
|
||||
exec /app/op-host-web-server \"$@\""]
|
||||
CMD ["sh", "-c", "if [ \"${OPENPENCIL_SERVE_MODE}\" = online ]; then set -- --online; else set --; fi; \
|
||||
exec /app/op-host-web-server --serve-web \"${OPENPENCIL_SERVE_PORT}\" --host 0.0.0.0 \"$@\""]
|
||||
|
|
|
|||
|
|
@ -135,6 +135,7 @@ pub mod scene_template_prompt;
|
|||
pub mod scene_vars;
|
||||
pub mod selection;
|
||||
pub mod selection_resolve;
|
||||
pub mod share_routes;
|
||||
pub mod state;
|
||||
pub mod statusbar_state;
|
||||
pub mod svg_import;
|
||||
|
|
|
|||
81
crates/op-editor-core/src/share_routes.rs
Normal file
81
crates/op-editor-core/src/share_routes.rs
Normal file
|
|
@ -0,0 +1,81 @@
|
|||
//! Tenant-sharing HTTP route paths, shared by the web shell (client) and the
|
||||
//! serve-web daemon (server).
|
||||
//!
|
||||
//! Only the multi-account online deployment serves these; the local and
|
||||
//! managed daemons have exactly one document and nobody to share it with.
|
||||
//! Keeping the paths in one wasm-clean crate both sides already depend on is
|
||||
//! the same arrangement `auth_routes` and `collab_routes` use.
|
||||
|
||||
/// Prefix shared by every route below (used by the server's sensitive-POST /
|
||||
/// CORS gating).
|
||||
pub const API_PREFIX: &str = "/api/share/";
|
||||
|
||||
/// `POST` — add an account to the caller's own access list.
|
||||
///
|
||||
/// Body: `{"userId":"<account id>"}`. The grantor is the request's verified
|
||||
/// identity and is never taken from the body.
|
||||
pub const GRANT: &str = "/api/share/grant";
|
||||
|
||||
/// `POST` — remove an account from the caller's own access list.
|
||||
pub const REVOKE: &str = "/api/share/revoke";
|
||||
|
||||
/// `GET` — who the caller shares with, and who shares with the caller.
|
||||
pub const LIST: &str = "/api/share/list";
|
||||
|
||||
/// Query parameter naming the tenant a request is addressed to.
|
||||
///
|
||||
/// A header would be the more usual choice, but `EventSource` cannot set
|
||||
/// request headers, and `/api/mcp/events` is exactly the route a visitor
|
||||
/// needs most — a shared document that does not push updates is not shared in
|
||||
/// any useful sense. Rather than split the mechanism (header for XHR, query
|
||||
/// for SSE) and have two places to get wrong, everything uses the query.
|
||||
///
|
||||
/// The value is an account id. It is a REQUEST for access, never a grant of
|
||||
/// it: the server still resolves the caller's own identity and checks it
|
||||
/// against the owner's access list.
|
||||
pub const TENANT_QUERY: &str = "tenant";
|
||||
|
||||
/// Read the tenant parameter out of a raw query string (no leading `?`).
|
||||
///
|
||||
/// Deliberately tiny and dependency-free so both the wasm shell and the
|
||||
/// daemon parse it identically. Percent-decoding is not attempted: an account
|
||||
/// id is an opaque token, and a value needing escapes is not one.
|
||||
pub fn tenant_from_query(query: &str) -> Option<&str> {
|
||||
query
|
||||
.split('&')
|
||||
.filter_map(|pair| pair.split_once('='))
|
||||
.find(|(name, _)| *name == TENANT_QUERY)
|
||||
.map(|(_, value)| value)
|
||||
.filter(|value| !value.is_empty())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn the_tenant_parameter_is_read_out_of_a_query_string() {
|
||||
assert_eq!(tenant_from_query("tenant=userA"), Some("userA"));
|
||||
assert_eq!(tenant_from_query("x=1&tenant=userA&y=2"), Some("userA"));
|
||||
assert_eq!(tenant_from_query("y=2&tenant=userA"), Some("userA"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_absent_or_empty_tenant_parameter_is_none() {
|
||||
for query in ["", "x=1", "tenant=", "tenants=userA", "atenant=userA"] {
|
||||
assert_eq!(tenant_from_query(query), None, "{query:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_the_exact_parameter_name_matches() {
|
||||
assert_eq!(tenant_from_query("Tenant=userA"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_route_sits_under_the_declared_prefix() {
|
||||
for route in [GRANT, REVOKE, LIST] {
|
||||
assert!(route.starts_with(API_PREFIX), "{route}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -375,6 +375,13 @@ pub struct HttpRequest {
|
|||
/// `Cookie` header value, verbatim, when present. The online daemon
|
||||
/// extracts its session cookie from it; every other mode ignores it.
|
||||
pub cookie: Option<String>,
|
||||
/// The raw query string (no leading `?`), when the target had one.
|
||||
///
|
||||
/// `path` keeps its query stripped so exact-path routing is unaffected;
|
||||
/// this is captured alongside so the online daemon can read the tenant
|
||||
/// parameter. `EventSource` cannot set headers, which is why the tenant
|
||||
/// travels in the query at all — see `share_routes::TENANT_QUERY`.
|
||||
pub query: Option<String>,
|
||||
}
|
||||
|
||||
/// Parse a capped HTTP header and then read exactly its declared body length.
|
||||
|
|
@ -418,13 +425,16 @@ pub fn read_http_request<S: std::io::Read>(stream: &mut S) -> Result<HttpRequest
|
|||
.to_ascii_uppercase();
|
||||
// Strip any `?query` from the request target so exact-path routing
|
||||
// (`/api/mcp/document`, `/mcp`, …) isn't defeated by `/api/mcp/document?x=1`.
|
||||
let path = request_parts
|
||||
let target = request_parts
|
||||
.next()
|
||||
.ok_or_else(|| McpServeError::Protocol("request path missing".into()))?
|
||||
.split('?')
|
||||
.next()
|
||||
.unwrap_or("")
|
||||
.to_string();
|
||||
.ok_or_else(|| McpServeError::Protocol("request path missing".into()))?;
|
||||
let (path, query) = match target.split_once('?') {
|
||||
Some((path, query)) => (
|
||||
path.to_string(),
|
||||
(!query.is_empty()).then(|| query.to_string()),
|
||||
),
|
||||
None => (target.to_string(), None),
|
||||
};
|
||||
// Parse `Content-Length` via `split_once(':')` — byte-slicing a `&str`
|
||||
// (e.g. `l[..15]`) would panic if a crafted header puts a multibyte UTF-8
|
||||
// boundary mid-slice; that panic would also bypass the live server's
|
||||
|
|
@ -519,6 +529,7 @@ pub fn read_http_request<S: std::io::Read>(stream: &mut S) -> Result<HttpRequest
|
|||
content_type,
|
||||
authorization,
|
||||
cookie,
|
||||
query,
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -495,9 +495,15 @@ pub fn handle_web_canvas_request(
|
|||
// `{running,port,localIp}` matches TS `server.get.ts`; the daemon
|
||||
// binds 127.0.0.1 (localhost-only) so localIp is loopback. Extra
|
||||
// `server`/`mode` fields are additive diagnostics.
|
||||
// `serveMode` is additive: the browser reads it to learn whether
|
||||
// the daemon is the sole sequencer for this document (online) or
|
||||
// merely a peer holding the operator's file (local/managed). That
|
||||
// decides whether a sync conflict may be auto-resolved — see
|
||||
// `op-host-web/src/live_sync_glue.rs::auto_resolve_is_safe`.
|
||||
body: format!(
|
||||
r#"{{"running":true,"port":{},"localIp":"127.0.0.1","server":"openpencil-mcp","mode":"web-canvas"}}"#,
|
||||
state.port
|
||||
r#"{{"running":true,"port":{},"localIp":"127.0.0.1","server":"openpencil-mcp","mode":"web-canvas","serveMode":"{}"}}"#,
|
||||
state.port,
|
||||
state.mode.wire_name()
|
||||
),
|
||||
},
|
||||
("POST", "/api/mcp/server") => update_mcp_server_settings(body, state),
|
||||
|
|
@ -733,8 +739,10 @@ mod online_run_loop;
|
|||
mod origin_guard;
|
||||
mod run_loop;
|
||||
mod serve_options;
|
||||
mod share_routes;
|
||||
pub mod tenant;
|
||||
pub mod tenant_auth;
|
||||
pub mod tenant_store;
|
||||
|
||||
pub use collab_state::DaemonMutationRefusal;
|
||||
pub use connect_routes::*;
|
||||
|
|
@ -747,10 +755,12 @@ pub use online_run_loop::*;
|
|||
use origin_guard::*;
|
||||
pub use run_loop::*;
|
||||
pub use serve_options::*;
|
||||
pub use tenant::{TenantError, TenantLimits, TenantRegistry};
|
||||
pub use share_routes::ShareError;
|
||||
pub use tenant::{TenantError, TenantLease, TenantLimits, TenantRegistry};
|
||||
pub use tenant_auth::{
|
||||
IdentityVerifier, OnlineAuthError, PresentedCredentials, ResolvedIdentity, StaticVerifier,
|
||||
};
|
||||
pub use tenant_store::{TenantStore, TenantStoreError};
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "web_canvas_server_tests.rs"]
|
||||
|
|
|
|||
|
|
@ -301,6 +301,7 @@ fn the_collab_routes_are_gated_as_sensitive_browser_posts() {
|
|||
token: None,
|
||||
authorization: None,
|
||||
cookie: None,
|
||||
query: None,
|
||||
};
|
||||
assert!(
|
||||
super::super::is_sensitive_browser_post(&request),
|
||||
|
|
|
|||
|
|
@ -28,6 +28,25 @@ pub enum ServeMode {
|
|||
}
|
||||
|
||||
impl ServeMode {
|
||||
/// Stable name for the wire, read by the browser shell.
|
||||
pub const fn wire_name(self) -> &'static str {
|
||||
match self {
|
||||
Self::Local => "local",
|
||||
Self::Managed => "managed",
|
||||
Self::Online => "online",
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether the daemon is the SOLE sequencer for the document it serves.
|
||||
///
|
||||
/// Online it is: every writer reaches the same in-memory document through
|
||||
/// the same version counter, and the SSE stream publishes the result to
|
||||
/// everyone. Locally it is not — the browser and the operator's file are
|
||||
/// two copies and the daemon arbitrates neither.
|
||||
pub const fn is_server_authoritative(self) -> bool {
|
||||
self.is_online()
|
||||
}
|
||||
|
||||
pub const fn is_online(self) -> bool {
|
||||
matches!(self, Self::Online)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -269,7 +269,19 @@ pub(super) fn serve_one_online<S: Read + Write>(
|
|||
)?;
|
||||
return Ok(false);
|
||||
}
|
||||
let lease = match registry.lease_for(&identity) {
|
||||
// Which tenant is this request addressed to? The query names an OWNER;
|
||||
// whether this caller may reach it is still decided by the owner's access
|
||||
// list, never by the parameter. Absent, it is the caller's own tenant.
|
||||
let requested_owner = req
|
||||
.query
|
||||
.as_deref()
|
||||
.and_then(op_editor_core::share_routes::tenant_from_query)
|
||||
.map(str::to_string);
|
||||
let lease = match requested_owner.as_deref() {
|
||||
Some(owner) => registry.lease_for_shared(owner, &identity),
|
||||
None => registry.lease_for(&identity),
|
||||
};
|
||||
let lease = match lease {
|
||||
Ok(lease) => lease,
|
||||
Err(error) => {
|
||||
crate::mcp_serve::write_mcp_http_response(
|
||||
|
|
@ -285,6 +297,43 @@ pub(super) fn serve_one_online<S: Read + Write>(
|
|||
return Ok(false);
|
||||
}
|
||||
};
|
||||
// Sharing is administered on the CALLER's own tenant, so it is answered
|
||||
// here rather than in the document tier — a visitor holding a `?tenant=`
|
||||
// lease on someone else's document must not be able to re-share it.
|
||||
if super::share_routes::is_share_route(&req.path) {
|
||||
let own = match registry.lease_for(&identity) {
|
||||
Ok(own) => own,
|
||||
Err(error) => {
|
||||
crate::mcp_serve::write_mcp_http_response_with_origin(
|
||||
stream,
|
||||
error.http_status(),
|
||||
&serde_json::json!({
|
||||
"ok": false,
|
||||
"error": error.code(),
|
||||
"message": error.to_string(),
|
||||
})
|
||||
.to_string(),
|
||||
cors_origin.as_deref(),
|
||||
)?;
|
||||
return Ok(false);
|
||||
}
|
||||
};
|
||||
let reply = super::share_routes::handle(
|
||||
&req.method,
|
||||
&req.path,
|
||||
&req.body,
|
||||
&identity,
|
||||
&own,
|
||||
registry,
|
||||
);
|
||||
crate::mcp_serve::write_mcp_http_response_with_origin(
|
||||
stream,
|
||||
reply.status,
|
||||
&reply.body,
|
||||
cors_origin.as_deref(),
|
||||
)?;
|
||||
return Ok(false);
|
||||
}
|
||||
// The lease outlives the dispatch (including a minutes-long SSE stream),
|
||||
// so the tenant these borrows point at cannot be evicted underneath them.
|
||||
dispatch(
|
||||
|
|
|
|||
|
|
@ -41,6 +41,9 @@ struct Request {
|
|||
content_type: Option<&'static str>,
|
||||
cookie: Option<&'static str>,
|
||||
origin: Option<&'static str>,
|
||||
/// Addresses the request at another account's tenant, as the browser does
|
||||
/// with `?tenant=` on the page URL.
|
||||
tenant: Option<&'static str>,
|
||||
}
|
||||
|
||||
impl Request {
|
||||
|
|
@ -53,6 +56,7 @@ impl Request {
|
|||
content_type: None,
|
||||
cookie: None,
|
||||
origin: None,
|
||||
tenant: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -97,11 +101,14 @@ impl Request {
|
|||
.origin
|
||||
.map(|o| format!("Origin: {o}\r\n"))
|
||||
.unwrap_or_default();
|
||||
let target = match self.tenant {
|
||||
Some(tenant) => format!("{}?tenant={tenant}", self.path),
|
||||
None => self.path.to_string(),
|
||||
};
|
||||
format!(
|
||||
"{} {} HTTP/1.1\r\nHost: canvas.example\r\n{auth}{cookie}{origin}{content_type}\
|
||||
"{} {target} HTTP/1.1\r\nHost: canvas.example\r\n{auth}{cookie}{origin}{content_type}\
|
||||
Content-Length: {}\r\n\r\n{}",
|
||||
self.method,
|
||||
self.path,
|
||||
self.body.len(),
|
||||
self.body
|
||||
)
|
||||
|
|
@ -747,3 +754,7 @@ fn a_disallowed_origin_gets_no_cors_header_at_all() {
|
|||
#[cfg(test)]
|
||||
#[path = "online_mcp_tests.rs"]
|
||||
mod mcp_profile;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "online_share_tests.rs"]
|
||||
mod share;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,409 @@
|
|||
//! Sharing, the tenant query parameter, and eviction persistence, exercised
|
||||
//! end to end through the online accept loop.
|
||||
//!
|
||||
//! Split out of `online_run_loop_tests.rs` at the 800-line cap; nested under
|
||||
//! it so `use super::*` still reaches the request builder and helpers.
|
||||
|
||||
use super::*;
|
||||
use crate::web_canvas_server::tenant_store::TenantStore;
|
||||
|
||||
/// A registry with a real on-disk store rooted in a temp directory.
|
||||
struct PersistentRegistry {
|
||||
root: std::path::PathBuf,
|
||||
registry: TenantRegistry,
|
||||
}
|
||||
|
||||
impl PersistentRegistry {
|
||||
fn new(label: &str) -> Self {
|
||||
let root = std::env::temp_dir().join(format!(
|
||||
"op-online-share-{label}-{}-{:?}",
|
||||
std::process::id(),
|
||||
std::thread::current().id()
|
||||
));
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
std::fs::create_dir_all(&root).expect("temp root");
|
||||
Self {
|
||||
registry: TenantRegistry::with_store(
|
||||
3102,
|
||||
TenantLimits {
|
||||
idle_evict_secs: 1,
|
||||
..TenantLimits::default()
|
||||
},
|
||||
vec![PUBLIC_ORIGIN.to_string()],
|
||||
TenantStore::new(Some(root.clone())),
|
||||
),
|
||||
root,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for PersistentRegistry {
|
||||
fn drop(&mut self) {
|
||||
let _ = std::fs::remove_dir_all(&self.root);
|
||||
}
|
||||
}
|
||||
|
||||
/// Address a request at another account's tenant.
|
||||
fn as_tenant(mut request: Request, owner: &'static str) -> Request {
|
||||
request.tenant = Some(owner);
|
||||
request
|
||||
}
|
||||
|
||||
fn share(
|
||||
registry: &TenantRegistry,
|
||||
token: &'static str,
|
||||
route: &'static str,
|
||||
target: &str,
|
||||
) -> String {
|
||||
serve(
|
||||
registry,
|
||||
&verifier(),
|
||||
Request::json(
|
||||
"POST",
|
||||
route,
|
||||
&serde_json::json!({ "userId": target }).to_string(),
|
||||
)
|
||||
.with_bearer(token),
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_visitor_reads_and_writes_the_owner_document_only_after_a_grant() {
|
||||
let registry = registry();
|
||||
let verifier = verifier();
|
||||
|
||||
// Before the grant, addressing userA's tenant is refused.
|
||||
let refused = serve(
|
||||
®istry,
|
||||
&verifier,
|
||||
as_tenant(
|
||||
Request::new("GET", "/api/mcp/document").with_bearer("tokB"),
|
||||
"userA",
|
||||
),
|
||||
);
|
||||
assert_eq!(status_line(&refused), "HTTP/1.1 403 Forbidden", "{refused}");
|
||||
assert_eq!(body(&refused)["error"], "tenant-not-shared");
|
||||
|
||||
// userA shares.
|
||||
let granted = share(
|
||||
®istry,
|
||||
"tokA",
|
||||
op_editor_core::share_routes::GRANT,
|
||||
"userB",
|
||||
);
|
||||
assert_eq!(status_line(&granted), "HTTP/1.1 200 OK", "{granted}");
|
||||
|
||||
// Now userB writes into userA's document…
|
||||
let pushed = serve(
|
||||
®istry,
|
||||
&verifier,
|
||||
as_tenant(
|
||||
Request::json("POST", "/api/mcp/document", SYNC_BODY).with_bearer("tokB"),
|
||||
"userA",
|
||||
),
|
||||
);
|
||||
assert_eq!(status_line(&pushed), "HTTP/1.1 200 OK", "{pushed}");
|
||||
|
||||
// …and userA sees it in their own tenant, with no parameter at all.
|
||||
let owner_view = serve(
|
||||
®istry,
|
||||
&verifier,
|
||||
Request::new("GET", "/api/mcp/document").with_bearer("tokA"),
|
||||
);
|
||||
assert!(owner_view.contains("Tenant Rect"), "{owner_view}");
|
||||
assert_eq!(body(&owner_view)["version"], 1);
|
||||
|
||||
// userB's OWN document is untouched — the parameter addressed a tenant,
|
||||
// it did not move the visitor into it.
|
||||
let visitor_own = serve(
|
||||
®istry,
|
||||
&verifier,
|
||||
Request::new("GET", "/api/mcp/document").with_bearer("tokB"),
|
||||
);
|
||||
assert_eq!(body(&visitor_own)["version"], 0, "{visitor_own}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_revoke_locks_the_visitor_out_again() {
|
||||
let registry = registry();
|
||||
share(
|
||||
®istry,
|
||||
"tokA",
|
||||
op_editor_core::share_routes::GRANT,
|
||||
"userB",
|
||||
);
|
||||
let allowed = serve(
|
||||
®istry,
|
||||
&verifier(),
|
||||
as_tenant(
|
||||
Request::new("GET", "/api/mcp/version").with_bearer("tokB"),
|
||||
"userA",
|
||||
),
|
||||
);
|
||||
assert_eq!(status_line(&allowed), "HTTP/1.1 200 OK");
|
||||
|
||||
share(
|
||||
®istry,
|
||||
"tokA",
|
||||
op_editor_core::share_routes::REVOKE,
|
||||
"userB",
|
||||
);
|
||||
let refused = serve(
|
||||
®istry,
|
||||
&verifier(),
|
||||
as_tenant(
|
||||
Request::new("GET", "/api/mcp/version").with_bearer("tokB"),
|
||||
"userA",
|
||||
),
|
||||
);
|
||||
assert_eq!(status_line(&refused), "HTTP/1.1 403 Forbidden", "{refused}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_tenant_parameter_naming_an_unshared_account_is_refused() {
|
||||
let registry = registry();
|
||||
for owner in ["userA", "nobody-at-all"] {
|
||||
let response = serve(
|
||||
®istry,
|
||||
&verifier(),
|
||||
Request {
|
||||
tenant: Some(Box::leak(owner.to_string().into_boxed_str())),
|
||||
..Request::new("GET", "/api/mcp/document")
|
||||
}
|
||||
.with_bearer("tokB"),
|
||||
);
|
||||
assert_eq!(status_line(&response), "HTTP/1.1 403 Forbidden", "{owner}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_account_may_always_address_its_own_tenant_explicitly() {
|
||||
let registry = registry();
|
||||
let response = serve(
|
||||
®istry,
|
||||
&verifier(),
|
||||
as_tenant(
|
||||
Request::new("GET", "/api/mcp/version").with_bearer("tokA"),
|
||||
"userA",
|
||||
),
|
||||
);
|
||||
assert_eq!(status_line(&response), "HTTP/1.1 200 OK", "{response}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_share_list_reports_both_directions_over_the_wire() {
|
||||
let registry = registry();
|
||||
share(
|
||||
®istry,
|
||||
"tokA",
|
||||
op_editor_core::share_routes::GRANT,
|
||||
"userB",
|
||||
);
|
||||
|
||||
let owner = serve(
|
||||
®istry,
|
||||
&verifier(),
|
||||
Request::new("GET", op_editor_core::share_routes::LIST).with_bearer("tokA"),
|
||||
);
|
||||
assert_eq!(body(&owner)["sharedWith"][0], "userB", "{owner}");
|
||||
|
||||
let visitor = serve(
|
||||
®istry,
|
||||
&verifier(),
|
||||
Request::new("GET", op_editor_core::share_routes::LIST).with_bearer("tokB"),
|
||||
);
|
||||
assert_eq!(body(&visitor)["sharedWithMe"][0], "userA", "{visitor}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_share_route_always_administers_the_callers_own_tenant() {
|
||||
// Even with a `?tenant=` parameter pointing at the owner, a visitor's
|
||||
// grant lands on the visitor's own access list.
|
||||
let registry = registry();
|
||||
share(
|
||||
®istry,
|
||||
"tokA",
|
||||
op_editor_core::share_routes::GRANT,
|
||||
"userB",
|
||||
);
|
||||
let response = serve(
|
||||
®istry,
|
||||
&verifier(),
|
||||
as_tenant(
|
||||
Request::json(
|
||||
"POST",
|
||||
op_editor_core::share_routes::GRANT,
|
||||
r#"{"userId":"userC"}"#,
|
||||
)
|
||||
.with_bearer("tokB"),
|
||||
"userA",
|
||||
),
|
||||
);
|
||||
assert_eq!(status_line(&response), "HTTP/1.1 200 OK", "{response}");
|
||||
|
||||
// userC still cannot reach userA.
|
||||
let stranger = serve(
|
||||
®istry,
|
||||
&verifier(),
|
||||
as_tenant(
|
||||
Request::new("GET", "/api/mcp/version").with_bearer("tokC"),
|
||||
"userA",
|
||||
),
|
||||
);
|
||||
assert_eq!(
|
||||
status_line(&stranger),
|
||||
"HTTP/1.1 401 Unauthorized",
|
||||
"{stranger}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_evicted_tenant_is_written_and_read_back() {
|
||||
let temp = PersistentRegistry::new("roundtrip");
|
||||
let verifier = verifier();
|
||||
|
||||
serve(
|
||||
&temp.registry,
|
||||
&verifier,
|
||||
Request::json("POST", "/api/mcp/document", SYNC_BODY).with_bearer("tokA"),
|
||||
);
|
||||
share(
|
||||
&temp.registry,
|
||||
"tokA",
|
||||
op_editor_core::share_routes::GRANT,
|
||||
"userB",
|
||||
);
|
||||
|
||||
assert_eq!(temp.registry.evict_idle(now_unix() + 3600), 1);
|
||||
assert_eq!(temp.registry.tenant_count(), 0);
|
||||
assert!(temp.registry.store().has_document("userA"));
|
||||
|
||||
// The document comes back…
|
||||
let restored = serve(
|
||||
&temp.registry,
|
||||
&verifier,
|
||||
Request::new("GET", "/api/mcp/document").with_bearer("tokA"),
|
||||
);
|
||||
assert!(restored.contains("Tenant Rect"), "{restored}");
|
||||
|
||||
// …and so does the access list, so a share survives a reclaim.
|
||||
let visitor = serve(
|
||||
&temp.registry,
|
||||
&verifier,
|
||||
as_tenant(
|
||||
Request::new("GET", "/api/mcp/version").with_bearer("tokB"),
|
||||
"userA",
|
||||
),
|
||||
);
|
||||
assert_eq!(status_line(&visitor), "HTTP/1.1 200 OK", "{visitor}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_grant_is_persisted_immediately_rather_than_at_eviction() {
|
||||
// A share the user was told had succeeded must survive a restart, and the
|
||||
// document it applies to may not be written for another half hour.
|
||||
let temp = PersistentRegistry::new("acl-now");
|
||||
share(
|
||||
&temp.registry,
|
||||
"tokA",
|
||||
op_editor_core::share_routes::GRANT,
|
||||
"userB",
|
||||
);
|
||||
assert!(
|
||||
temp.registry.store().load_acl("userA").contains("userB"),
|
||||
"the grant must be on disk before any eviction"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_corrupt_stored_document_yields_a_starter_and_keeps_the_bytes() {
|
||||
let temp = PersistentRegistry::new("corrupt");
|
||||
let verifier = verifier();
|
||||
serve(
|
||||
&temp.registry,
|
||||
&verifier,
|
||||
Request::json("POST", "/api/mcp/document", SYNC_BODY).with_bearer("tokA"),
|
||||
);
|
||||
assert_eq!(temp.registry.evict_idle(now_unix() + 3600), 1);
|
||||
|
||||
let dir = temp.registry.store().tenant_dir("userA").expect("dir");
|
||||
std::fs::write(dir.join("current.op"), b"not a document").expect("corrupt");
|
||||
|
||||
let served = serve(
|
||||
&temp.registry,
|
||||
&verifier,
|
||||
Request::new("GET", "/api/mcp/document").with_bearer("tokA"),
|
||||
);
|
||||
assert_eq!(status_line(&served), "HTTP/1.1 200 OK", "{served}");
|
||||
assert!(
|
||||
!served.contains("Tenant Rect"),
|
||||
"an unreadable document must yield a starter, not a failure: {served}"
|
||||
);
|
||||
let quarantined: Vec<String> = std::fs::read_dir(&dir)
|
||||
.expect("read dir")
|
||||
.filter_map(|entry| entry.ok())
|
||||
.map(|entry| entry.file_name().to_string_lossy().into_owned())
|
||||
.filter(|name| name.contains("corrupt"))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
quarantined.len(),
|
||||
1,
|
||||
"the bytes must be kept, not overwritten"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_account_id_full_of_traversal_cannot_escape_the_data_directory() {
|
||||
let temp = PersistentRegistry::new("traversal");
|
||||
let hostile = "../../../../etc/op-escape";
|
||||
let verifier = StaticVerifier::parse(&format!("tokX={hostile}"));
|
||||
|
||||
serve(
|
||||
&temp.registry,
|
||||
&verifier,
|
||||
Request::json("POST", "/api/mcp/document", SYNC_BODY).with_bearer("tokX"),
|
||||
);
|
||||
assert_eq!(temp.registry.evict_idle(now_unix() + 3600), 1);
|
||||
|
||||
let dir = temp.registry.store().tenant_dir(hostile).expect("dir");
|
||||
assert!(
|
||||
dir.starts_with(&temp.root),
|
||||
"{dir:?} escaped {:?}",
|
||||
temp.root
|
||||
);
|
||||
assert!(
|
||||
dir.join("current.op").is_file(),
|
||||
"the document still round-trips"
|
||||
);
|
||||
// Nothing was created outside the store.
|
||||
assert!(!std::path::Path::new("/etc/op-escape").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_tenant_that_cannot_be_written_stays_resident_rather_than_losing_its_document() {
|
||||
let temp = PersistentRegistry::new("unwritable");
|
||||
let verifier = verifier();
|
||||
serve(
|
||||
&temp.registry,
|
||||
&verifier,
|
||||
Request::json("POST", "/api/mcp/document", SYNC_BODY).with_bearer("tokA"),
|
||||
);
|
||||
// Make the store root a file so `create_dir_all` cannot succeed.
|
||||
std::fs::remove_dir_all(&temp.root).expect("clear root");
|
||||
std::fs::write(&temp.root, b"not a directory").expect("block the root");
|
||||
|
||||
assert_eq!(
|
||||
temp.registry.evict_idle(now_unix() + 3600),
|
||||
0,
|
||||
"reclaiming memory must not be worth discarding a document"
|
||||
);
|
||||
assert_eq!(temp.registry.tenant_count(), 1);
|
||||
let still_there = serve(
|
||||
&temp.registry,
|
||||
&verifier,
|
||||
Request::new("GET", "/api/mcp/document").with_bearer("tokA"),
|
||||
);
|
||||
assert!(still_there.contains("Tenant Rect"), "{still_there}");
|
||||
|
||||
let _ = std::fs::remove_file(&temp.root);
|
||||
}
|
||||
188
crates/op-host-services/src/web_canvas_server/share_routes.rs
Normal file
188
crates/op-host-services/src/web_canvas_server/share_routes.rs
Normal file
|
|
@ -0,0 +1,188 @@
|
|||
//! `/api/share/*` handlers — who else may open this account's document.
|
||||
//!
|
||||
//! Online only. The local and managed daemons have one document and one
|
||||
//! operator, so there is nothing to share and these routes are not mounted.
|
||||
//!
|
||||
//! ## The one invariant
|
||||
//!
|
||||
//! The grantor is always the request's VERIFIED identity. The body names the
|
||||
//! account being granted, never the account doing the granting — otherwise
|
||||
//! any caller could add themselves to any document's access list, which is
|
||||
//! the whole security property inverted.
|
||||
//!
|
||||
//! Grants run on the connection thread rather than under the document lock:
|
||||
//! the access list is its own mutex on the tenant, so sharing is answerable
|
||||
//! while a large document push is in flight.
|
||||
|
||||
use op_editor_core::share_routes;
|
||||
|
||||
use super::tenant::{TenantLease, TenantRegistry};
|
||||
use super::tenant_auth::ResolvedIdentity;
|
||||
use super::WebReply;
|
||||
|
||||
/// Longest body either POST accepts. Both are a single short account id.
|
||||
const MAX_SHARE_BODY_BYTES: usize = 4 * 1024;
|
||||
|
||||
/// Longest account id accepted in a body.
|
||||
const MAX_ACCOUNT_ID_CHARS: usize = 256;
|
||||
|
||||
/// Why a share request was refused.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum ShareError {
|
||||
BodyTooLarge,
|
||||
MalformedRequest,
|
||||
/// The body named the caller's own account.
|
||||
SelfShare,
|
||||
}
|
||||
|
||||
impl ShareError {
|
||||
pub const fn code(self) -> &'static str {
|
||||
match self {
|
||||
Self::BodyTooLarge => "payload-too-large",
|
||||
Self::MalformedRequest => "malformed-share-request",
|
||||
Self::SelfShare => "cannot-share-with-self",
|
||||
}
|
||||
}
|
||||
|
||||
pub const fn http_status(self) -> &'static str {
|
||||
match self {
|
||||
Self::BodyTooLarge => "413 Payload Too Large",
|
||||
Self::MalformedRequest | Self::SelfShare => "400 Bad Request",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for ShareError {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.write_str(match self {
|
||||
Self::BodyTooLarge => "body too large",
|
||||
Self::MalformedRequest => "malformed share request",
|
||||
Self::SelfShare => "an account already has access to its own document",
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for ShareError {}
|
||||
|
||||
/// Whether `path` is one of the share routes.
|
||||
pub(super) fn is_share_route(path: &str) -> bool {
|
||||
matches!(
|
||||
path,
|
||||
share_routes::GRANT | share_routes::REVOKE | share_routes::LIST
|
||||
)
|
||||
}
|
||||
|
||||
/// Dispatch one `/api/share/*` request.
|
||||
///
|
||||
/// `lease` is the CALLER's own tenant: grant and revoke edit the caller's
|
||||
/// access list, never the tenant a `?tenant=` parameter pointed at. A visitor
|
||||
/// cannot re-share a document they were merely given access to.
|
||||
pub(super) fn handle(
|
||||
method: &str,
|
||||
path: &str,
|
||||
body: &str,
|
||||
identity: &ResolvedIdentity,
|
||||
lease: &TenantLease,
|
||||
registry: &TenantRegistry,
|
||||
) -> WebReply {
|
||||
match (method, path) {
|
||||
("POST", share_routes::GRANT) => mutate(body, identity, lease, registry, true),
|
||||
("POST", share_routes::REVOKE) => mutate(body, identity, lease, registry, false),
|
||||
("GET", share_routes::LIST) => list(identity, lease, registry),
|
||||
_ => WebReply {
|
||||
status: "405 Method Not Allowed",
|
||||
body: crate::mcp_serve::rest_error_body("method not allowed for this share route"),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn mutate(
|
||||
body: &str,
|
||||
identity: &ResolvedIdentity,
|
||||
lease: &TenantLease,
|
||||
registry: &TenantRegistry,
|
||||
granting: bool,
|
||||
) -> WebReply {
|
||||
let account = match parse_account(body, &identity.user_id) {
|
||||
Ok(account) => account,
|
||||
Err(error) => return error_reply(error),
|
||||
};
|
||||
let changed = if granting {
|
||||
lease.tenant().grant(&account)
|
||||
} else {
|
||||
lease.tenant().revoke(&account)
|
||||
};
|
||||
if changed {
|
||||
// Persisted immediately rather than at eviction: a share the user was
|
||||
// told had succeeded must survive a restart, and the document it
|
||||
// applies to may not be written for another half hour.
|
||||
registry.persist_acl(lease.owner_id(), lease.tenant());
|
||||
}
|
||||
WebReply {
|
||||
status: "200 OK",
|
||||
body: serde_json::json!({
|
||||
"ok": true,
|
||||
"changed": changed,
|
||||
"sharedWith": lease.tenant().shared_with().into_iter().collect::<Vec<_>>(),
|
||||
})
|
||||
.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
fn list(identity: &ResolvedIdentity, lease: &TenantLease, registry: &TenantRegistry) -> WebReply {
|
||||
WebReply {
|
||||
status: "200 OK",
|
||||
body: serde_json::json!({
|
||||
"ok": true,
|
||||
// Who may open this account's document…
|
||||
"sharedWith": lease.tenant().shared_with().into_iter().collect::<Vec<_>>(),
|
||||
// …and whose documents this account may open. Resident owners
|
||||
// only; see `TenantRegistry::shared_with_visitor`.
|
||||
"sharedWithMe": registry.shared_with_visitor(&identity.user_id),
|
||||
})
|
||||
.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Pull the target account out of a share body.
|
||||
///
|
||||
/// The account id is not validated against the hub: this deployment has no
|
||||
/// user-lookup endpoint yet, so an id that belongs to nobody simply grants
|
||||
/// access to nobody. M5 should resolve it through the hub so a typo is
|
||||
/// reported at grant time instead of silently doing nothing.
|
||||
fn parse_account(body: &str, caller: &str) -> Result<String, ShareError> {
|
||||
if body.len() > MAX_SHARE_BODY_BYTES {
|
||||
return Err(ShareError::BodyTooLarge);
|
||||
}
|
||||
let parsed: serde_json::Value =
|
||||
serde_json::from_str(body).map_err(|_| ShareError::MalformedRequest)?;
|
||||
let account = parsed
|
||||
.get("userId")
|
||||
.and_then(|value| value.as_str())
|
||||
.map(str::trim)
|
||||
.filter(|value| !value.is_empty())
|
||||
.ok_or(ShareError::MalformedRequest)?;
|
||||
if account.chars().count() > MAX_ACCOUNT_ID_CHARS {
|
||||
return Err(ShareError::MalformedRequest);
|
||||
}
|
||||
if account == caller {
|
||||
return Err(ShareError::SelfShare);
|
||||
}
|
||||
Ok(account.to_string())
|
||||
}
|
||||
|
||||
fn error_reply(error: ShareError) -> WebReply {
|
||||
WebReply {
|
||||
status: error.http_status(),
|
||||
body: serde_json::json!({
|
||||
"ok": false,
|
||||
"error": error.code(),
|
||||
"message": error.to_string(),
|
||||
})
|
||||
.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "share_routes_tests.rs"]
|
||||
mod tests;
|
||||
|
|
@ -0,0 +1,247 @@
|
|||
//! Tests for the share administration routes and their admission rules.
|
||||
|
||||
use super::*;
|
||||
use crate::mcp_serve::tool_profile::McpScopes;
|
||||
use crate::web_canvas_server::tenant::{TenantError, TenantLimits};
|
||||
use crate::web_canvas_server::tenant_auth::IdentityVia;
|
||||
|
||||
fn identity(user_id: &str) -> ResolvedIdentity {
|
||||
ResolvedIdentity {
|
||||
user_id: user_id.into(),
|
||||
username: user_id.into(),
|
||||
display_name: user_id.into(),
|
||||
via: IdentityVia::ApiToken,
|
||||
scopes: McpScopes::FULL,
|
||||
}
|
||||
}
|
||||
|
||||
fn registry() -> TenantRegistry {
|
||||
TenantRegistry::with_store(
|
||||
3102,
|
||||
TenantLimits::default(),
|
||||
Vec::new(),
|
||||
super::super::tenant_store::TenantStore::new(None),
|
||||
)
|
||||
}
|
||||
|
||||
fn body_of(reply: &WebReply) -> serde_json::Value {
|
||||
serde_json::from_str(&reply.body).expect("json body")
|
||||
}
|
||||
|
||||
fn grant(registry: &TenantRegistry, owner: &str, target: &str) -> WebReply {
|
||||
let identity = identity(owner);
|
||||
let lease = registry.lease_for(&identity).expect("lease");
|
||||
handle(
|
||||
"POST",
|
||||
share_routes::GRANT,
|
||||
&serde_json::json!({ "userId": target }).to_string(),
|
||||
&identity,
|
||||
&lease,
|
||||
registry,
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_grant_admits_the_named_account_and_nobody_else() {
|
||||
let registry = registry();
|
||||
let reply = grant(®istry, "userA", "userB");
|
||||
assert_eq!(reply.status, "200 OK");
|
||||
assert_eq!(body_of(&reply)["changed"], true);
|
||||
|
||||
let visitor = identity("userB");
|
||||
assert!(registry.lease_for_shared("userA", &visitor).is_ok());
|
||||
|
||||
let stranger = identity("userC");
|
||||
assert_eq!(
|
||||
registry.lease_for_shared("userA", &stranger).unwrap_err(),
|
||||
TenantError::NotShared
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_revoke_takes_effect_on_the_next_request() {
|
||||
let registry = registry();
|
||||
grant(®istry, "userA", "userB");
|
||||
let visitor = identity("userB");
|
||||
assert!(registry.lease_for_shared("userA", &visitor).is_ok());
|
||||
|
||||
let owner = identity("userA");
|
||||
let lease = registry.lease_for(&owner).expect("lease");
|
||||
let reply = handle(
|
||||
"POST",
|
||||
share_routes::REVOKE,
|
||||
r#"{"userId":"userB"}"#,
|
||||
&owner,
|
||||
&lease,
|
||||
®istry,
|
||||
);
|
||||
assert_eq!(reply.status, "200 OK");
|
||||
assert_eq!(body_of(&reply)["changed"], true);
|
||||
|
||||
// The access list is consulted per request, so this is immediate — no
|
||||
// session to expire first.
|
||||
assert_eq!(
|
||||
registry.lease_for_shared("userA", &visitor).unwrap_err(),
|
||||
TenantError::NotShared
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_owner_always_reaches_their_own_document() {
|
||||
let registry = registry();
|
||||
let owner = identity("userA");
|
||||
assert!(registry.lease_for_shared("userA", &owner).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_repeated_grant_is_acknowledged_without_changing_anything() {
|
||||
let registry = registry();
|
||||
assert_eq!(
|
||||
body_of(&grant(®istry, "userA", "userB"))["changed"],
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
body_of(&grant(®istry, "userA", "userB"))["changed"],
|
||||
false
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn revoking_an_account_that_was_never_granted_is_not_an_error() {
|
||||
let registry = registry();
|
||||
let owner = identity("userA");
|
||||
let lease = registry.lease_for(&owner).expect("lease");
|
||||
let reply = handle(
|
||||
"POST",
|
||||
share_routes::REVOKE,
|
||||
r#"{"userId":"nobody"}"#,
|
||||
&owner,
|
||||
&lease,
|
||||
®istry,
|
||||
);
|
||||
assert_eq!(reply.status, "200 OK");
|
||||
assert_eq!(body_of(&reply)["changed"], false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_list_reports_both_directions() {
|
||||
let registry = registry();
|
||||
grant(®istry, "userA", "userB");
|
||||
grant(®istry, "userC", "userB");
|
||||
|
||||
let visitor = identity("userB");
|
||||
let lease = registry.lease_for(&visitor).expect("lease");
|
||||
let reply = handle("GET", share_routes::LIST, "", &visitor, &lease, ®istry);
|
||||
let body = body_of(&reply);
|
||||
assert_eq!(reply.status, "200 OK");
|
||||
// userB has shared with nobody…
|
||||
assert_eq!(body["sharedWith"].as_array().map(Vec::len), Some(0));
|
||||
// …and two accounts have shared with userB.
|
||||
let mine: Vec<&str> = body["sharedWithMe"]
|
||||
.as_array()
|
||||
.expect("array")
|
||||
.iter()
|
||||
.filter_map(|value| value.as_str())
|
||||
.collect();
|
||||
assert_eq!(mine, vec!["userA", "userC"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_visitor_cannot_reshare_the_document_they_were_given() {
|
||||
// The route always edits the CALLER's own tenant, so a grant issued by a
|
||||
// visitor lands on the visitor's own document, never the owner's.
|
||||
let registry = registry();
|
||||
grant(®istry, "userA", "userB");
|
||||
grant(®istry, "userB", "userC");
|
||||
|
||||
let stranger = identity("userC");
|
||||
assert_eq!(
|
||||
registry.lease_for_shared("userA", &stranger).unwrap_err(),
|
||||
TenantError::NotShared,
|
||||
"userB must not be able to widen userA's access list"
|
||||
);
|
||||
assert!(
|
||||
registry.lease_for_shared("userB", &stranger).is_ok(),
|
||||
"userB may of course share their own document"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_account_cannot_share_with_itself() {
|
||||
let registry = registry();
|
||||
let reply = grant(®istry, "userA", "userA");
|
||||
assert_eq!(reply.status, "400 Bad Request");
|
||||
assert_eq!(body_of(&reply)["error"], "cannot-share-with-self");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_malformed_share_body_is_refused() {
|
||||
let registry = registry();
|
||||
let owner = identity("userA");
|
||||
let lease = registry.lease_for(&owner).expect("lease");
|
||||
for body in [
|
||||
"",
|
||||
"{}",
|
||||
"not json",
|
||||
r#"{"userId":""}"#,
|
||||
r#"{"userId":123}"#,
|
||||
r#"{"user":"x"}"#,
|
||||
] {
|
||||
let reply = handle("POST", share_routes::GRANT, body, &owner, &lease, ®istry);
|
||||
assert_eq!(reply.status, "400 Bad Request", "{body:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_oversized_share_body_is_refused_before_it_is_parsed() {
|
||||
let registry = registry();
|
||||
let owner = identity("userA");
|
||||
let lease = registry.lease_for(&owner).expect("lease");
|
||||
let body = format!(r#"{{"userId":"{}"}}"#, "x".repeat(MAX_SHARE_BODY_BYTES));
|
||||
let reply = handle(
|
||||
"POST",
|
||||
share_routes::GRANT,
|
||||
&body,
|
||||
&owner,
|
||||
&lease,
|
||||
®istry,
|
||||
);
|
||||
assert_eq!(reply.status, "413 Payload Too Large");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_wrong_method_on_a_share_route_is_405() {
|
||||
let registry = registry();
|
||||
let owner = identity("userA");
|
||||
let lease = registry.lease_for(&owner).expect("lease");
|
||||
let reply = handle("GET", share_routes::GRANT, "", &owner, &lease, ®istry);
|
||||
assert_eq!(reply.status, "405 Method Not Allowed");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_share_routes_are_recognised_and_nothing_else_is() {
|
||||
for route in [
|
||||
share_routes::GRANT,
|
||||
share_routes::REVOKE,
|
||||
share_routes::LIST,
|
||||
] {
|
||||
assert!(is_share_route(route), "{route}");
|
||||
}
|
||||
for other in ["/api/mcp/document", "/api/share", "/api/share/", "/mcp"] {
|
||||
assert!(!is_share_route(other), "{other}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_forbidden_share_and_an_unknown_one_answer_identically() {
|
||||
// Otherwise the difference tells a caller which accounts exist.
|
||||
assert_eq!(TenantError::NotShared.http_status(), "403 Forbidden");
|
||||
let registry = registry();
|
||||
let stranger = identity("userC");
|
||||
let unknown = registry
|
||||
.lease_for_shared("nobody-at-all", &stranger)
|
||||
.unwrap_err();
|
||||
grant(®istry, "userA", "userB");
|
||||
let forbidden = registry.lease_for_shared("userA", &stranger).unwrap_err();
|
||||
assert_eq!(unknown, forbidden);
|
||||
}
|
||||
|
|
@ -27,13 +27,14 @@
|
|||
//! victim, and removes it only after re-checking that the entry in the map is
|
||||
//! still the exact `Arc` it decided to evict.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::collections::{BTreeSet, HashMap};
|
||||
use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use op_editor_core::EditorState;
|
||||
|
||||
use super::tenant_auth::ResolvedIdentity;
|
||||
use super::tenant_store::TenantStore;
|
||||
use super::{SseHub, WebCanvasState};
|
||||
|
||||
/// Global connection ceiling for the online daemon.
|
||||
|
|
@ -119,6 +120,9 @@ pub enum TenantError {
|
|||
/// This account already holds [`TenantLimits::max_conns_per_tenant`]
|
||||
/// connections.
|
||||
TooManyConnections,
|
||||
/// The caller asked for another account's tenant and is not on its
|
||||
/// access list.
|
||||
NotShared,
|
||||
}
|
||||
|
||||
impl TenantError {
|
||||
|
|
@ -126,11 +130,17 @@ impl TenantError {
|
|||
match self {
|
||||
Self::TooManyTenants => "server-busy",
|
||||
Self::TooManyConnections => "too-many-connections",
|
||||
Self::NotShared => "tenant-not-shared",
|
||||
}
|
||||
}
|
||||
|
||||
pub const fn http_status(self) -> &'static str {
|
||||
"503 Service Unavailable"
|
||||
match self {
|
||||
Self::TooManyTenants | Self::TooManyConnections => "503 Service Unavailable",
|
||||
// A forbidden share and a non-existent one answer the same way:
|
||||
// otherwise the difference is an oracle for which accounts exist.
|
||||
Self::NotShared => "403 Forbidden",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -141,6 +151,7 @@ impl std::fmt::Display for TenantError {
|
|||
Self::TooManyConnections => {
|
||||
f.write_str("too many concurrent connections for this account")
|
||||
}
|
||||
Self::NotShared => f.write_str("this document is not shared with you"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -155,6 +166,13 @@ pub struct Tenant {
|
|||
/// This account's SSE subscribers. Separate per tenant so a version bump
|
||||
/// is only ever broadcast to the account that caused it.
|
||||
pub(crate) hub: SseHub,
|
||||
/// Accounts this tenant's owner has shared the document with.
|
||||
///
|
||||
/// A `BTreeSet` rather than a `HashSet` so the persisted list has a
|
||||
/// stable order and two saves of the same ACL produce the same bytes.
|
||||
/// Separate from the state mutex because admission is checked on every
|
||||
/// request while the document lock may be held by a long push.
|
||||
shared_with: Mutex<BTreeSet<String>>,
|
||||
/// Live leases. Non-zero means "do not evict".
|
||||
leases: AtomicUsize,
|
||||
/// Unix seconds of the last lease acquire or release.
|
||||
|
|
@ -162,19 +180,58 @@ pub struct Tenant {
|
|||
}
|
||||
|
||||
impl Tenant {
|
||||
fn new(port: u16, allow_origins: &[String], now_unix: u64) -> Self {
|
||||
let mut state = WebCanvasState::new_for_tenant(EditorState::starter(), port);
|
||||
fn new(
|
||||
port: u16,
|
||||
allow_origins: &[String],
|
||||
editor: EditorState,
|
||||
shared_with: BTreeSet<String>,
|
||||
now_unix: u64,
|
||||
) -> Self {
|
||||
let mut state = WebCanvasState::new_for_tenant(editor, port);
|
||||
// Every tenant answers for the same public origin; the allowlist is a
|
||||
// deployment property, not an account one.
|
||||
state.allow_origins = allow_origins.to_vec();
|
||||
Self {
|
||||
state: Mutex::new(state),
|
||||
hub: SseHub::default(),
|
||||
shared_with: Mutex::new(shared_with),
|
||||
leases: AtomicUsize::new(0),
|
||||
last_active_unix: AtomicU64::new(now_unix),
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether `visitor` may reach this tenant's document.
|
||||
pub fn admits(&self, visitor: &str) -> bool {
|
||||
self.shared_with
|
||||
.lock()
|
||||
.unwrap_or_else(|p| p.into_inner())
|
||||
.contains(visitor)
|
||||
}
|
||||
|
||||
/// Add an account to the access list. Returns whether it was new.
|
||||
pub fn grant(&self, visitor: &str) -> bool {
|
||||
self.shared_with
|
||||
.lock()
|
||||
.unwrap_or_else(|p| p.into_inner())
|
||||
.insert(visitor.to_string())
|
||||
}
|
||||
|
||||
/// Remove an account. Returns whether it had been granted.
|
||||
pub fn revoke(&self, visitor: &str) -> bool {
|
||||
self.shared_with
|
||||
.lock()
|
||||
.unwrap_or_else(|p| p.into_inner())
|
||||
.remove(visitor)
|
||||
}
|
||||
|
||||
/// A snapshot of the access list.
|
||||
pub fn shared_with(&self) -> BTreeSet<String> {
|
||||
self.shared_with
|
||||
.lock()
|
||||
.unwrap_or_else(|p| p.into_inner())
|
||||
.clone()
|
||||
}
|
||||
|
||||
/// Live lease count. Zero is the only evictable value.
|
||||
pub fn lease_count(&self) -> usize {
|
||||
self.leases.load(Ordering::Acquire)
|
||||
|
|
@ -196,6 +253,9 @@ impl Tenant {
|
|||
/// of it stay valid.
|
||||
pub struct TenantLease {
|
||||
tenant: Arc<Tenant>,
|
||||
/// The account this lease resolved to — the OWNER of the document, which
|
||||
/// for a shared visit is not the visitor.
|
||||
user_id: String,
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for TenantLease {
|
||||
|
|
@ -210,6 +270,12 @@ impl std::fmt::Debug for TenantLease {
|
|||
}
|
||||
|
||||
impl TenantLease {
|
||||
/// The owning account id. For a shared visit this is the owner, not the
|
||||
/// visitor — it is the tenant's key, not the caller's identity.
|
||||
pub fn owner_id(&self) -> &str {
|
||||
&self.user_id
|
||||
}
|
||||
|
||||
pub fn tenant(&self) -> &Tenant {
|
||||
&self.tenant
|
||||
}
|
||||
|
|
@ -241,18 +307,34 @@ pub struct TenantRegistry {
|
|||
port: u16,
|
||||
/// Public origins this deployment answers for, stamped onto every tenant.
|
||||
allow_origins: Vec<String>,
|
||||
/// Where an evicted tenant is written and a returning one is read from.
|
||||
store: TenantStore,
|
||||
}
|
||||
|
||||
impl TenantRegistry {
|
||||
pub fn new(port: u16, limits: TenantLimits, allow_origins: Vec<String>) -> Self {
|
||||
Self::with_store(port, limits, allow_origins, TenantStore::from_env())
|
||||
}
|
||||
|
||||
pub fn with_store(
|
||||
port: u16,
|
||||
limits: TenantLimits,
|
||||
allow_origins: Vec<String>,
|
||||
store: TenantStore,
|
||||
) -> Self {
|
||||
Self {
|
||||
tenants: Mutex::new(HashMap::new()),
|
||||
limits,
|
||||
port,
|
||||
allow_origins,
|
||||
store,
|
||||
}
|
||||
}
|
||||
|
||||
pub const fn store(&self) -> &TenantStore {
|
||||
&self.store
|
||||
}
|
||||
|
||||
/// The deployment's public origin allowlist.
|
||||
pub fn allow_origins(&self) -> &[String] {
|
||||
&self.allow_origins
|
||||
|
|
@ -273,21 +355,72 @@ impl TenantRegistry {
|
|||
/// on [`super::tenant_auth`] for why no request-supplied value may ever
|
||||
/// reach this argument.
|
||||
///
|
||||
/// A brand new tenant starts from [`EditorState::starter`]. M1 does not
|
||||
/// persist, so an evicted tenant's document is gone and a returning
|
||||
/// account gets a fresh starter document; that is the documented M1
|
||||
/// semantic, and M4 replaces it with load-on-create.
|
||||
/// A tenant that is not in memory is restored from disk when the store
|
||||
/// holds one, and starts from [`EditorState::starter`] otherwise — so an
|
||||
/// eviction is invisible to the account beyond the first request's cost.
|
||||
pub fn lease_for(&self, identity: &ResolvedIdentity) -> Result<TenantLease, TenantError> {
|
||||
self.lease_tenant(&identity.user_id)
|
||||
}
|
||||
|
||||
/// Take a lease on `owner_id`'s tenant on behalf of `visitor`.
|
||||
///
|
||||
/// The owner always passes. Anyone else must appear in the owner's access
|
||||
/// list — and note that the list is consulted on EVERY request, so a
|
||||
/// revoke takes effect on the visitor's next call rather than whenever
|
||||
/// some session expires.
|
||||
///
|
||||
/// A tenant that is not resident is restored first: a visitor must be
|
||||
/// able to open a shared document whose owner is offline, and refusing
|
||||
/// until the owner next signs in would make sharing useless.
|
||||
pub fn lease_for_shared(
|
||||
&self,
|
||||
owner_id: &str,
|
||||
visitor: &ResolvedIdentity,
|
||||
) -> Result<TenantLease, TenantError> {
|
||||
if owner_id == visitor.user_id {
|
||||
return self.lease_tenant(owner_id);
|
||||
}
|
||||
let lease = self.lease_tenant(owner_id)?;
|
||||
if !lease.tenant().admits(&visitor.user_id) {
|
||||
return Err(TenantError::NotShared);
|
||||
}
|
||||
Ok(lease)
|
||||
}
|
||||
|
||||
/// Who has shared with `visitor`, across every resident tenant.
|
||||
///
|
||||
/// Resident only, and deliberately: a full answer would mean reading every
|
||||
/// directory in the store on every call. The owners a visitor is actually
|
||||
/// working with are resident by definition, and the visitor can always
|
||||
/// open a share they were told about directly.
|
||||
pub fn shared_with_visitor(&self, visitor: &str) -> Vec<String> {
|
||||
let tenants = self.lock();
|
||||
let mut owners: Vec<String> = tenants
|
||||
.iter()
|
||||
.filter(|(owner, tenant)| owner.as_str() != visitor && tenant.admits(visitor))
|
||||
.map(|(owner, _)| owner.clone())
|
||||
.collect();
|
||||
owners.sort_unstable();
|
||||
owners
|
||||
}
|
||||
|
||||
fn lease_tenant(&self, user_id: &str) -> Result<TenantLease, TenantError> {
|
||||
let now = now_unix();
|
||||
let mut tenants = self.lock();
|
||||
let tenant = match tenants.get(&identity.user_id) {
|
||||
let tenant = match tenants.get(user_id) {
|
||||
Some(existing) => Arc::clone(existing),
|
||||
None => {
|
||||
if tenants.len() >= self.limits.max_tenants {
|
||||
return Err(TenantError::TooManyTenants);
|
||||
}
|
||||
let created = Arc::new(Tenant::new(self.port, &self.allow_origins, now));
|
||||
tenants.insert(identity.user_id.clone(), Arc::clone(&created));
|
||||
let created = Arc::new(Tenant::new(
|
||||
self.port,
|
||||
&self.allow_origins,
|
||||
self.restore_editor(user_id),
|
||||
self.store.load_acl(user_id),
|
||||
now,
|
||||
));
|
||||
tenants.insert(user_id.to_string(), Arc::clone(&created));
|
||||
created
|
||||
}
|
||||
};
|
||||
|
|
@ -299,7 +432,41 @@ impl TenantRegistry {
|
|||
// resolved above and being leased here.
|
||||
tenant.leases.fetch_add(1, Ordering::AcqRel);
|
||||
tenant.touch(now);
|
||||
Ok(TenantLease { tenant })
|
||||
Ok(TenantLease {
|
||||
tenant,
|
||||
user_id: user_id.to_string(),
|
||||
})
|
||||
}
|
||||
|
||||
/// The document to open a tenant with.
|
||||
///
|
||||
/// A stored document that will not load has already been moved aside by
|
||||
/// the store, so the account gets a starter rather than a failed request —
|
||||
/// losing a document is bad, but refusing to serve the account at all
|
||||
/// because of it is worse.
|
||||
fn restore_editor(&self, user_id: &str) -> EditorState {
|
||||
match self.store.load_document(user_id) {
|
||||
Ok(state) => state,
|
||||
Err(super::tenant_store::TenantStoreError::Disabled)
|
||||
| Err(super::tenant_store::TenantStoreError::NotStored) => EditorState::starter(),
|
||||
Err(error) => {
|
||||
eprintln!(
|
||||
"openpencil --serve-web --online: starting a fresh document for an \
|
||||
account whose stored one could not be loaded ({error})"
|
||||
);
|
||||
EditorState::starter()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Persist a tenant's access list, if persistence is on.
|
||||
pub fn persist_acl(&self, user_id: &str, tenant: &Tenant) {
|
||||
if !self.store.is_enabled() {
|
||||
return;
|
||||
}
|
||||
if let Err(error) = self.store.save_acl_for(user_id, &tenant.shared_with()) {
|
||||
eprintln!("openpencil --serve-web --online: could not persist a share list ({error})");
|
||||
}
|
||||
}
|
||||
|
||||
/// Reclaim tenants that hold no lease and have been idle past the limit.
|
||||
|
|
@ -321,10 +488,23 @@ impl TenantRegistry {
|
|||
.collect();
|
||||
let mut evicted = 0;
|
||||
for (id, victim) in victims {
|
||||
// M4 persistence hook: write `victim.state` to
|
||||
// `$OPENPENCIL_ONLINE_DATA_DIR/<id>/current.op` HERE, before the
|
||||
// compare-and-remove below, so a returning account either finds
|
||||
// the old tenant still mapped or a file it can load.
|
||||
// Write BEFORE the compare-and-remove, so at no instant is the
|
||||
// tenant both absent from the map and absent from disk: a request
|
||||
// arriving mid-eviction either finds the resident tenant (the
|
||||
// registry lock is held, so it waits) or, afterwards, the file.
|
||||
if self.store.is_enabled() {
|
||||
let guard = victim.state.lock().unwrap_or_else(|p| p.into_inner());
|
||||
if let Err(error) = self.store.save(&id, &guard.editor, &victim.shared_with()) {
|
||||
// A tenant that cannot be written is kept resident. Evicting
|
||||
// it anyway would discard the document to reclaim memory,
|
||||
// which is the wrong trade for the user whose work it is.
|
||||
eprintln!(
|
||||
"openpencil --serve-web --online: keeping a tenant resident because \
|
||||
it could not be persisted ({error})"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
let still_the_same = tenants
|
||||
.get(&id)
|
||||
.is_some_and(|current| Arc::ptr_eq(current, &victim));
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ fn request_with(authorization: Option<&str>, cookie: Option<&str>) -> HttpReques
|
|||
content_type: None,
|
||||
authorization: authorization.map(str::to_string),
|
||||
cookie: cookie.map(str::to_string),
|
||||
query: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
262
crates/op-host-services/src/web_canvas_server/tenant_store.rs
Normal file
262
crates/op-host-services/src/web_canvas_server/tenant_store.rs
Normal file
|
|
@ -0,0 +1,262 @@
|
|||
//! Where an evicted tenant's document goes, and how it comes back.
|
||||
//!
|
||||
//! M1 kept tenants purely in memory, so eviction lost the document and a
|
||||
//! returning account got a starter. This is the other half: eviction writes,
|
||||
//! and a cache miss reads.
|
||||
//!
|
||||
//! ## Directory naming
|
||||
//!
|
||||
//! The directory is `SHA-256(user_id)`, first 16 hex characters — never the
|
||||
//! account id itself. An account id is a string the daemon did not choose,
|
||||
//! and joining an unvetted string onto a path is how `../` gets to walk out
|
||||
//! of the data directory. Hashing removes the question entirely: the output
|
||||
//! alphabet is `[0-9a-f]`, so there is no traversal to defend against, no
|
||||
//! case-collision on a case-insensitive filesystem, and no length limit to
|
||||
//! worry about.
|
||||
//!
|
||||
//! ## Writing
|
||||
//!
|
||||
//! Temp file in the same directory, then `rename`. A crash therefore leaves
|
||||
//! either the previous document or the new one, never a half-written file
|
||||
//! that would fail to load on the account's next visit.
|
||||
//!
|
||||
//! ## Reading a file that will not load
|
||||
//!
|
||||
//! Renamed to `.corrupt` and the account gets a starter document. Deleting it
|
||||
//! would destroy the only copy of whatever the user had; silently overwriting
|
||||
//! it does the same thing one save later. Keeping it costs a few bytes and is
|
||||
//! the difference between "we can look at it" and "it is gone".
|
||||
|
||||
use std::collections::BTreeSet;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use op_editor_core::EditorState;
|
||||
use sha2::{Digest, Sha256};
|
||||
|
||||
/// Root of the on-disk tenant store. Unset disables persistence entirely,
|
||||
/// which is what a demo or a test wants.
|
||||
pub const DATA_DIR_ENV: &str = "OPENPENCIL_ONLINE_DATA_DIR";
|
||||
|
||||
/// The document file inside a tenant directory.
|
||||
const DOCUMENT_FILE: &str = "current.op";
|
||||
/// The access list inside a tenant directory.
|
||||
const ACL_FILE: &str = "acl.json";
|
||||
/// Suffix a file that would not load is moved aside under.
|
||||
const CORRUPT_SUFFIX: &str = "corrupt";
|
||||
|
||||
/// Longest access list that is written or read back.
|
||||
///
|
||||
/// A share list is a handful of accounts; a larger one is a bug or an attempt
|
||||
/// to make the daemon allocate on every eviction.
|
||||
const MAX_SHARED_ACCOUNTS: usize = 256;
|
||||
|
||||
/// Why a tenant could not be persisted or restored.
|
||||
///
|
||||
/// Every variant is non-fatal at the call site: a failed save leaves the
|
||||
/// previous file, and a failed load yields a starter document. Persistence
|
||||
/// must never be able to take the service down.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum TenantStoreError {
|
||||
/// No data directory is configured, so there is nothing to read or write.
|
||||
Disabled,
|
||||
/// The tenant has nothing stored yet.
|
||||
NotStored,
|
||||
/// The stored file could not be read or written.
|
||||
Io(String),
|
||||
/// The stored document exists but could not be loaded. It has been moved
|
||||
/// aside; the caller should start fresh.
|
||||
Unreadable(String),
|
||||
}
|
||||
|
||||
impl std::fmt::Display for TenantStoreError {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
Self::Disabled => f.write_str("tenant persistence is not configured"),
|
||||
Self::NotStored => f.write_str("no stored document for this account"),
|
||||
Self::Io(detail) => write!(f, "tenant store IO failed: {detail}"),
|
||||
Self::Unreadable(detail) => {
|
||||
write!(f, "stored document could not be loaded: {detail}")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for TenantStoreError {}
|
||||
|
||||
/// The on-disk tenant store.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct TenantStore {
|
||||
root: Option<PathBuf>,
|
||||
}
|
||||
|
||||
impl TenantStore {
|
||||
/// Build from the environment. No data directory means no persistence.
|
||||
pub fn from_env() -> Self {
|
||||
Self::new(
|
||||
std::env::var(DATA_DIR_ENV)
|
||||
.ok()
|
||||
.map(|value| value.trim().to_string())
|
||||
.filter(|value| !value.is_empty())
|
||||
.map(PathBuf::from),
|
||||
)
|
||||
}
|
||||
|
||||
pub const fn new(root: Option<PathBuf>) -> Self {
|
||||
Self { root }
|
||||
}
|
||||
|
||||
pub const fn is_enabled(&self) -> bool {
|
||||
self.root.is_some()
|
||||
}
|
||||
|
||||
/// The directory holding `user_id`'s tenant, if persistence is on.
|
||||
///
|
||||
/// See the module docs for why this is a hash and not the id.
|
||||
pub fn tenant_dir(&self, user_id: &str) -> Option<PathBuf> {
|
||||
self.root.as_ref().map(|root| root.join(dir_name(user_id)))
|
||||
}
|
||||
|
||||
/// Write a tenant's document and access list.
|
||||
///
|
||||
/// Called during eviction, while the registry lock is held and the tenant
|
||||
/// has no leases — so nothing can be writing the document underneath it.
|
||||
pub fn save(
|
||||
&self,
|
||||
user_id: &str,
|
||||
state: &EditorState,
|
||||
shared_with: &BTreeSet<String>,
|
||||
) -> Result<(), TenantStoreError> {
|
||||
let Some(dir) = self.tenant_dir(user_id) else {
|
||||
return Err(TenantStoreError::Disabled);
|
||||
};
|
||||
std::fs::create_dir_all(&dir).map_err(|error| TenantStoreError::Io(error.to_string()))?;
|
||||
// The document goes through the same streaming, atomic writer desktop
|
||||
// Save uses, so a large document with embedded images is not built in
|
||||
// memory twice and a crash cannot truncate the live file.
|
||||
crate::doc_io::save_to_path(state, &dir.join(DOCUMENT_FILE))
|
||||
.map_err(|error| TenantStoreError::Io(error.to_string()))?;
|
||||
self.save_acl(&dir, shared_with)
|
||||
}
|
||||
|
||||
/// Write just the access list. Used when a grant or revoke should survive
|
||||
/// a restart even though the document has not changed.
|
||||
pub fn save_acl_for(
|
||||
&self,
|
||||
user_id: &str,
|
||||
shared_with: &BTreeSet<String>,
|
||||
) -> Result<(), TenantStoreError> {
|
||||
let Some(dir) = self.tenant_dir(user_id) else {
|
||||
return Err(TenantStoreError::Disabled);
|
||||
};
|
||||
std::fs::create_dir_all(&dir).map_err(|error| TenantStoreError::Io(error.to_string()))?;
|
||||
self.save_acl(&dir, shared_with)
|
||||
}
|
||||
|
||||
fn save_acl(&self, dir: &Path, shared_with: &BTreeSet<String>) -> Result<(), TenantStoreError> {
|
||||
let bounded: Vec<&String> = shared_with.iter().take(MAX_SHARED_ACCOUNTS).collect();
|
||||
let body = serde_json::json!({ "sharedWith": bounded }).to_string();
|
||||
atomic_write(&dir.join(ACL_FILE), body.as_bytes())
|
||||
}
|
||||
|
||||
/// Restore a tenant's document, if one was stored.
|
||||
///
|
||||
/// A file that will not load is moved aside (see the module docs) and
|
||||
/// reported as [`TenantStoreError::Unreadable`], so the caller starts the
|
||||
/// account fresh rather than failing its request.
|
||||
pub fn load_document(&self, user_id: &str) -> Result<EditorState, TenantStoreError> {
|
||||
let Some(dir) = self.tenant_dir(user_id) else {
|
||||
return Err(TenantStoreError::Disabled);
|
||||
};
|
||||
let path = dir.join(DOCUMENT_FILE);
|
||||
if !path.is_file() {
|
||||
return Err(TenantStoreError::NotStored);
|
||||
}
|
||||
match crate::doc_io::load_editor_state(&path, op_editor_core::Locale::EnUs) {
|
||||
Ok(state) => Ok(state),
|
||||
Err(error) => {
|
||||
let detail = error.to_string();
|
||||
quarantine(&path);
|
||||
Err(TenantStoreError::Unreadable(detail))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Restore a tenant's access list. A missing or unreadable list is an
|
||||
/// empty one — failing closed, since the list only ever grants access.
|
||||
pub fn load_acl(&self, user_id: &str) -> BTreeSet<String> {
|
||||
let Some(dir) = self.tenant_dir(user_id) else {
|
||||
return BTreeSet::new();
|
||||
};
|
||||
let path = dir.join(ACL_FILE);
|
||||
let Ok(body) = std::fs::read_to_string(&path) else {
|
||||
return BTreeSet::new();
|
||||
};
|
||||
let Ok(parsed) = serde_json::from_str::<serde_json::Value>(&body) else {
|
||||
quarantine(&path);
|
||||
return BTreeSet::new();
|
||||
};
|
||||
parsed
|
||||
.get("sharedWith")
|
||||
.and_then(|value| value.as_array())
|
||||
.map(|entries| {
|
||||
entries
|
||||
.iter()
|
||||
.filter_map(|entry| entry.as_str())
|
||||
.filter(|entry| !entry.trim().is_empty())
|
||||
.take(MAX_SHARED_ACCOUNTS)
|
||||
.map(str::to_string)
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Whether anything is stored for `user_id`.
|
||||
pub fn has_document(&self, user_id: &str) -> bool {
|
||||
self.tenant_dir(user_id)
|
||||
.is_some_and(|dir| dir.join(DOCUMENT_FILE).is_file())
|
||||
}
|
||||
}
|
||||
|
||||
/// The directory name for `user_id`: the first 16 hex characters of its
|
||||
/// SHA-256. See the module docs for why this is not the id itself.
|
||||
pub fn dir_name(user_id: &str) -> String {
|
||||
let digest = Sha256::digest(user_id.as_bytes());
|
||||
digest
|
||||
.iter()
|
||||
.take(8)
|
||||
.map(|byte| format!("{byte:02x}"))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Write `bytes` to `path` via a same-directory temp file plus rename.
|
||||
fn atomic_write(path: &Path, bytes: &[u8]) -> Result<(), TenantStoreError> {
|
||||
let temp = path.with_extension("tmp");
|
||||
std::fs::write(&temp, bytes).map_err(|error| TenantStoreError::Io(error.to_string()))?;
|
||||
std::fs::rename(&temp, path).map_err(|error| {
|
||||
// Leaving the temp behind would accumulate one file per failed write.
|
||||
let _ = std::fs::remove_file(&temp);
|
||||
TenantStoreError::Io(error.to_string())
|
||||
})
|
||||
}
|
||||
|
||||
/// Move a file that would not parse out of the way, preserving it.
|
||||
///
|
||||
/// Best effort: if the rename fails there is nothing useful left to do, and
|
||||
/// the caller is already returning a starter document. A pre-existing
|
||||
/// `.corrupt` from an earlier failure is kept — the timestamp suffix means a
|
||||
/// second bad file does not overwrite the first.
|
||||
fn quarantine(path: &Path) {
|
||||
let stamp = crate::web_canvas_server::tenant::now_unix();
|
||||
let target = path.with_extension(format!("{CORRUPT_SUFFIX}.{stamp}"));
|
||||
if std::fs::rename(path, &target).is_err() {
|
||||
return;
|
||||
}
|
||||
eprintln!(
|
||||
"openpencil --serve-web --online: kept an unreadable tenant file at {}",
|
||||
target.display()
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "tenant_store_tests.rs"]
|
||||
mod tests;
|
||||
|
|
@ -0,0 +1,257 @@
|
|||
//! Tests for the on-disk tenant store.
|
||||
|
||||
use super::*;
|
||||
use op_editor_core::EditorState;
|
||||
|
||||
/// A store rooted in a fresh temp directory, removed when the guard drops.
|
||||
struct TempStore {
|
||||
root: PathBuf,
|
||||
store: TenantStore,
|
||||
}
|
||||
|
||||
impl TempStore {
|
||||
fn new(label: &str) -> Self {
|
||||
let root = std::env::temp_dir().join(format!(
|
||||
"op-tenant-store-{label}-{}-{:?}",
|
||||
std::process::id(),
|
||||
std::thread::current().id()
|
||||
));
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
std::fs::create_dir_all(&root).expect("temp root");
|
||||
Self {
|
||||
store: TenantStore::new(Some(root.clone())),
|
||||
root,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for TempStore {
|
||||
fn drop(&mut self) {
|
||||
let _ = std::fs::remove_dir_all(&self.root);
|
||||
}
|
||||
}
|
||||
|
||||
/// A document carrying one recognisable node, built through the same
|
||||
/// canonical loader the daemon uses.
|
||||
///
|
||||
/// The round trip is asserted on DOCUMENT content, not on editor UI state —
|
||||
/// the `.op` format deliberately does not carry the latter, so a marker like
|
||||
/// `file_name_display` would silently "fail" every time.
|
||||
fn named_document(name: &str) -> EditorState {
|
||||
let json = serde_json::json!({
|
||||
"version": "1.0.0",
|
||||
"children": [{
|
||||
"id": "n1", "type": "rectangle", "name": name,
|
||||
"x": 1, "y": 2, "width": 80, "height": 40,
|
||||
}],
|
||||
})
|
||||
.to_string();
|
||||
let loaded = op_pen_loader::load_canonical(&json).expect("canonical document");
|
||||
let mut state = EditorState::starter();
|
||||
state.replace_document(loaded.value);
|
||||
state
|
||||
}
|
||||
|
||||
/// The name of the document's first node, read off the serialized form so
|
||||
/// this does not depend on `PenNode`'s variant shape.
|
||||
fn node_name(state: &EditorState) -> Option<String> {
|
||||
let json = serde_json::to_value(state.doc.children.first()?).ok()?;
|
||||
json.get("name")?.as_str().map(str::to_string)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_saved_document_comes_back() {
|
||||
let temp = TempStore::new("roundtrip");
|
||||
let mut acl = BTreeSet::new();
|
||||
acl.insert("userB".to_string());
|
||||
|
||||
temp.store
|
||||
.save("userA", &named_document("kept.op"), &acl)
|
||||
.expect("save");
|
||||
|
||||
let restored = temp.store.load_document("userA").expect("load");
|
||||
assert_eq!(node_name(&restored).as_deref(), Some("kept.op"));
|
||||
assert_eq!(temp.store.load_acl("userA"), acl);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_account_with_nothing_stored_reports_so() {
|
||||
let temp = TempStore::new("empty");
|
||||
assert_eq!(
|
||||
temp.store.load_document("userA").unwrap_err(),
|
||||
TenantStoreError::NotStored
|
||||
);
|
||||
assert!(temp.store.load_acl("userA").is_empty());
|
||||
assert!(!temp.store.has_document("userA"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_disabled_store_reads_and_writes_nothing() {
|
||||
let store = TenantStore::new(None);
|
||||
assert!(!store.is_enabled());
|
||||
assert_eq!(
|
||||
store.load_document("userA").unwrap_err(),
|
||||
TenantStoreError::Disabled
|
||||
);
|
||||
assert_eq!(
|
||||
store
|
||||
.save("userA", &EditorState::starter(), &BTreeSet::new())
|
||||
.unwrap_err(),
|
||||
TenantStoreError::Disabled
|
||||
);
|
||||
assert!(store.tenant_dir("userA").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_corrupt_document_is_kept_aside_and_the_account_starts_fresh() {
|
||||
let temp = TempStore::new("corrupt");
|
||||
temp.store
|
||||
.save("userA", &named_document("original.op"), &BTreeSet::new())
|
||||
.expect("save");
|
||||
let dir = temp.store.tenant_dir("userA").expect("dir");
|
||||
let document = dir.join("current.op");
|
||||
std::fs::write(&document, b"this is not a document").expect("corrupt it");
|
||||
|
||||
let error = temp.store.load_document("userA").unwrap_err();
|
||||
assert!(
|
||||
matches!(error, TenantStoreError::Unreadable(_)),
|
||||
"{error:?}"
|
||||
);
|
||||
assert!(
|
||||
!document.exists(),
|
||||
"the unreadable file must be moved aside, not left to fail every visit"
|
||||
);
|
||||
let kept: Vec<_> = std::fs::read_dir(&dir)
|
||||
.expect("read dir")
|
||||
.filter_map(Result::ok)
|
||||
.map(|entry| entry.file_name().to_string_lossy().into_owned())
|
||||
.filter(|name| name.contains("corrupt"))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
kept.len(),
|
||||
1,
|
||||
"the bytes must be preserved, not deleted: {kept:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_second_corrupt_file_does_not_overwrite_the_first() {
|
||||
let temp = TempStore::new("corrupt-twice");
|
||||
let dir = temp.store.tenant_dir("userA").expect("dir");
|
||||
std::fs::create_dir_all(&dir).expect("dir");
|
||||
let document = dir.join("current.op");
|
||||
|
||||
// Two separate quarantines. The stamp is second-resolution, so this
|
||||
// asserts only that the first is never destroyed.
|
||||
std::fs::write(&document, b"garbage one").expect("write");
|
||||
let _ = temp.store.load_document("userA");
|
||||
std::fs::write(&document, b"garbage two").expect("write");
|
||||
let _ = temp.store.load_document("userA");
|
||||
|
||||
let kept: Vec<_> = std::fs::read_dir(&dir)
|
||||
.expect("read dir")
|
||||
.filter_map(Result::ok)
|
||||
.filter(|entry| entry.file_name().to_string_lossy().contains("corrupt"))
|
||||
.collect();
|
||||
assert!(!kept.is_empty(), "at least the first must survive");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_corrupt_access_list_reads_as_empty_rather_than_granting_anyone() {
|
||||
let temp = TempStore::new("bad-acl");
|
||||
let dir = temp.store.tenant_dir("userA").expect("dir");
|
||||
std::fs::create_dir_all(&dir).expect("dir");
|
||||
std::fs::write(dir.join("acl.json"), b"{not json").expect("write");
|
||||
assert!(
|
||||
temp.store.load_acl("userA").is_empty(),
|
||||
"an unreadable access list must grant nobody"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_account_id_never_becomes_a_path_component() {
|
||||
// The whole point of hashing: an id full of traversal cannot address a
|
||||
// directory outside the store root.
|
||||
let temp = TempStore::new("traversal");
|
||||
for hostile in [
|
||||
"../../../../etc/passwd",
|
||||
"..",
|
||||
"/etc/shadow",
|
||||
"a/b/c",
|
||||
"..\\..\\windows",
|
||||
"userA\0evil",
|
||||
] {
|
||||
let dir = temp.store.tenant_dir(hostile).expect("dir");
|
||||
assert!(
|
||||
dir.starts_with(&temp.root),
|
||||
"{hostile:?} escaped the data directory: {dir:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
dir.parent(),
|
||||
Some(temp.root.as_path()),
|
||||
"{hostile:?} produced a nested path"
|
||||
);
|
||||
let name = dir
|
||||
.file_name()
|
||||
.expect("name")
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
assert_eq!(name.len(), 16, "{hostile:?} -> {name}");
|
||||
assert!(
|
||||
name.chars().all(|c| c.is_ascii_hexdigit()),
|
||||
"{hostile:?} -> {name}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_hostile_account_id_still_round_trips_its_own_document() {
|
||||
// Hashing must not break the account, only the path injection.
|
||||
let temp = TempStore::new("hostile-roundtrip");
|
||||
let hostile = "../../../../etc/passwd";
|
||||
temp.store
|
||||
.save(hostile, &named_document("hostile.op"), &BTreeSet::new())
|
||||
.expect("save");
|
||||
let restored = temp.store.load_document(hostile).expect("load");
|
||||
assert_eq!(node_name(&restored).as_deref(), Some("hostile.op"));
|
||||
// And it did not collide with a different account.
|
||||
assert_eq!(
|
||||
temp.store.load_document("userA").unwrap_err(),
|
||||
TenantStoreError::NotStored
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn directory_names_are_stable_and_distinct() {
|
||||
assert_eq!(dir_name("userA"), dir_name("userA"));
|
||||
assert_ne!(dir_name("userA"), dir_name("userB"));
|
||||
assert_eq!(dir_name("userA").len(), 16);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_write_leaves_no_temp_file_behind() {
|
||||
let temp = TempStore::new("atomic");
|
||||
temp.store
|
||||
.save("userA", &EditorState::starter(), &BTreeSet::new())
|
||||
.expect("save");
|
||||
let dir = temp.store.tenant_dir("userA").expect("dir");
|
||||
let strays: Vec<_> = std::fs::read_dir(&dir)
|
||||
.expect("read dir")
|
||||
.filter_map(Result::ok)
|
||||
.filter(|entry| entry.file_name().to_string_lossy().ends_with(".tmp"))
|
||||
.collect();
|
||||
assert!(
|
||||
strays.is_empty(),
|
||||
"a completed write must leave no temp file"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saving_the_access_list_alone_does_not_require_a_document() {
|
||||
let temp = TempStore::new("acl-only");
|
||||
let mut acl = BTreeSet::new();
|
||||
acl.insert("userB".to_string());
|
||||
temp.store.save_acl_for("userA", &acl).expect("save acl");
|
||||
assert_eq!(temp.store.load_acl("userA"), acl);
|
||||
assert!(!temp.store.has_document("userA"));
|
||||
}
|
||||
|
|
@ -435,6 +435,7 @@ fn sensitive_browser_posts_include_credentials_and_ai_routes() {
|
|||
content_type: None,
|
||||
authorization: None,
|
||||
cookie: None,
|
||||
query: None,
|
||||
};
|
||||
assert!(is_sensitive_browser_post(&request), "path={path}");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -632,7 +632,9 @@ fn ensure_event_stream(base: &str) {
|
|||
if now_ms_f64() < SSE_RETRY_AT_MS.get() {
|
||||
return;
|
||||
}
|
||||
let Ok(stream) = web_sys::EventSource::new(&format!("{base}/api/mcp/events")) else {
|
||||
let Ok(stream) = web_sys::EventSource::new(&crate::daemon_base::with_tenant_param(&format!(
|
||||
"{base}/api/mcp/events"
|
||||
))) else {
|
||||
note_stream_failure();
|
||||
return;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -107,3 +107,85 @@ mod tests {
|
|||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The tenant this tab is viewing, when the page URL names one.
|
||||
///
|
||||
/// A visitor opens a shared document with `?tenant=<ownerId>` on the page
|
||||
/// URL, and every daemon request this shell makes must carry the same
|
||||
/// parameter — otherwise the poll, the push and the event stream would each
|
||||
/// address a different document. Read once (the value cannot change without
|
||||
/// a navigation, which reloads the shell) and appended by
|
||||
/// [`with_tenant_param`], which every request path funnels through.
|
||||
///
|
||||
/// This is a REQUEST, not a credential: the daemon still resolves the
|
||||
/// caller's own identity and checks it against the owner's access list, so a
|
||||
/// hand-edited parameter buys nothing.
|
||||
pub fn tenant_param() -> Option<String> {
|
||||
thread_local! {
|
||||
static TENANT: std::cell::OnceCell<Option<String>> = const {
|
||||
std::cell::OnceCell::new()
|
||||
};
|
||||
}
|
||||
TENANT.with(|cell| {
|
||||
cell.get_or_init(|| {
|
||||
let search = web_sys::window()?.location().search().ok()?;
|
||||
let query = search.strip_prefix('?').unwrap_or(&search);
|
||||
op_editor_core::share_routes::tenant_from_query(query).map(str::to_string)
|
||||
})
|
||||
.clone()
|
||||
})
|
||||
}
|
||||
|
||||
/// Append the tab's tenant parameter to `url`, if it has one.
|
||||
///
|
||||
/// Pure so the query-joining rule is testable without a DOM; the browser
|
||||
/// wrapper is [`with_tenant_param`].
|
||||
pub fn append_tenant_param(url: &str, tenant: Option<&str>) -> String {
|
||||
let Some(tenant) = tenant.filter(|value| !value.is_empty()) else {
|
||||
return url.to_string();
|
||||
};
|
||||
let separator = if url.contains('?') { '&' } else { '?' };
|
||||
format!(
|
||||
"{url}{separator}{}={tenant}",
|
||||
op_editor_core::share_routes::TENANT_QUERY
|
||||
)
|
||||
}
|
||||
|
||||
/// Browser wrapper over [`append_tenant_param`].
|
||||
///
|
||||
/// Applied inside the HTTP helpers rather than at each call site, so a route
|
||||
/// added later cannot forget it and silently address the wrong document.
|
||||
pub fn with_tenant_param(url: &str) -> String {
|
||||
append_tenant_param(url, tenant_param().as_deref())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tenant_param_tests {
|
||||
use super::append_tenant_param;
|
||||
|
||||
#[test]
|
||||
fn a_url_without_a_query_gains_one() {
|
||||
assert_eq!(
|
||||
append_tenant_param("http://x/api/mcp/document", Some("userA")),
|
||||
"http://x/api/mcp/document?tenant=userA"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_url_that_already_has_a_query_is_extended() {
|
||||
assert_eq!(
|
||||
append_tenant_param("http://x/api/mcp/document?v=2", Some("userA")),
|
||||
"http://x/api/mcp/document?v=2&tenant=userA"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_tenant_leaves_the_url_exactly_as_it_was() {
|
||||
for tenant in [None, Some("")] {
|
||||
assert_eq!(
|
||||
append_tenant_param("http://x/api/mcp/document", tenant),
|
||||
"http://x/api/mcp/document"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -93,7 +93,10 @@ pub fn get(url: &str, on_response: Rc<dyn Fn(String)>) -> bool {
|
|||
let Ok(xhr) = web_sys::XmlHttpRequest::new() else {
|
||||
return false;
|
||||
};
|
||||
if xhr.open_with_async("GET", url, true).is_err() {
|
||||
if xhr
|
||||
.open_with_async("GET", &crate::daemon_base::with_tenant_param(url), true)
|
||||
.is_err()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
attach_daemon_headers(&xhr, url);
|
||||
|
|
@ -119,7 +122,10 @@ pub fn get_with_status(url: &str, on_response: Rc<dyn Fn(u16, String)>) -> bool
|
|||
let Ok(xhr) = web_sys::XmlHttpRequest::new() else {
|
||||
return false;
|
||||
};
|
||||
if xhr.open_with_async("GET", url, true).is_err() {
|
||||
if xhr
|
||||
.open_with_async("GET", &crate::daemon_base::with_tenant_param(url), true)
|
||||
.is_err()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
attach_daemon_headers(&xhr, url);
|
||||
|
|
@ -146,7 +152,10 @@ pub fn post_json(url: &str, body: &str, on_response: Option<Rc<dyn Fn(String)>>)
|
|||
let Ok(xhr) = web_sys::XmlHttpRequest::new() else {
|
||||
return false;
|
||||
};
|
||||
if xhr.open_with_async("POST", url, true).is_err() {
|
||||
if xhr
|
||||
.open_with_async("POST", &crate::daemon_base::with_tenant_param(url), true)
|
||||
.is_err()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
attach_daemon_headers(&xhr, url);
|
||||
|
|
@ -173,7 +182,10 @@ pub fn post_json_with_status(url: &str, body: &str, on_response: Rc<dyn Fn(u16,
|
|||
let Ok(xhr) = web_sys::XmlHttpRequest::new() else {
|
||||
return false;
|
||||
};
|
||||
if xhr.open_with_async("POST", url, true).is_err() {
|
||||
if xhr
|
||||
.open_with_async("POST", &crate::daemon_base::with_tenant_param(url), true)
|
||||
.is_err()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
attach_daemon_headers(&xhr, url);
|
||||
|
|
|
|||
219
crates/op-host-web/src/live_sync_conflict.rs
Normal file
219
crates/op-host-web/src/live_sync_conflict.rs
Normal file
|
|
@ -0,0 +1,219 @@
|
|||
//! When a sync conflict may resolve itself.
|
||||
//!
|
||||
//! Split out of `live_sync_glue.rs` at the 800-line cap. It is one decision
|
||||
//! plus the deployment probe that feeds it, and it is worth reading as a unit:
|
||||
//! getting it wrong either silently discards a user's unpushed work or leaves
|
||||
//! a shared document permanently latched.
|
||||
|
||||
use op_editor_core::CollabConnectionPhase;
|
||||
|
||||
/// The safety decision behind [`maybe_auto_resolve_conflict_in_session`],
|
||||
/// separated so it can be tested without a live shell.
|
||||
///
|
||||
/// Two situations qualify, for the same underlying reason — there is an
|
||||
/// authoritative document to accept, so accepting it converges rather than
|
||||
/// destroys:
|
||||
///
|
||||
/// 1. **A live collaboration session** (`Active`). The session core sequences
|
||||
/// every edit, and a rejected local edit is projected into
|
||||
/// `collab.discarded_edit` for the panel to replay.
|
||||
///
|
||||
/// 2. **A server-authoritative deployment** (`serveMode: online`). The daemon
|
||||
/// owns the one in-memory document every writer reaches, and its version
|
||||
/// counter is the total order. A 409 there means only "someone else's push
|
||||
/// landed between this tab's read and its write" — and the SSE stream is
|
||||
/// already delivering that newer document, so re-reading it is not a
|
||||
/// choice between two candidate truths, it is catching up to the one.
|
||||
///
|
||||
/// Why overwriting is acceptable in case 2: the lost window is a single
|
||||
/// push's diff — at most the ~2 s since this tab last synced — and the
|
||||
/// alternative is the latch, which in a shared tenant never clears, because
|
||||
/// nothing outside a session ever resolves it. A visitor would simply be
|
||||
/// frozen out of the document. Demo-grade concurrency (409 → refetch) is the
|
||||
/// documented semantic for a shared online tenant; a latch that requires a
|
||||
/// session to lift is not a stricter version of that, it is a hang.
|
||||
///
|
||||
/// Outside both — a local or managed daemon with no session — the daemon is a
|
||||
/// peer holding the operator's file, not an authority, and nothing preserves
|
||||
/// the losing edit. The latch stays and explicit resolution is untouched.
|
||||
pub(super) const fn auto_resolve_is_safe(
|
||||
has_conflict: bool,
|
||||
phase: CollabConnectionPhase,
|
||||
server_authoritative: bool,
|
||||
) -> bool {
|
||||
has_conflict && (server_authoritative || matches!(phase, CollabConnectionPhase::Active))
|
||||
}
|
||||
|
||||
/// Whether the daemon this shell talks to is the sole sequencer for the
|
||||
/// document.
|
||||
///
|
||||
/// Learned once from `GET /api/mcp/server`'s `serveMode` (see
|
||||
/// [`probe_serve_mode`]) and cached, because it is a property of the
|
||||
/// deployment and cannot change without a reload. Defaults to `false` until
|
||||
/// the probe answers, so the conservative behaviour is what runs during
|
||||
/// start-up rather than the permissive one.
|
||||
pub(super) fn server_is_authoritative() -> bool {
|
||||
SERVER_AUTHORITATIVE.with(|flag| flag.get())
|
||||
}
|
||||
|
||||
thread_local! {
|
||||
static SERVER_AUTHORITATIVE: std::cell::Cell<bool> = const {
|
||||
std::cell::Cell::new(false)
|
||||
};
|
||||
}
|
||||
|
||||
/// Record what `GET /api/mcp/server` said about the deployment.
|
||||
///
|
||||
/// Split from the fetch so the parse is testable without a DOM.
|
||||
fn note_serve_mode(body: &str) {
|
||||
if let Some(authoritative) = parse_server_authoritative(body) {
|
||||
SERVER_AUTHORITATIVE.with(|flag| flag.set(authoritative));
|
||||
}
|
||||
}
|
||||
|
||||
/// Read `serveMode` out of the health response.
|
||||
///
|
||||
/// `None` when the field is absent — an older daemon, which is by definition
|
||||
/// not an online one, so the caller leaves the conservative default in place.
|
||||
fn parse_server_authoritative(body: &str) -> Option<bool> {
|
||||
let parsed: serde_json::Value = serde_json::from_str(body).ok()?;
|
||||
let mode = parsed.get("serveMode")?.as_str()?;
|
||||
Some(mode == "online")
|
||||
}
|
||||
|
||||
/// Ask the daemon once, at start-up, which deployment this is.
|
||||
pub(super) fn probe_serve_mode(base: &str) {
|
||||
crate::live_sync::get(
|
||||
&format!("{base}/api/mcp/server"),
|
||||
std::rc::Rc::new(|body: String| note_serve_mode(&body)),
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod auto_resolve_tests {
|
||||
use super::*;
|
||||
use op_editor_core::CollabConnectionPhase;
|
||||
|
||||
#[test]
|
||||
fn a_conflict_inside_an_active_session_resolves_itself() {
|
||||
assert!(auto_resolve_is_safe(
|
||||
true,
|
||||
CollabConnectionPhase::Active,
|
||||
false
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_conflict_means_nothing_to_resolve() {
|
||||
for phase in [
|
||||
CollabConnectionPhase::Idle,
|
||||
CollabConnectionPhase::Active,
|
||||
CollabConnectionPhase::ReadOnly,
|
||||
] {
|
||||
assert!(!auto_resolve_is_safe(false, phase, false));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_non_active_phase_keeps_the_latch() {
|
||||
// Outside an Active session there is no authoritative server document
|
||||
// to accept and no `discarded_edit` projection to recover the losing
|
||||
// edit from, so auto-accepting would silently destroy unpushed work.
|
||||
// `Reconnecting` and `ReadOnly` look session-ish and are deliberately
|
||||
// included: neither can sequence an edit.
|
||||
for phase in [
|
||||
CollabConnectionPhase::Idle,
|
||||
CollabConnectionPhase::Starting,
|
||||
CollabConnectionPhase::Discovering,
|
||||
CollabConnectionPhase::Joining,
|
||||
CollabConnectionPhase::Authenticating,
|
||||
CollabConnectionPhase::Reconnecting,
|
||||
CollabConnectionPhase::ReadOnly,
|
||||
CollabConnectionPhase::Ended,
|
||||
] {
|
||||
assert!(
|
||||
!auto_resolve_is_safe(true, phase, false),
|
||||
"{phase:?} must keep the existing explicit-resolution semantics"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod server_authority_tests {
|
||||
use super::{auto_resolve_is_safe, parse_server_authoritative};
|
||||
use op_editor_core::CollabConnectionPhase;
|
||||
|
||||
#[test]
|
||||
fn an_online_daemon_is_authoritative() {
|
||||
assert_eq!(
|
||||
parse_server_authoritative(r#"{"running":true,"port":3100,"serveMode":"online"}"#),
|
||||
Some(true)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_local_or_managed_daemon_is_not() {
|
||||
for mode in ["local", "managed"] {
|
||||
assert_eq!(
|
||||
parse_server_authoritative(&format!(r#"{{"serveMode":"{mode}"}}"#)),
|
||||
Some(false),
|
||||
"{mode}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_daemon_that_does_not_report_a_mode_leaves_the_default_alone() {
|
||||
// An older daemon has no `serveMode`; it is by definition not an
|
||||
// online one, so the conservative default must survive the probe.
|
||||
for body in [r#"{"running":true}"#, "not json", "", "{}"] {
|
||||
assert_eq!(parse_server_authoritative(body), None, "{body:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_server_authoritative_deployment_auto_resolves_outside_a_session() {
|
||||
// This is the M4 case: an online shared tenant has no collaboration
|
||||
// session, so without this the 409 latch would never lift and the
|
||||
// visitor would be frozen out of the document.
|
||||
for phase in [
|
||||
CollabConnectionPhase::Idle,
|
||||
CollabConnectionPhase::Starting,
|
||||
CollabConnectionPhase::Reconnecting,
|
||||
CollabConnectionPhase::ReadOnly,
|
||||
] {
|
||||
assert!(
|
||||
auto_resolve_is_safe(true, phase, true),
|
||||
"{phase:?} must auto-resolve when the server is the sequencer"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_conflict_never_resolves_however_authoritative_the_server_is() {
|
||||
assert!(!auto_resolve_is_safe(
|
||||
false,
|
||||
CollabConnectionPhase::Idle,
|
||||
true
|
||||
));
|
||||
assert!(!auto_resolve_is_safe(
|
||||
false,
|
||||
CollabConnectionPhase::Active,
|
||||
true
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_peer_daemon_outside_a_session_still_latches() {
|
||||
// The local daemon holds the operator's file and arbitrates nothing;
|
||||
// auto-accepting there would silently discard unpushed work.
|
||||
for phase in [
|
||||
CollabConnectionPhase::Idle,
|
||||
CollabConnectionPhase::Reconnecting,
|
||||
CollabConnectionPhase::ReadOnly,
|
||||
] {
|
||||
assert!(!auto_resolve_is_safe(true, phase, false), "{phase:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -233,6 +233,9 @@ fn push_reasons(
|
|||
pub(crate) fn start<C: RepaintContext + 'static>(inner: &Rc<RefCell<C>>, sync: SharedSync) {
|
||||
ACTIVE_SYNC.with(|slot| *slot.borrow_mut() = Some(Rc::downgrade(&sync)));
|
||||
let base = crate::daemon_base::daemon_base();
|
||||
// One-shot: learn whether this daemon is the document's sole sequencer,
|
||||
// which decides whether a sync conflict may auto-resolve.
|
||||
probe_serve_mode(&base);
|
||||
// One document fetch / one push at a time; ticks observing an in-flight
|
||||
// request skip (the TS hook queues at most one — same effective shape).
|
||||
// `fetch_busy` stays a plain Cell (pull-side, local to this module);
|
||||
|
|
@ -371,7 +374,7 @@ fn maybe_auto_resolve_conflict_in_session<C: RepaintContext + 'static>(
|
|||
.try_borrow()
|
||||
.map(|context| context.host().editor_state().editor_ui.collab.phase)
|
||||
.unwrap_or(op_editor_core::CollabConnectionPhase::Idle);
|
||||
if !auto_resolve_is_safe(has_conflict, phase) {
|
||||
if !auto_resolve_is_safe(has_conflict, phase, server_is_authoritative()) {
|
||||
return;
|
||||
}
|
||||
// Re-opens the pull for THIS pair only; the resolving pull's apply calls
|
||||
|
|
@ -382,21 +385,6 @@ fn maybe_auto_resolve_conflict_in_session<C: RepaintContext + 'static>(
|
|||
}
|
||||
}
|
||||
|
||||
/// The safety decision behind [`maybe_auto_resolve_conflict_in_session`],
|
||||
/// separated so it can be tested without a live shell.
|
||||
///
|
||||
/// `Active` and nothing else. Every other phase — including `Reconnecting` and
|
||||
/// `ReadOnly`, which look session-ish — either has no authoritative server
|
||||
/// document to accept or no `discarded_edit` projection to recover the losing
|
||||
/// edit from, so auto-accepting there would be the silent data loss this is
|
||||
/// specifically avoiding.
|
||||
const fn auto_resolve_is_safe(
|
||||
has_conflict: bool,
|
||||
phase: op_editor_core::CollabConnectionPhase,
|
||||
) -> bool {
|
||||
has_conflict && matches!(phase, op_editor_core::CollabConnectionPhase::Active)
|
||||
}
|
||||
|
||||
/// Apply a `GET /api/mcp/document` response to the live shell.
|
||||
/// `WebSyncClient::sync_with_editor_meta` runs the apply closure only for a
|
||||
/// newer document and commits that exact version only when the closure returns
|
||||
|
|
@ -782,48 +770,6 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod auto_resolve_tests {
|
||||
use super::auto_resolve_is_safe;
|
||||
use op_editor_core::CollabConnectionPhase;
|
||||
|
||||
#[test]
|
||||
fn a_conflict_inside_an_active_session_resolves_itself() {
|
||||
assert!(auto_resolve_is_safe(true, CollabConnectionPhase::Active));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_conflict_means_nothing_to_resolve() {
|
||||
for phase in [
|
||||
CollabConnectionPhase::Idle,
|
||||
CollabConnectionPhase::Active,
|
||||
CollabConnectionPhase::ReadOnly,
|
||||
] {
|
||||
assert!(!auto_resolve_is_safe(false, phase));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_non_active_phase_keeps_the_latch() {
|
||||
// Outside an Active session there is no authoritative server document
|
||||
// to accept and no `discarded_edit` projection to recover the losing
|
||||
// edit from, so auto-accepting would silently destroy unpushed work.
|
||||
// `Reconnecting` and `ReadOnly` look session-ish and are deliberately
|
||||
// included: neither can sequence an edit.
|
||||
for phase in [
|
||||
CollabConnectionPhase::Idle,
|
||||
CollabConnectionPhase::Starting,
|
||||
CollabConnectionPhase::Discovering,
|
||||
CollabConnectionPhase::Joining,
|
||||
CollabConnectionPhase::Authenticating,
|
||||
CollabConnectionPhase::Reconnecting,
|
||||
CollabConnectionPhase::ReadOnly,
|
||||
CollabConnectionPhase::Ended,
|
||||
] {
|
||||
assert!(
|
||||
!auto_resolve_is_safe(true, phase),
|
||||
"{phase:?} must keep the existing explicit-resolution semantics"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
#[path = "live_sync_conflict.rs"]
|
||||
mod live_sync_conflict;
|
||||
use live_sync_conflict::{auto_resolve_is_safe, probe_serve_mode, server_is_authoritative};
|
||||
|
|
|
|||
Loading…
Reference in a new issue