feat: sabre webdav server implementation to replace external webdav a…#1330
Open
feat: sabre webdav server implementation to replace external webdav a…#1330
Conversation
db8222c to
d79d6ae
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Implements an internal (sabre/dav-based) WebDAV endpoint in the internal app to support the legacy MS Word RTF editing journey, removing dependency on the external Apache WebDAV service and proxying document reads/writes through CQRS/API.
Changes:
- Add
/internal-dav/nginx routing and a dedicatedwebdav.phpfront controller to handle MS Word WebDAV requests. - Introduce WebDAV services (JWT verification, virtual DAV nodes, Redis locking/cache) and wire controllers to generate internal WebDAV links behind a feature toggle.
- Extend the API/document-store integration to support overwriting document contents (
OverwriteContent+DocumentStoreInterface::update) and add related validation.
Reviewed changes
Copilot reviewed 45 out of 48 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/docker/internal/internal.conf | Routes /internal-dav/ to webdav.php and adds DAV-compliance headers on / for MS Word |
| app/internal/public/webdav.php | New WebDAV entrypoint: validates JWT, restores session tokens from Redis, runs sabre/dav server |
| app/internal/module/Olcs/src/Service/WebDav/* | Redis factory + DAV virtual filesystem + Redis lock backend + JWT verification service/factory |
| app/internal/module/Olcs/src/Service/Helper/WebDavJsonWebTokenGenerationService* | Adds internal WebDAV URL pattern + extra JWT claims to support the integrated flow |
| app/internal/module//Controller/ | Uses WebDavSessionTrait to generate internal/legacy DAV links and cache tokens when enabled |
| app/api/module/Api/src/Domain/CommandHandler/Document/OverwriteContent.php | Adds API command handler to overwrite stored document content (toggle-gated) |
| app/api/module/DocumentShare/src/Service/WebDavClient.php + DocumentStoreInterface.php | Adds update()/updateStream() support for overwriting remote-stored files |
| app/internal/test/* + app/api/test/* | Adds/updates tests for the new WebDAV components and API validation/handler logic |
| docs/app/features/webdav.md | Documents the new internal WebDAV architecture and key components |
Comments suppressed due to low confidence (3)
app/internal/module/Olcs/src/Service/WebDav/VirtualFile.php:118
getLastModified()falls back totime()whenever no cached value exists, so the same document can appear to have a different last-modified timestamp on every request/PROPFIND. This can confuse WebDAV clients (including MS Office) and break caching/conflict detection. Consider sourcing this from document metadata (or keeping a stable value for the lifetime of the WebDAV session) instead of using the current time.
app/internal/test/Olcs/src/Service/WebDav/WebDavRedisFactoryTest.php:59invokeUsesDefaultsWhenConfigMissing()is non-deterministic and can slow/flap in CI because it attempts a real connection tolocalhost:6379(and may wait up to the 5s timeout). Prefer making this test deterministic (e.g. configure an invalid host/port and assertnull, or refactor the factory to allow injecting/mocking the Redis client).
app/internal/public/webdav.php:143- The JWT
docclaim is treated as a file extension ($extension = $documentPath) and concatenated into$filenamewithout any sanitization. If this claim ever contains unexpected values (e.g. including/,.., or multiple dots), it could create surprising paths/nodes in the DAV tree. Consider renaming$documentPathto$extensionand validating against an allowlist like{rtf,doc,docx}(or at least stripping non-alphanumerics) before building the filename.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…pache server to support legacy MSword RTF edit journey
d79d6ae to
c236134
Compare
…lementation # Conflicts: # app/internal/composer.lock # app/selfserve/composer.lock
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
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.
…pache server to support legacy MSword RTF edit journey
Description
Implements just enough webdav server in app/internal application to support the caseworkers document editing in MSWord journey to continue, abstracted from the underlying file-store. All Document reads/writes in this new flow are handled via CQRS to the API, and as such are agnostic to the ultimate file storage backend.
Related issue: VOL-6960
Before submitting (or marking as "ready for review")