diff --git a/src/interfaces/api/handlers/wopi_handler.rs b/src/interfaces/api/handlers/wopi_handler.rs index ab4f98df..1ce94960 100644 --- a/src/interfaces/api/handlers/wopi_handler.rs +++ b/src/interfaces/api/handlers/wopi_handler.rs @@ -20,10 +20,13 @@ use axum::{ use serde::{Deserialize, Serialize}; use std::sync::Arc; +use crate::application::ports::authorization_ports::AuthorizationEngine; use crate::application::ports::file_ports::{FileRetrievalUseCase, FileUploadUseCase}; use crate::application::services::wopi_lock_service::WopiLockService; use crate::application::services::wopi_token_service::WopiTokenService; use crate::domain::repositories::drive_repository::DriveRepository; +use crate::domain::services::authorization::{Permission, Resource, Subject}; +use crate::infrastructure::services::pg_acl_engine::PgAclEngine; use crate::infrastructure::services::wopi_discovery_service::WopiDiscoveryService; /// Shared state for WOPI handlers. @@ -64,6 +67,37 @@ pub struct CheckFileInfoResponse { pub close_url: String, } +/// Enforce that the WOPI caller (`claims.sub`) still has `perm` on the +/// file at redemption time — not just at token-mint time. +/// +/// **Why every verb needs this.** WOPI tokens are validated locally +/// (HMAC over claims), so a token that was legitimately minted stays +/// verify-able until its TTL. If a grant is revoked after mint, or the +/// token was minted for view but is used to POST content, the token's +/// signature alone doesn't catch it. This helper re-checks against the +/// live authorization engine on every verb — the memory note +/// `wopi-authz-bypass` calls out the class of bugs this fences. +/// +/// Returns 404 (anti-enumeration — same shape as "file doesn't exist") +/// on both bad UUID and authorization denial. The engine emits a +/// structured `audit` line on denial internally, so ops sees the real +/// reason without the attacker being able to distinguish "gone" from +/// "revoked". +async fn require_wopi_perm( + authz: &PgAclEngine, + caller_sub: &str, + file_id: &str, + perm: Permission, +) -> Result<(uuid::Uuid, uuid::Uuid), StatusCode> { + let caller_uuid = uuid::Uuid::parse_str(caller_sub).map_err(|_| StatusCode::UNAUTHORIZED)?; + let file_uuid = uuid::Uuid::parse_str(file_id).map_err(|_| StatusCode::NOT_FOUND)?; + authz + .require(Subject::User(caller_uuid), perm, Resource::File(file_uuid)) + .await + .map_err(|_| StatusCode::NOT_FOUND)?; + Ok((caller_uuid, file_uuid)) +} + /// GET /wopi/files/{file_id} — CheckFileInfo async fn check_file_info( Path(file_id): Path, @@ -82,6 +116,19 @@ async fn check_file_info( return StatusCode::UNAUTHORIZED.into_response(); } + // Redemption-time authz: even with a valid token, the caller must + // still hold Read on this file. Catches revoked-grant-mid-session. + if let Err(status) = require_wopi_perm( + state.app_state.authorization.as_ref(), + &claims.sub, + &file_id, + Permission::Read, + ) + .await + { + return status.into_response(); + } + // Fetch file metadata let file = match state .app_state @@ -99,6 +146,24 @@ async fn check_file_info( .map(|dt| dt.to_rfc3339()) .unwrap_or_default(); + // `user_can_write` = actual current Update permission ∧ token's + // can_write flag. If the caller's Update was revoked since the + // token was minted (e.g. their grant was downgraded from Editor + // to Viewer), the editor sees the file as read-only and won't + // even attempt PutFile. The stricter `require_wopi_perm(Update)` + // in put_file is the actual gate; this field is a UI hint. + let can_write_now = claims.can_write + && state + .app_state + .authorization + .check( + Subject::User(uuid::Uuid::parse_str(&claims.sub).unwrap_or(uuid::Uuid::nil())), + Permission::Update, + Resource::File(uuid::Uuid::parse_str(&file_id).unwrap_or(uuid::Uuid::nil())), + ) + .await + .unwrap_or(false); + let response = CheckFileInfoResponse { base_file_name: file.name.clone(), // WOPI's `OwnerId` field is required. Post-D7 the DTO no @@ -112,9 +177,9 @@ async fn check_file_info( user_id: claims.sub.clone(), version: file.modified_at.to_string(), supports_locks: true, - supports_update: claims.can_write, + supports_update: can_write_now, supports_rename: false, - user_can_write: claims.can_write, + user_can_write: can_write_now, user_friendly_name: claims.username.clone(), post_message_origin: state.public_base_url.clone(), last_modified_time: last_modified, @@ -145,6 +210,18 @@ async fn get_file( return StatusCode::UNAUTHORIZED.into_response(); } + // Redemption-time authz — see require_wopi_perm docstring. + if let Err(status) = require_wopi_perm( + state.app_state.authorization.as_ref(), + &claims.sub, + &file_id, + Permission::Read, + ) + .await + { + return status.into_response(); + } + match state .app_state .applications @@ -184,6 +261,21 @@ async fn put_file( return StatusCode::UNAUTHORIZED.into_response(); } + // Redemption-time authz: the token says the caller could write when + // it was minted, but Update permission may have been revoked since. + // Re-check now so a stale write-capable token can't survive a + // downgrade / share removal / drive-membership change until its TTL. + if let Err(status) = require_wopi_perm( + state.app_state.authorization.as_ref(), + &claims.sub, + &file_id, + Permission::Update, + ) + .await + { + return status.into_response(); + } + // Check lock let request_lock = headers .get("X-WOPI-Lock") @@ -302,6 +394,22 @@ async fn file_operations( return StatusCode::UNAUTHORIZED.into_response(); } + // Every lock op mutates shared state (LOCK / UNLOCK / REFRESH_LOCK + // change the lock; GET_LOCK reads it but the read is only useful + // to a caller who could subsequently take a write action — so gate + // on Update uniformly rather than splitting per-op). A Viewer with + // a stale token must not be able to hold or contend for a lock. + if let Err(status) = require_wopi_perm( + state.app_state.authorization.as_ref(), + &claims.sub, + &file_id, + Permission::Update, + ) + .await + { + return status.into_response(); + } + let override_header = headers .get("X-WOPI-Override") .and_then(|v| v.to_str().ok()) @@ -374,25 +482,71 @@ pub struct EditorUrlResponse { pub access_token_ttl: i64, } -/// Determines if `caller_id` can access `file_id` and with what permissions. +/// Resolve the WOPI mint target: gate on real permissions and derive +/// the `can_write` flag from the caller's ACTUAL Update rights. /// -/// Uses the SQL-level ownership check (`get_file_owned`) so that files -/// belonging to other users — or non-existent files — both return `NOT_FOUND`, -/// avoiding existence-leak oracles. +/// Prior behaviour used a naive `requested_action != "view"` heuristic +/// so a Viewer clicking "Edit in Collabora" received a write-capable +/// token, promoting themselves to Editor for the token's TTL. The +/// memory note `wopi-authz-bypass` fix #12 calls this out explicitly. /// -/// Returns `(FileDto, can_write)` on success. +/// Contract: +/// +/// 1. **Read** is the bar to open the file in any mode. If the caller +/// has no Read grant, return 404 (anti-enum — same shape as "no such +/// file"). +/// 2. **Update** determines the returned `can_write` bit — INDEPENDENT +/// of what the client's `requested_action` said. A Viewer who +/// requested `action=edit` gets `can_write=false` and Collabora +/// opens in view mode; the token stays authorised for view-only +/// ops and put_file will 404 at redemption regardless. +/// 3. `requested_action == "view"` is respected as a downgrade — an +/// Editor can explicitly request view mode (co-browsing a doc +/// without accidentally editing) and get `can_write=false`. +/// +/// The `PgAclEngine::require`/`check` calls emit structured audit +/// lines on denial (`authz.denied` event), so a Viewer's "edit" +/// attempt shows up in the audit stream as a rejected Update check. async fn authorize_wopi_access( + authz: &PgAclEngine, file_retrieval: &S, file_id: &str, caller_id: uuid::Uuid, requested_action: &str, ) -> Result<(crate::application::dtos::file_dto::FileDto, bool), StatusCode> { - let file = file_retrieval - .get_file_with_perms(file_id, caller_id) + let file_uuid = uuid::Uuid::parse_str(file_id).map_err(|_| StatusCode::NOT_FOUND)?; + + // Step 1 — Read is required to even open the file. + authz + .require( + Subject::User(caller_id), + Permission::Read, + Resource::File(file_uuid), + ) .await .map_err(|_| StatusCode::NOT_FOUND)?; - // Owner verified — grant write unless explicitly requesting view-only. - let can_write = requested_action != "view"; + + let file = file_retrieval + .get_file(file_id) + .await + .map_err(|_| StatusCode::NOT_FOUND)?; + + // Step 2 — can_write reflects real Update, not the client's + // action-string. `check` returns bool without throwing; failure + // just means the caller lacks Update, so we degrade the token to + // read-only. Deliberately no `require` here — a Viewer opening + // the file is legitimate; only the write claim is suppressed. + let has_update = authz + .check( + Subject::User(caller_id), + Permission::Update, + Resource::File(file_uuid), + ) + .await + .unwrap_or(false); + + // Step 3 — allow explicit view-mode downgrade for Editors. + let can_write = has_update && requested_action != "view"; Ok((file, can_write)) } @@ -409,6 +563,7 @@ pub async fn get_editor_url( let username = &auth_user.username; // Verify the caller owns the file (SQL-level check, no existence leak). let (file, can_write) = match authorize_wopi_access( + state.app_state.authorization.as_ref(), state.app_state.applications.file_retrieval_service.as_ref(), ¶ms.file_id, user_id, @@ -494,7 +649,8 @@ async fn host_page( Ok(u) => u, Err(_) => return StatusCode::UNAUTHORIZED.into_response(), }; - let file = match authorize_wopi_access( + let (file, can_write_now) = match authorize_wopi_access( + state.app_state.authorization.as_ref(), state.app_state.applications.file_retrieval_service.as_ref(), &file_id, caller_uuid, @@ -502,7 +658,7 @@ async fn host_page( ) .await { - Ok((f, _)) => f, + Ok((f, cw)) => (f, cw), Err(status) => return status.into_response(), }; @@ -519,11 +675,15 @@ async fn host_page( _ => return StatusCode::INTERNAL_SERVER_ERROR.into_response(), }; + // Use the freshly-computed `can_write_now` (real Update permission + // ∧ requested_action) rather than the incoming token's `can_write` + // flag. Otherwise a Viewer who somehow reached this host page with + // a stale edit-capable token would get another one re-minted. let (token, ttl) = match state.token_service.generate_token( &file_id, &claims.sub, &claims.username, - claims.can_write, + can_write_now, ) { Ok(t) => t, Err(_) => return StatusCode::INTERNAL_SERVER_ERROR.into_response(), diff --git a/tests/api/run.sh b/tests/api/run.sh index 6fa42e63..e0f0e992 100755 --- a/tests/api/run.sh +++ b/tests/api/run.sh @@ -38,17 +38,34 @@ wait_for_http() { SERVER_PID="" +WOPI_MOCK_PID="" + cleanup() { if [[ -n "$SERVER_PID" ]]; then log "Stopping OxiCloud server (pid $SERVER_PID)..." kill "$SERVER_PID" 2>/dev/null || true wait "$SERVER_PID" 2>/dev/null || true fi + if [[ -n "$WOPI_MOCK_PID" ]]; then + log "Stopping WOPI mock discovery (pid $WOPI_MOCK_PID)..." + kill "$WOPI_MOCK_PID" 2>/dev/null || true + wait "$WOPI_MOCK_PID" 2>/dev/null || true + fi bash "$COMMON/stop-db.sh" } trap cleanup EXIT +# ── 0. WOPI mock discovery ──────────────────────────────────────────────────── +# Serves the static discovery.xml `OXICLOUD_WOPI_DISCOVERY_URL` +# points at (server.env pins port 9100). Started BEFORE OxiCloud so +# the server's cache-fill on first WOPI request finds it. The mock +# is stdlib-only Python (no deps) — see the file header for what it +# returns and why it's cheap. +log "Starting WOPI mock discovery on port 9100..." +node "$COMMON/wopi_mock_discovery.js" > /tmp/wopi-mock-discovery.log 2>&1 & +WOPI_MOCK_PID=$! + # ── 1. Start postgres ───────────────────────────────────────────────────────── bash "$COMMON/spawn-db.sh" @@ -168,7 +185,8 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test "$API_DIR/cross_drive_move.hurl" \ "$API_DIR/cross_drive_copy.hurl" \ "$API_DIR/webdav_dead_properties.hurl" \ - "$API_DIR/webdav_nested_move_cascade.hurl" + "$API_DIR/webdav_nested_move_cascade.hurl" \ + "$API_DIR/wopi_authz.hurl" #bash "$API_DIR/dedup_bulk_upload.sh" diff --git a/tests/api/wopi_authz.hurl b/tests/api/wopi_authz.hurl new file mode 100644 index 00000000..144e0df7 --- /dev/null +++ b/tests/api/wopi_authz.hurl @@ -0,0 +1,377 @@ +# ============================================================= +# OxiCloud — WOPI authorization at token redemption +# ============================================================= +# Regression coverage for the WOPI verb-handler bypass documented in +# memory note `wopi-authz-bypass`. Two bugs closed: +# +# 1. Verb handlers (check_file_info, get_file, put_file, +# file_operations, host_page) previously did NOT call +# `AuthorizationEngine::require` at redemption. A grant +# revoked between mint-time and request-time silently kept +# working until the token TTL expired. +# +# 2. The mint helper decided `can_write` from the client's +# `requested_action` string (`!= "view"` → write). A Viewer +# clicking "Edit in Collabora" received a write-capable +# token because the string was "edit". +# +# The fix wires `authz.require` on every verb and derives +# `can_write` from the caller's actual Update permission. This +# suite hits both paths through the real HTTP surface. +# +# Note on infra: +# * `OXICLOUD_WOPI_ENABLED=true` in tests/common/server.env +# * `OXICLOUD_WOPI_SECRET` pinned so the tokens the server mints +# round-trip verify-able through the suite +# * WOPI discovery served by `tests/common/wopi_mock_discovery.py` +# started by run.sh — mock URL points at a black-hole editor +# so we only assert on OxiCloud's own responses +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 — Login as admin (owner) and capture home folder id +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "{{username}}", "password": "{{password}}" } + +HTTP 200 +[Captures] +alice_token: jsonpath "$.access_token" +alice_user_id: jsonpath "$.user.id" + + +GET {{base_url}}/api/folders +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Captures] +alice_home_id: jsonpath "$[0].id" + + +# ───────────────────────────────────────────────────────────── +# Step 2 — Create a Bob user (Viewer under test) via admin API +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/admin/users +Authorization: Bearer {{alice_token}} +Content-Type: application/json +{ + "username": "wopi-bob", + "password": "WopiBobPassword1!", + "email": "wopi-bob@example.com", + "role": "user" +} + +HTTP 201 +[Captures] +bob_user_id: jsonpath "$.id" + + +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "wopi-bob", "password": "WopiBobPassword1!" } + +HTTP 200 +[Captures] +bob_token: jsonpath "$.access_token" + + +# ───────────────────────────────────────────────────────────── +# Step 3 — Alice uploads a plain-text file the WOPI verbs will +# target. `text/plain` is in the mock discovery XML so +# `/api/wopi/editor-url` resolves to a real (black-hole) +# editor URL — the endpoint returns 200 with an +# access_token we can then poke at the verbs. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/files/upload +Authorization: Bearer {{alice_token}} +[MultipartFormData] +folder_id: {{alice_home_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 201 +[Captures] +file_id: jsonpath "$.id" +[Asserts] +jsonpath "$.mime_type" == "text/plain" + + +# ───────────────────────────────────────────────────────────── +# Step 4 — Alice mints an editor-URL for her own file with +# `action=edit`. Owner has Update → can_write=true. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/wopi/editor-url?file_id={{file_id}}&action=edit +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Captures] +alice_edit_token: jsonpath "$.access_token" +[Asserts] +jsonpath "$.access_token" isString +jsonpath "$.editor_url" contains "edit" + + +# ───────────────────────────────────────────────────────────── +# Step 5 — CheckFileInfo with the owner's edit token. Verb +# re-checks Read → allowed. `user_can_write=true` +# reflects real Update. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/wopi/files/{{file_id}}?access_token={{alice_edit_token}} + +HTTP 200 +[Asserts] +jsonpath "$.UserId" == "{{alice_user_id}}" +jsonpath "$.UserCanWrite" == true +jsonpath "$.SupportsUpdate" == true + + +# ───────────────────────────────────────────────────────────── +# Step 6 — GetFile with the owner's edit token. Verb re-checks +# Read → 200 with body. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/wopi/files/{{file_id}}/contents?access_token={{alice_edit_token}} + +HTTP 200 +[Asserts] +body contains "Hello" + + +# ───────────────────────────────────────────────────────────── +# Step 7 — PutFile with the owner's edit token. Verb re-checks +# Update → 200. The owner overwrites her own file. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/wopi/files/{{file_id}}/contents?access_token={{alice_edit_token}} +Content-Type: application/octet-stream +``` +owner overwrite via WOPI PutFile +``` + +HTTP 200 + + +# ───────────────────────────────────────────────────────────── +# Step 8 — Alice explicitly requests view mode. Even the owner +# gets `can_write=false` — the token respects the +# client's downgrade so Collabora can open a doc +# "read-only for co-browsing". +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/wopi/editor-url?file_id={{file_id}}&action=view +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Captures] +alice_view_token: jsonpath "$.access_token" + + +GET {{base_url}}/wopi/files/{{file_id}}?access_token={{alice_view_token}} + +HTTP 200 +[Asserts] +# Owner explicitly requested view — supports_update flips off. +jsonpath "$.UserCanWrite" == false +jsonpath "$.SupportsUpdate" == false + + +# View token trying to write → 401 (token's can_write bit says no +# before the authz.require ever runs). +POST {{base_url}}/wopi/files/{{file_id}}/contents?access_token={{alice_view_token}} +Content-Type: application/octet-stream +``` +owner trying to write with view token +``` + +HTTP 401 + + +# ───────────────────────────────────────────────────────────── +# Step 9 — SECURITY: Bob has NO grant on Alice's file. Requests +# an edit-URL. The mint helper's Read gate fires → 404 +# (anti-enum). This is the pre-fix behaviour holding +# — mint-time Read was already enforced via +# get_file_with_perms. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/wopi/editor-url?file_id={{file_id}}&action=edit +Authorization: Bearer {{bob_token}} + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step 10 — Alice grants Bob the Viewer role on the file. +# Capture the grant id off the POST response so Step +# 13's revoke doesn't need to LIST + filter (the LIST +# endpoint returns a bare JSON array, not +# `.grants[?...]`, and Hurl's single-match filter +# capture behaviour is quirky — see memory note +# `feedback_hurl_jsonpath_filter_empty`). +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/grants +Authorization: Bearer {{alice_token}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{bob_user_id}}" }, + "resource": { "type": "file", "id": "{{file_id}}" }, + "role": "viewer" +} + +HTTP 201 + + +# ───────────────────────────────────────────────────────────── +# Step 11 — SECURITY: Bob (Viewer) requests an EDIT token. Fix +# #12: mint helper derives `can_write` from real +# Update permission, not from the requested_action +# string. Bob has Read but not Update → token is +# minted with `can_write=false` even though he asked +# for "edit". +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/wopi/editor-url?file_id={{file_id}}&action=edit +Authorization: Bearer {{bob_token}} + +HTTP 200 +[Captures] +bob_forged_edit_token: jsonpath "$.access_token" + + +# CheckFileInfo with Bob's "edit" token shows UserCanWrite=false +# because the token's can_write bit was scrubbed at mint. Prior +# to the fix this was `true` — a Viewer editing Alice's file. +GET {{base_url}}/wopi/files/{{file_id}}?access_token={{bob_forged_edit_token}} + +HTTP 200 +[Asserts] +jsonpath "$.UserId" == "{{bob_user_id}}" +jsonpath "$.UserCanWrite" == false +jsonpath "$.SupportsUpdate" == false + + +# Bob attempting PutFile with his "edit" token → 401. The +# token's own can_write=false is the outer gate; even if the +# token had somehow been forged with can_write=true, the +# redemption-time authz.require(Update) would return 404. +POST {{base_url}}/wopi/files/{{file_id}}/contents?access_token={{bob_forged_edit_token}} +Content-Type: application/octet-stream +``` +Bob trying to write as Viewer +``` + +HTTP 401 + + +# Bob CAN read (his Read grant is real). +GET {{base_url}}/wopi/files/{{file_id}}/contents?access_token={{bob_forged_edit_token}} + +HTTP 200 + + +# ───────────────────────────────────────────────────────────── +# Step 12 — SECURITY: promote Bob to Editor. Now he legitimately +# holds Update, so an edit token becomes truly write- +# capable. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/grants +Authorization: Bearer {{alice_token}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{bob_user_id}}" }, + "resource": { "type": "file", "id": "{{file_id}}" }, + "role": "editor" +} + +# The engine's `ON CONFLICT UPDATE` collapses one role row per +# (subject, resource), so this Editor grant REPLACES the Viewer +# grant from Step 10 rather than stacking. Bob now holds +# Editor alone; revoking it in Step 13 leaves him with no +# grants at all. +HTTP 201 +[Captures] +bob_grant_id: jsonpath "$.grants[0].id" + + +GET {{base_url}}/api/wopi/editor-url?file_id={{file_id}}&action=edit +Authorization: Bearer {{bob_token}} + +HTTP 200 +[Captures] +bob_real_edit_token: jsonpath "$.access_token" + + +GET {{base_url}}/wopi/files/{{file_id}}?access_token={{bob_real_edit_token}} + +HTTP 200 +[Asserts] +# Bob is a real Editor now → can_write flips to true. +jsonpath "$.UserCanWrite" == true +jsonpath "$.SupportsUpdate" == true + + +POST {{base_url}}/wopi/files/{{file_id}}/contents?access_token={{bob_real_edit_token}} +Content-Type: application/octet-stream +``` +Bob as Editor legitimately writes +``` + +HTTP 200 + + +# ───────────────────────────────────────────────────────────── +# Step 13 — SECURITY: revoke Bob's grant AFTER his edit token was +# minted. The token stays cryptographically valid until +# TTL, but every subsequent verb call must hit the +# authorization engine and reject. +# +# This is the CORE bug the memory note describes: prior +# to the fix Bob's PutFile still succeeded here because +# the verb handlers trusted the token in isolation. +# +# The Editor grant from Step 12 REPLACED the Viewer +# grant from Step 10 (engine's ON CONFLICT UPDATE — +# one role row per subject/resource). So revoking the +# Editor grant leaves Bob with no grants at all; every +# verb — Read AND Update — must refuse. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/grants/{{bob_grant_id}} +Authorization: Bearer {{alice_token}} + +HTTP 204 + + +# CheckFileInfo — no Read → 404. Prior to the fix the verb +# handler trusted the token and returned 200 with the file's +# metadata. +GET {{base_url}}/wopi/files/{{file_id}}?access_token={{bob_real_edit_token}} + +HTTP 404 + + +# GetFile — no Read → 404. Prior to the fix Bob could still +# download the file content until the token TTL expired. +GET {{base_url}}/wopi/files/{{file_id}}/contents?access_token={{bob_real_edit_token}} + +HTTP 404 + + +# PutFile — no Update → 404 (verb-side require_wopi_perm), OR +# 401 if the token's own `!claims.can_write` gate happened to +# fire first. The important assertion is "not 200" — a revoked +# grant must never let the caller through. +POST {{base_url}}/wopi/files/{{file_id}}/contents?access_token={{bob_real_edit_token}} +Content-Type: application/octet-stream +``` +Bob post-revoke tries to write +``` + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Cleanup — delete the test file so subsequent Hurl files don't +# see it. Bob user stays; other tests may reuse the `wopi-bob` +# username, but the grants that made this test meaningful are +# gone. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/files/{{file_id}} +Authorization: Bearer {{alice_token}} + +HTTP 204 diff --git a/tests/common/server.env b/tests/common/server.env index 8cc4f118..6c27ee39 100644 --- a/tests/common/server.env +++ b/tests/common/server.env @@ -13,7 +13,17 @@ OXICLOUD_ENABLE_SEARCH=true OXICLOUD_ENABLE_FILE_SHARING=true OXICLOUD_ENABLE_MUSIC=true OXICLOUD_EXPOSE_SYSTEM_USERS=true -OXICLOUD_WOPI_ENABLED=false +OXICLOUD_WOPI_ENABLED=true +# Fixed secret so the Hurl WOPI test can hand-craft valid access +# tokens with a known signing key. Prod deployments MUST override +# this to a random per-deployment value. +OXICLOUD_WOPI_SECRET=test-wopi-secret-do-not-use-in-prod-do-not-use-in-prod +# Discovery URL points at a black hole — VERB endpoints don't need +# discovery, and the WOPI Hurl suite deliberately does NOT touch +# `/api/wopi/editor-url` (the only path that would fetch it), so +# an unreachable URL keeps startup fast and hermetic. +OXICLOUD_WOPI_DISCOVERY_URL=http://127.0.0.1:9100/discovery.xml +OXICLOUD_WOPI_TOKEN_TTL_SECS=3600 OXICLOUD_OIDC_ENABLED=false OXICLOUD_NEXTCLOUD_ENABLED=true diff --git a/tests/common/wopi_mock_discovery.js b/tests/common/wopi_mock_discovery.js new file mode 100644 index 00000000..88005321 --- /dev/null +++ b/tests/common/wopi_mock_discovery.js @@ -0,0 +1,68 @@ +#!/usr/bin/env node +// Minimal mock WOPI discovery server for the Hurl WOPI suite. +// +// Serves a valid RFC-shaped discovery XML on `GET /discovery.xml` so +// `OXICLOUD_WOPI_DISCOVERY_URL=http://127.0.0.1:/discovery.xml` +// resolves to a real editor URL when `/api/wopi/editor-url` fetches it. +// +// The `urlsrc` we hand back points at a black-hole host so no real +// editor process needs to be running — the Hurl suite only asserts on +// OxiCloud's own responses (token contents, HTTP status codes, +// headers). The mock exists purely to let `get_editor_url` succeed +// end-to-end so we can exercise the mint-time authz path (Viewer- +// clicks-Edit gets a read-only token). +// +// Node stdlib only — matches the tooling used by tests/oidc/fake_idp +// (both are stdlib-free apart from `node-oidc-provider` on that side). +// No package.json, no npm install, no extra dependency for the api +// test suite. Started + reaped by `tests/api/run.sh`. Port comes from +// `WOPI_MOCK_PORT` env var (default 9100). + +'use strict'; + +const http = require('http'); + +const DISCOVERY_XML = ` + + + + + + + + + + + + + + + +`; + +const port = Number(process.env.WOPI_MOCK_PORT || 9100); + +const server = http.createServer((req, res) => { + if (req.method === 'GET' && req.url === '/discovery.xml') { + res.writeHead(200, { + 'Content-Type': 'application/xml; charset=utf-8', + 'Content-Length': Buffer.byteLength(DISCOVERY_XML), + }); + res.end(DISCOVERY_XML); + return; + } + res.writeHead(404); + res.end(); +}); + +// SIGTERM from `kill` in run.sh cleanup — exit quietly so the test +// runner's tail-of-log stays clean. +for (const sig of ['SIGTERM', 'SIGINT']) { + process.on(sig, () => server.close(() => process.exit(0))); +} + +server.listen(port, '127.0.0.1', () => { + console.log(`wopi-mock-discovery listening on 127.0.0.1:${port}`); +});