mirror of
https://github.com/alexta69/metube.git
synced 2026-09-21 13:35:01 +00:00
fix: let named CORS origins send credentials (closes #155)
A bookmarklet could never reach an instance behind reverse-proxy auth. on_prepare echoed Access-Control-Allow-Origin but never sent Access-Control-Allow-Credentials, and hardcoded Allow-Headers to Content-Type, so both approaches the reporter suggested in 2022 still failed in a browser today: credentials:'include' was rejected for the missing Allow-Credentials, and an explicit Authorization header was rejected as not allowed by the preflight. Naming an origin in CORS_ALLOWED_ORIGINS is a deliberate trust grant, so a named origin may now send credentials and an Authorization header. The '*' wildcard is not such a grant: it matches origins the operator never enumerated, and since credentials require echoing the origin back rather than sending '*', pairing the two would let any site the user visits drive their instance with their own session. The wildcard therefore keeps byte-for-byte the uncredentialed behaviour it has always had, and a '*' anywhere in the list disables credentials for every origin in it, with a startup warning so that combination is not silently confusing. This matches the boundary socket.io already enforces: engineio defaults cors_credentials to True, and its wildcard test is against the string '*' while we pass a list, so it too grants credentials only to explicitly listed origins. Also sets Vary: Origin, appending rather than clobbering, so a shared cache cannot hand one origin's Allow-Origin to another. Verified in a real browser across the matrix: with the caller's origin named, plain/credentialed/Authorization requests all succeed; under '*' the credentialed and Authorization requests stay blocked; from an unlisted origin everything stays blocked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -98,7 +98,7 @@ Enabling `writeinfojson` or `writethumbnail` in `YTDL_OPTIONS` also writes a fee
|
|||||||
* __HTTPS__: Use `https` instead of `http` (__CERTFILE__ and __KEYFILE__ required). Defaults to `false`.
|
* __HTTPS__: Use `https` instead of `http` (__CERTFILE__ and __KEYFILE__ required). Defaults to `false`.
|
||||||
* __CERTFILE__: HTTPS certificate file path.
|
* __CERTFILE__: HTTPS certificate file path.
|
||||||
* __KEYFILE__: HTTPS key file path.
|
* __KEYFILE__: HTTPS key file path.
|
||||||
* __CORS_ALLOWED_ORIGINS__: Comma-separated list of origins permitted to make cross-origin requests to the MeTube API; `*` allows all. When unset or empty, all cross-origin requests are denied. Required for browser extensions and bookmarklets — see [Sending links to MeTube](#-sending-links-to-metube).
|
* __CORS_ALLOWED_ORIGINS__: Comma-separated list of origins permitted to make cross-origin requests to the MeTube API; `*` allows all. When unset or empty, all cross-origin requests are denied. Required for browser extensions and bookmarklets — see [Sending links to MeTube](#-sending-links-to-metube). Naming origins explicitly also lets them send credentials (a login cookie, or the `Authorization` header a reverse proxy checks), which `*` deliberately does not: it would let any site you visit drive your instance with your own session.
|
||||||
* __ROBOTS_TXT__: A path to a `robots.txt` file mounted in the container.
|
* __ROBOTS_TXT__: A path to a `robots.txt` file mounted in the container.
|
||||||
|
|
||||||
## 🎛️ Configuring yt-dlp options
|
## 🎛️ Configuring yt-dlp options
|
||||||
@@ -243,7 +243,7 @@ __Browser extensions__ allow right-clicking videos and sending them directly to
|
|||||||
* __Chrome:__ contributed by [Rpsl](https://github.com/rpsl) — install from the [Chrome Webstore](https://chrome.google.com/webstore/detail/metube-downloader/fbmkmdnlhacefjljljlbhkodfmfkijdh) or [from sources](https://github.com/Rpsl/metube-browser-extension).
|
* __Chrome:__ contributed by [Rpsl](https://github.com/rpsl) — install from the [Chrome Webstore](https://chrome.google.com/webstore/detail/metube-downloader/fbmkmdnlhacefjljljlbhkodfmfkijdh) or [from sources](https://github.com/Rpsl/metube-browser-extension).
|
||||||
* __Firefox:__ contributed by [nanocortex](https://github.com/nanocortex) — install from [Firefox Addons](https://addons.mozilla.org/en-US/firefox/addon/metube-downloader) or get sources [here](https://github.com/nanocortex/metube-firefox-addon).
|
* __Firefox:__ contributed by [nanocortex](https://github.com/nanocortex) — install from [Firefox Addons](https://addons.mozilla.org/en-US/firefox/addon/metube-downloader) or get sources [here](https://github.com/nanocortex/metube-firefox-addon).
|
||||||
|
|
||||||
__Bookmarklets__ send the currently open page to MeTube with one click. Add the origins of the sites where you use them to `CORS_ALLOWED_ORIGINS`, e.g. `https://www.youtube.com,https://www.vimeo.com`. The code (Chrome and Firefox variants, contributed by [kushfest](https://github.com/kushfest) and [shoonya75](https://github.com/shoonya75)) is in the [Bookmarklets wiki page](https://github.com/alexta69/metube/wiki/Bookmarklets).
|
__Bookmarklets__ send the currently open page to MeTube with one click. Add the origins of the sites where you use them to `CORS_ALLOWED_ORIGINS`, e.g. `https://www.youtube.com,https://www.vimeo.com`. If your instance sits behind authentication, list the origins individually rather than using `*` — only named origins are allowed to send credentials. The code (Chrome and Firefox variants, contributed by [kushfest](https://github.com/kushfest) and [shoonya75](https://github.com/shoonya75)) is in the [Bookmarklets wiki page](https://github.com/alexta69/metube/wiki/Bookmarklets).
|
||||||
|
|
||||||
__iOS Shortcut:__ [rithask](https://github.com/rithask) created an [iOS shortcut](https://www.icloud.com/shortcuts/66627a9f334c467baabdb2769763a1a6) for sending URLs to MeTube from Safari's share menu; it prompts for your instance address on first use.
|
__iOS Shortcut:__ [rithask](https://github.com/rithask) created an [iOS shortcut](https://www.icloud.com/shortcuts/66627a9f334c467baabdb2769763a1a6) for sending URLs to MeTube from Safari's share menu; it prompts for your instance address on first use.
|
||||||
|
|
||||||
|
|||||||
+44
-3
@@ -335,6 +335,12 @@ async def state_dir_guard(request, handler):
|
|||||||
|
|
||||||
app = web.Application(middlewares=[state_dir_guard])
|
app = web.Application(middlewares=[state_dir_guard])
|
||||||
_cors_origins = [o.strip() for o in config.CORS_ALLOWED_ORIGINS.split(',') if o.strip()] if config.CORS_ALLOWED_ORIGINS else []
|
_cors_origins = [o.strip() for o in config.CORS_ALLOWED_ORIGINS.split(',') if o.strip()] if config.CORS_ALLOWED_ORIGINS else []
|
||||||
|
if '*' in _cors_origins and len(_cors_origins) > 1:
|
||||||
|
log.warning(
|
||||||
|
"CORS_ALLOWED_ORIGINS mixes '*' with named origins %s. '*' wins, and credentialed "
|
||||||
|
"cross-origin requests stay disabled for every origin in the list. Remove '*' if you "
|
||||||
|
"need a bookmarklet to reach an authenticated instance.",
|
||||||
|
[o for o in _cors_origins if o != '*'])
|
||||||
sio = socketio.AsyncServer(cors_allowed_origins=_cors_origins if _cors_origins else [])
|
sio = socketio.AsyncServer(cors_allowed_origins=_cors_origins if _cors_origins else [])
|
||||||
sio.attach(app, socketio_path=config.URL_PREFIX + 'socket.io')
|
sio.attach(app, socketio_path=config.URL_PREFIX + 'socket.io')
|
||||||
routes = web.RouteTableDef()
|
routes = web.RouteTableDef()
|
||||||
@@ -1286,9 +1292,44 @@ app.router.add_route('OPTIONS', config.URL_PREFIX + 'delete-cookies', add_cors)
|
|||||||
|
|
||||||
async def on_prepare(request, response):
|
async def on_prepare(request, response):
|
||||||
origin = request.headers.get('Origin')
|
origin = request.headers.get('Origin')
|
||||||
if origin and _cors_origins and ('*' in _cors_origins or origin in _cors_origins):
|
if not origin or not _cors_origins:
|
||||||
response.headers['Access-Control-Allow-Origin'] = origin
|
return
|
||||||
response.headers['Access-Control-Allow-Headers'] = 'Content-Type'
|
|
||||||
|
# Naming an origin in CORS_ALLOWED_ORIGINS is a deliberate trust grant, so
|
||||||
|
# such an origin may send credentials: the cookie or Authorization header
|
||||||
|
# that a reverse proxy in front of MeTube checks. Without this a bookmarklet
|
||||||
|
# cannot reach an authenticated instance at all (issue #155).
|
||||||
|
#
|
||||||
|
# The '*' wildcard is emphatically not such a grant — it matches origins the
|
||||||
|
# operator never enumerated, including every site the user happens to visit.
|
||||||
|
# Echoing the origin back (which we must do, since '*' is illegal alongside
|
||||||
|
# credentials) and allowing credentials would let any page drive the user's
|
||||||
|
# instance with the user's own session. So the wildcard keeps exactly the
|
||||||
|
# uncredentialed behaviour it has always had, and a wildcard anywhere in the
|
||||||
|
# list disables credentials for every origin in it.
|
||||||
|
#
|
||||||
|
# Derived here rather than held in a second module global so the wildcard
|
||||||
|
# test and the membership test can never disagree about the same list.
|
||||||
|
wildcard = '*' in _cors_origins
|
||||||
|
trusted = not wildcard and origin in _cors_origins
|
||||||
|
if not (wildcard or trusted):
|
||||||
|
return
|
||||||
|
|
||||||
|
response.headers['Access-Control-Allow-Origin'] = origin
|
||||||
|
# Authorization rides on the same grant: allowing it under the wildcard
|
||||||
|
# would let an arbitrary page attempt credentials against an instance it
|
||||||
|
# can already reach, from inside the victim's network.
|
||||||
|
response.headers['Access-Control-Allow-Headers'] = 'Content-Type, Authorization' if trusted else 'Content-Type'
|
||||||
|
if trusted:
|
||||||
|
response.headers['Access-Control-Allow-Credentials'] = 'true'
|
||||||
|
|
||||||
|
# The response now differs per Origin, so a shared cache must not hand one
|
||||||
|
# origin's Allow-Origin to another.
|
||||||
|
vary = response.headers.get('Vary')
|
||||||
|
if not vary:
|
||||||
|
response.headers['Vary'] = 'Origin'
|
||||||
|
elif 'origin' not in (v.strip().lower() for v in vary.split(',')):
|
||||||
|
response.headers['Vary'] = f'{vary}, Origin'
|
||||||
|
|
||||||
app.on_response_prepare.append(on_prepare)
|
app.on_response_prepare.append(on_prepare)
|
||||||
|
|
||||||
|
|||||||
@@ -525,3 +525,130 @@ async def test_download_blocks_state_dir_files(monkeypatch):
|
|||||||
(download_dir / "video.mp4").unlink(missing_ok=True)
|
(download_dir / "video.mp4").unlink(missing_ok=True)
|
||||||
(download_dir / percent_filename).unlink(missing_ok=True)
|
(download_dir / percent_filename).unlink(missing_ok=True)
|
||||||
state_dir.rmdir()
|
state_dir.rmdir()
|
||||||
|
|
||||||
|
# --- CORS (issue #155) -------------------------------------------------------
|
||||||
|
#
|
||||||
|
# The security property under test: credentials are granted only to an origin
|
||||||
|
# the operator named explicitly, and never under the '*' wildcard. Each test
|
||||||
|
# builds a fresh Application because main.app binds to the first event loop
|
||||||
|
# that runs it; the logic under test lives entirely in main.on_prepare, and the
|
||||||
|
# real main.add_cors preflight handler is mounted so the preflight path is the
|
||||||
|
# production one.
|
||||||
|
|
||||||
|
async def _cors_version(request):
|
||||||
|
return web.Response(text="v")
|
||||||
|
|
||||||
|
|
||||||
|
def _cors_app():
|
||||||
|
app = web.Application()
|
||||||
|
app.router.add_route("OPTIONS", "/add", main.add_cors)
|
||||||
|
app.router.add_get("/version", _cors_version)
|
||||||
|
app.on_response_prepare.append(main.on_prepare)
|
||||||
|
return app
|
||||||
|
|
||||||
|
|
||||||
|
async def _cors_headers(monkeypatch, origins, origin, path="/add", method="OPTIONS"):
|
||||||
|
monkeypatch.setattr(main, "_cors_origins", origins)
|
||||||
|
async with TestClient(TestServer(_cors_app())) as client:
|
||||||
|
resp = await client.request(
|
||||||
|
method, path,
|
||||||
|
headers={
|
||||||
|
"Origin": origin,
|
||||||
|
"Access-Control-Request-Method": "POST",
|
||||||
|
"Access-Control-Request-Headers": "content-type,authorization",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
return resp.headers
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_cors_listed_origin_gets_credentials(monkeypatch):
|
||||||
|
h = await _cors_headers(monkeypatch, ["https://www.youtube.com"], "https://www.youtube.com")
|
||||||
|
assert h["Access-Control-Allow-Origin"] == "https://www.youtube.com"
|
||||||
|
assert h["Access-Control-Allow-Credentials"] == "true"
|
||||||
|
assert "Authorization" in h["Access-Control-Allow-Headers"]
|
||||||
|
assert "Origin" in h["Vary"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_cors_wildcard_never_grants_credentials(monkeypatch):
|
||||||
|
h = await _cors_headers(monkeypatch, ["*"], "https://evil.example")
|
||||||
|
# The wildcard still reflects the origin, exactly as before...
|
||||||
|
assert h["Access-Control-Allow-Origin"] == "https://evil.example"
|
||||||
|
# ...but must not hand out the user's session, nor let a page attempt
|
||||||
|
# credentials of its own.
|
||||||
|
assert "Access-Control-Allow-Credentials" not in h
|
||||||
|
assert h["Access-Control-Allow-Headers"] == "Content-Type"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_cors_wildcard_mixed_with_named_origin_still_denies_credentials(monkeypatch):
|
||||||
|
# '*' anywhere in the list disables credentials for everyone in it, so a
|
||||||
|
# stray wildcard cannot silently widen a named grant.
|
||||||
|
h = await _cors_headers(monkeypatch, ["*", "https://www.youtube.com"], "https://www.youtube.com")
|
||||||
|
assert h["Access-Control-Allow-Origin"] == "https://www.youtube.com"
|
||||||
|
assert "Access-Control-Allow-Credentials" not in h
|
||||||
|
assert h["Access-Control-Allow-Headers"] == "Content-Type"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_cors_unlisted_origin_gets_nothing(monkeypatch):
|
||||||
|
h = await _cors_headers(monkeypatch, ["https://www.youtube.com"], "https://evil.example")
|
||||||
|
assert "Access-Control-Allow-Origin" not in h
|
||||||
|
assert "Access-Control-Allow-Credentials" not in h
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_cors_disabled_by_default(monkeypatch):
|
||||||
|
h = await _cors_headers(monkeypatch, [], "https://www.youtube.com")
|
||||||
|
assert "Access-Control-Allow-Origin" not in h
|
||||||
|
assert "Access-Control-Allow-Credentials" not in h
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_cors_credentials_apply_to_actual_response_not_just_preflight(monkeypatch):
|
||||||
|
# The browser checks Allow-Credentials on the real response too, so a
|
||||||
|
# preflight-only grant would still fail.
|
||||||
|
h = await _cors_headers(
|
||||||
|
monkeypatch, ["https://www.youtube.com"], "https://www.youtube.com",
|
||||||
|
path="/version", method="GET")
|
||||||
|
assert h["Access-Control-Allow-Credentials"] == "true"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_cors_origin_match_is_exact(monkeypatch):
|
||||||
|
# Substring or suffix matching here would be a bypass.
|
||||||
|
for impostor in (
|
||||||
|
"https://www.youtube.com.evil.example",
|
||||||
|
"https://evilwww.youtube.com",
|
||||||
|
"http://www.youtube.com",
|
||||||
|
"https://www.youtube.com:8443",
|
||||||
|
):
|
||||||
|
h = await _cors_headers(monkeypatch, ["https://www.youtube.com"], impostor)
|
||||||
|
assert "Access-Control-Allow-Origin" not in h, impostor
|
||||||
|
assert "Access-Control-Allow-Credentials" not in h, impostor
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_cors_null_origin_is_not_trusted(monkeypatch):
|
||||||
|
# Sandboxed iframes and some file:// contexts send Origin: null.
|
||||||
|
h = await _cors_headers(monkeypatch, ["https://www.youtube.com"], "null")
|
||||||
|
assert "Access-Control-Allow-Origin" not in h
|
||||||
|
assert "Access-Control-Allow-Credentials" not in h
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_cors_vary_appends_to_existing_value(monkeypatch):
|
||||||
|
# Static responses can already carry a Vary; clobbering it would break
|
||||||
|
# content negotiation.
|
||||||
|
monkeypatch.setattr(main, "_cors_origins", ["https://www.youtube.com"])
|
||||||
|
|
||||||
|
async def handler(request):
|
||||||
|
return web.Response(text="x", headers={"Vary": "Accept-Encoding"})
|
||||||
|
|
||||||
|
app = web.Application()
|
||||||
|
app.router.add_get("/v", handler)
|
||||||
|
app.on_response_prepare.append(main.on_prepare)
|
||||||
|
async with TestClient(TestServer(app)) as client:
|
||||||
|
resp = await client.get("/v", headers={"Origin": "https://www.youtube.com"})
|
||||||
|
assert resp.headers["Vary"] == "Accept-Encoding, Origin"
|
||||||
|
|||||||
Reference in New Issue
Block a user