fix: only navigate to a validated absolute http(s) return URL

[oc-tmueller]

Summary

The value that tells the editor where to navigate when it is closed is now
supplied by the server side and validated, instead of being taken from the
server URL parameter in documents.js.

• DocumentController::federated() validates server as an absolute http(s)
  URL with a non-empty host and returns the usual responseError() otherwise,
  alongside the existing request guards. The validated value is handed to the
  template as return_to_server.
• index() and public() emit an empty return_to_server — only a federated
  share ever returns to a remote server. All three methods render the same
  template, so all three set the key.
• templates/documents.php emits it as a hidden input, following the existing
  #wopi-url idiom.
• documents.js reads that hidden input rather than the URL parameter, and
  re-checks the value with new URL() before using it as a navigation target,
  falling back to the document list if it does not parse. A path component is
  accepted, so subdirectory installs keep working.

Separately, the WOPI postMessage target origin is narrowed from '*' to the
Collabora Online origin derived from the discovery urlsrc, and both message
listeners now ignore events that do not come from that origin.

Core's shared getURLParameter() is deliberately untouched — it is a global
also used by apps/files.

Testing

20 new PHPUnit cases in DocumentControllerTest:

• federated() rejects values that are not absolute http(s) URLs, including
  scheme-relative ones, and accepts well-formed ones — in particular a
  subdirectory install and a non-default port.
• the validated value reaches the template, and public() always emits an
  empty one.
• public() accepts no server parameter at all.

Full unit suite: 78 tests, 199 assertions, green on PHP 8.3.
php-cs-fixer (ownCloud coding standard): clean. phpstan level 5 on appinfo lib:
no errors.

The JS side is verified manually — this repo has no JS test harness
(package.json has lint and build only, and lint covers src/ only, not
js/).

🤖 Generated with Claude Code

[kw-fscheuer]

Reviewed as a security fix for the DOM XSS → account takeover finding tracked internally as OC10-163 (YesWeHack #YWH-PGM8721-594, CVSS:3.1 8.7). Approving — this closes it, and the approach is the right one.

What I checked and liked:

• The source is removed, not filtered. Dropping getURLParameter('server') in favour of a server-populated hidden input fixes the root cause instead of blocking one payload. Having public() and index() emit an empty return_to_server is what actually closes the reported page — the vulnerable route can no longer produce a navigable value at all.
• No render site was missed. TemplateResponse('richdocuments', 'documents', …) occurs at exactly three places (index, public, federated) and all three now set the key, so the template never renders with it undefined.
• _absoluteHttpUrl() gets the subtle part right. It does not rely on new URL() throwing — it would not, since new URL('javascript:alert(1)') parses successfully — but explicitly checks url.protocol and url.host. That is the single easiest thing to get wrong here. Keeping the path component so subdirectory installs work is the right call too.
• isValidServerUrl() is sound: requiring parse_url() to yield both a scheme and a non-empty host rejects javascript:/data: (no host) and scheme-relative //evil.tld (no scheme), and the scheme comparison is case-insensitive.
• Both message listeners are guarded, including editorInitListener — our original assessment only cited the second one, so thanks for catching that — and narrowing the outbound postMessage target from '*' is a genuine improvement beyond the reported issue.
• Failure modes are closed throughout: messages are dropped when the origin cannot be determined, and onClose() falls through to the document list rather than navigating on a value that does not validate.
• Test coverage is solid. Asserting getDocumentByFederatedToken is never() called on the invalid cases proves the request stops at the guard rather than merely erroring later, which is the right thing to pin.

Three non-blocking hardening notes, none of which should hold up the merge:

1. _wopiOrigin() falls back to our own origin when urlsrc is empty. new URL(undefined, window.location.href) does not throw — undefined stringifies to "undefined" and resolves as a relative path — so the helper returns the ownCloud origin rather than null, and the listeners would then accept messages from our own origin. Not exploitable (posting from that origin already implies script execution there, and the editor pages always set rd_urlsrc), but the failure mode is worth closing:

   _wopiOrigin: function() {
       if (!documentsMain.urlsrc) {
           return null;
       }
       try {
           return new URL(documentsMain.urlsrc, window.location.href).origin;
       } catch (exc) { … }
   }

2. Worth a comment on the ordering dependency. isValidServerUrl() accepts any absolute http(s) host, so on its own it would allow server=http://attacker.tld as a navigation target. What actually prevents that is the federation allowlist downstream — getWopiForToken() → FederationService::isServerAllowed(), which fails closed on an empty richdocuments.federation_allowlist and makes federated() return responseError() before rendering. That layering is correct, but implicit; a one-line comment at the new guard noting that host trust comes from isServerAllowed() and this check is scheme/shape only would stop a future refactor from quietly turning it into an open redirect.

3. The allowInlineScript(true) opt-out remains (index, public, federated). Correctly out of scope for this PR, but it is what made this sink reachable at all, so until it goes the next DOM sink here will also fail open. This PR conveniently demonstrates the pattern for retiring it — #return-to-server joining #wopi-url. Worth its own issue.

Noted on testing: the JS side having no harness is a real gap for items like (1), since lint covers src/ only, not js/. Not something to solve in this PR.

[oc-tmueller]

Notes 1 and 2 are addressed in the force-pushed 3e633bb, mirrored byte for byte in #621 (4f4173f). Note 3 is now tracked as OC10-167, linked to OC10-163.

Note 1 — _wopiOrigin() falls back to our own origin. Confirmed, and there is a second way in that we both missed. new URL() throws for neither of them:

urlsrc	old return value
'' / undefined / null	our own origin
javascript:… / data:…	the string 'null'

The second row is the worse one: an opaque origin serializes to the string 'null', which is exactly what a sandboxed frame or a data:/blob: frame reports as e.origin — so the listener would have accepted it, and the outbound postMessage(msg, 'null') would have thrown. Not reachable today, since urlsrc comes from the admin's discovery XML rather than from a request, but DiscoveryService::getWopiSrc() does return ['urlsrc' => null] when discovery cannot be read, which is the first row.

Both now yield no origin at all. The guard reuses _absoluteHttpUrl() so that "absolute http(s) with a host" stays defined in one place, and it validates the resolved URL so that a urlsrc configured relative to this server keeps working:

_wopiOrigin: function() {
	if (!documentsMain.urlsrc) {
		return null;
	}

	var resolved;
	try {
		resolved = new URL(documentsMain.urlsrc, window.location.href);
	} catch (exc) { … return null; }

	if (!documentsMain._absoluteHttpUrl(resolved.href)) {
		return null;
	}

	return resolved.origin;
}

The callers already handled null correctly — both listeners drop the message and WOPIPostMessage() returns without posting.

Note 2 — the ordering dependency. Confirmed and now written down at the guard: it checks the shape only, and host trust comes from isServerAllowed() via getWopiForToken(), which fails closed on an empty richdocuments.federation_allowlist and returns before the template response is built. The comment says not to drop that call or move it behind the render, because on its own the new check accepts any host.

Note 3 — allowInlineScript(true). Agreed, and OC10-167 records it: the three call sites, that this is what made the sink reachable rather than inert, and the retirement path #620 demonstrates (moving the remaining rd_* inline globals to hidden inputs the way #return-to-server joined #wopi-url).

On the testing gap. Fair, and it is the reason note 1 could sit there unnoticed. There is no JS harness on these branches, so this change has no CI coverage — I verified it against a local karma/jasmine harness instead, and all six new _wopiOrigin cases fail against the pre-fix code with exactly the values in the table above (http://localhost:9876 for the empty cases, 'null' for javascript:/data:). That harness is not part of this PR; it will come separately so the security fix stays minimal and backportable.