Feat/wopi phase 4#6
Draft
moodyjmz wants to merge 49 commits into
Draft
Conversation
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
…tion Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
…n validation Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
After stripping WOPI template placeholders the URL may have no '?' yet, so appending with '&' produced a malformed URL like `https://editor/path&wopisrc=...`. Check for an existing '?' and use '?' as the separator when absent. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
File extensions are user-controlled (derived from the filename) and
were interpolated directly into XPath predicates. Validate the extension
against a strict [a-zA-Z0-9]{1,20} allowlist before use.
Also pass LIBXML_NONET | LIBXML_NOCDATA to SimpleXMLElement to block
network requests during XML parsing of the discovery response.
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
…urface An unconstrained wopi_url allowed any scheme (file://, gopher://) and any host including cloud metadata endpoints. Reject non-http/https schemes and reject URLs that embed credentials. allow_local_address remains enabled intentionally: self-hosted deployments often run the editor and Nextcloud on the same host/network. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
… putFile Range requests with start >= fileSize now return 416 (Range Not Satisfiable) per RFC 7233 §4.4 instead of silently streaming wrong data. Added try/finally around both range-stream handles so they are closed on exception. Also wrapped php://input in try/finally in putFile so the handle is always released even when early-return or an exception fires. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Without a cleanup job the office_wopi table grows without bound — one row per file open per 10-hour TTL window. Add a TimedJob that runs hourly and batch-deletes expired rows (500 at a time) via a new WopiMapper::deleteByIds() helper. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
WOPI locks are file-level (one opaque lock_id per file, 30-minute TTL) and distinct from the per-token session data in office_wopi. Separate table keeps the semantics clean. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
WopiLock stores a single opaque lock_id per file with a 30-minute TTL (WOPI spec). WopiLockMapper provides findByFileId, upsertLock (create or refresh), getExpiredLockIds, and deleteByIds for the cleanup job. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
POST wopi/files/{fileId} dispatches on X-WOPI-Override header:
- LOCK: acquire lock; idempotent for same lock_id; 409 on conflict
- UNLOCK: verify lock_id then release; 409 on mismatch
- REFRESH_LOCK: extend TTL; 409 on mismatch
- GET_LOCK: return current lock_id (empty string if unlocked)
- LOCK + X-WOPI-OldLock: UnlockAndRelock (atomic swap)
All conflict responses carry X-WOPI-Lock and X-WOPI-LockFailureReason
headers per WOPI spec so the editor can surface a meaningful error.
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Without these guards PutFile accepted any write regardless of whether another client held the file lock, silently discarding concurrent edits. - X-WOPI-Lock must match the current stored lock when one exists; returns 409 with X-WOPI-Lock + X-WOPI-LockFailureReason on mismatch. - X-WOPI-ItemVersion (when provided) must match the file mtime; returns 409 with current X-WOPI-ItemVersion when file changed out-of-band. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
The same hourly job now clears both office_wopi (expired tokens) and office_wopi_locks (expired file locks) so neither table grows without bound. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
NC35 removed registerJob() and registerSettings() from IRegistrationContext. Declare the CleanupJob background job and the Admin settings class in appinfo/info.xml instead, which is the supported mechanism from NC35 onwards. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
NC's putContent() closes the stream handle internally. The finally block called fclose() on an already-invalid resource, producing a PHP warning and a 500 response. Guard with is_resource() before closing. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
…ponses StreamResponse defaults to text/html; override on all GetFile paths (full, range, zero-byte) for WOPI spec compliance. Signed-off-by: James Manuel <james.manuel@nextcloud.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
executeOperation dispatched LOCK/UNLOCK/REFRESH_LOCK without checking canwrite, allowing read-only tokens (including read-only guest shares) to acquire and hold locks. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Migration 1002 adds hide_download BOOLEAN (default false) to oc_office_wopi. WopiMapper::generateGuestToken and TokenManager:: generateGuestToken accept the flag; CheckFileInfo derives HideExportOption, DisablePrint, DisableExport and UserCanNotWriteRelative from it at response time. Signed-off-by: James Manuel <james.manuel@nextcloud.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
GET /apps/office/open/share/{shareToken} opens a file in the WOPI editor
via a Nextcloud public share link. Handles:
- File shares (share node is a File)
- Folder shares (resolve target by fileId or path parameter)
- Password-protected shares (checks public_link_authenticated session
key in both legacy string and current array-of-IDs formats)
Generates a guest WOPI token stamped with the share's canWrite and
hideDownload flags. Returns the same editor TemplateResponse shape
as EditorController::open.
Signed-off-by: James Manuel <james.manuel@nextcloud.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
HideExportOption, DisablePrint and DisableExport are now derived from the token's hide_download field. UserCanNotWriteRelative is true for both guests and hideDownload tokens, preventing Save As from writing back to the owner's storage. Signed-off-by: James Manuel <james.manuel@nextcloud.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
- Strip control chars and cap guestName at 64 bytes in ShareController to prevent log injection and oversized display names - Add boundary comment on getFirstNodeById in resolveFile - Document three known gaps in PHASE3_DECISIONS.md (password UI, authenticated-user-through-share, federated shares) Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Parses app/@name attributes from the cached discovery XML to return all MIME types the editor advertises. Returns [] on cache miss or parse failure so callers degrade gracefully. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
KG2: authenticated NC users visiting a share link now get a full user
token via TokenManager::generateToken() rather than a guest token.
hideDownload is not applied on the user path — download restrictions
target unauthenticated third parties, not collaborators with direct access.
KG1: password-protected share without a session now redirects to
/s/{token} for the NC password challenge instead of returning 401 JSON.
Authenticated users bypass the password check entirely.
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Register LoadAdditionalScriptsListener to load the office-file-actions script on every Files app page. Supported MIME types from DiscoveryService are injected as initial state so the file action can filter correctly without a per-file HTTP round-trip. Falls back to an empty MIME list if discovery is unavailable, preventing the listener from blocking page render. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Adds a file-actions.ts entry that registers a DEFAULT file action for all MIME types advertised by the editor's discovery XML. MIME list is injected as initial state by LoadAdditionalScriptsListener. - Authenticated file access routes to EditorController::open - Public share context (isPublicShare()) routes to ShareController using getSharingToken() from @nextcloud/sharing/public Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
window.close() only works for popup windows. For full-page navigation, fall back to the office overview if there is no history to go back to. Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Covers: features, local dev setup, WOPI protocol flow, key classes, Phase 3 public share support, known gaps, and architecture decisions. Signed-off-by: James Manuel <james.manuel@nextcloud.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
- LoadAdditionalScriptsListener: replace deprecated IInitialStateService with IInitialState (drops app-ID arg from provideInitialState) - SettingsController: fix Settings\Admin namespace (was resolving to wrong FQN), use strict === false check on parse_url result - EditorController + ShareController: replace invalid 'blank' TemplateResponse render type with 'base' - ShareController: add RedirectResponse to return type union; use !== '' for password check instead of truthy comparison - WopiController: guard fopen() calls against false before passing to StreamResponse; add explicit return type on putFile closure; type usort callback parameters; cast range $length to int - Application: remove manual TokenManager service registration — NC DI autowires it including the ?string $userId convention - TokenManager: remove unused $logger property - DiscoveryService: use strict === false / === [] checks on xpath() results Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Psalm 5.x crashes on PHP 8.5 with "null as array offset" in its own internals. Upgrade vendor-bin/psalm to ^6.0 (6.16.1) to resolve. Generate psalm-baseline.xml to suppress ~130 known false positives: UnusedClass for DI-registered services, MissingDependency for NC internal oc\hooks\emitter, QBMapper entity magic methods, and Doctrine DBAL docblock type widening. Genuine bugs fixed in previous commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Required by CI npm install/build/lint steps. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
moodyjmz
force-pushed
the
feat/wopi-phase-4
branch
from
e3ac239 to
933de98
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add WOPI support - kind of PoC, but works well