diff --git a/.dockerignore b/.dockerignore index cce76ad..b9b0f9d 100644 --- a/.dockerignore +++ b/.dockerignore @@ -5,3 +5,4 @@ node_modules .shadow-cljs resources/public/js .env +data diff --git a/.env-template b/.env-template index b665290..8c354cb 100644 --- a/.env-template +++ b/.env-template @@ -25,6 +25,11 @@ 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 + # --- nREPL debug server (POST /nrepl) ----------------------------------------- # nREPL has no authentication and evaluates arbitrary code, so it listens on loopback # only by default. Changing NREPL_BIND to 0.0.0.0 exposes it to the network: only do that diff --git a/.gitignore b/.gitignore index 846f5ce..536c805 100644 --- a/.gitignore +++ b/.gitignore @@ -5,5 +5,7 @@ node_modules # Compiled UI (script/ui-build) resources/public/js .nrepl-port +# Local SQLite database for the todo group (TODO_DB_PATH) +data # Local secrets (copy .env-template to .env) .env diff --git a/README.md b/README.md index 3747fad..9a765a1 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,11 @@ docker run --rm -p 3000:3000 tbd | `GET` | `/auth/callback` | OAuth redirect target: completes the login, sets the session cookie, redirects to `/` | | `POST` | `/auth/logout` | Ends the session | | `GET` | `/auth/me` | The logged-in user, `{:login "alice" :id 7 :admin? true}` (401 if not logged in) | +| `GET` | `/api/todos` | **Login required.** Your todos: `{:todos [{:id 1 :owner-id 7 :title "buy milk" :done false :created-at 1700000000000 :updated-at 1700000000000}]}` | +| `POST` | `/api/todos` | **Login required.** Body `{"title": "buy milk"}` creates a todo and returns it with `201` (400 if the title is blank or over 200 characters) | +| `GET` | `/api/todos/:id` | **Login required.** One todo (404 if it doesn't exist or isn't yours) | +| `PATCH` | `/api/todos/:id` | **Login required.** Body with `title` and/or `done` (`{"done": true}`) updates the todo and returns it (400 for an empty or invalid change, 404 as above) | +| `DELETE` | `/api/todos/:id` | **Login required.** Deletes the todo: `{:deleted true}` (404 as above) | | `GET` | `/nrepl` | **Admin only.** Whether the nREPL debug server is on: `{:enabled false}`, or `{:enabled true :bind "127.0.0.1" :port 7888}` | | `POST` | `/nrepl` | **Admin only.** Body `{"enabled": true}` (JSON or EDN) starts the nREPL debug server, `false` stops it; returns `{:enabled true :bind "127.0.0.1" :port 7888}` | @@ -57,6 +62,24 @@ curl localhost:3000/api/ping # pong curl 'localhost:3000/api/add?x=2&y=3' # {:result 5} ``` +## Todos + +Each logged-in user has their own todo list, keyed by their Gitea user id. Every query is +filtered by that id, so users can't see or change each other's todos; someone else's todo answers +`404`, exactly like a missing one. Without a login the todo routes answer `401`. + +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. + +```bash +# after logging in in a browser, copy the `tbd-session` cookie value: +curl -XPOST localhost:3000/api/todos -H 'Cookie: tbd-session=...' \ + -H 'Content-Type: application/json' -d '{"title": "buy milk"}' +curl localhost:3000/api/todos -H 'Cookie: tbd-session=...' +``` + ## Authentication and the admin API Login is OAuth2 (authorization-code flow) against a [Gitea](https://about.gitea.com/) instance. @@ -75,6 +98,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` | | `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`) | | `NREPL_PORT`, `NREPL_BIND` | Where the debug nREPL listens (default `7888` on `127.0.0.1`) | Sessions live in an encrypted, signed, `HttpOnly`, `SameSite=Lax` cookie and expire after 8 hours @@ -156,6 +180,7 @@ src/tbd/ /core.clj Business logic for the group, with no HTTP concerns auth/ Gitea OAuth (core), session + admin middleware (middleware), /auth routes nrepl/ On-demand nREPL server (core) and the admin-only `/nrepl` routes + todo/ Per-user todos: pure rules (core), SQLite storage (db), `/api/todos` routes ui/ ClojureScript re-frame app (shadow-cljs project); ui/src/tbd/ui/{db,events,subs,views,core}.cljs .env-template Settings the server reads from the environment; copy to .env (git-ignored) resources/public/ index.html (the shell served at /) and js/ (compiled UI output, git-ignored) @@ -166,4 +191,6 @@ script/ Bash entry points (server, ui-build, test, coverage, repl, To add a new group, create `tbd..routes` with a `routes` fn that returns a reitit route 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? 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`. diff --git a/deps.edn b/deps.edn index 50eaa58..c2ff839 100644 --- a/deps.edn +++ b/deps.edn @@ -5,7 +5,10 @@ ring/ring-defaults {:mvn/version "0.4.0"} metosin/reitit {:mvn/version "0.7.2"} ;; Runtime dep: POST /nrepl starts an nREPL server on demand (see tbd.nrepl.core). - nrepl/nrepl {:mvn/version "1.2.0"}} + nrepl/nrepl {:mvn/version "1.2.0"} + ;; The todo group's storage (see tbd.todo.db): a local SQLite file. + com.github.seancorfield/next.jdbc {:mvn/version "1.3.1002"} + org.xerial/sqlite-jdbc {:mvn/version "3.46.1.3"}} :aliases {:dev diff --git a/integration-test/integration_test.bb b/integration-test/integration_test.bb index c0b6388..8088504 100755 --- a/integration-test/integration_test.bb +++ b/integration-test/integration_test.bb @@ -157,6 +157,12 @@ (assert-eq "A query-string admin flag is no bypass" 401 (:status (post-json* (str base-url "/nrepl?admin=true") "{\"enabled\":true}"))) (println) + (println "Test 15: the todo API without a session returns 401") + (assert-eq "GET list" 401 (:status (get* (str base-url "/api/todos")))) + (assert-eq "POST create" 401 (:status (post-json* (str base-url "/api/todos") "{\"title\":\"x\"}"))) + (assert-eq "GET one" 401 (:status (get* (str base-url "/api/todos/1")))) + (println) + (println "================================") (println (format " Results: %d passed, %d failed" @pass @fail)) (println "================================") diff --git a/src/tbd/auth/middleware.clj b/src/tbd/auth/middleware.clj index 84df19e..6efd7c8 100644 --- a/src/tbd/auth/middleware.clj +++ b/src/tbd/auth/middleware.clj @@ -2,7 +2,7 @@ (:require [ring.middleware.session :refer [wrap-session]] [ring.middleware.session.cookie :refer [cookie-store]] [tbd.auth.core :as auth] - [tbd.http :refer [edn-response]])) + [tbd.http :refer [respond]])) (def session-ttl-ms (* 8 60 60 1000)) @@ -42,5 +42,18 @@ (if-let [user (current-user req)] (if (auth/admin? config (:login user)) (handler (assoc req :user user)) - (edn-response 403 {:error "admin access required"})) - (edn-response 401 {:error "login required"}))))))}) + (respond 403 {:error "admin access required"})) + (respond 401 {:error "login required"}))))))}) + +(def login-middleware + "Reitit middleware for routes whose data has `:login? true`: 401 without a login, otherwise + the user goes on as `:user` ({:login :id ...}). Like admin-middleware, mount it ahead of the + parsing/coercion middleware." + {:name ::login + :compile (fn [route-data _opts] + (when (:login? route-data) + (fn [handler] + (fn [req] + (if-let [user (current-user req)] + (handler (assoc req :user user)) + (respond 401 {:error "login required"}))))))}) diff --git a/src/tbd/auth/routes.clj b/src/tbd/auth/routes.clj index eb6f530..c61ee5b 100644 --- a/src/tbd/auth/routes.clj +++ b/src/tbd/auth/routes.clj @@ -2,10 +2,10 @@ (:require [ring.util.response :as response] [tbd.auth.core :as auth] [tbd.auth.middleware :as mw] - [tbd.http :refer [edn-response]])) + [tbd.http :refer [respond]])) (def ^:private not-configured - (edn-response 503 {:error "OAuth is not configured (set GITEA_URL, GITEA_CLIENT_ID, GITEA_CLIENT_SECRET, BASE_URL)"})) + (respond 503 {:error "OAuth is not configured (set GITEA_URL, GITEA_CLIENT_ID, GITEA_CLIENT_SECRET, BASE_URL)"})) (defn- login-handler [config] (fn [{:keys [session]}] @@ -20,9 +20,9 @@ (let [{:keys [code state error]} (:query parameters)] (cond (not (auth/configured? config)) not-configured - (not (auth/state-matches? (:oauth-state session) state)) (edn-response 400 {:error "invalid OAuth state"}) - error (edn-response 403 {:error "login was denied"}) - (nil? code) (edn-response 400 {:error "missing authorization code"}) + (not (auth/state-matches? (:oauth-state session) state)) (respond 400 {:error "invalid OAuth state"}) + error (respond 403 {:error "login was denied"}) + (nil? code) (respond 400 {:error "missing authorization code"}) :else (try (let [user (some->> (auth/exchange-code config code) (auth/fetch-user config))] @@ -31,16 +31,16 @@ (-> (response/redirect "/") (assoc :session {:user (assoc user :expires-at (+ (System/currentTimeMillis) mw/session-ttl-ms))})) - (edn-response 401 {:error "login failed"}))) + (respond 401 {:error "login failed"}))) (catch Exception e (println "[tbd] Gitea login failed:" (.getMessage e)) - (edn-response 502 {:error "could not complete login with Gitea"}))))))) + (respond 502 {:error "could not complete login with Gitea"}))))))) (defn- me-handler [config] (fn [req] (if-let [{:keys [login id]} (mw/current-user req)] - (edn-response 200 {:login login :id id :admin? (auth/admin? config login)}) - (edn-response 401 {:error "login required"})))) + (respond 200 {:login login :id id :admin? (auth/admin? config login)}) + (respond 401 {:error "login required"})))) (defn routes [config] ["/auth" @@ -57,7 +57,7 @@ ["/logout" {:post {:tags ["auth"] :summary "End the session" :handler (fn [_req] - (assoc (edn-response 200 {:logged-out true}) :session nil))}}] + (assoc (respond 200 {:logged-out true}) :session nil))}}] ["/me" {:get {:tags ["auth"] :summary "The logged-in user (401 if not logged in)" :handler (me-handler config)}}]]) diff --git a/src/tbd/core.clj b/src/tbd/core.clj index ec3d718..2953791 100644 --- a/src/tbd/core.clj +++ b/src/tbd/core.clj @@ -20,7 +20,9 @@ [tbd.http :as http] [tbd.math.routes :as math] [tbd.nrepl.core :as nrepl] - [tbd.nrepl.routes :as nrepl-routes])) + [tbd.nrepl.routes :as nrepl-routes] + [tbd.todo.db :as todo-db] + [tbd.todo.routes :as todo-routes])) (defn- swagger-handler "reitit emits `\"definitions\": null` when no route declares a schema, which @@ -34,12 +36,10 @@ (update :body dissoc :definitions)))))) (defn- coercion-error-handler - "Invalid request params -> 400 in the same EDN shape the routes use." + "Invalid request params -> 400, in the same shape (and format) the routes use." [e _req] - {:status 400 - :headers {"Content-Type" "application/edn; charset=utf-8"} - :body (pr-str {:error "invalid request parameters" - :details (:humanized (reitit.coercion/encode-error (ex-data e)))})}) + (http/respond 400 {:error "invalid request parameters" + :details (:humanized (reitit.coercion/encode-error (ex-data e)))})) (defn- index-handler "The HTML shell for the re-frame UI (ui/); it loads the compiled JS from /js." @@ -48,11 +48,9 @@ (response/content-type "text/html; charset=utf-8"))) (defn- decode-error-handler - "A request body that isn't valid for its Content-Type -> 400 in the same EDN shape." + "A request body that isn't valid for its Content-Type -> 400, in the same shape (and format) the routes use." [_e _req] - {:status 400 - :headers {"Content-Type" "application/edn; charset=utf-8"} - :body (pr-str {:error "malformed request body"})}) + (http/respond 400 {:error "malformed request body"})) (def ^:private exception-middleware (exception/create-exception-middleware @@ -62,10 +60,12 @@ ;; 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]}] +(defn make-router [{:keys [auth nrepl todo]}] (ring/router [["/swagger.json" - {:get {:no-doc true + ;; The spec is JSON, whatever the API default is (Swagger UI can't read EDN). + {:muuntaja m/instance + :get {:no-doc true :swagger {:info {:title "tbd API" :version "1.0.0"}} :handler (swagger-handler)}}] ["/" {:get {:no-doc true @@ -73,17 +73,21 @@ (health/routes) (math/routes) (auth-routes/routes auth) - (nrepl-routes/routes nrepl)] - {:data {:muuntaja m/instance + (nrepl-routes/routes nrepl) + (todo-routes/routes todo)] + {:data {:muuntaja http/muuntaja :coercion malli/coercion - ;; Admin auth goes first so unauthenticated callers to `:admin? true` routes - ;; are turned away before any body parsing or coercion runs. The muuntaja - ;; middleware is split around the exception middleware (rather than the combined - ;; format-middleware) so a malformed request body is caught and answered with a 400. - :middleware [(auth-mw/admin-middleware auth) - parameters/parameters-middleware - muuntaja-middleware/format-negotiate-middleware + ;; Handlers (and the auth/error middleware) return plain data; the muuntaja + ;; middleware encodes it as the Accept header asks, so it wraps everything else. + ;; Auth comes next, so unauthenticated callers to `:admin? true` / `:login? true` + ;; routes are turned away before any body parsing or coercion runs. The request + ;; half of muuntaja is split off below the exception middleware (rather than using the + ;; combined format-middleware) so a malformed request body is caught and answered with a 400. + :middleware [muuntaja-middleware/format-negotiate-middleware muuntaja-middleware/format-response-middleware + (auth-mw/admin-middleware auth) + auth-mw/login-middleware + parameters/parameters-middleware exception-middleware muuntaja-middleware/format-request-middleware coercion/coerce-request-middleware]}})) @@ -102,7 +106,8 @@ (def config {:auth (auth/env-config) - :nrepl (nrepl/env-config)}) + :nrepl (nrepl/env-config) + :todo (todo-db/env-config)}) (def app (make-app config)) diff --git a/src/tbd/http.clj b/src/tbd/http.clj index aa827b9..ca297a5 100644 --- a/src/tbd/http.clj +++ b/src/tbd/http.clj @@ -1,9 +1,20 @@ -(ns tbd.http) +(ns tbd.http + (:require [muuntaja.core :as m])) -(defn edn-response [status body] - {:status status - :headers {"Content-Type" "application/edn; charset=utf-8"} - :body (pr-str body)}) +(def muuntaja + "Content negotiation for the API. The request body is decoded by its Content-Type and the + response is encoded by the Accept header: JSON, EDN or Transit. When a request doesn't say + (no Content-Type, no Accept, or an Accept nothing here satisfies, like `*/*`) it is EDN." + (m/create + (-> m/default-options + (assoc :default-format "application/edn") + (assoc-in [:http :extract-content-type] + #(or (m/extract-content-type-ring %) "application/edn"))))) + +(defn respond + "A response whose `body` is plain data; the muuntaja middleware encodes it as the client asked." + [status body] + {:status status :body body}) (defn wrap-browser-viewable-edn "Browsers download `application/edn` instead of showing it. When the request comes from a diff --git a/src/tbd/math/routes.clj b/src/tbd/math/routes.clj index 5b6e79d..c89beb1 100644 --- a/src/tbd/math/routes.clj +++ b/src/tbd/math/routes.clj @@ -1,18 +1,14 @@ (ns tbd.math.routes - (:require [tbd.math.core :as math])) + (:require [tbd.http :as http] + [tbd.math.core :as math])) ;; :int first so "2" stays a long (a bare `number?` or :double would decode it to 2.0). ;; The :swagger/* props make the spec say plain `number` instead of the first :or branch. (def ^:private number-schema [:or {:swagger/type "number" :swagger/format "double"} :int :double]) -(defn- edn-response [status body] - {:status status - :headers {"Content-Type" "application/edn; charset=utf-8"} - :body (pr-str body)}) - (defn- add-handler [{{{:keys [x y]} :query} :parameters}] - (edn-response 200 {:result (math/add x y)})) + (http/respond 200 {:result (math/add x y)})) (defn routes [] ["/api/add" diff --git a/src/tbd/nrepl/routes.clj b/src/tbd/nrepl/routes.clj index fdcaf53..7731e23 100644 --- a/src/tbd/nrepl/routes.clj +++ b/src/tbd/nrepl/routes.clj @@ -1,5 +1,5 @@ (ns tbd.nrepl.routes - (:require [tbd.http :refer [edn-response]] + (:require [tbd.http :refer [respond]] [tbd.nrepl.core :as nrepl])) (defn- toggle-handler [config] @@ -10,13 +10,13 @@ (if enabled "enabled" "disabled") login (if enabled (format " on %s:%d" (:bind status) (:port status)) ""))) - (edn-response 200 status)) + (respond 200 status)) (catch Exception e (println "[tbd] nREPL toggle failed:" (.getMessage e)) - (edn-response 500 {:error (str "could not change nREPL state: " (.getMessage e))}))))) + (respond 500 {:error (str "could not change nREPL state: " (.getMessage e))}))))) (defn- status-handler [_req] - (edn-response 200 (nrepl/status))) + (respond 200 (nrepl/status))) (defn routes [config] ["/nrepl" diff --git a/src/tbd/todo/core.clj b/src/tbd/todo/core.clj new file mode 100644 index 0000000..e8dbe92 --- /dev/null +++ b/src/tbd/todo/core.clj @@ -0,0 +1,40 @@ +(ns tbd.todo.core + "Business logic for the todo group: validation and state changes on plain maps. No HTTP and + no database (see tbd.todo.db); anything time-dependent takes `now` (epoch ms) as an argument. + + Fallible fns return {:ok value} or {:error message}." + (:require [clojure.string :as str])) + +(def max-title-length 200) + +(defn validate-title + "Titles are trimmed, and must be non-blank and at most `max-title-length` characters." + [title] + (let [title (some-> title str/trim)] + (cond + (str/blank? title) {:error "title must not be blank"} + (> (count title) max-title-length) {:error (format "title must be at most %d characters" max-title-length)} + :else {:ok title}))) + +(defn new-todo + "A todo for `owner-id`, not yet stored (so no :id)." + [owner-id title now] + (let [{:keys [ok error]} (validate-title title)] + (if error + {:error error} + {:ok {:owner-id owner-id :title ok :done false :created-at now :updated-at now}}))) + +(defn apply-changes + "`todo` with `changes` (a map with :title and/or :done) applied. Anything else in `changes` + is ignored; a change with neither is an error." + [todo changes now] + (let [changes (select-keys changes [:title :done]) + {:keys [ok error]} (if (contains? changes :title) + (validate-title (:title changes)) + {:ok (:title todo)})] + (cond + (empty? changes) {:error "nothing to change: give a title and/or done"} + error {:error error} + :else {:ok (-> todo + (assoc :title ok :updated-at now) + (cond-> (contains? changes :done) (assoc :done (boolean (:done changes)))))}))) diff --git a/src/tbd/todo/db.clj b/src/tbd/todo/db.clj new file mode 100644 index 0000000..637ce8f --- /dev/null +++ b/src/tbd/todo/db.clj @@ -0,0 +1,79 @@ +(ns tbd.todo.db + "SQLite persistence for the todo group, in a local file. 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] + [next.jdbc.result-set :as rs])) + +(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))}))) + +(defn- migrate! [ds] + (jdbc/execute! ds ["CREATE TABLE IF NOT EXISTS todos ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + owner_id INTEGER NOT NULL, + title TEXT NOT NULL, + done INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL)"]) + (jdbc/execute! ds ["CREATE INDEX IF NOT EXISTS todos_by_owner ON todos (owner_id, id)"])) + +(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." + [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!)) + +(def ^:private opts {:builder-fn rs/as-unqualified-kebab-maps}) + +;; SQLite has no boolean type; `done` is stored as 0/1. +(defn- row->todo [row] + (some-> row (update :done #(= 1 %)))) + +(defn insert! + "Stores a todo (as built by tbd.todo.core/new-todo) and returns it with its :id." + [ds {:keys [owner-id title done created-at updated-at]}] + (row->todo + (jdbc/execute-one! ds ["INSERT INTO todos (owner_id, title, done, created_at, updated_at) + VALUES (?, ?, ?, ?, ?) RETURNING *" + owner-id title (if done 1 0) created-at updated-at] + opts))) + +(defn list-todos [ds owner-id] + (mapv row->todo + (jdbc/execute! ds ["SELECT * FROM todos WHERE owner_id = ? ORDER BY id" owner-id] opts))) + +(defn find-todo + "The owner's todo with this id, or nil (including when it belongs to someone else)." + [ds owner-id id] + (row->todo + (jdbc/execute-one! ds ["SELECT * FROM todos WHERE owner_id = ? AND id = ?" owner-id id] opts))) + +(defn update! + "Writes the todo's :title, :done and :updated-at; returns the stored row, or nil if the owner has no such todo." + [ds owner-id {:keys [id title done updated-at]}] + (row->todo + (jdbc/execute-one! ds ["UPDATE todos SET title = ?, done = ?, updated_at = ? + WHERE owner_id = ? AND id = ? RETURNING *" + title (if done 1 0) updated-at owner-id id] + opts))) + +(defn delete! + "True if the owner's todo existed and is now gone." + [ds owner-id id] + (pos? (::jdbc/update-count + (jdbc/execute-one! ds ["DELETE FROM todos WHERE owner_id = ? AND id = ?" owner-id id])))) diff --git a/src/tbd/todo/routes.clj b/src/tbd/todo/routes.clj new file mode 100644 index 0000000..ea12f35 --- /dev/null +++ b/src/tbd/todo/routes.clj @@ -0,0 +1,71 @@ +(ns tbd.todo.routes + (:require [tbd.http :refer [respond]] + [tbd.todo.core :as todo] + [tbd.todo.db :as db])) + +(def ^:private not-found (respond 404 {:error "todo not found"})) + +;; Todos belong to the logged-in user (:login? true puts them on the request). Someone else's +;; todo is answered exactly like a missing one, so ids can't be probed. +(defn- owner-id [req] (get-in req [:user :id])) + +(defn- now [] (System/currentTimeMillis)) + +(defn- list-handler [ds] + (fn [req] + (respond 200 {:todos (db/list-todos @ds (owner-id req))}))) + +(defn- create-handler [ds] + (fn [{{{:keys [title]} :body} :parameters :as req}] + (let [{:keys [ok error]} (todo/new-todo (owner-id req) title (now))] + (if error + (respond 400 {:error error}) + (respond 201 (db/insert! @ds ok)))))) + +(defn- get-handler [ds] + (fn [{{{:keys [id]} :path} :parameters :as req}] + (if-let [found (db/find-todo @ds (owner-id req) id)] + (respond 200 found) + not-found))) + +(defn- patch-handler [ds] + (fn [{{{:keys [id]} :path changes :body} :parameters :as req}] + (let [owner (owner-id req)] + (if-let [found (db/find-todo @ds owner id)] + (let [{:keys [ok error]} (todo/apply-changes found changes (now))] + (if error + (respond 400 {:error error}) + (respond 200 (db/update! @ds owner ok)))) + not-found)))) + +(defn- delete-handler [ds] + (fn [{{{:keys [id]} :path} :parameters :as req}] + (if (db/delete! @ds (owner-id req) id) + (respond 200 {:deleted true}) + not-found))) + +(defn routes + "`db-path` is opened (and migrated) on the first request, not when the app loads." + [{:keys [db-path]}] + (let [ds (delay (db/open db-path)) + id-param {:path [:map [:id {:description "Todo id"} :int]]}] + ["/api/todos" + {:tags ["todo"] :login? true} + ["" + {:get {:summary "List your todos (login required)" + :handler (list-handler ds)} + :post {:summary (format "Create a todo; the title is trimmed, non-blank, at most %d characters" todo/max-title-length) + :parameters {:body [:map [:title :string]]} + :handler (create-handler ds)}}] + ["/:id" + {:get {:summary "One of your todos (404 if it isn't yours)" + :parameters id-param + :handler (get-handler ds)} + :patch {:summary "Change the title and/or done flag of one of your todos" + :parameters (assoc id-param :body [:map + [:title {:optional true} :string] + [:done {:optional true} :boolean]]) + :handler (patch-handler ds)} + :delete {:summary "Delete one of your todos" + :parameters id-param + :handler (delete-handler ds)}}]])) diff --git a/test/tbd/auth/routes_test.clj b/test/tbd/auth/routes_test.clj index 96d1413..4c3e828 100644 --- a/test/tbd/auth/routes_test.clj +++ b/test/tbd/auth/routes_test.clj @@ -1,20 +1,24 @@ (ns tbd.auth.routes-test - (:require [clojure.edn :as edn] - [clojure.string :as str] + (:require [clojure.string :as str] [clojure.test :refer [deftest is testing]] [tbd.auth.middleware :as mw] - [tbd.core :as core])) + [tbd.core :as core] + [tbd.test-util :as test-util])) (defn gitea-stub - "Stands in for Gitea: any code is exchanged for a token that resolves to `login`." - [login] - (fn [{:keys [url]}] - (cond - (str/ends-with? url "/login/oauth/access_token") {:status 200 :body "{\"access_token\":\"tok\"}"} - (str/ends-with? url "/api/v1/user") {:status 200 :body (format "{\"login\":\"%s\",\"id\":7}" login)}))) + "Stands in for Gitea: any code is exchanged for a token that resolves to `login` (with Gitea id 7, or `id`)." + ([login] (gitea-stub login 7)) + ([login id] + (fn [{:keys [url]}] + (cond + (str/ends-with? url "/login/oauth/access_token") {:status 200 :body "{\"access_token\":\"tok\"}"} + (str/ends-with? url "/api/v1/user") {:status 200 :body (format "{\"login\":\"%s\",\"id\":%d}" login id)})))) -(defn make-app [http & {:as overrides}] - (core/make-app {:auth (merge {:gitea-url "https://gitea.test" +(defn make-app + "The app against a Gitea stub. `:todo` (a todo config) is passed through; other overrides go into the auth config." + [http & {:keys [todo] :as overrides}] + (core/make-app {:todo todo + :auth (merge {:gitea-url "https://gitea.test" :client-id "cid" :client-secret "sec" :redirect-uri "http://tbd.test/auth/callback" @@ -22,16 +26,13 @@ :session-secret "test-secret" :secure-cookies? false :http http} - overrides) + (dissoc overrides :todo)) :nrepl {:port 0 :bind "127.0.0.1"}})) (defn request [app method uri & {:keys [cookie query]}] (let [resp (app (cond-> {:request-method method :uri uri :query-string query :headers {}} cookie (assoc-in [:headers "cookie"] cookie)))] - (cond-> resp - (and (string? (:body resp)) - (str/includes? (get-in resp [:headers "Content-Type"] "") "edn")) - (update :body edn/read-string)))) + (test-util/decode-body resp))) (defn set-cookie "The `name=value` pair from a response's Set-Cookie header." diff --git a/test/tbd/math/routes_test.clj b/test/tbd/math/routes_test.clj index ba6af95..02a8fd2 100644 --- a/test/tbd/math/routes_test.clj +++ b/test/tbd/math/routes_test.clj @@ -1,14 +1,12 @@ (ns tbd.math.routes-test - (:require [clojure.edn :as edn] - [clojure.test :refer [deftest is testing]] + (:require [clojure.test :refer [deftest is testing]] [muuntaja.core :as m] - [tbd.core :as core])) + [tbd.core :as core] + [tbd.test-util :as test-util])) ;; Coercion/validation is configured on the router in tbd.core, so go through the real app. (defn add [query] - (let [resp (core/app {:request-method :get :uri "/api/add" :query-string query})] - (-> resp - (update :body edn/read-string)))) + (test-util/decode-body (core/app {:request-method :get :uri "/api/add" :query-string query}))) (deftest add-route-test (testing "integers" @@ -31,7 +29,7 @@ (let [resp (core/app {:request-method :get :uri "/api/add" :query-string "x=2&y=3" :headers {"accept" "text/html,application/xhtml+xml,*/*;q=0.8"}})] (is (= "text/plain; charset=utf-8" (get-in resp [:headers "Content-Type"]))) - (is (= {:result 5} (edn/read-string (:body resp)))))) + (is (= {:result 5} (:body (test-util/decode-body resp)))))) (testing "programmatic clients still get application/edn" (let [resp (core/app {:request-method :get :uri "/api/add" :query-string "x=2&y=3" :headers {"accept" "*/*"}})] diff --git a/test/tbd/nrepl/routes_test.clj b/test/tbd/nrepl/routes_test.clj index 21b6d15..ae09278 100644 --- a/test/tbd/nrepl/routes_test.clj +++ b/test/tbd/nrepl/routes_test.clj @@ -1,8 +1,8 @@ (ns tbd.nrepl.routes-test - (:require [clojure.edn :as edn] - [clojure.test :refer [deftest is testing use-fixtures]] + (:require [clojure.test :refer [deftest is testing use-fixtures]] [muuntaja.core :as m] [tbd.auth.routes-test :as t] + [tbd.test-util :as test-util] [tbd.nrepl.core :as nrepl]) (:import (java.io ByteArrayInputStream) (java.net InetAddress ServerSocket))) @@ -20,12 +20,12 @@ :headers (cond-> {"content-type" content-type} cookie (assoc "cookie" cookie))} body (assoc :body (ByteArrayInputStream. (.getBytes ^String body))))))) - (cond-> @resp (string? (:body @resp)) (update :body edn/read-string))))) + (test-util/decode-body @resp)))) (defn get-nrepl [app cookie] (let [resp (app {:request-method :get :uri "/nrepl" :headers (cond-> {} cookie (assoc "cookie" cookie))})] - (cond-> resp (string? (:body resp)) (update :body edn/read-string)))) + (test-util/decode-body resp))) (deftest anonymous-test (let [app (t/make-app (t/gitea-stub "alice"))] diff --git a/test/tbd/test_util.clj b/test/tbd/test_util.clj new file mode 100644 index 0000000..894ed3d --- /dev/null +++ b/test/tbd/test_util.clj @@ -0,0 +1,13 @@ +(ns tbd.test-util + (:require [muuntaja.core :as m] + [tbd.http :as http])) + +(defn decode-body + "Reads a response's body back into data by the response's own Content-Type (JSON, EDN or + Transit). Bodies that aren't one of those, like text/plain, are left alone." + [resp] + (let [content-type (get-in resp [:headers "Content-Type"] "") + body (:body resp)] + (if (and body (not (string? body)) (re-find #"json|edn|transit" content-type)) + (update resp :body #(m/decode http/muuntaja content-type %)) + resp))) diff --git a/test/tbd/todo/core_test.clj b/test/tbd/todo/core_test.clj new file mode 100644 index 0000000..19d8c67 --- /dev/null +++ b/test/tbd/todo/core_test.clj @@ -0,0 +1,41 @@ +(ns tbd.todo.core-test + (:require [clojure.test :refer [deftest is testing]] + [tbd.todo.core :as todo])) + +(deftest validate-title-test + (testing "trims" + (is (= {:ok "buy milk"} (todo/validate-title " buy milk\n")))) + (testing "blank or missing" + (doseq [title [nil "" " " "\t\n"]] + (is (= {:error "title must not be blank"} (todo/validate-title title)) (pr-str title)))) + (testing "length is checked after trimming" + (let [longest (apply str (repeat todo/max-title-length "x"))] + (is (= {:ok longest} (todo/validate-title (str " " longest " ")))) + (is (contains? (todo/validate-title (str longest "x")) :error))))) + +(deftest new-todo-test + (is (= {:ok {:owner-id 7 :title "buy milk" :done false :created-at 1000 :updated-at 1000}} + (todo/new-todo 7 " buy milk " 1000))) + (is (= {:error "title must not be blank"} (todo/new-todo 7 " " 1000)))) + +(def stored {:id 1 :owner-id 7 :title "buy milk" :done false :created-at 1000 :updated-at 1000}) + +(deftest apply-changes-test + (testing "title" + (is (= {:ok (assoc stored :title "buy oat milk" :updated-at 2000)} + (todo/apply-changes stored {:title " buy oat milk "} 2000)))) + (testing "done, both ways" + (let [done (:ok (todo/apply-changes stored {:done true} 2000))] + (is (= (assoc stored :done true :updated-at 2000) done)) + (is (= (assoc stored :updated-at 3000) (:ok (todo/apply-changes done {:done false} 3000)))))) + (testing "both at once" + (is (= {:ok (assoc stored :title "x" :done true :updated-at 2000)} + (todo/apply-changes stored {:title "x" :done true} 2000)))) + (testing "an invalid title changes nothing" + (is (= {:error "title must not be blank"} (todo/apply-changes stored {:title " " :done true} 2000)))) + (testing "only :title and :done can change; the rest of the todo is untouched" + (is (= {:ok (assoc stored :done true :updated-at 2000)} + (todo/apply-changes stored {:done true :id 99 :owner-id 8 :created-at 0} 2000)))) + (testing "no changes" + (doseq [changes [{} {:id 99} nil]] + (is (contains? (todo/apply-changes stored changes 2000) :error) (pr-str changes))))) diff --git a/test/tbd/todo/db_test.clj b/test/tbd/todo/db_test.clj new file mode 100644 index 0000000..11b520b --- /dev/null +++ b/test/tbd/todo/db_test.clj @@ -0,0 +1,69 @@ +(ns tbd.todo.db-test + (:require [clojure.java.io :as io] + [clojure.test :refer [deftest is testing]] + [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")) + +(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)] + (db/open path) + (is (.exists (io/file path))))) + (testing "reopening keeps the data" + (let [path (temp-db-path) + saved (db/insert! (db/open path) (todo 7 "buy milk"))] + (is (= [saved] (db/list-todos (db/open path) 7))))) + (testing "a path is required" + (doseq [path [nil "" " "]] + (is (thrown? clojure.lang.ExceptionInfo (db/open path)))))) + +(deftest crud-test + (let [ds (db/open (temp-db-path)) + saved (db/insert! ds (todo 7 "buy milk")) + id (:id saved)] + (testing "insert returns the stored todo with an id" + (is (= (assoc (todo 7 "buy milk") :id id) saved)) + (is (not= id (:id (db/insert! ds (todo 7 "walk dog")))))) + (testing "find and list" + (is (= saved (db/find-todo ds 7 id))) + (is (= ["buy milk" "walk dog"] (map :title (db/list-todos ds 7))))) + (testing "update" + (is (= (assoc saved :title "buy oat milk" :done true :updated-at 2000) + (db/update! ds 7 (assoc saved :title "buy oat milk" :done true :updated-at 2000)))) + (is (= true (:done (db/find-todo ds 7 id))))) + (testing "missing ids" + (is (nil? (db/find-todo ds 7 999))) + (is (nil? (db/update! ds 7 (assoc saved :id 999)))) + (is (false? (db/delete! ds 7 999)))) + (testing "delete" + (is (true? (db/delete! ds 7 id))) + (is (nil? (db/find-todo ds 7 id))) + (is (false? (db/delete! ds 7 id)))))) + +(deftest owner-scoping-test + (let [ds (db/open (temp-db-path)) + mine (db/insert! ds (todo 7 "mine")) + theirs (db/insert! ds (todo 8 "theirs"))] + (is (= [mine] (db/list-todos ds 7))) + (is (= [theirs] (db/list-todos ds 8))) + (is (= [] (db/list-todos ds 9))) + (testing "another owner's todo can't be found, changed, or deleted by its id" + (is (nil? (db/find-todo ds 8 (:id mine)))) + (is (nil? (db/update! ds 8 (assoc mine :title "hijacked")))) + (is (false? (db/delete! ds 8 (:id mine)))) + (is (= mine (db/find-todo ds 7 (:id mine))))))) diff --git a/test/tbd/todo/routes_test.clj b/test/tbd/todo/routes_test.clj new file mode 100644 index 0000000..ef50e2b --- /dev/null +++ b/test/tbd/todo/routes_test.clj @@ -0,0 +1,106 @@ +(ns tbd.todo.routes-test + (:require [clojure.java.io :as io] + [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]) + (:import (java.io ByteArrayInputStream))) + +(defn request + "`body` is a JSON string (or nil). The response body is decoded by its Content-Type." + [app method uri cookie & [body]] + (let [resp (app (cond-> {:request-method method :uri uri :headers (cond-> {} cookie (assoc "cookie" cookie))} + body (-> (assoc :body (ByteArrayInputStream. (.getBytes ^String body))) + (assoc-in [:headers "content-type"] "application/json"))))] + (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})) + +(def ^:private todos "/api/todos") + +(deftest anonymous-test + (let [path (db-test/temp-db-path) + app (app-for path "alice" 7)] + (doseq [[method uri body] [[:get todos nil] + [:post todos "{\"title\":\"x\"}"] + [:post todos "nope"] + [:get (str todos "/1") nil] + [:patch (str todos "/1") "{\"done\":true}"] + [:delete (str todos "/1") nil]]] + (let [resp (request app method uri nil body)] + (is (= 401 (:status resp)) (str method " " uri)) + (is (= {:error "login required"} (:body resp))))) + (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) + cookie (t/login! app)] + (testing "starts empty" + (let [resp (request app :get todos cookie)] + (is (= 200 (:status resp))) + (is (= "application/edn; charset=utf-8" (get-in resp [:headers "Content-Type"]))) + (is (= {:todos []} (:body resp))))) + (testing "create" + (let [resp (request app :post todos cookie "{\"title\":\" buy milk \"}") + {:keys [id] :as created} (:body resp)] + (is (= 201 (:status resp))) + (is (= {:owner-id 7 :title "buy milk" :done false} (select-keys created [:owner-id :title :done]))) + (is (pos? id)) + (testing "get" + (is (= created (:body (request app :get (str todos "/" id) cookie))))) + (testing "patch" + (let [resp (request app :patch (str todos "/" id) cookie "{\"done\":true}")] + (is (= 200 (:status resp))) + (is (= (dissoc (assoc created :done true) :updated-at) (dissoc (:body resp) :updated-at)))) + (is (= "oat milk" (:title (:body (request app :patch (str todos "/" id) cookie "{\"title\":\"oat milk\"}")))))) + (testing "list" + (request app :post todos cookie "{\"title\":\"walk dog\"}") + (is (= ["oat milk" "walk dog"] (map :title (:todos (:body (request app :get todos cookie))))))) + (testing "delete" + (is (= {:status 200 :body {:deleted true}} + (select-keys (request app :delete (str todos "/" id) cookie) [:status :body]))) + (is (= 404 (:status (request app :get (str todos "/" id) cookie)))) + (is (= 404 (:status (request app :delete (str todos "/" id) cookie)))) + (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) + cookie (t/login! app) + created (:body (request app :post todos cookie "{\"title\":\"keep\"}")) + one (str todos "/" (:id created))] + (testing "bad titles on create" + (doseq [body ["{\"title\":\"\"}" "{\"title\":\" \"}" "{}" "{\"title\":5}" "{\"title\":null}" "nope" + (format "{\"title\":\"%s\"}" (apply str (repeat 201 "x")))]] + (let [resp (request app :post todos cookie body)] + (is (= 400 (:status resp)) body) + (is (contains? (:body resp) :error))))) + (testing "bad changes" + (doseq [body ["{}" "{\"title\":\" \"}" "{\"done\":\"yes\"}" "{\"id\":99}"]] + (is (= 400 (:status (request app :patch one cookie body))) body)) + (is (= 400 (:status (request app :patch one cookie))) "no body")) + (testing "a non-numeric id" + (doseq [method [:get :patch :delete]] + (is (= 400 (:status (request app method (str todos "/abc") cookie "{\"done\":true}"))) (str method)))) + (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) + alice-app (app-for path "alice" 7) + bob-app (app-for path "bob" 8) + alice (t/login! alice-app) + bob (t/login! bob-app) + {:keys [id] :as mine} (:body (request alice-app :post todos alice "{\"title\":\"alice's\"}")) + one (str todos "/" id)] + (testing "each user lists only their own" + (is (= [] (:todos (:body (request bob-app :get todos bob))))) + (request bob-app :post todos bob "{\"title\":\"bob's\"}") + (is (= ["alice's"] (map :title (:todos (:body (request alice-app :get todos alice)))))) + (is (= ["bob's"] (map :title (:todos (:body (request bob-app :get todos bob))))))) + (testing "someone else's todo is indistinguishable from a missing one" + (let [missing (request bob-app :get (str todos "/999999") bob)] + (doseq [[method body] [[:get nil] [:patch "{\"title\":\"hijacked\"}"] [:delete nil]]] + (let [resp (request bob-app method one bob body)] + (is (= 404 (:status resp)) (str method)) + (is (= (:body missing) (:body resp))))))) + (is (= mine (:body (request alice-app :get one alice))) "alice's todo is untouched")))