feat(files): share-link tokens are kept only hashed and encrypted, and every link change is audited
A /watch link's token is the credential, and it sat in the database as written. Migration 0107 replaces the column with the token's SHA-256 (the lookup) and a Fernet-encrypted copy (so the owner can copy the same link again); every existing address keeps working, and the downgrade decrypts them back. Without a copy this key can read, a link still opens but its address cannot be shown: "Copy link" then takes the newest link that has one or makes a new one, and the Share dialog says so in that row instead of a broken URL. Making, changing and revoking a link are audited, never with its token or password.
This commit is contained in:
@@ -0,0 +1,66 @@
|
||||
"""Share-link tokens no longer stored in the clear
|
||||
|
||||
Revision ID: 0107_link_token_at_rest
|
||||
Revises: 0106_api_tokens
|
||||
Create Date: 2026-10-15
|
||||
|
||||
A `/watch` link's token is the credential itself, and it sat in `download_links.token` as written,
|
||||
so a copy of the database was every shared file. Now a link is found by the token's SHA-256
|
||||
(`token_hash`), and the token is kept only Fernet-encrypted (`token_enc`, TOKEN_ENCRYPTION_KEY) so its
|
||||
owner can copy the same link again. Without a key the copy is not kept: the link still opens, its
|
||||
address just cannot be shown again (`app/downloads/links.py`).
|
||||
|
||||
Every existing link is converted here, so the addresses already sent keep working. The downgrade
|
||||
writes the tokens back from their encrypted copies; a link with none left (no key, or a key since
|
||||
changed) gets a fresh random token, which is the same as revoking it.
|
||||
"""
|
||||
|
||||
import hashlib
|
||||
import secrets
|
||||
|
||||
import sqlalchemy as sa
|
||||
from alembic import op
|
||||
|
||||
from app import security
|
||||
|
||||
revision: str = "0107_link_token_at_rest"
|
||||
down_revision: str | None = "0106_api_tokens"
|
||||
branch_labels: str | None = None
|
||||
depends_on: str | None = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.add_column("download_links", sa.Column("token_hash", sa.String(64)))
|
||||
op.add_column("download_links", sa.Column("token_enc", sa.Text()))
|
||||
conn = op.get_bind()
|
||||
sealing = security.can_encrypt()
|
||||
for link_id, token in conn.execute(sa.text("SELECT id, token FROM download_links")).fetchall():
|
||||
conn.execute(
|
||||
sa.text("UPDATE download_links SET token_hash = :h, token_enc = :e WHERE id = :id"),
|
||||
{
|
||||
"id": link_id,
|
||||
"h": hashlib.sha256(token.encode()).hexdigest(),
|
||||
"e": security.encrypt(token) if sealing else None,
|
||||
},
|
||||
)
|
||||
op.alter_column("download_links", "token_hash", nullable=False)
|
||||
op.create_unique_constraint("uq_download_link_token_hash", "download_links", ["token_hash"])
|
||||
op.drop_constraint("uq_download_link_token", "download_links", type_="unique")
|
||||
op.drop_column("download_links", "token")
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.add_column("download_links", sa.Column("token", sa.String(64)))
|
||||
conn = op.get_bind()
|
||||
rows = conn.execute(sa.text("SELECT id, token_enc FROM download_links")).fetchall()
|
||||
for link_id, enc in rows:
|
||||
token = security.decrypt_optional(enc) or secrets.token_urlsafe(24)
|
||||
conn.execute(
|
||||
sa.text("UPDATE download_links SET token = :t WHERE id = :id"),
|
||||
{"id": link_id, "t": token},
|
||||
)
|
||||
op.alter_column("download_links", "token", nullable=False)
|
||||
op.create_unique_constraint("uq_download_link_token", "download_links", ["token"])
|
||||
op.drop_constraint("uq_download_link_token_hash", "download_links", type_="unique")
|
||||
op.drop_column("download_links", "token_enc")
|
||||
op.drop_column("download_links", "token_hash")
|
||||
@@ -73,6 +73,11 @@ class AuditAction:
|
||||
# What a token then did is the file rows above, their summary naming it.
|
||||
API_TOKEN_CREATE = "api_token_create"
|
||||
API_TOKEN_REVOKE = "api_token_revoke"
|
||||
# A public `/watch` link to a file (`routes.downloads`): made, its settings changed, revoked.
|
||||
# Never its token or password — only whether it has one.
|
||||
DOWNLOAD_LINK_CREATE = "download_link_create"
|
||||
DOWNLOAD_LINK_UPDATE = "download_link_update"
|
||||
DOWNLOAD_LINK_REVOKE = "download_link_revoke"
|
||||
|
||||
|
||||
def record(
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
"""Public "share by link" helpers for the download center.
|
||||
|
||||
A DownloadLink is a capability URL: the unguessable `token` grants anyone read access to one
|
||||
download's file on the login-free `/watch/{token}` page. Password-protected links additionally
|
||||
require a short-lived signed **grant** on the file request — because a `<video src>` can't send a
|
||||
header, the password is exchanged once at `/unlock` for an HMAC grant appended to the file URL,
|
||||
so the raw password never rides in the media URL (or server logs).
|
||||
A DownloadLink is a capability URL: the unguessable token grants anyone read access to one
|
||||
download's file on the login-free `/watch/{token}` page. So the token is never stored as written:
|
||||
the link is found by its SHA-256, and the owner's copy is Fernet-encrypted — a database copy alone
|
||||
opens nothing. Without that copy (no TOKEN_ENCRYPTION_KEY, or a key since changed) the link still
|
||||
opens; only its address cannot be shown again, and the owner makes a new link to share.
|
||||
|
||||
Password-protected links additionally require a short-lived signed **grant** on the file request —
|
||||
because a `<video src>` can't send a header, the password is exchanged once at `/unlock` for an HMAC
|
||||
grant appended to the file URL, so the raw password never rides in the media URL (or server logs).
|
||||
"""
|
||||
|
||||
import hashlib
|
||||
@@ -12,6 +16,10 @@ import hmac
|
||||
import secrets
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app import security
|
||||
from app.config import settings
|
||||
from app.models import DownloadLink
|
||||
|
||||
@@ -22,6 +30,24 @@ def new_token() -> str:
|
||||
return secrets.token_urlsafe(24) # ~32 chars, 192 bits
|
||||
|
||||
|
||||
def seal(link: DownloadLink, token: str) -> None:
|
||||
"""Store `token` on `link` the only ways it is kept: its hash, and its encrypted copy."""
|
||||
link.token_hash = security.hash_token(token)
|
||||
link.token_enc = security.encrypt(token) if security.can_encrypt() else None
|
||||
|
||||
|
||||
def find(db: Session, token: str) -> DownloadLink | None:
|
||||
"""The link a `/watch` token names, by its hash."""
|
||||
return db.execute(
|
||||
select(DownloadLink).where(DownloadLink.token_hash == security.hash_token(token))
|
||||
).scalar_one_or_none()
|
||||
|
||||
|
||||
def recover(link: DownloadLink) -> str | None:
|
||||
"""The link's token from its encrypted copy, or None when there is none this key can read."""
|
||||
return security.decrypt_optional(link.token_enc)
|
||||
|
||||
|
||||
def is_expired(link: DownloadLink) -> bool:
|
||||
return link.expires_at is not None and link.expires_at <= datetime.now(timezone.utc)
|
||||
|
||||
@@ -61,12 +87,14 @@ def check_grant(token: str, grant: str | None, password_version: int) -> bool:
|
||||
# --- serialization -------------------------------------------------------------------------
|
||||
|
||||
|
||||
def owner_view(link: DownloadLink, base_url: str = "") -> dict:
|
||||
"""What the file owner sees when managing their links."""
|
||||
def owner_view(link: DownloadLink, base_url: str = "", token: str | None = None) -> dict:
|
||||
"""What the file owner sees when managing their links. `token` is the one just made (it is in
|
||||
hand even where no copy is kept); else it is read back, and `url` is None when it cannot be."""
|
||||
token = token or recover(link)
|
||||
return {
|
||||
"id": link.id,
|
||||
"token": link.token,
|
||||
"url": f"{base_url}/watch/{link.token}",
|
||||
"token": token,
|
||||
"url": f"{base_url}/watch/{token}" if token else None,
|
||||
"allow_download": link.allow_download,
|
||||
"has_password": link.password_hash is not None,
|
||||
"expires_at": link.expires_at.isoformat() if link.expires_at else None,
|
||||
|
||||
@@ -11,13 +11,12 @@ import html
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.config import settings
|
||||
from app.downloads import links as linksmod
|
||||
from app.downloads import storage
|
||||
from app.models import DownloadJob, DownloadLink, MediaAsset
|
||||
from app.models import DownloadJob, MediaAsset
|
||||
|
||||
_OG_BLOCK = re.compile(r"<!--OG:START-->.*?<!--OG:END-->", re.DOTALL)
|
||||
|
||||
@@ -74,7 +73,7 @@ def _jpeg_size(path: Path) -> tuple[int, int] | None:
|
||||
|
||||
def _watch_tags(token: str, db: Session) -> str | None:
|
||||
"""Build the OG/Twitter meta block for a watch token, or None to keep the generic card."""
|
||||
link = db.execute(select(DownloadLink).where(DownloadLink.token == token)).scalar_one_or_none()
|
||||
link = linksmod.find(db, token)
|
||||
if link is None or linksmod.is_expired(link):
|
||||
return None
|
||||
url = f"{settings.app_base}/watch/{token}"
|
||||
|
||||
@@ -1131,8 +1131,10 @@ class DownloadQuota(Base, UpdatedAtMixin):
|
||||
class DownloadLink(Base, TimestampMixin):
|
||||
"""A public capability link to one download — the "share by link" surface.
|
||||
|
||||
Anyone holding the unguessable `token` can watch the file on the login-free `/watch/{token}`
|
||||
player page (Google-Drive style). Optional per-link controls: `expires_at`, `allow_download`
|
||||
Anyone holding the unguessable token can watch the file on the login-free `/watch/{token}`
|
||||
player page (Google-Drive style). The token itself is never stored as written: a link is found
|
||||
by its SHA-256 (`token_hash`), and `token_enc` is the Fernet-encrypted copy its owner copies the
|
||||
link from again — absent without a key (`downloads/links.py`). Optional per-link controls: `expires_at`, `allow_download`
|
||||
(stream-only vs downloadable), and a `password_hash` (argon2, like a user password). Revoke =
|
||||
delete the row. Distinct from `DownloadShare`, which grants a *registered* user access via the
|
||||
in-app "Shared with me" list."""
|
||||
@@ -1143,7 +1145,8 @@ class DownloadLink(Base, TimestampMixin):
|
||||
job_id: Mapped[int] = mapped_column(
|
||||
ForeignKey("download_jobs.id", ondelete="CASCADE"), index=True
|
||||
)
|
||||
token: Mapped[str] = mapped_column(String(64), unique=True)
|
||||
token_hash: Mapped[str] = mapped_column(String(64), unique=True)
|
||||
token_enc: Mapped[str | None] = mapped_column(Text) # Fernet ciphertext — never returned/logged
|
||||
created_by: Mapped[int | None] = mapped_column(ForeignKey("users.id", ondelete="CASCADE"))
|
||||
allow_download: Mapped[bool] = mapped_column(Boolean, default=False, server_default="false")
|
||||
password_hash: Mapped[str | None] = mapped_column(Text)
|
||||
|
||||
@@ -62,6 +62,7 @@ from app.routes.files import (
|
||||
)
|
||||
from app.security import hash_password
|
||||
from app.userscope import is_messageable_user, messageable_clauses
|
||||
from app.utils import iso
|
||||
from app.refusal import refused
|
||||
|
||||
router = APIRouter(prefix="/api/downloads", tags=["downloads"])
|
||||
@@ -769,18 +770,37 @@ def create_link(
|
||||
if job.status != JobStatus.DONE or job.asset_id is None:
|
||||
raise refused(409, "share_unfinished")
|
||||
pw = body.password.strip()
|
||||
token = linksmod.new_token()
|
||||
link = DownloadLink(
|
||||
job_id=job_id,
|
||||
token=linksmod.new_token(),
|
||||
created_by=user.id,
|
||||
allow_download=body.allow_download,
|
||||
password_hash=hash_password(pw) if pw else None,
|
||||
expires_at=_link_expiry(body.expires_days),
|
||||
)
|
||||
linksmod.seal(link, token)
|
||||
db.add(link)
|
||||
db.flush()
|
||||
audit.record(
|
||||
db,
|
||||
user.id,
|
||||
audit.AuditAction.DOWNLOAD_LINK_CREATE,
|
||||
target_type="download_link",
|
||||
target_id=link.id,
|
||||
after={"job_id": job_id, **_link_audit(link)},
|
||||
)
|
||||
db.commit()
|
||||
db.refresh(link)
|
||||
return linksmod.owner_view(link)
|
||||
return linksmod.owner_view(link, token=token)
|
||||
|
||||
|
||||
def _link_audit(link: DownloadLink) -> dict:
|
||||
"""A link's settings as the audit records them: whether it has a password, never which."""
|
||||
return {
|
||||
"allow_download": link.allow_download,
|
||||
"has_password": link.password_hash is not None,
|
||||
"expires_at": iso(link.expires_at),
|
||||
}
|
||||
|
||||
|
||||
class UpdateLinkIn(BaseModel):
|
||||
@@ -797,6 +817,7 @@ def update_link(
|
||||
db: Session = Depends(get_db),
|
||||
) -> dict:
|
||||
link = _own_link(db, user, link_id)
|
||||
before = _link_audit(link)
|
||||
sent = body.model_fields_set
|
||||
if "allow_download" in sent:
|
||||
link.allow_download = bool(body.allow_download)
|
||||
@@ -808,6 +829,17 @@ def update_link(
|
||||
# Bump the version so any grant issued under the previous password stops working now, rather
|
||||
# than living out its 6h TTL (setting/clearing/changing the password all count as a rotation).
|
||||
link.password_version += 1
|
||||
after = _link_audit(link)
|
||||
if after != before or "password" in sent:
|
||||
audit.record(
|
||||
db,
|
||||
user.id,
|
||||
audit.AuditAction.DOWNLOAD_LINK_UPDATE,
|
||||
target_type="download_link",
|
||||
target_id=link_id,
|
||||
before=before,
|
||||
after=after,
|
||||
)
|
||||
db.commit()
|
||||
return linksmod.owner_view(link)
|
||||
|
||||
@@ -818,8 +850,16 @@ def revoke_link(
|
||||
) -> dict:
|
||||
link = _own_link(db, user, link_id)
|
||||
db.delete(link)
|
||||
audit.record(
|
||||
db,
|
||||
user.id,
|
||||
audit.AuditAction.DOWNLOAD_LINK_REVOKE,
|
||||
target_type="download_link",
|
||||
target_id=link_id,
|
||||
after={"job_id": link.job_id},
|
||||
)
|
||||
db.commit()
|
||||
return {"deleted": link_id}
|
||||
return {"ok": True, "deleted": link_id}
|
||||
|
||||
|
||||
# --- admin ---------------------------------------------------------------------------------
|
||||
|
||||
@@ -9,7 +9,6 @@ a short-lived signed grant that the media URL carries (a `<video src>` can't sen
|
||||
from fastapi import APIRouter, Depends, Request
|
||||
from fastapi.responses import FileResponse
|
||||
from pydantic import BaseModel
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.config import settings
|
||||
@@ -38,7 +37,7 @@ _unlock_failures = RateLimiter(max_events=50, window_seconds=300)
|
||||
|
||||
|
||||
def _link_or_404(db: Session, token: str) -> DownloadLink:
|
||||
link = db.execute(select(DownloadLink).where(DownloadLink.token == token)).scalar_one_or_none()
|
||||
link = linksmod.find(db, token)
|
||||
if link is None or linksmod.is_expired(link):
|
||||
raise refused(404, "link_gone")
|
||||
return link
|
||||
|
||||
@@ -46,6 +46,12 @@ _fernet: Fernet | None = (
|
||||
)
|
||||
|
||||
|
||||
def can_encrypt() -> bool:
|
||||
"""Whether a value can be stored encrypted here: TOKEN_ENCRYPTION_KEY is set. Production
|
||||
refuses to start without it (`config`); a local box may run without one."""
|
||||
return _fernet is not None
|
||||
|
||||
|
||||
def _require_fernet() -> Fernet:
|
||||
if _fernet is None:
|
||||
raise RuntimeError("TOKEN_ENCRYPTION_KEY is not configured")
|
||||
|
||||
@@ -0,0 +1,187 @@
|
||||
"""A `/watch` link's token at rest (downloads/links.py, migration 0107), DB-lane.
|
||||
|
||||
The token is the credential, so the database keeps only its SHA-256 (to find the link by) and a
|
||||
Fernet-encrypted copy (for its owner to copy the same link again). Without a copy this key can read,
|
||||
the link still opens and only its address cannot be shown again. Making, changing and revoking a
|
||||
link is audited, never with its token or password.
|
||||
"""
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
from alembic import command
|
||||
from alembic.config import Config
|
||||
from cryptography.fernet import Fernet
|
||||
from sqlalchemy import select, text
|
||||
|
||||
from app import security
|
||||
from app.config import settings
|
||||
from app.db import engine
|
||||
from app.downloads import probe as probemod
|
||||
from app.models import AuditLog, DownloadLink
|
||||
|
||||
_VIDEO = {
|
||||
"streams": [
|
||||
{"codec_type": "video", "codec_name": "h264", "pix_fmt": "yuv420p"},
|
||||
{"codec_type": "audio", "codec_name": "aac"},
|
||||
],
|
||||
"format": {"format_name": "mov,mp4", "duration": "5"},
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def key(monkeypatch):
|
||||
monkeypatch.setattr(security, "_fernet", Fernet(Fernet.generate_key()))
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def owner_job(as_user, make_user, tmp_path, monkeypatch, upload_headers):
|
||||
monkeypatch.setattr(settings, "files_root", str(tmp_path))
|
||||
monkeypatch.setattr(probemod, "probe_json", lambda _p: _VIDEO)
|
||||
owner = as_user(make_user())
|
||||
job = owner.post("/api/files/upload", content=b"fake-video", headers=upload_headers("clip.mp4"))
|
||||
assert job.status_code == 200, job.text
|
||||
return owner, job.json()["id"]
|
||||
|
||||
|
||||
def _row(db, link_id: int) -> dict:
|
||||
db.expire_all()
|
||||
row = db.execute(
|
||||
text("SELECT * FROM download_links WHERE id = :id"), {"id": link_id}
|
||||
).mappings()
|
||||
return dict(row.one())
|
||||
|
||||
|
||||
def _audits(db, action: str) -> list[AuditLog]:
|
||||
db.expire_all()
|
||||
return list(db.execute(select(AuditLog).where(AuditLog.action == action)).scalars())
|
||||
|
||||
|
||||
def test_the_token_is_kept_only_hashed_and_encrypted(db, key, owner_job, client):
|
||||
owner, job_id = owner_job
|
||||
made = owner.post(f"/api/downloads/{job_id}/links", json={}).json()
|
||||
token = made["token"]
|
||||
assert made["url"] == f"/watch/{token}"
|
||||
|
||||
row = _row(db, made["id"])
|
||||
assert "token" not in row
|
||||
assert token not in json.dumps(row, default=str)
|
||||
assert row["token_hash"] == security.hash_token(token)
|
||||
assert security.decrypt(row["token_enc"]) == token
|
||||
|
||||
# The address it was given opens it, and its owner can copy the same one again.
|
||||
assert client.get(f"/api/public/watch/{token}").status_code == 200
|
||||
listed = owner.get(f"/api/downloads/{job_id}/links").json()
|
||||
assert [entry["url"] for entry in listed] == [f"/watch/{token}"]
|
||||
|
||||
|
||||
def test_a_token_naming_no_link_finds_none(db, key, owner_job, client):
|
||||
owner, job_id = owner_job
|
||||
token = owner.post(f"/api/downloads/{job_id}/links", json={}).json()["token"]
|
||||
other = token[:-1] + ("A" if token[-1] != "A" else "B")
|
||||
assert client.get(f"/api/public/watch/{other}").status_code == 404
|
||||
# Its hash is no key either: the lookup hashes what it is given.
|
||||
hashed = security.hash_token(token)
|
||||
assert client.get(f"/api/public/watch/{hashed}").status_code == 404
|
||||
|
||||
|
||||
def test_without_a_key_the_link_opens_but_its_address_is_not_kept(db, owner_job, client):
|
||||
assert not security.can_encrypt()
|
||||
owner, job_id = owner_job
|
||||
made = owner.post(f"/api/downloads/{job_id}/links", json={}).json()
|
||||
# The one who made it has it in hand…
|
||||
assert made["url"] == f"/watch/{made['token']}"
|
||||
assert _row(db, made["id"])["token_enc"] is None
|
||||
assert client.get(f"/api/public/watch/{made['token']}").status_code == 200
|
||||
# …but it cannot be shown again.
|
||||
listed = owner.get(f"/api/downloads/{job_id}/links").json()
|
||||
assert [(entry["token"], entry["url"]) for entry in listed] == [(None, None)]
|
||||
|
||||
|
||||
def test_a_changed_key_hides_the_address_and_the_link_still_opens(
|
||||
db, key, owner_job, client, monkeypatch
|
||||
):
|
||||
owner, job_id = owner_job
|
||||
token = owner.post(f"/api/downloads/{job_id}/links", json={}).json()["token"]
|
||||
monkeypatch.setattr(security, "_fernet", Fernet(Fernet.generate_key()))
|
||||
listed = owner.get(f"/api/downloads/{job_id}/links").json()
|
||||
assert [entry["url"] for entry in listed] == [None]
|
||||
assert client.get(f"/api/public/watch/{token}").status_code == 200
|
||||
|
||||
|
||||
def test_making_changing_and_revoking_a_link_is_audited_without_its_secrets(db, key, owner_job):
|
||||
owner, job_id = owner_job
|
||||
pw = "a secret word"
|
||||
made = owner.post(
|
||||
f"/api/downloads/{job_id}/links", json={"password": pw, "allow_download": False}
|
||||
).json()
|
||||
link_id = made["id"]
|
||||
patch = f"/api/downloads/links/{link_id}"
|
||||
assert owner.patch(patch, json={"allow_download": True}).status_code == 200
|
||||
assert owner.patch(patch, json={"password": "another one"}).status_code == 200
|
||||
# Nothing changed: nothing to record.
|
||||
assert owner.patch(patch, json={"allow_download": True}).status_code == 200
|
||||
gone = owner.delete(f"/api/downloads/links/{link_id}")
|
||||
assert gone.json() == {"ok": True, "deleted": link_id}
|
||||
|
||||
(create,) = _audits(db, "download_link_create")
|
||||
assert create.target_id == str(link_id)
|
||||
assert create.after == {
|
||||
"job_id": job_id,
|
||||
"allow_download": False,
|
||||
"has_password": True,
|
||||
"expires_at": None,
|
||||
}
|
||||
updates = _audits(db, "download_link_update")
|
||||
assert [(u.before["allow_download"], u.after["allow_download"]) for u in updates] == [
|
||||
(False, True),
|
||||
(True, True), # the password changed: a rotation is recorded though the flags are equal
|
||||
]
|
||||
(revoke,) = _audits(db, "download_link_revoke")
|
||||
assert revoke.target_id == str(link_id)
|
||||
assert revoke.after == {"job_id": job_id}
|
||||
|
||||
trail = json.dumps(
|
||||
[[a.summary, a.before, a.after] for a in [create, *updates, revoke]], default=str
|
||||
)
|
||||
for secret in (made["token"], pw, "another one"):
|
||||
assert secret not in trail
|
||||
|
||||
|
||||
def test_the_migration_keeps_every_address_already_sent(db, key, owner_job, client):
|
||||
owner, job_id = owner_job
|
||||
db.commit() # an open transaction's locks would hold the downgrade's ALTER TABLE forever
|
||||
cfg = Config("alembic.ini")
|
||||
command.downgrade(cfg, "0106_api_tokens")
|
||||
try:
|
||||
with engine.begin() as conn:
|
||||
conn.execute(
|
||||
text("INSERT INTO download_links (job_id, token) VALUES (:j, 'an-old-token-sent')"),
|
||||
{"j": job_id},
|
||||
)
|
||||
finally:
|
||||
command.upgrade(cfg, "head")
|
||||
|
||||
(row,) = db.execute(text("SELECT token_hash, token_enc FROM download_links")).mappings().all()
|
||||
assert row["token_hash"] == security.hash_token("an-old-token-sent")
|
||||
assert security.decrypt(row["token_enc"]) == "an-old-token-sent"
|
||||
assert client.get("/api/public/watch/an-old-token-sent").status_code == 200
|
||||
listed = owner.get(f"/api/downloads/{job_id}/links").json()
|
||||
assert [entry["url"] for entry in listed] == ["/watch/an-old-token-sent"]
|
||||
|
||||
|
||||
def test_the_downgrade_writes_the_tokens_back(db, key, owner_job):
|
||||
owner, job_id = owner_job
|
||||
token = owner.post(f"/api/downloads/{job_id}/links", json={}).json()["token"]
|
||||
db.commit() # an open transaction's locks would hold the downgrade's ALTER TABLE forever
|
||||
cfg = Config("alembic.ini")
|
||||
command.downgrade(cfg, "0106_api_tokens")
|
||||
try:
|
||||
with engine.connect() as conn:
|
||||
back = conn.execute(text("SELECT token FROM download_links")).scalars().all()
|
||||
finally:
|
||||
command.upgrade(cfg, "head")
|
||||
assert back == [token]
|
||||
assert db.execute(select(DownloadLink.token_hash)).scalars().all() == [
|
||||
security.hash_token(token)
|
||||
]
|
||||
@@ -4,7 +4,7 @@
|
||||
// Revoke is the destructive one. Both colours once sat on the button, where the ghost variant's own
|
||||
// colour class outranks them (clsx does not merge Tailwind), so a password-protected link looked
|
||||
// unprotected and Revoke hovered neutral. They live on the ICON now; this pins them there.
|
||||
import { describe, it, expect, afterEach, vi } from "vitest";
|
||||
import { describe, it, expect, afterEach, beforeEach, vi } from "vitest";
|
||||
import { render, screen, cleanup } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import type { ReactNode } from "react";
|
||||
@@ -17,31 +17,33 @@ vi.mock("react-i18next", async (importOriginal) => ({
|
||||
}));
|
||||
vi.mock("./ui/Modal", () => ({ default: ({ children }: { children: ReactNode }) => children }));
|
||||
vi.mock("./confirmContext", () => ({ useConfirm: () => async () => false }));
|
||||
const h = vi.hoisted(() => ({ links: null as unknown[] | null }));
|
||||
vi.mock("../lib/api", () => ({
|
||||
api: {
|
||||
shareRecipients: async () => [],
|
||||
downloadLinks: async () => [
|
||||
{
|
||||
id: 1,
|
||||
token: "a",
|
||||
url: "/watch/a",
|
||||
allow_download: false,
|
||||
has_password: true,
|
||||
expires_at: null,
|
||||
view_count: 0,
|
||||
created_at: null,
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
token: "b",
|
||||
url: "/watch/b",
|
||||
allow_download: false,
|
||||
has_password: false,
|
||||
expires_at: null,
|
||||
view_count: 0,
|
||||
created_at: null,
|
||||
},
|
||||
],
|
||||
downloadLinks: async () =>
|
||||
h.links ?? [
|
||||
{
|
||||
id: 1,
|
||||
token: "a",
|
||||
url: "/watch/a",
|
||||
allow_download: false,
|
||||
has_password: true,
|
||||
expires_at: null,
|
||||
view_count: 0,
|
||||
created_at: null,
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
token: "b",
|
||||
url: "/watch/b",
|
||||
allow_download: false,
|
||||
has_password: false,
|
||||
expires_at: null,
|
||||
view_count: 0,
|
||||
created_at: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -49,6 +51,9 @@ import ShareDialog from "./ShareDialog";
|
||||
import type { DownloadJob } from "../lib/api";
|
||||
|
||||
afterEach(cleanup);
|
||||
beforeEach(() => {
|
||||
h.links = null;
|
||||
});
|
||||
|
||||
const iconClass = (button: HTMLElement) => button.querySelector("svg")?.getAttribute("class") ?? "";
|
||||
|
||||
@@ -71,3 +76,33 @@ describe("ShareDialog · a link row's state colours", () => {
|
||||
expect(iconClass(revoke)).toContain("text-danger");
|
||||
});
|
||||
});
|
||||
|
||||
describe("ShareDialog · a link whose address cannot be shown again", () => {
|
||||
it("says so in place of the address, with nothing to copy", async () => {
|
||||
const link = (id: number, url: string | null) => ({
|
||||
id,
|
||||
token: url && url.slice("/watch/".length),
|
||||
url,
|
||||
allow_download: false,
|
||||
has_password: false,
|
||||
expires_at: null,
|
||||
view_count: 0,
|
||||
created_at: null,
|
||||
});
|
||||
h.links = [link(3, null), link(2, "/watch/b")];
|
||||
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
render(
|
||||
<QueryClientProvider client={qc}>
|
||||
<ShareDialog job={{ id: 9 } as DownloadJob} onClose={() => {}} />
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
expect(await screen.findByText("downloads.share.urlHidden")).toBeTruthy();
|
||||
// One row has an address to show and copy; the hidden one shows none (not "…/null").
|
||||
expect(screen.getAllByRole("button", { name: "downloads.share.copy" })).toHaveLength(1);
|
||||
const shown = screen.getAllByRole("textbox").map((el) => (el as HTMLInputElement).value);
|
||||
expect(shown).toContain(`${window.location.origin}/watch/b`);
|
||||
expect(shown.some((v) => v.includes("null"))).toBe(false);
|
||||
// Its other controls stay: it can still be protected, made downloadable, or revoked.
|
||||
expect(screen.getAllByRole("button", { name: "downloads.share.revoke" })).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -70,10 +70,10 @@ function LinkRow({ link, onChanged }: { link: ShareLink; onChanged: () => void }
|
||||
const [copied, setCopied] = useState(false);
|
||||
const [editPass, setEditPass] = useState(false);
|
||||
const [newPass, setNewPass] = useState("");
|
||||
const fullUrl = window.location.origin + link.url;
|
||||
const fullUrl = link.url && window.location.origin + link.url;
|
||||
|
||||
const copy = async () => {
|
||||
if (await copyText(fullUrl)) {
|
||||
if (fullUrl && (await copyText(fullUrl))) {
|
||||
setCopied(true);
|
||||
setTimeout(() => setCopied(false), 1500);
|
||||
} else {
|
||||
@@ -114,10 +114,20 @@ function LinkRow({ link, onChanged }: { link: ShareLink; onChanged: () => void }
|
||||
<div className={`${SURFACE.card} p-2.5 space-y-2`}>
|
||||
<div className="flex items-center gap-2 rounded-md focus-within:outline-2 focus-within:outline-accent/60 focus-within:outline-offset-2">
|
||||
<Link2 className="w-4 h-4 text-muted shrink-0" />
|
||||
<InnerInput className="text-xs text-muted truncate" readOnly value={fullUrl} />
|
||||
<IconButton label={t("downloads.share.copy")} size="sm" variant="ghost" onClick={copy}>
|
||||
{copied ? <Check className="w-4 h-4 text-success" /> : <Copy className="w-4 h-4" />}
|
||||
</IconButton>
|
||||
{fullUrl ? (
|
||||
<>
|
||||
<InnerInput className="text-xs text-muted truncate" readOnly value={fullUrl} />
|
||||
<IconButton label={t("downloads.share.copy")} size="sm" variant="ghost" onClick={copy}>
|
||||
{copied ? <Check className="w-4 h-4 text-success" /> : <Copy className="w-4 h-4" />}
|
||||
</IconButton>
|
||||
</>
|
||||
) : (
|
||||
// The server keeps no copy of this link's address it can read: the link still opens for
|
||||
// whoever has it, but it cannot be shared again from here.
|
||||
<span className="flex-1 min-w-0 text-xs text-muted">
|
||||
{t("downloads.share.urlHidden")}
|
||||
</span>
|
||||
)}
|
||||
{/* All three on the same rung. Only Copy was migrated at first, which left one 2rem button
|
||||
between two 1.75rem ones with a different radius and hover fill — and, on a coarse
|
||||
pointer, `.touch-target` grew ONLY the migrated one to 2.75rem while the destructive
|
||||
|
||||
@@ -33,5 +33,8 @@
|
||||
"file_folder_retention": "Folder retention changed",
|
||||
"file_folder_delete": "Folder deleted",
|
||||
"api_token_create": "API token created",
|
||||
"api_token_revoke": "API token revoked"
|
||||
"api_token_revoke": "API token revoked",
|
||||
"download_link_create": "Share link created",
|
||||
"download_link_update": "Share link changed",
|
||||
"download_link_revoke": "Share link revoked"
|
||||
}
|
||||
|
||||
@@ -105,6 +105,7 @@
|
||||
"password": "Password (optional)",
|
||||
"copy": "Copy link",
|
||||
"copyFailed": "Couldn’t copy",
|
||||
"urlHidden": "This link still opens, but its address can no longer be shown. To share the file again, make a new link.",
|
||||
"revoke": "Revoke",
|
||||
"revokeConfirm": "This link will stop working immediately.",
|
||||
"views_one": "{{count}} view",
|
||||
|
||||
@@ -33,5 +33,8 @@
|
||||
"file_folder_retention": "Mappa megőrzési ideje módosítva",
|
||||
"file_folder_delete": "Mappa törölve",
|
||||
"api_token_create": "API-token létrehozva",
|
||||
"api_token_revoke": "API-token visszavonva"
|
||||
"api_token_revoke": "API-token visszavonva",
|
||||
"download_link_create": "Megosztási link létrehozva",
|
||||
"download_link_update": "Megosztási link módosítva",
|
||||
"download_link_revoke": "Megosztási link visszavonva"
|
||||
}
|
||||
|
||||
@@ -105,6 +105,7 @@
|
||||
"password": "Jelszó (opcionális)",
|
||||
"copy": "Link másolása",
|
||||
"copyFailed": "Nem sikerült másolni",
|
||||
"urlHidden": "Ez a link továbbra is működik, de a címe már nem jeleníthető meg. Ha újra meg akarod osztani a fájlt, készíts új linket.",
|
||||
"revoke": "Visszavonás",
|
||||
"revokeConfirm": "A link azonnal használhatatlanná válik.",
|
||||
"views_one": "{{count}} megtekintés",
|
||||
|
||||
@@ -140,8 +140,10 @@ export interface ShareRecipient {
|
||||
|
||||
export interface ShareLink {
|
||||
id: number;
|
||||
token: string;
|
||||
url: string; // path like "/watch/<token>" — prepend window.location.origin to share
|
||||
// Null when the server cannot show this link's address again (no encrypted copy its key reads): the
|
||||
// link still opens for whoever has it; to share it anew, make a new link.
|
||||
token: string | null;
|
||||
url: string | null; // path like "/watch/<token>" — prepend window.location.origin to share
|
||||
allow_download: boolean;
|
||||
has_password: boolean;
|
||||
expires_at: string | null;
|
||||
@@ -266,7 +268,7 @@ export const downloadsApi = {
|
||||
patch: { allow_download?: boolean; expires_days?: number | null; password?: string },
|
||||
): Promise<ShareLink> =>
|
||||
req(`/api/downloads/links/${linkId}`, { method: "PATCH", body: JSON.stringify(patch) }),
|
||||
revokeDownloadLink: (linkId: number): Promise<{ deleted: number }> =>
|
||||
revokeDownloadLink: (linkId: number): Promise<{ ok: true; deleted: number }> =>
|
||||
req(`/api/downloads/links/${linkId}`, { method: "DELETE" }),
|
||||
// The library player's resume checkpoint. A position too early or too close to the end clears it.
|
||||
saveDownloadProgress: (
|
||||
|
||||
@@ -19,6 +19,7 @@ vi.mock("./clipboard", () => ({ copyText }));
|
||||
vi.mock("./notifications", () => ({ notify }));
|
||||
|
||||
import { quickShareLink } from "./downloads";
|
||||
import { qk } from "./queryKeys";
|
||||
|
||||
const t = (k: string) => k;
|
||||
const origin = window.location.origin;
|
||||
@@ -71,6 +72,34 @@ describe("quickShareLink", () => {
|
||||
expect(firstToast().level).toBe("success");
|
||||
});
|
||||
|
||||
it("passes over a link whose address the server can no longer show", async () => {
|
||||
downloadLinks.mockResolvedValue([
|
||||
{ id: 4, url: null },
|
||||
{ id: 3, url: "/watch/older" },
|
||||
]);
|
||||
|
||||
const url = await quickShareLink(qc(), { id: 1 }, t);
|
||||
|
||||
expect(url).toBe(`${origin}/watch/older`);
|
||||
expect(createDownloadLink).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("mints a new link when none it has can be shown, keeping the old ones listed", async () => {
|
||||
const hidden = { id: 4, url: null };
|
||||
downloadLinks.mockResolvedValue([hidden]);
|
||||
createDownloadLink.mockResolvedValue({ id: 8, url: "/watch/new" });
|
||||
const client = qc();
|
||||
|
||||
const url = await quickShareLink(client, { id: 6 }, t);
|
||||
|
||||
expect(url).toBe(`${origin}/watch/new`);
|
||||
expect(copyText).toHaveBeenCalledWith(`${origin}/watch/new`);
|
||||
expect(client.getQueryData(qk.downloadLinks(6))).toEqual([
|
||||
{ id: 8, url: "/watch/new" },
|
||||
hidden,
|
||||
]);
|
||||
});
|
||||
|
||||
it("still toasts (warning) with the open action when the clipboard copy fails", async () => {
|
||||
downloadLinks.mockResolvedValue([{ id: 5, url: "/watch/x" }]);
|
||||
copyText.mockResolvedValue(false);
|
||||
|
||||
@@ -27,22 +27,25 @@ export async function quickShareLink(
|
||||
job: { id: number },
|
||||
t: (key: string) => string,
|
||||
): Promise<string | null> {
|
||||
let link;
|
||||
let path;
|
||||
try {
|
||||
const existing = await qc.fetchQuery({
|
||||
queryKey: qk.downloadLinks(job.id),
|
||||
queryFn: () => api.downloadLinks(job.id),
|
||||
});
|
||||
link = existing[0];
|
||||
if (!link) {
|
||||
link = await api.createDownloadLink(job.id, {}); // omit allow_download → backend default (true)
|
||||
qc.setQueryData(qk.downloadLinks(job.id), [link]);
|
||||
// The newest link whose address can still be shown; one that cannot is no use to copy.
|
||||
path = existing.find((l) => l.url)?.url;
|
||||
if (!path) {
|
||||
const made = await api.createDownloadLink(job.id, {}); // omit allow_download → backend default (true)
|
||||
qc.setQueryData(qk.downloadLinks(job.id), [made, ...existing]);
|
||||
path = made.url;
|
||||
}
|
||||
if (!path) throw new Error("the new link came back without its address");
|
||||
} catch (e) {
|
||||
notifyActionError(e, t("downloads.share.linkFailed"));
|
||||
return null;
|
||||
}
|
||||
const url = window.location.origin + link.url;
|
||||
const url = window.location.origin + path;
|
||||
const action = {
|
||||
label: t("downloads.share.openNewTab"),
|
||||
onClick: () => window.open(url, "_blank", "noopener,noreferrer"),
|
||||
|
||||
Reference in New Issue
Block a user