Repository: simonw/datasette-auth-tokens (language: python)

Issue:
SQL configuration option to enable storing tokens in a database
Can use a similar pattern to https://github.com/simonw/datasette-permissions-sql
SQL configuration option to enable storing tokens in a database
Can use a similar pattern to https://github.com/simonw/datasette-permissions-sql
SQL configuration option to enable storing tokens in a database
Can use a similar pattern to https://github.com/simonw/datasette-permissions-sql


Discussion / hints:
The tricky detail here is to discourage people from accidentally introducing timing attacks.

If someone builds such that a token auth check ends up doing `select * from tokens where token = ?` they will have accidentally introduced a timing attack - a sophisticated attacker could time their requests and derive a working token from how long the queries are taking.

To protect against that, a token needs to look more like this:

`12314-aoeuaeocugaoethkatoenuhn`

Where `12314` is used to lookup the token in the database, and then the remainder is compared using `secrets.compare_digest`.
So i think I'll enforce the design of the tokens for this feature: you HAVE to have a token design where the bit before the first hyphen is the identifier and the bit afterwards is used for a `compare_digest` comparison.
So maybe you get to provide a SQL configuration query like this:
```sql
select shared_secret from tokens where id = :id
```
The plugin then splits off the `:id` part of the incoming token, runs that query with it, and does a `compare_digest` against the value that was returned from the query.

Needs to be a bit more involved than this though because I want to be able to set a `actor` dictionary based on the result of the query.

Maybe this:
```sql
select
  user_id as actor_id, 'bot' as actor_type, users.name as actor_name, shared_secret
from
  tokens join users on tokens.user_id = users.id
where
  token_id = :id
```
This would work tables like this:

**tokens**
| token_id | user_id | shared_secret |
|----------|---------|---------------|
| 441      | 223     | 32847oue2034  |

**users**
| id | name |
|----------|---------|
| 223      | simon     |

And would result in an `actor` dictionary like this:
```json
{
  "id": 223,
  "type": "bot",
  "name": "simon"
}
```
I don't like `shared_secret` as the column name here. I think `token_secret` is better.
Current configuration syntax looks like this:
```json
{
    "plugins": {
        "datasette-auth-tokens": [
            {
                "token": {
                    "$env": "BOT_TOKEN"
                },
                "actor": {
                    "bot_id": "my-bot"
                }
            }
        ]
    }
}
```
This is a problem, because the `"datasette-auth-tokens"` value is a list, not a dictionary - which means I can't add additional keys for things like SQL configuration.

I'm going to change it to a dictionary like this one:

```json
{
    "plugins": {
        "datasette-auth-tokens": {
            "query": {
                "sql": "select actor_id, token_secret from tokens where id = :token_id"
            },
            "tokens": [
                {
                    "token": {
                        "$env": "BOT_TOKEN"
                    },
                    "actor": {
                        "bot_id": "my-bot"
                    }
                }
            ]
        }
    }
}
```
The tricky detail here is to discourage people from accidentally introducing timing attacks.

If someone builds such that a token auth check ends up doing `select * from tokens where token = ?` they will have accidentally introduced a timing attack - a sophisticated attacker could time their requests and derive a working token from how long the queries are taking.

To protect against that, a token needs to look more like this:

`12314-aoeuaeocugaoethkatoenuhn`

Where `12314` is used to lookup the token in the database, and then the remainder is compared using `secrets.compare_digest`.
So i think I'll enforce the design of the tokens for this feature: you HAVE to have a token design where the bit before the first hyphen is the identifier and the bit afterwards is used for a `compare_digest` comparison.
So maybe you get to provide a SQL configuration query like this:
```sql
select shared_secret from tokens where id = :id
```
The plugin then splits off the `:id` part of the incoming token, runs that query with it, and does a `compare_digest` against the value that was returned from the query.

Needs to be a bit more involved than this though because I want to be able to set a `actor` dictionary based on the result of the query.

Maybe this:
```sql
select
  user_id as actor_id, 'bot' as actor_type, users.name as actor_name, shared_secret
from
  tokens join users on tokens.user_id = users.id
where
  token_id = :id
```
This would work tables like this:

**tokens**
| token_id | user_id | shared_secret |
|----------|---------|---------------|
| 441      | 223     | 32847oue2034  |

**users**
| id | name |
|----------|---------|
| 223      | simon     |

And would result in an `actor` dictionary like this:
```json
{
  "id": 223,
  "type": "bot",
  "name": "simon"
}
```
I don't like `shared_secret` as the column name here. I think `token_secret` is better.
Current configuration syntax looks like this:
```json
{
    "plugins": {
        "datasette-auth-tokens": [
            {
                "token": {
                    "$env": "BOT_TOKEN"
                },
                "actor": {
                    "bot_id": "my-bot"
                }
            }
        ]
    }
}
```
This is a problem, because the `"datasette-auth-tokens"` value is a list, not a dictionary - which means I can't add additional keys for things like SQL configuration.

I'm going to change it to a dictionary like this one:

```json
{
    "plugins": {
        "datasette-auth-tokens": {
            "query": {
                "sql": "select actor_id, token_secret from tokens where id = :token_id"
            },
            "tokens": [
                {
                    "token": {
                        "$env": "BOT_TOKEN"
                    },
                    "actor": {
                        "bot_id": "my-bot"
                    }
                }
            ]
        }
    }
}
```
The tricky detail here is to discourage people from accidentally introducing timing attacks.

If someone builds such that a token auth check ends up doing `select * from tokens where token = ?` they will have accidentally introduced a timing attack - a sophisticated attacker could time their requests and derive a working token from how long the queries are taking.

To protect against that, a token needs to look more like this:

`12314-aoeuaeocugaoethkatoenuhn`

Where `12314` is used to lookup the token in the database, and then the remainder is compared using `secrets.compare_digest`.
So i think I'll enforce the design of the tokens for this feature: you HAVE to have a token design where the bit before the first hyphen is the identifier and the bit afterwards is used for a `compare_digest` comparison.
So maybe you get to provide a SQL configuration query like this:
```sql
select shared_secret from tokens where id = :id
```
The plugin then splits off the `:id` part of the incoming token, runs that query with it, and does a `compare_digest` against the value that was returned from the query.

Needs to be a bit more involved than this though because I want to be able to set a `actor` dictionary based on the result of the query.

Maybe this:
```sql
select
  user_id as actor_id, 'bot' as actor_type, users.name as actor_name, shared_secret
from
  tokens join users on tokens.user_id = users.id
where
  token_id = :id
```
This would work tables like this:

**tokens**
| token_id | user_id | shared_secret |
|----------|---------|---------------|
| 441      | 223     | 32847oue2034  |

**users**
| id | name |
|----------|---------|
| 223      | simon     |

And would result in an `actor` dictionary like this:
```json
{
  "id": 223,
  "type": "bot",
  "name": "simon"
}
```
I don't like `shared_secret` as the column name here. I think `token_secret` is better.
Current configuration syntax looks like this:
```json
{
    "plugins": {
        "datasette-auth-tokens": [
            {
                "token": {
                    "$env": "BOT_TOKEN"
                },
                "actor": {
                    "bot_id": "my-bot"
                }
            }
        ]
    }
}
```
This is a problem, because the `"datasette-auth-tokens"` value is a list, not a dictionary - which means I can't add additional keys for things like SQL configuration.

I'm going to change it to a dictionary like this one:

```json
{
    "plugins": {
        "datasette-auth-tokens": {
            "query": {
                "sql": "select actor_id, token_secret from tokens where id = :token_id"
            },
            "tokens": [
                {
                    "token": {
                        "$env": "BOT_TOKEN"
                    },
                    "actor": {
                        "bot_id": "my-bot"
                    }
                }
            ]
        }
    }
}
```

Fix patch:
diff --git a/README.md b/README.md
index da60f96..2bc7b92 100644
--- a/README.md
+++ b/README.md
@@ -12,7 +12,7 @@ Install this plugin in the same environment as Datasette.
 
     $ pip install datasette-auth-tokens
 
-## Simple usage: a hard-coded list of tokens
+## Hard-coded tokens
 
 Read about Datasette's [authentication and permissions system](https://datasette.readthedocs.io/en/latest/authentication.html).
 
@@ -36,16 +36,18 @@ You can then use `"allow"` blocks to provide that token with permission to acces
 ```json
 {
     "plugins": {
-        "datasette-auth-tokens": [
-            {
-                "token": {
-                    "$env": "BOT_TOKEN"
-                },
-                "actor": {
-                    "bot_id": "my-bot"
+        "datasette-auth-tokens": {
+            "tokens": [
+                {
+                    "token": {
+                        "$env": "BOT_TOKEN"
+                    },
+                    "actor": {
+                        "bot_id": "my-bot"
+                    }
                 }
-            }
-        ]
+            ]
+        }
     },
     "databases": {
         ":memory:": {
@@ -60,7 +62,6 @@ You can then use `"allow"` blocks to provide that token with permission to acces
         }
     }
 }
-
 ```
 This uses Datasette's [secret configuration values mechanism](https://datasette.readthedocs.io/en/stable/plugins.html#secret-configuration-values) to allow the secret token to be passed as an environment variable.
 
@@ -74,3 +75,33 @@ You can now run authenticated API queries like this:
     $ curl -H 'Authorization: Bearer this-is-the-secret-token' \
       'http://127.0.0.1:8001/:memory:/show_version.json?_shape=array'
     [{"sqlite_version()": "3.31.1"}]
+
+## Tokens from your database
+
+As an alternative (or in addition) to the hard-coded list of tokens you can store tokens in a database table and configure the plugin to access them using a SQL query.
+
+Your query needs to take a `:token_id` parameter and return at least two columns: one called `token_secret` and one called `actor_*` - usually `actor_id`. Further `actor_` prefixed columns can be returned to provide more details for the authenticated actor.
+
+Here's a simple example of a configuration query:
+
+```sql
+select actor_id, actor_name, token_secret from tokens where token_id = :token_id
+```
+
+This can run against a table like this one:
+
+| token_id | token_secret | actor_id | actor_name |
+| -------- | ------------ | -------- | ---------- |
+| 1        | bd3c94f51fcd | 78       | Cleopaws   |
+| 2        | 86681b4d6f66 | 32       | Pancakes   |
+
+The tokens are formed as the token id, then a hyphen, then the token secret. For example:
+
+- `1-bd3c94f51fcd`
+- `2-86681b4d6f66`
+
+The SQL query will be executed with the portion before the hyphen as the `:token_id` parameter.
+
+The `token_secret` value returned by the query will be comepared to the portion of the token after the hyphen to check if the token is valid.
+
+Columns with a prefix of `actor_` will be used to populate the actor dictionary. In the above example, a token of `2-86681b4d6f66` will become an actor dictionary of `{"id": 32, "name": "Pancakes"}`.
diff --git a/datasette_auth_tokens/__init__.py b/datasette_auth_tokens/__init__.py
index 41ae678..4bf6bfc 100644
--- a/datasette_auth_tokens/__init__.py
+++ b/datasette_auth_tokens/__init__.py
@@ -4,13 +4,42 @@
 
 @hookimpl
 def actor_from_request(datasette, request):
-    allowed_tokens = datasette.plugin_config("datasette-auth-tokens") or []
-    authorization = request.headers.get("authorization")
-    if not authorization:
-        return None
-    if not authorization.startswith("Bearer "):
-        return None
-    incoming_token = authorization[len("Bearer ") :]
-    for token in allowed_tokens:
-        if secrets.compare_digest(token["token"], incoming_token):
-            return token["actor"]
+    async def inner():
+        config = datasette.plugin_config("datasette-auth-tokens") or {}
+        allowed_tokens = config.get("tokens") or []
+        authorization = request.headers.get("authorization")
+        if not authorization:
+            return None
+        if not authorization.startswith("Bearer "):
+            return None
+        incoming_token = authorization[len("Bearer ") :]
+        # First try hard-coded tokens in the list
+        for token in allowed_tokens:
+            if secrets.compare_digest(token["token"], incoming_token):
+                return token["actor"]
+        # Now try the SQL query, if present
+        query = config.get("query")
+        if query:
+            if "-" not in incoming_token:
+                # Invalid token
+                return None
+            token_id, token_secret = incoming_token.split("-", 2)
+            sql = query["sql"]
+            database = query.get("database")
+            db = datasette.get_database(database)
+            results = await db.execute(sql, {"token_id": token_id})
+            if not results:
+                return None
+            row = results.first()
+            assert (
+                "token_secret" in row.keys()
+            ), "Returned row must contain a token_secret"
+            if secrets.compare_digest(row["token_secret"], token_secret):
+                # Set actor based on actor_* columns
+                return {
+                    k.replace("actor_", ""): row[k]
+                    for k in row.keys()
+                    if k.startswith("actor_")
+                }
+
+    return inner
diff --git a/setup.py b/setup.py
index a233a48..b0a14c4 100644
--- a/setup.py
+++ b/setup.py
@@ -24,6 +24,6 @@ def get_long_description():
     packages=["datasette_auth_tokens"],
     entry_points={"datasette": ["auth_tokens = datasette_auth_tokens"]},
     install_requires=["datasette>=0.44",],
-    extras_require={"test": ["pytest", "pytest-asyncio", "httpx"]},
+    extras_require={"test": ["pytest", "pytest-asyncio", "httpx", "sqlite-utils"]},
     tests_require=["datasette-auth-tokens[test]"],
 )


Test patch:
diff --git a/tests/test_auth_tokens.py b/tests/test_auth_tokens.py
index 4aea0fc..aa80a6a 100644
--- a/tests/test_auth_tokens.py
+++ b/tests/test_auth_tokens.py
@@ -1,21 +1,49 @@
 from datasette.app import Datasette
 import httpx
 import pytest
+import sqlite_utils
 
 
 @pytest.fixture
-async def ds():
+async def ds(tmp_path_factory):
+    db_directory = tmp_path_factory.mktemp("dbs")
+    db_path1 = db_directory / "test1.db"
+    sqlite_utils.Database(db_path1)["foo"].insert({"bar": 1})
+    db_path2 = db_directory / "test2.db"
+    db = sqlite_utils.Database(db_path2)
+    db["tokens"].insert_all(
+        [
+            {
+                "id": 1,
+                "actor_id": "one",
+                "actor_name": "Cleo",
+                "token_secret": "oneone",
+            },
+            {
+                "id": 2,
+                "actor_id": "two",
+                "actor_name": "Pancakes",
+                "token_secret": "twotwo",
+            },
+        ],
+        pk="id",
+    )
     return Datasette(
-        [],
-        memory=True,
+        [db_path1, db_path2],
         metadata={
             "plugins": {
-                "datasette-auth-tokens": [
-                    {"token": "one", "actor": {"id": "one"}},
-                    {"token": "two", "actor": {"id": "two"}},
-                ]
+                "datasette-auth-tokens": {
+                    "query": {
+                        "sql": "select actor_id, actor_name, token_secret from tokens where id = :token_id",
+                        "database": "test2",
+                    },
+                    "tokens": [
+                        {"token": "one", "actor": {"id": "one"}},
+                        {"token": "two", "actor": {"id": "two"}},
+                    ],
+                }
             },
-            "databases": {":memory:": {"allow_sql": {"id": "one"},}},
+            "databases": {"test1": {"allow_sql": {"id": "one"},}},
         },
     )
     return ds
@@ -25,18 +53,60 @@ async def ds():
     "token,path,expected_status",
     [
         ("", "/", 200),
-        ("", "/:memory:?sql=select+1", 403),
+        ("", "/test1?sql=select+1", 403),
         ("one", "/", 200),
-        ("one", "/:memory:?sql=select+1", 200),
+        ("one", "/test1?sql=select+1", 200),
         ("two", "/", 200),
-        ("two", "/:memory:?sql=select+1", 403),
+        ("two", "/test1?sql=select+1", 403),
     ],
 )
 @pytest.mark.asyncio
-async def test_fallback(ds, token, path, expected_status):
+async def test_token(ds, token, path, expected_status):
     async with httpx.AsyncClient(app=ds.app()) as client:
         response = await client.get(
             "http://localhost{}".format(path),
             headers={"Authorization": "Bearer {}".format(token)},
         )
         assert expected_status == response.status_code
+
+
+@pytest.mark.parametrize(
+    "token,path,expected_status",
+    [
+        ("", "/", 200),
+        ("", "/test1?sql=select+1", 403),
+        ("1-oneone", "/", 200),
+        ("1-oneone", "/test1?sql=select+1", 200),
+        ("2-twotwo", "/", 200),
+        ("2-twotwo", "/test1?sql=select+1", 403),
+        ("invalid", "/", 200),
+        ("invalid", "/test1?sql=select+1", 403),
+    ],
+)
+@pytest.mark.asyncio
+async def test_query(ds, token, path, expected_status):
+    async with httpx.AsyncClient(app=ds.app()) as client:
+        response = await client.get(
+            "http://localhost{}".format(path),
+            headers={"Authorization": "Bearer {}".format(token)},
+        )
+        assert expected_status == response.status_code
+
+
+@pytest.mark.parametrize(
+    "token,expected_actor",
+    [
+        ("1-oneone", {"id": "one", "name": "Cleo"}),
+        ("2-twotwo", {"id": "two", "name": "Pancakes"}),
+        ("invalid", None),
+        ("invalid", None),
+    ],
+)
+@pytest.mark.asyncio
+async def test_actor(ds, token, expected_actor):
+    async with httpx.AsyncClient(app=ds.app()) as client:
+        response = await client.get(
+            "http://localhost/-/actor.json",
+            headers={"Authorization": "Bearer {}".format(token)},
+        )
+        assert {"actor": expected_actor} == response.json()