diff --git a/.env-template b/.env-template index ded184e..807d9be 100644 --- a/.env-template +++ b/.env-template @@ -25,16 +25,13 @@ SESSION_SECRET= # Empty = nobody can. ADMIN_USERS= -# --- Todo database -------------------------------------------------------- -# SQLite file for the todo group; the directory is created on first use. In Docker, mount a -# volume here or the todos vanish with the container. -# TODO_DB_PATH=data/todo.db - -# --- MCP OAuth database ----------------------------------------------------- -# SQLite file for the MCP server's OAuth state: registered clients, and the tokens (hashed) -# of every client a user has approved. In Docker, mount a volume here too, or every MCP client -# has to be approved again after a restart. BASE_URL (above) must be the address MCP clients use. -# OAUTH_DB_PATH=data/oauth.db +# --- Database ----------------------------------------------------------------- +# The one SQLite file the app keeps everything in: the todos, and the MCP server's OAuth state +# (registered clients, and the hashed tokens of every client a user has approved). The directory +# is created on first use. In Docker, mount a volume here or the todos vanish with the container +# and every MCP client has to be approved again after a restart. BASE_URL (above) must be the +# address MCP clients use. +# DB_PATH=data/tbd.db # --- Chatbot (LM Studio) ------------------------------------------------------ # The chat widget in the UI (the green square, bottom left) talks to an LM Studio server through diff --git a/.gitignore b/.gitignore index c3113a9..58f959c 100644 --- a/.gitignore +++ b/.gitignore @@ -8,7 +8,7 @@ resources/public/js # clj-kondo / clojure-lsp caches .clj-kondo .lsp -# Local SQLite databases (TODO_DB_PATH, OAUTH_DB_PATH) +# Local SQLite database (DB_PATH) data # Local secrets (copy .env-template to .env) .env diff --git a/README.md b/README.md index a97a762..b8e6470 100644 --- a/README.md +++ b/README.md @@ -90,10 +90,11 @@ and the [MCP tools](#mcp-server) both call, so they behave the same and are test tool only translates its request into a service call and the result into a response, and never touches the database. -Todos are stored in a local [SQLite](https://www.sqlite.org/) file, `data/todo.db` by default -(git-ignored), which is created and migrated on the first todo request. Set `TODO_DB_PATH` to put it -elsewhere. In Docker, mount a volume at that path (e.g. `docker run -v tbd-data:/app/data ...`), or the -todos are lost with the container. +Everything the app stores (todos, and the MCP OAuth state) lives in one local +[SQLite](https://www.sqlite.org/) file, `data/tbd.db` by default (git-ignored), which is created and +migrated on first use. Each group owns its own tables (`tbd.todo.db`, `tbd.oauth.db`); `tbd.sqlite` opens the +file and runs their migrations. Set `DB_PATH` to put it elsewhere. In Docker, mount a volume at that path +(e.g. `docker run -v tbd-data:/app/data ...`), or the todos and MCP approvals are lost with the container. Every id the app issues (a todo's, an OAuth grant's) is a UUID, never a number: a model that reads `7` as a todo's id mixes it up with a 7 in a title. (`owner-id` is the user's id in Gitea, which it issues, so it stays @@ -164,7 +165,7 @@ The consent page is what stops a stranger's client (anyone can register one) fro sending you a link: nothing is issued without your click. Redirect URIs must be `https`, `http` on `localhost`/`127.0.0.1`/`[::1]` (the port may vary), or an app's own scheme (`cursor://...`). -What's stored is in `OAUTH_DB_PATH`: registered clients, and for each approval the user and a *hash* of the +What's stored is in the app's database (`DB_PATH`): registered clients, and for each approval the user and a *hash* of the current access and refresh tokens. To cut a client off, delete its row from `grants`. Admin-ness isn't in the token: it is looked up in `ADMIN_USERS` on every request, so removing someone from the list takes effect at once. A Gitea account that is disabled keeps working until its refresh token expires (up to 30 @@ -188,8 +189,7 @@ login, **403** for a logged-in non-admin. With no allowlist, nobody is an admin. | `BASE_URL` | Public URL of this app; an `https://` one also marks the session cookie `Secure`. It is also the address of the MCP server: the OAuth issuer, and what MCP clients are told to use (`/mcp`) | | `ADMIN_USERS` | Comma-separated Gitea usernames (case-insensitive) allowed to use the admin API | | `SESSION_SECRET` | Signs and encrypts session cookies. Unset = a random key per start, so logins don't survive a restart | -| `TODO_DB_PATH` | SQLite file for the todo group (default `data/todo.db`) | -| `OAUTH_DB_PATH` | SQLite file for the MCP OAuth server: registered clients, codes and tokens (default `data/oauth.db`). Mount a volume for it in Docker too, or every MCP client has to be approved again after each restart | +| `DB_PATH` | The app's SQLite file: todos, and the MCP OAuth server's registered clients, codes and tokens (default `data/tbd.db`). Mount a volume for it in Docker, or todos are lost and every MCP client has to be approved again after each restart | | `LMSTUDIO_URL`, `LMSTUDIO_MODEL`, `LMSTUDIO_API_KEY`, `LMSTUDIO_TIMEOUT_SECONDS` | The LM Studio server behind the [chatbot](#chatbot). Only the URL is needed | | `CHAT_TRACE_FILE` | Optional. A file to append a JSON trace of every chatbot turn to, [with the model's full reasoning and replies](#watching-the-chatbot-think). Off by default | | `NREPL_PORT`, `NREPL_BIND` | Where the debug nREPL listens (default `7888` on `127.0.0.1`) | @@ -500,4 +500,5 @@ To add a new group, create `tbd..routes` with a `routes` fn that returns vector, then add it to the router in `src/tbd/core.clj`. To make a route admin-only, put `:admin? true` in its route data (admin allowlist), or `:login? true` (any logged-in user; the user is passed to the handler as `:user`). A group with storage keeps it in its own `tbd..db`, apart -from its business logic in `core.clj`. +from its business logic in `core.clj`, with a `migrate!` fn for its tables in the app's one database; +add it to the `sqlite/open` call in `make-router`. diff --git a/src/tbd/core.clj b/src/tbd/core.clj index a0b0c7c..3b097d7 100644 --- a/src/tbd/core.clj +++ b/src/tbd/core.clj @@ -27,6 +27,7 @@ [tbd.nrepl.routes :as nrepl-routes] [tbd.oauth.db :as oauth-db] [tbd.oauth.routes :as oauth-routes] + [tbd.sqlite :as sqlite] [tbd.todo.db :as todo-db] [tbd.todo.routes :as todo-routes])) @@ -70,10 +71,9 @@ ;; Each logical group (health, users, orders, ...) owns a `tbd..routes` ;; namespace that returns its own route subtree; they're concatenated here. -(defn make-router [{:keys [auth nrepl todo oauth webmcp chat]}] - (let [oauth-ds (delay (oauth-db/open (:db-path oauth))) - ;; Opened (and migrated) on first use, not when the app loads. - todo-ds (delay (todo-db/open (:db-path todo)))] +(defn make-router [{:keys [auth nrepl db webmcp chat]}] + ;; One database for every group's tables, opened (and migrated) on first use, not when the app loads. + (let [ds (delay (sqlite/open (:db-path db) todo-db/migrate! oauth-db/migrate!))] (ring/router [["/swagger.json" ;; The spec is JSON, whatever the API default is (Swagger UI can't read EDN). @@ -85,12 +85,12 @@ :handler (index-handler webmcp)}}] (health/routes) (math/routes) - (mcp-routes/routes auth oauth-ds todo-ds) - (oauth-routes/routes auth oauth-ds) + (mcp-routes/routes auth ds) + (oauth-routes/routes auth ds) (auth-routes/routes auth) (chat-routes/routes chat) (nrepl-routes/routes nrepl) - (todo-routes/routes todo-ds)] + (todo-routes/routes ds)] {:data {:muuntaja http/muuntaja :coercion malli/coercion ;; Handlers (and the auth/error middleware) return plain data; the muuntaja @@ -124,8 +124,7 @@ {:auth (auth/env-config) :nrepl (nrepl/env-config) :chat (chat/env-config) - :todo (todo-db/env-config) - :oauth (oauth-db/env-config) + :db (sqlite/env-config) :webmcp {:origin-trial-token (some-> (System/getenv "WEBMCP_ORIGIN_TRIAL_TOKEN") str/trim not-empty)}}) (def app (make-app config)) diff --git a/src/tbd/mcp/routes.clj b/src/tbd/mcp/routes.clj index 8fe624c..ea358b3 100644 --- a/src/tbd/mcp/routes.clj +++ b/src/tbd/mcp/routes.clj @@ -18,9 +18,9 @@ (defn make-handler "A Ring handler speaking MCP over Streamable HTTP, for callers with a valid access token - (`auth` is the auth config, `ds` a delay of the OAuth database, `todo-ds` one of the todo database). It keeps its MCP sessions in + (`auth` is the auth config, `ds` a delay of the app's database). It keeps its MCP sessions in memory, so each handler is its own server: sessions don't outlive it, and aren't shared between nodes." - [auth ds todo-ds] + [auth ds] (let [{:keys [runtime jsonrpc-handler] :as options} (mcp-server/make-server-options {:info (es/make-info "tbd" "1.0.0" "tbd MCP server") @@ -29,14 +29,14 @@ :traffic-logger traffic-logger/nop-traffic-logger})] (-> (mcp-http/make-ring-handler runtime jsonrpc-handler options) ;; What the tools that touch todos need besides the caller (see tbd.mcp.tools). - (wrap-runtime {tools/todo-ds-key todo-ds}) + (wrap-runtime {tools/ds-key ds}) ;; Jetty hands over the body as a stream; plumcp reads the message through `:on-msg`. mcp-http/wrap-request-body-reader ;; Outermost, so a request without a token is turned away before its body is read. (oauth-mw/wrap-bearer-auth auth ds)))) -(defn routes [auth ds todo-ds] - (let [handler (make-handler auth ds todo-ds)] +(defn routes [auth ds] + (let [handler (make-handler auth ds)] ["/mcp" ;; plumcp does its own content negotiation, JSON parsing and (via the wrapper above) body ;; reading, so it must not sit behind the API's muuntaja/coercion middleware chain. ^:replace diff --git a/src/tbd/mcp/tools.clj b/src/tbd/mcp/tools.clj index c66c62c..6d111ad 100644 --- a/src/tbd/mcp/tools.clj +++ b/src/tbd/mcp/tools.clj @@ -18,10 +18,10 @@ [tbd.todo.schema :as todo-schema] [tbd.todo.service :as todo])) -(def todo-ds-key - "Where the todo database (a delay, as for the HTTP routes) is in the request's plumcp runtime - bag; tbd.mcp.routes puts it there." - :tbd/todo-ds) +(def ds-key + "Where the database (a delay, as for the HTTP routes) is in the request's plumcp runtime bag; + tbd.mcp.routes puts it there." + :tbd/ds) (defn- current-user "Who is calling: {:login :id :admin? :client-id}, the user whose access token the request @@ -39,7 +39,7 @@ (if error (throw (ex-info error {})) ok)) ;; Todos belong to the caller, and tbd.todo.service scopes every operation to them. -(defn- todo-ds [ctx] (ctx-get ctx todo-ds-key)) +(defn- ds [ctx] (ctx-get ctx ds-key)) (def primitives {:tools @@ -55,20 +55,20 @@ (format "%s (Gitea user %d)%s" login id (if admin? ", admin" ""))))) (tool "list_todos" "Lists your todos (JSON: id, title, done, created-at, updated-at), oldest first" [:map] - (fn [_ ctx] (->json (ok! (todo/list-todos (todo-ds ctx) (current-user ctx)))))) + (fn [_ ctx] (->json (ok! (todo/list-todos (ds ctx) (current-user ctx)))))) (tool "get_todo" "Gets one of your todos by id (JSON)" todo-schema/IdParam - (fn [{:keys [id]} ctx] (->json (ok! (todo/find-todo (todo-ds ctx) (current-user ctx) id))))) + (fn [{:keys [id]} ctx] (->json (ok! (todo/find-todo (ds ctx) (current-user ctx) id))))) (tool "create_todo" "Creates a todo for you and returns it (JSON)" todo-schema/NewTodo - (fn [{:keys [title]} ctx] (->json (ok! (todo/create! (todo-ds ctx) (current-user ctx) title))))) + (fn [{:keys [title]} ctx] (->json (ok! (todo/create! (ds ctx) (current-user ctx) title))))) (tool "update_todo" "Changes the title and/or done flag of one of your todos and returns it (JSON)" todo-schema/UpdateTodo (fn [{:keys [id] :as input} ctx] - (->json (ok! (todo/update! (todo-ds ctx) (current-user ctx) id (dissoc input :id)))))) + (->json (ok! (todo/update! (ds ctx) (current-user ctx) id (dissoc input :id)))))) (tool "delete_todo" "Deletes one of your todos" todo-schema/IdParam (fn [{:keys [id]} ctx] - (ok! (todo/delete! (todo-ds ctx) (current-user ctx) id)) + (ok! (todo/delete! (ds ctx) (current-user ctx) id)) "deleted"))]}) diff --git a/src/tbd/oauth/db.clj b/src/tbd/oauth/db.clj index c36b31b..d12ea1d 100644 --- a/src/tbd/oauth/db.clj +++ b/src/tbd/oauth/db.clj @@ -1,23 +1,12 @@ (ns tbd.oauth.db - "SQLite persistence for the MCP OAuth server, in a local file. Storage only: what is valid, and + "SQLite persistence for the MCP OAuth server, in the app's database (see tbd.sqlite). Storage only: what is valid, and when, lives in tbd.oauth.core and tbd.oauth.routes. Codes and tokens are stored as hashes (see tbd.oauth.core/hash-token), so callers pass hashes here, never the secrets themselves." (:require [clojure.edn :as edn] - [clojure.java.io :as io] - [clojure.string :as str] [next.jdbc :as jdbc] [next.jdbc.result-set :as rs] [tbd.sqlite :as sqlite])) -(def default-path "data/oauth.db") - -(defn env-config - "OAUTH_DB_PATH: where the SQLite file lives (default data/oauth.db; its directory is created)." - ([] (env-config (System/getenv))) - ([env] - (let [path (get env "OAUTH_DB_PATH")] - {:db-path (if (str/blank? path) default-path (str/trim path))}))) - ;; A user's approval of a client: the current access and refresh token (hashed), replaced ;; together when the refresh token is used. Deleting the row revokes the approval. `id` is a ;; UUID (see tbd.sqlite/new-id); `user_id` is the user's id in Gitea, which the app doesn't issue. @@ -34,7 +23,9 @@ refresh_expires_at INTEGER NOT NULL, created_at INTEGER NOT NULL)") -(defn- migrate! [ds] +(defn migrate! + "Creates the OAuth tables in the database `ds`, or brings them up to date." + [ds] ;; Clients register themselves (RFC 7591), so a row here says nothing about who the user is. (jdbc/execute! ds ["CREATE TABLE IF NOT EXISTS clients ( id TEXT PRIMARY KEY, @@ -56,15 +47,10 @@ (jdbc/execute! ds ["CREATE INDEX IF NOT EXISTS grants_by_client ON grants (client_id)"])) (defn open - "A datasource for the database file at `path`, created and migrated if need be; connections - and locking work as in tbd.todo.db." + "A database with just the OAuth tables, at `path`; the app opens one with every group's tables + (see tbd.core), which this is for tests of the group on its own." [path] - (when (str/blank? path) - (throw (ex-info "no OAuth database path configured (set OAUTH_DB_PATH)" {}))) - (some-> (io/file path) .getAbsoluteFile .getParentFile .mkdirs) - (doto (jdbc/get-datasource {:dbtype "sqlite" :dbname path - :journal_mode "WAL" :busy_timeout 5000}) - migrate!)) + (sqlite/open path migrate!)) (def ^:private opts {:builder-fn rs/as-unqualified-kebab-maps}) diff --git a/src/tbd/oauth/routes.clj b/src/tbd/oauth/routes.clj index 9ac63c9..968f433 100644 --- a/src/tbd/oauth/routes.clj +++ b/src/tbd/oauth/routes.clj @@ -303,7 +303,7 @@ (token-error "unsupported_grant_type" "grant_type must be authorization_code or refresh_token")))) (defn routes - "`ds` is a delay of the datasource (see tbd.oauth.db/open), so the database isn't touched until needed." + "`ds` is a delay of the datasource (see tbd.sqlite/open), so the database isn't touched until needed." [config ds] (let [guarded #(only-when-configured config %)] ["" {:no-doc true diff --git a/src/tbd/sqlite.clj b/src/tbd/sqlite.clj index c669057..03be4c8 100644 --- a/src/tbd/sqlite.clj +++ b/src/tbd/sqlite.clj @@ -1,10 +1,35 @@ (ns tbd.sqlite - "What the app's SQLite databases (tbd.todo.db, tbd.oauth.db) share: the ids of the rows they - own, and moving a table off the integer ids it used to have." - (:require [clojure.string :as str] + "The app's one SQLite database, which every group with storage (tbd.todo.db, tbd.oauth.db) + keeps its tables in: where it lives, opening it, the ids of the rows the groups own, and + moving a table off the integer ids it used to have." + (:require [clojure.java.io :as io] + [clojure.string :as str] [next.jdbc :as jdbc] [next.jdbc.result-set :as rs])) +(def default-path "data/tbd.db") + +(defn env-config + "DB_PATH: where the SQLite file lives (default data/tbd.db; its directory is created)." + ([] (env-config (System/getenv))) + ([env] + (let [path (get env "DB_PATH")] + {:db-path (if (str/blank? path) default-path (str/trim path))}))) + +(defn open + "A datasource for the database file at `path`, created if need be and migrated by each of + `migrations` (fns of the datasource, each creating and updating a group's own tables). Each + query opens its own connection (cheap for a local file); WAL mode plus a busy timeout let + concurrent requests wait on each other's writes instead of failing." + [path & migrations] + (when (str/blank? path) + (throw (ex-info "no database path configured (set DB_PATH)" {}))) + (some-> (io/file path) .getAbsoluteFile .getParentFile .mkdirs) + (let [ds (jdbc/get-datasource {:dbtype "sqlite" :dbname path + :journal_mode "WAL" :busy_timeout 5000})] + (run! #(% ds) migrations) + ds)) + (defn new-id "An id for a row this app owns: a random UUID, as lowercase text. Never an integer: an LLM that reads `7` as a todo's id confuses it with a 7 in a title, and a UUID is nothing else." diff --git a/src/tbd/todo/db.clj b/src/tbd/todo/db.clj index 83efd34..f19d5f9 100644 --- a/src/tbd/todo/db.clj +++ b/src/tbd/todo/db.clj @@ -1,22 +1,11 @@ (ns tbd.todo.db - "SQLite persistence for the todo group, in a local file. Storage only: validation and other + "SQLite persistence for the todo group, in the app's database (see tbd.sqlite). Storage only: validation and other rules live in tbd.todo.core. Every query takes an `owner-id` and filters on it, so one user's todos can't be read or changed through another user's ids." - (:require [clojure.java.io :as io] - [clojure.string :as str] - [next.jdbc :as jdbc] + (:require [next.jdbc :as jdbc] [next.jdbc.result-set :as rs] [tbd.sqlite :as sqlite])) -(def default-path "data/todo.db") - -(defn env-config - "TODO_DB_PATH: where the SQLite file lives (default data/todo.db; its directory is created)." - ([] (env-config (System/getenv))) - ([env] - (let [path (get env "TODO_DB_PATH")] - {:db-path (if (str/blank? path) default-path (str/trim path))}))) - (def ^:private create-todos ;; `id` is a UUID (see tbd.sqlite/new-id); `owner_id` is the user's id in Gitea, which the app ;; doesn't issue, so it stays the integer Gitea gave. @@ -28,22 +17,18 @@ created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL)") -(defn- migrate! [ds] +(defn migrate! + "Creates the todo tables in the database `ds`, or brings them up to date." + [ds] (sqlite/migrate-integer-id! ds "todos" create-todos) (jdbc/execute! ds [create-todos]) (jdbc/execute! ds ["CREATE INDEX IF NOT EXISTS todos_by_owner ON todos (owner_id, created_at)"])) (defn open - "A datasource for the database file at `path`, created and migrated if need be. Each query - opens its own connection (cheap for a local file); WAL mode plus a busy timeout let - concurrent requests wait on each other's writes instead of failing." + "A database with just the todo tables, at `path`; the app opens one with every group's tables + (see tbd.core), which this is for tests of the group on its own." [path] - (when (str/blank? path) - (throw (ex-info "no todo database path configured (set TODO_DB_PATH)" {}))) - (some-> (io/file path) .getAbsoluteFile .getParentFile .mkdirs) - (doto (jdbc/get-datasource {:dbtype "sqlite" :dbname path - :journal_mode "WAL" :busy_timeout 5000}) - migrate!)) + (sqlite/open path migrate!)) (def ^:private opts {:builder-fn rs/as-unqualified-kebab-maps}) diff --git a/src/tbd/todo/routes.clj b/src/tbd/todo/routes.clj index 51a8d51..dd5e13f 100644 --- a/src/tbd/todo/routes.clj +++ b/src/tbd/todo/routes.clj @@ -39,7 +39,7 @@ (reply 200 (fn [_] {:deleted true}) (service/delete! ds user id)))) (defn routes - "`ds` is a delay of the todo database (see tbd.todo.db/open), shared with the MCP tools." + "`ds` is a delay of the app's database (see tbd.sqlite/open), shared with the MCP tools." [ds] (let [id-param {:path schema/IdParam}] ["/api/todos" diff --git a/src/tbd/todo/service.clj b/src/tbd/todo/service.clj index cde156a..9365c3f 100644 --- a/src/tbd/todo/service.clj +++ b/src/tbd/todo/service.clj @@ -5,7 +5,7 @@ same on both and are written (and tested) once. A front end only translates: its request into arguments, and the result into a response. - `ds` is the delay of the todo database the app shares (opened on first use, so a caller who is + `ds` is the delay of the database the app shares (opened on first use, so a caller who is turned away earlier never opens it). `user` is the authenticated caller, {:id ...} at least (however they proved who they are: a session or an access token); a todo is only ever reached through its owner's id, and someone else's is reported exactly like a missing one. diff --git a/test/tbd/auth/routes_test.clj b/test/tbd/auth/routes_test.clj index a0709c1..96d786e 100644 --- a/test/tbd/auth/routes_test.clj +++ b/test/tbd/auth/routes_test.clj @@ -15,10 +15,9 @@ (str/ends-with? url "/api/v1/user") {:status 200 :body (format "{\"login\":\"%s\",\"id\":%d}" login id)})))) (defn make-app - "The app against a Gitea stub. `:todo`, `:oauth` and `:chat` (their configs) are passed through; other overrides go into the auth config." - [http & {:keys [todo oauth chat] :as overrides}] - (core/make-app {:todo todo - :oauth oauth + "The app against a Gitea stub. `:db` and `:chat` (their configs) are passed through; other overrides go into the auth config." + [http & {:keys [db chat] :as overrides}] + (core/make-app {:db db :chat chat :auth (merge {:base-url "http://tbd.test" :gitea-url "https://gitea.test" @@ -29,7 +28,7 @@ :session-secret "test-secret" :secure-cookies? false :http http} - (dissoc overrides :todo :oauth :chat)) + (dissoc overrides :db :chat)) :nrepl {:port 0 :bind "127.0.0.1"}})) (defn request [app method uri & {:keys [cookie query]}] diff --git a/test/tbd/http_test.clj b/test/tbd/http_test.clj index 046c7e6..5ca76e8 100644 --- a/test/tbd/http_test.clj +++ b/test/tbd/http_test.clj @@ -5,7 +5,7 @@ [tbd.core :as core] [tbd.http :as http] [tbd.test-util :as test-util] - [tbd.todo.db-test :as db-test]) + [tbd.sqlite-test :as sqlite-test]) (:import (java.io ByteArrayInputStream))) (defn call @@ -65,7 +65,7 @@ (is (= "application/edn; charset=utf-8" (content-type (call app :get "/api/todos" {})))))))) (deftest request-format-test - (let [app (t/make-app (t/gitea-stub "alice" 7) :todo {:db-path (db-test/temp-db-path)}) + (let [app (t/make-app (t/gitea-stub "alice" 7) :db {:db-path (sqlite-test/temp-db-path)}) cookie (t/login! app) post (fn [headers body] (call app :post "/api/todos" (assoc headers "cookie" cookie) body))] (testing "JSON in, EDN out by default" diff --git a/test/tbd/mcp/routes_test.clj b/test/tbd/mcp/routes_test.clj index eed9e64..5a16516 100644 --- a/test/tbd/mcp/routes_test.clj +++ b/test/tbd/mcp/routes_test.clj @@ -6,7 +6,7 @@ [malli.core :as malli] [tbd.oauth.core :as oauth] [tbd.oauth.db :as db] - [tbd.oauth.db-test :as db-test] + [tbd.sqlite-test :as sqlite-test] [tbd.oauth.routes-test :as o] [tbd.auth.routes-test :as t]) (:import (java.io ByteArrayInputStream))) @@ -80,9 +80,9 @@ (defn- fixture "An app on a fresh database, with the tokens of an admin (alice) and a regular user (bob)." [] - (let [path (db-test/temp-db-path)] + (let [path (sqlite-test/temp-db-path)] {:path path - :app (t/make-app (t/gitea-stub "alice" 7) :oauth {:db-path path} :todo {:db-path (db-test/temp-db-path)}) + :app (t/make-app (t/gitea-stub "alice" 7) :db {:db-path path}) :alice (mint-token! path "alice" 7) :bob (mint-token! path "bob" 8)})) @@ -177,22 +177,22 @@ (is (= 401 (:status resp))))))) (deftest admin-follows-the-allowlist-test - (let [path (db-test/temp-db-path) + (let [path (sqlite-test/temp-db-path) token (mint-token! path "alice" 7) - app-with (fn [admins] (t/make-app (t/gitea-stub "alice") :oauth {:db-path path} :admin-users admins))] + app-with (fn [admins] (t/make-app (t/gitea-stub "alice") :db {:db-path path} :admin-users admins))] (is (= "alice (Gitea user 7), admin" (text (call-tool (app-with #{"alice"}) token (initialize (app-with #{"alice"}) token) "whoami" {})))) (testing "it is decided per request, not when the token was issued" (let [app (app-with #{})] (is (= "alice (Gitea user 7)" (text (call-tool app token (initialize app token) "whoami" {})))))))) (deftest unconfigured-test - (let [app (t/make-app (t/gitea-stub "alice") :oauth {:db-path (db-test/temp-db-path)} :gitea-url nil) + (let [app (t/make-app (t/gitea-stub "alice") :db {:db-path (sqlite-test/temp-db-path)} :gitea-url nil) resp (rpc app "anything" {:jsonrpc "2.0" :id 1 :method "initialize"})] (is (= 503 (:status resp)) "with no login there is no way to get a token, so /mcp isn't open instead"))) (deftest end-to-end-test (testing "a client that does the whole OAuth dance ends up as the user who approved it" - (let [app (o/app-for (db-test/temp-db-path) "carol" 9) + (let [app (o/app-for (sqlite-test/temp-db-path) "carol" 9) {:keys [client-id tokens]} (o/get-tokens! app) whoami (fn [access-token] (text (call-tool app access-token (initialize app access-token) "whoami" {})))] (is (= "carol (Gitea user 9)" (whoami (:access_token tokens)))) diff --git a/test/tbd/oauth/db_test.clj b/test/tbd/oauth/db_test.clj index e2be5e4..954ac75 100644 --- a/test/tbd/oauth/db_test.clj +++ b/test/tbd/oauth/db_test.clj @@ -2,12 +2,8 @@ (:require [clojure.java.io :as io] [clojure.test :refer [deftest is testing]] [next.jdbc :as jdbc] - [tbd.oauth.db :as db]) - (:import (java.nio.file Files) - (java.nio.file.attribute FileAttribute))) - -(defn temp-db-path [] - (str (Files/createTempDirectory "tbd-oauth" (make-array FileAttribute 0)) "/data/oauth.db")) + [tbd.oauth.db :as db] + [tbd.sqlite-test :refer [temp-db-path]])) (defn client [id created-at] {:id id :name "Test client" :redirect-uris ["https://c.example.com/cb" "http://localhost/cb"] :created-at created-at}) @@ -18,11 +14,6 @@ :refresh-hash (str "refresh-" n) :refresh-expires-at 9000 :created-at 1000}) -(deftest env-config-test - (is (= {:db-path "data/oauth.db"} (db/env-config {}))) - (is (= {:db-path "data/oauth.db"} (db/env-config {"OAUTH_DB_PATH" " "}))) - (is (= {:db-path "/var/oauth.db"} (db/env-config {"OAUTH_DB_PATH" " /var/oauth.db "})))) - (deftest open-test (testing "creates the file and its directory" (let [path (temp-db-path)] diff --git a/test/tbd/oauth/routes_test.clj b/test/tbd/oauth/routes_test.clj index 09ceff7..92f2b88 100644 --- a/test/tbd/oauth/routes_test.clj +++ b/test/tbd/oauth/routes_test.clj @@ -6,7 +6,7 @@ [tbd.auth.routes-test :as t] [tbd.oauth.core :as oauth] [tbd.oauth.db :as db] - [tbd.oauth.db-test :as db-test] + [tbd.sqlite-test :as sqlite-test] [tbd.test-util :as test-util]) (:import (java.io ByteArrayInputStream) (java.net URLDecoder))) @@ -23,7 +23,7 @@ (defn app-for "An app whose OAuth database is `path`; the Gitea user is alice (id 7), an admin, unless told otherwise." ([path] (app-for path "alice" 7)) - ([path login id] (t/make-app (t/gitea-stub login id) :oauth {:db-path path}))) + ([path login id] (t/make-app (t/gitea-stub login id) :db {:db-path path}))) (defn call "Runs a request through `app`. `:form` (a map) is sent as a urlencoded body, `:body` (a string) as JSON." @@ -94,7 +94,7 @@ {:client-id client-id :cookie cookie :tokens (:body (token! app client-id code))})) (deftest discovery-test - (let [app (app-for (db-test/temp-db-path))] + (let [app (app-for (sqlite-test/temp-db-path))] (testing "protected resource metadata, at the root and at the resource's own path" (doseq [uri ["/.well-known/oauth-protected-resource" "/.well-known/oauth-protected-resource/mcp"]] (let [resp (call app :get uri)] @@ -112,8 +112,8 @@ [:headers "Content-Type"])))))) (deftest unconfigured-test - (let [path (db-test/temp-db-path) - app (t/make-app (t/gitea-stub "alice") :oauth {:db-path path} :gitea-url nil)] + (let [path (sqlite-test/temp-db-path) + app (t/make-app (t/gitea-stub "alice") :db {:db-path path} :gitea-url nil)] (doseq [[method uri] [[:get "/.well-known/oauth-protected-resource"] [:get "/.well-known/oauth-authorization-server"] [:post "/oauth/register"] @@ -123,7 +123,7 @@ (is (= 503 (:status (call app method uri))) (str method " " uri))))) (deftest register-test - (let [path (db-test/temp-db-path) + (let [path (sqlite-test/temp-db-path) app (app-for path)] (testing "a client is registered as a public client" (let [resp (call app :post "/oauth/register" @@ -168,7 +168,7 @@ (is (= error (get-in resp [:body :error])) why)))))) (deftest authorize-request-errors-test - (let [app (app-for (db-test/temp-db-path)) + (let [app (app-for (sqlite-test/temp-db-path)) client-id (register! app) cookie (t/login! app) open (fn [params] (call app :get "/oauth/authorize" :cookie cookie :query (auth/form-encode params)))] @@ -206,7 +206,7 @@ :query (str "client_id=" client-id "&client_id=" client-id)))))))) (deftest login-then-consent-test - (let [app (app-for (db-test/temp-db-path)) + (let [app (app-for (sqlite-test/temp-db-path)) client-id (register! app) query (auth/form-encode (authorize-params client-id))] (testing "a logged-out user is sent to log in first, and back afterwards" @@ -224,7 +224,7 @@ (is (str/includes? (:body page) "Test client"))))))))) (deftest consent-page-test - (let [app (app-for (db-test/temp-db-path)) + (let [app (app-for (sqlite-test/temp-db-path)) client-id (register! app {:client_name ""}) {:keys [page csrf]} (consent! app (t/login! app) (authorize-params client-id))] (is (= 200 (:status page))) @@ -243,7 +243,7 @@ (is (= "no-store" (get-in page [:headers "Cache-Control"])))))) (deftest consent-decision-test - (let [app (app-for (db-test/temp-db-path)) + (let [app (app-for (sqlite-test/temp-db-path)) client-id (register! app) params (authorize-params client-id) cookie (t/login! app)] @@ -282,7 +282,7 @@ (is (nil? (get-in resp [:headers "Location"]))))))) (deftest code-exchange-test - (let [path (db-test/temp-db-path) + (let [path (sqlite-test/temp-db-path) app (app-for path) client-id (register! app) other-client-id (register! app) @@ -340,7 +340,7 @@ "the token endpoint takes forms, as RFC 6749 says")))) (deftest refresh-test - (let [path (db-test/temp-db-path) + (let [path (sqlite-test/temp-db-path) app (app-for path) {:keys [client-id tokens]} (get-tokens! app) refresh! (fn [refresh-token & {:as overrides}] diff --git a/test/tbd/sqlite_test.clj b/test/tbd/sqlite_test.clj new file mode 100644 index 0000000..8a59087 --- /dev/null +++ b/test/tbd/sqlite_test.clj @@ -0,0 +1,44 @@ +(ns tbd.sqlite-test + (:require [clojure.java.io :as io] + [clojure.test :refer [deftest is testing]] + [next.jdbc :as jdbc] + [tbd.oauth.db :as oauth-db] + [tbd.sqlite :as sqlite] + [tbd.todo.db :as todo-db]) + (:import (java.nio.file Files) + (java.nio.file.attribute FileAttribute))) + +(defn temp-db-path + "A path for a database file inside a fresh temp directory (which `open` has to create a + subdirectory of, as with the default data/tbd.db)." + [] + (str (Files/createTempDirectory "tbd" (make-array FileAttribute 0)) "/data/tbd.db")) + +(deftest env-config-test + (is (= {:db-path "data/tbd.db"} (sqlite/env-config {}))) + (is (= {:db-path "data/tbd.db"} (sqlite/env-config {"DB_PATH" " "}))) + (is (= {:db-path "/var/tbd.db"} (sqlite/env-config {"DB_PATH" " /var/tbd.db "})))) + +(deftest open-test + (testing "creates the file and its directory" + (let [path (temp-db-path)] + (sqlite/open path todo-db/migrate!) + (is (.exists (io/file path))))) + (testing "no path is an error, not a database in the working directory" + (is (thrown? clojure.lang.ExceptionInfo (sqlite/open nil))) + (is (thrown? clojure.lang.ExceptionInfo (sqlite/open " ")))) + (testing "every migration runs on the datasource, and again (harmlessly) on reopening" + (let [path (temp-db-path) + tables #(set (map :name (jdbc/execute! % ["SELECT name FROM sqlite_master WHERE type = 'table'"] + {:builder-fn next.jdbc.result-set/as-unqualified-maps})))] + (is (every? (tables (sqlite/open path todo-db/migrate! oauth-db/migrate!)) + ["todos" "clients" "codes" "grants"])) + (is (every? (tables (sqlite/open path todo-db/migrate! oauth-db/migrate!)) + ["todos" "clients" "codes" "grants"])))) + (testing "the groups keep their data apart in the one file" + (let [path (temp-db-path) + ds (sqlite/open path todo-db/migrate! oauth-db/migrate!)] + (todo-db/insert! ds {:owner-id 7 :title "buy milk" :done false :created-at 1 :updated-at 1}) + (oauth-db/insert-client! ds {:id "c1" :name "Test client" :redirect-uris ["http://localhost/cb"] :created-at 1}) + (is (= ["buy milk"] (map :title (todo-db/list-todos ds 7)))) + (is (= "c1" (:id (oauth-db/find-client ds "c1"))))))) diff --git a/test/tbd/todo/db_test.clj b/test/tbd/todo/db_test.clj index 00b3041..f70c202 100644 --- a/test/tbd/todo/db_test.clj +++ b/test/tbd/todo/db_test.clj @@ -2,24 +2,12 @@ (:require [clojure.java.io :as io] [clojure.test :refer [deftest is testing]] [next.jdbc :as jdbc] - [tbd.todo.db :as db]) - (:import (java.nio.file Files) - (java.nio.file.attribute FileAttribute))) - -(defn temp-db-path - "A path for a database file inside a fresh temp directory (which `open` has to create a - subdirectory of, as with the default data/todo.db)." - [] - (str (Files/createTempDirectory "tbd-todo" (make-array FileAttribute 0)) "/data/todo.db")) + [tbd.sqlite-test :refer [temp-db-path]] + [tbd.todo.db :as db])) (defn todo [owner-id title] {:owner-id owner-id :title title :done false :created-at 1000 :updated-at 1000}) -(deftest env-config-test - (is (= {:db-path "data/todo.db"} (db/env-config {}))) - (is (= {:db-path "data/todo.db"} (db/env-config {"TODO_DB_PATH" " "}))) - (is (= {:db-path "/var/todo.db"} (db/env-config {"TODO_DB_PATH" " /var/todo.db "})))) - (deftest open-test (testing "creates the file and its directory" (let [path (temp-db-path)] diff --git a/test/tbd/todo/routes_test.clj b/test/tbd/todo/routes_test.clj index 1232f61..b2e0ef8 100644 --- a/test/tbd/todo/routes_test.clj +++ b/test/tbd/todo/routes_test.clj @@ -3,7 +3,7 @@ [clojure.test :refer [deftest is testing]] [tbd.auth.routes-test :as t] [tbd.test-util :as test-util] - [tbd.todo.db-test :as db-test]) + [tbd.sqlite-test :as sqlite-test]) (:import (java.io ByteArrayInputStream))) (defn request @@ -15,12 +15,12 @@ (test-util/decode-body resp))) (defn app-for [db-path login id] - (t/make-app (t/gitea-stub login id) :todo {:db-path db-path})) + (t/make-app (t/gitea-stub login id) :db {:db-path db-path})) (def ^:private todos "/api/todos") (deftest anonymous-test - (let [path (db-test/temp-db-path) + (let [path (sqlite-test/temp-db-path) app (app-for path "alice" 7)] (doseq [[method uri body] [[:get todos nil] [:post todos "{\"title\":\"x\"}"] @@ -34,7 +34,7 @@ (is (not (.exists (io/file path))) "the database isn't even opened"))) (deftest lifecycle-test - (let [app (app-for (db-test/temp-db-path) "alice" 7) + (let [app (app-for (sqlite-test/temp-db-path) "alice" 7) cookie (t/login! app)] (testing "starts empty" (let [resp (request app :get todos cookie)] @@ -65,7 +65,7 @@ (is (= ["walk dog"] (map :title (:todos (:body (request app :get todos cookie))))))))))) (deftest validation-test - (let [app (app-for (db-test/temp-db-path) "alice" 7) + (let [app (app-for (sqlite-test/temp-db-path) "alice" 7) cookie (t/login! app) created (:body (request app :post todos cookie "{\"title\":\"keep\"}")) one (str todos "/" (:id created))] @@ -86,7 +86,7 @@ (is (= [created] (:todos (:body (request app :get todos cookie)))) "nothing above changed the todo"))) (deftest per-user-scoping-test - (let [path (db-test/temp-db-path) + (let [path (sqlite-test/temp-db-path) alice-app (app-for path "alice" 7) bob-app (app-for path "bob" 8) alice (t/login! alice-app) diff --git a/test/tbd/todo/service_test.clj b/test/tbd/todo/service_test.clj index ac00e31..938622d 100644 --- a/test/tbd/todo/service_test.clj +++ b/test/tbd/todo/service_test.clj @@ -2,7 +2,7 @@ (:require [clojure.test :refer [deftest is testing]] [tbd.todo.core :as todo] [tbd.todo.db :as db] - [tbd.todo.db-test :as db-test] + [tbd.sqlite-test :as sqlite-test] [tbd.todo.service :as service])) ;; The rules every front end (HTTP routes, MCP tools) gets: ownership, not-found and validation. @@ -11,7 +11,7 @@ (def alice {:id 7 :login "alice"}) (def bob {:id 8 :login "bob"}) -(defn- fresh-ds [] (let [path (db-test/temp-db-path)] (delay (db/open path)))) +(defn- fresh-ds [] (let [path (sqlite-test/temp-db-path)] (delay (db/open path)))) (def ^:private not-found {:error "todo not found" :kind :not-found}) @@ -68,7 +68,7 @@ (deftest database-is-opened-on-use-test (testing "nothing opens the database until a service fn needs it" (let [opened (atom 0) - ds (delay (swap! opened inc) (db/open (db-test/temp-db-path)))] + ds (delay (swap! opened inc) (db/open (sqlite-test/temp-db-path)))] (is (zero? @opened)) (service/list-todos ds alice) (service/list-todos ds alice)