mirror of
https://github.com/simonw/datasette.git
synced 2026-07-09 17:14:35 +02:00
Return 401 for invalid or expired bearer tokens
Invalid dstok_ tokens - bad signature, malformed payload, expired, or presented while allow_signed_tokens is off - previously degraded the request to anonymous, so clients saw a 403 permission error or worse, a 200 with anonymous-visible data. Token handlers can now raise TokenInvalid for tokens they recognize but reject; Datasette responds with 401, the canonical JSON error body and a WWW-Authenticate: Bearer error="invalid_token" header, even when a valid cookie is also present. Bearer tokens no registered handler recognizes are still ignored, so authentication plugins with their own token formats keep working. TokenInvalid is exported from the datasette package for use by plugin token handlers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GrHZSypDfMnym1tM5XJAFZ
This commit is contained in:
parent
b2cdc81d34
commit
aaaffe45b8
11 changed files with 214 additions and 45 deletions
|
|
@ -1,7 +1,7 @@
|
|||
from datasette.permissions import Permission # noqa
|
||||
from datasette.version import __version_info__, __version__ # noqa
|
||||
from datasette.events import Event # noqa
|
||||
from datasette.tokens import TokenHandler, TokenRestrictions # noqa
|
||||
from datasette.tokens import TokenHandler, TokenInvalid, TokenRestrictions # noqa
|
||||
from datasette.utils.asgi import Forbidden, NotFound, Request, Response # noqa
|
||||
from datasette.utils import actor_matches_allow # noqa
|
||||
from datasette.views import Context # noqa
|
||||
|
|
|
|||
|
|
@ -111,7 +111,9 @@ from .utils import (
|
|||
baseconv,
|
||||
call_with_supported_arguments,
|
||||
detect_json1,
|
||||
add_cors_headers,
|
||||
display_actor,
|
||||
error_body,
|
||||
escape_css_string,
|
||||
escape_sqlite,
|
||||
find_spatialite,
|
||||
|
|
@ -130,6 +132,7 @@ from .utils import (
|
|||
redact_keys,
|
||||
row_sql_params_pks,
|
||||
)
|
||||
from .tokens import TokenInvalid
|
||||
from .utils.asgi import (
|
||||
AsgiLifespan,
|
||||
Forbidden,
|
||||
|
|
@ -905,7 +908,9 @@ class Datasette:
|
|||
Verify an API token by trying all registered token handlers.
|
||||
|
||||
Returns an actor dict from the first handler that recognizes the
|
||||
token, or None if no handler accepts it.
|
||||
token, or None if no handler accepts it. A handler may raise
|
||||
TokenInvalid for a token it recognizes but rejects (bad signature,
|
||||
expired) - Datasette turns that into a 401 response.
|
||||
"""
|
||||
for token_handler in self._token_handlers():
|
||||
result = await token_handler.verify_token(self, token)
|
||||
|
|
@ -2887,13 +2892,24 @@ class DatasetteRouter:
|
|||
# Handle authentication
|
||||
default_actor = scope.get("actor") or None
|
||||
actor = None
|
||||
token_error = None
|
||||
results = pm.hook.actor_from_request(datasette=self.ds, request=request)
|
||||
for result in results:
|
||||
result = await await_me_maybe(result)
|
||||
try:
|
||||
result = await await_me_maybe(result)
|
||||
except TokenInvalid as ex:
|
||||
# A presented token was recognized but rejected - fail the
|
||||
# request with a 401 even if another credential is valid,
|
||||
# but keep awaiting the remaining coroutines first
|
||||
if token_error is None:
|
||||
token_error = ex
|
||||
continue
|
||||
if result and actor is None:
|
||||
actor = result
|
||||
# Don't break — we must await all coroutines to avoid
|
||||
# "coroutine was never awaited" warnings
|
||||
if token_error is not None:
|
||||
return await self.handle_401(request, send, token_error)
|
||||
scope_modifications["actor"] = actor or default_actor
|
||||
scope = dict(scope, **scope_modifications)
|
||||
|
||||
|
|
@ -2925,6 +2941,17 @@ class DatasetteRouter:
|
|||
except Exception as exception:
|
||||
return await self.handle_exception(request, send, exception)
|
||||
|
||||
async def handle_401(self, request, send, exception):
|
||||
# A presented bearer token was recognized by a handler but rejected.
|
||||
# Bearer tokens are API credentials, so this is always JSON.
|
||||
headers = {"www-authenticate": 'Bearer error="invalid_token"'}
|
||||
if self.ds.cors:
|
||||
add_cors_headers(headers)
|
||||
response = Response.json(
|
||||
error_body([str(exception)], 401), status=401, headers=headers
|
||||
)
|
||||
await response.asgi_send(send)
|
||||
|
||||
async def handle_404(self, request, send, exception=None):
|
||||
# If path contains % encoding, redirect to tilde encoding
|
||||
if "%" in request.path:
|
||||
|
|
|
|||
|
|
@ -18,6 +18,21 @@ if TYPE_CHECKING:
|
|||
from datasette.app import Datasette
|
||||
|
||||
|
||||
class TokenInvalid(Exception):
|
||||
"""
|
||||
Raised by a TokenHandler when a token it recognizes is invalid -
|
||||
for example a bad signature, malformed payload or expired token.
|
||||
|
||||
Datasette responds to this with an HTTP 401 error. Handlers should
|
||||
return None instead for tokens they do not recognize at all, so that
|
||||
other registered handlers get a chance to verify them.
|
||||
"""
|
||||
|
||||
def __init__(self, message="Invalid token"):
|
||||
self.message = message
|
||||
super().__init__(message)
|
||||
|
||||
|
||||
@dataclasses.dataclass
|
||||
class TokenRestrictions:
|
||||
"""
|
||||
|
|
@ -108,8 +123,12 @@ class TokenHandler:
|
|||
|
||||
async def verify_token(self, datasette: "Datasette", token: str) -> Optional[dict]:
|
||||
"""
|
||||
Verify a token and return an actor dict, or None if this handler
|
||||
does not recognize the token.
|
||||
Verify a token and return an actor dict.
|
||||
|
||||
Return None if this handler does not recognize the token at all,
|
||||
so other handlers can try it. Raise TokenInvalid if the token is
|
||||
recognized but invalid (bad signature, malformed, expired) - the
|
||||
request will fail with a 401 error.
|
||||
"""
|
||||
raise NotImplementedError
|
||||
|
||||
|
|
@ -147,29 +166,32 @@ class SignedTokenHandler(TokenHandler):
|
|||
async def verify_token(self, datasette: "Datasette", token: str) -> Optional[dict]:
|
||||
prefix = "dstok_"
|
||||
|
||||
if not datasette.setting("allow_signed_tokens"):
|
||||
if not token.startswith(prefix):
|
||||
# Not one of our tokens - leave it for other handlers
|
||||
return None
|
||||
|
||||
if not datasette.setting("allow_signed_tokens"):
|
||||
raise TokenInvalid(
|
||||
"Signed tokens are not enabled for this Datasette instance"
|
||||
)
|
||||
|
||||
max_signed_tokens_ttl = datasette.setting("max_signed_tokens_ttl")
|
||||
|
||||
if not token.startswith(prefix):
|
||||
return None
|
||||
|
||||
raw = token[len(prefix) :]
|
||||
try:
|
||||
decoded = datasette.unsign(raw, namespace="token")
|
||||
except itsdangerous.BadSignature:
|
||||
return None
|
||||
raise TokenInvalid("Invalid token signature")
|
||||
|
||||
if "t" not in decoded:
|
||||
return None
|
||||
raise TokenInvalid("Invalid token: no timestamp")
|
||||
created = decoded["t"]
|
||||
if not isinstance(created, int):
|
||||
return None
|
||||
raise TokenInvalid("Invalid token: invalid timestamp")
|
||||
|
||||
duration = decoded.get("d")
|
||||
if duration is not None and not isinstance(duration, int):
|
||||
return None
|
||||
raise TokenInvalid("Invalid token: invalid duration")
|
||||
|
||||
if (duration is None and max_signed_tokens_ttl) or (
|
||||
duration is not None
|
||||
|
|
@ -180,7 +202,7 @@ class SignedTokenHandler(TokenHandler):
|
|||
|
||||
if duration:
|
||||
if time.time() - created > duration:
|
||||
return None
|
||||
raise TokenInvalid("Token has expired")
|
||||
|
||||
actor = {"id": decoded["a"], "token": "dstok"}
|
||||
|
||||
|
|
|
|||
|
|
@ -991,7 +991,9 @@ The ``/-/create-token`` page cannot be accessed by actors that are authenticated
|
|||
|
||||
Datasette plugins that implement their own form of API token authentication should follow this convention.
|
||||
|
||||
You can disable the signed token feature entirely using the :ref:`allow_signed_tokens <setting_allow_signed_tokens>` setting.
|
||||
If a request presents a token that a token handler recognizes but rejects - an invalid signature, a malformed payload or an expired token - Datasette responds with a ``401`` status, the :ref:`standard JSON error format <json_api_errors>` and a ``WWW-Authenticate: Bearer error="invalid_token"`` header. This means API clients can distinguish "your token needs to be renewed" (``401``) from "your token does not grant this permission" (``403``). A ``Bearer`` token that no registered handler recognizes at all is ignored, since it may be intended for an authentication plugin.
|
||||
|
||||
You can disable the signed token feature entirely using the :ref:`allow_signed_tokens <setting_allow_signed_tokens>` setting. Requests presenting a ``dstok_`` token while the feature is disabled receive a ``401``.
|
||||
|
||||
.. _authentication_cli_create_token:
|
||||
|
||||
|
|
|
|||
|
|
@ -2546,6 +2546,10 @@ The default ``SignedTokenHandler`` uses itsdangerous signed tokens (``dstok_`` p
|
|||
|
||||
async def verify_token(self, datasette, token):
|
||||
# Look up token in database, return actor dict or None
|
||||
# if this handler does not recognize the token. Raise
|
||||
# datasette.TokenInvalid for a token this handler
|
||||
# recognizes but rejects (revoked, expired) - Datasette
|
||||
# will respond with a 401 error.
|
||||
...
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1156,14 +1156,17 @@ registered via `register_token_handler`; the default is
|
|||
- **Format:** `dstok_` + itsdangerous-signed payload (namespace `token`)
|
||||
containing `a` (actor id), `t` (creation Unix time), optional `d`
|
||||
(duration seconds), optional `_r` (restrictions).
|
||||
- **Verification** returns no actor when: `allow_signed_tokens` is off, the
|
||||
signature is invalid, `t` is missing/non-integer, or the token is expired.
|
||||
The effective duration is `d` capped by `max_signed_tokens_ttl` (default 0
|
||||
= no cap; a non-zero setting also imposes a TTL on tokens without `d`).
|
||||
- **Verification:** a `dstok_`-prefixed token that fails verification —
|
||||
`allow_signed_tokens` off, invalid signature, missing/non-integer `t`,
|
||||
malformed `d`, or expired — raises `TokenInvalid`, and the request fails
|
||||
with **401**, the canonical error body and a
|
||||
`WWW-Authenticate: Bearer error="invalid_token"` header (even if a valid
|
||||
`ds_actor` cookie is also present). Tokens with prefixes no registered
|
||||
handler recognizes are ignored (they may belong to an auth plugin). The
|
||||
effective duration is `d` capped by `max_signed_tokens_ttl` (default 0 =
|
||||
no cap; a non-zero setting also imposes a TTL on tokens without `d`).
|
||||
- **Resulting actor:** `{"id": <a>, "token": "dstok"}` plus `"_r"` and
|
||||
`"token_expires"` when applicable. Invalid/expired tokens silently produce
|
||||
an anonymous request (no 401) — the failure then surfaces as a 403 from
|
||||
whatever permission check the request hits.
|
||||
`"token_expires"` when applicable.
|
||||
|
||||
**Restrictions (`_r`)** (default_permissions/restrictions.py):
|
||||
|
||||
|
|
|
|||
|
|
@ -96,19 +96,23 @@ request prefers JSON, mirroring `handle_exception`.
|
|||
completed, SQL is invalid" is defensible but should then not reuse the `ok`
|
||||
key — see §2.)
|
||||
|
||||
### 1c. Wrong-status outliers (P2)
|
||||
### 1c. Wrong-status outliers (P2) — ✅ IMPLEMENTED
|
||||
|
||||
- ~~Row **delete** write failures return **500** (views/row.py:757) while row
|
||||
**update** write failures return **400** (views/row.py:832-835). Same
|
||||
failure class, different status; pick 400 (or 409 for constraint
|
||||
violations) for both.~~ ✅ **Done** — delete now returns 400, matching
|
||||
update and the rest of the write API.
|
||||
- Invalid or expired bearer tokens silently degrade the request to anonymous,
|
||||
- ~~Invalid or expired bearer tokens silently degrade the request to anonymous,
|
||||
so clients see a 403 permission error (or worse, anonymous-permitted data)
|
||||
rather than a 401 (tokens.py:147-193). For 1.0, a malformed/expired
|
||||
`Authorization: Bearer dstok_...` header should produce **401** with a
|
||||
distinguishable error, so clients can tell "renew your token" apart from
|
||||
"you lack permission".
|
||||
"you lack permission".~~ ✅ **Done** — token handlers can raise
|
||||
`TokenInvalid`; Datasette responds 401 with the canonical body and a
|
||||
`WWW-Authenticate: Bearer error="invalid_token"` header. Unrecognized
|
||||
token prefixes still fall through to anonymous so auth plugins keep
|
||||
working.
|
||||
|
||||
---
|
||||
|
||||
|
|
@ -391,7 +395,8 @@ Two details make tiering urgent rather than optional:
|
|||
objects (§2).~~ ✅ Done.
|
||||
5. ~~Filter `/-/databases.json` by `view-database` or gate it behind
|
||||
`permissions-debug` (§6).~~ ✅ Done.
|
||||
6. 401 (not silent-anonymous) for invalid/expired bearer tokens (§1c).
|
||||
6. ~~401 (not silent-anonymous) for invalid/expired bearer tokens (§1c).~~
|
||||
✅ Done.
|
||||
7. Publish explicit stability tiers, including extras and pagination-token
|
||||
opacity (§9).
|
||||
8. Resolve the looks-like-a-bug list (§8), especially trusted-query delete
|
||||
|
|
|
|||
|
|
@ -202,8 +202,8 @@ async def test_insert_rows(ds_write, return_rows):
|
|||
"/data/docs/-/insert",
|
||||
{"rows": [{"title": "Test"} for i in range(10)]},
|
||||
"bad_token",
|
||||
403,
|
||||
["Permission denied"],
|
||||
401,
|
||||
["Invalid token signature"],
|
||||
),
|
||||
(
|
||||
"/data/docs/-/insert",
|
||||
|
|
@ -410,12 +410,13 @@ async def test_insert_or_upsert_row_errors(
|
|||
},
|
||||
)
|
||||
|
||||
actor_response = (
|
||||
await ds_write.client.get("/-/actor.json", headers=kwargs["headers"])
|
||||
).json()
|
||||
assert set((actor_response["actor"] or {}).get("_r", {}).get("a") or []) == set(
|
||||
token_permissions
|
||||
)
|
||||
if special_case != "bad_token":
|
||||
actor_response = (
|
||||
await ds_write.client.get("/-/actor.json", headers=kwargs["headers"])
|
||||
).json()
|
||||
assert set((actor_response["actor"] or {}).get("_r", {}).get("a") or []) == set(
|
||||
token_permissions
|
||||
)
|
||||
|
||||
if special_case == "invalid_json":
|
||||
del kwargs["json"]
|
||||
|
|
|
|||
|
|
@ -236,7 +236,9 @@ def test_auth_create_token(
|
|||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auth_create_token_not_allowed_for_tokens(ds_client):
|
||||
ds_tok = ds_client.ds.sign({"a": "test", "token": "dstok"}, "token")
|
||||
ds_tok = ds_client.ds.sign(
|
||||
{"a": "test", "token": "dstok", "t": int(time.time())}, "token"
|
||||
)
|
||||
response = await ds_client.get(
|
||||
"/-/create-token",
|
||||
headers={"Authorization": "Bearer dstok_{}".format(ds_tok)},
|
||||
|
|
@ -304,8 +306,16 @@ async def test_auth_with_dstok_token(ds_client, scenario, should_work):
|
|||
assert actor["token"] == "dstok"
|
||||
if scenario != "valid_unlimited_token":
|
||||
assert isinstance(actor["token_expires"], int)
|
||||
else:
|
||||
elif scenario == "no_token":
|
||||
# No credentials presented - request proceeds as anonymous
|
||||
assert response.json() == {"ok": True, "actor": None}
|
||||
else:
|
||||
# Invalid credentials presented - hard 401
|
||||
assert response.status_code == 401
|
||||
data = response.json()
|
||||
assert data["ok"] is False
|
||||
assert data["status"] == 401
|
||||
assert response.headers["www-authenticate"].startswith("Bearer")
|
||||
finally:
|
||||
ds_client.ds._settings["allow_signed_tokens"] = True
|
||||
|
||||
|
|
@ -339,8 +349,9 @@ def test_cli_create_token(app_client, expires):
|
|||
expected_actor["token_expires"] = details["t"] + expires
|
||||
assert response.json == {"ok": True, "actor": expected_actor}
|
||||
else:
|
||||
expected_actor = None
|
||||
assert response.json == {"ok": True, "actor": expected_actor}
|
||||
# Expired token - hard 401
|
||||
assert response.status == 401
|
||||
assert response.json["ok"] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ https://github.com/simonw/datasette/issues - 1.0 API consistency
|
|||
"""
|
||||
|
||||
import pytest
|
||||
import time
|
||||
from datasette.app import Datasette
|
||||
from datasette.utils import sqlite3
|
||||
|
||||
|
|
@ -395,3 +396,90 @@ async def test_row_delete_write_failure_is_400(tmp_path_factory):
|
|||
assert "deletes are blocked" in data["error"]
|
||||
finally:
|
||||
ds.close()
|
||||
|
||||
|
||||
# Invalid bearer tokens must produce 401, not silent anonymous access
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_expired_token_returns_401(ds_error_shape):
|
||||
token = "dstok_{}".format(
|
||||
ds_error_shape.sign(
|
||||
{"a": "root", "t": int(time.time()) - 2000, "d": 1000},
|
||||
namespace="token",
|
||||
)
|
||||
)
|
||||
response = await ds_error_shape.client.get(
|
||||
"/-/actor.json", headers={"Authorization": "Bearer {}".format(token)}
|
||||
)
|
||||
data = assert_canonical_error(response, 401)
|
||||
assert "expired" in data["error"].lower()
|
||||
assert response.headers["www-authenticate"].startswith("Bearer")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bad_signature_token_returns_401(ds_error_shape):
|
||||
response = await ds_error_shape.client.get(
|
||||
"/-/actor.json", headers={"Authorization": "Bearer dstok_garbage"}
|
||||
)
|
||||
data = assert_canonical_error(response, 401)
|
||||
assert response.headers["www-authenticate"].startswith("Bearer")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unrecognized_token_prefix_stays_anonymous(ds_error_shape):
|
||||
# No registered handler claims this token - it might belong to a
|
||||
# plugin's actor_from_request hook, so it must not hard-fail
|
||||
response = await ds_error_shape.client.get(
|
||||
"/-/actor.json", headers={"Authorization": "Bearer sometoken_abc"}
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"ok": True, "actor": None}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_valid_token_still_authenticates(ds_error_shape):
|
||||
token = "dstok_{}".format(
|
||||
ds_error_shape.sign(
|
||||
{"a": "root", "t": int(time.time())},
|
||||
namespace="token",
|
||||
)
|
||||
)
|
||||
response = await ds_error_shape.client.get(
|
||||
"/-/actor.json", headers={"Authorization": "Bearer {}".format(token)}
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json()["actor"]["id"] == "root"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bad_token_beats_valid_cookie(ds_error_shape):
|
||||
# A malformed Authorization header is a hard error even if a valid
|
||||
# ds_actor cookie is also present
|
||||
response = await ds_error_shape.client.get(
|
||||
"/-/actor.json",
|
||||
headers={"Authorization": "Bearer dstok_garbage"},
|
||||
cookies={"ds_actor": ds_error_shape.client.actor_cookie({"id": "root"})},
|
||||
)
|
||||
assert_canonical_error(response, 401)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_when_signed_tokens_disabled_returns_401(tmp_path_factory):
|
||||
db_directory = tmp_path_factory.mktemp("dbs")
|
||||
db_path = str(db_directory / "data.db")
|
||||
conn = sqlite3.connect(db_path)
|
||||
conn.execute("vacuum")
|
||||
conn.close()
|
||||
ds = Datasette([db_path], settings={"allow_signed_tokens": False})
|
||||
try:
|
||||
token = "dstok_{}".format(
|
||||
ds.sign({"a": "root", "t": int(time.time())}, namespace="token")
|
||||
)
|
||||
response = await ds.client.get(
|
||||
"/-/actor.json", headers={"Authorization": "Bearer {}".format(token)}
|
||||
)
|
||||
data = assert_canonical_error(response, 401)
|
||||
assert "not enabled" in data["error"]
|
||||
finally:
|
||||
ds.close()
|
||||
|
|
|
|||
|
|
@ -5,7 +5,12 @@ Tests for the register_token_handler plugin hook.
|
|||
from datasette.app import Datasette
|
||||
from datasette.hookspecs import hookimpl
|
||||
from datasette.plugins import pm
|
||||
from datasette.tokens import TokenHandler, TokenRestrictions, SignedTokenHandler
|
||||
from datasette.tokens import (
|
||||
TokenHandler,
|
||||
TokenInvalid,
|
||||
TokenRestrictions,
|
||||
SignedTokenHandler,
|
||||
)
|
||||
import pytest
|
||||
|
||||
|
||||
|
|
@ -66,10 +71,10 @@ async def test_verify_token_unknown_returns_none(datasette):
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_verify_token_bad_signature_returns_none(datasette):
|
||||
"""verify_token() should return None for tokens with bad signatures."""
|
||||
result = await datasette.verify_token("dstok_tampered_data_here")
|
||||
assert result is None
|
||||
async def test_verify_token_bad_signature_raises(datasette):
|
||||
"""verify_token() should raise TokenInvalid for tokens with bad signatures."""
|
||||
with pytest.raises(TokenInvalid):
|
||||
await datasette.verify_token("dstok_tampered_data_here")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -334,5 +339,6 @@ async def test_signed_tokens_disabled():
|
|||
ds = Datasette(settings={"allow_signed_tokens": False})
|
||||
with pytest.raises(ValueError, match="Signed tokens are not enabled"):
|
||||
await ds.create_token("test_actor", handler="signed")
|
||||
# verify_token should return None rather than raising
|
||||
assert await ds.verify_token("dstok_anything") is None
|
||||
# verify_token should raise TokenInvalid for a dstok_ token
|
||||
with pytest.raises(TokenInvalid, match="not enabled"):
|
||||
await ds.verify_token("dstok_anything")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue