From 670b8a2de0732cb68a4e5ce0d60a1fc50bcf5204 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Tue, 29 Oct 2024 15:15:36 -0400 Subject: [PATCH 01/61] Add new /admin/verify endpoint --- src/main/lrsql/admin/interceptors/account.clj | 8 ++++++++ src/main/lrsql/admin/routes.clj | 14 +++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 71e821dd2..1c00d7fe1 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -250,6 +250,14 @@ :response {:status 200 :body result})))})) +(def bodyless + "Return a 204 OK response without a body." + (interceptor + {:name ::get-bodyless + :enter + (fn get-account [ctx] + (assoc ctx :response {:status 204}))})) + (defn generate-jwt "Upon account login, generate a new JSON web token." [secret exp] diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 170d9022e..c7db6cb95 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -101,7 +101,7 @@ (gs/a (gs/o {:account-id :t#string :username :t#string}))) 401 (g/rref :error-401)}}) - ;; Get my accounts + ;; Get my account (gc/annotate ["/admin/me" :get (conj common-interceptors (ji/validate-jwt @@ -116,6 +116,18 @@ (gs/o {:account-id :t#string :username :t#string}) ) 401 (g/rref :error-401)}}) + ;; Check that I am logged in + (gc/annotate + ["/admin/verify" :get (conj common-interceptors + (ji/validate-jwt + jwt-secret jwt-leeway no-val-opts) + ji/validate-jwt-account + ai/bodyless)] + {:description "Verify that querying account is logged in" + :operationId :verify-own-account + :security [{:bearerAuth []}] + :responses {204 (g/response "No body") + 401 (g/rref :error-401)}}) ;; Delete account (and associated credentials) (gc/annotate ["/admin/account" :delete (conj common-interceptors From 39d6c15d4ade1a404116d3c03c3bcb016e12a61a Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Tue, 29 Oct 2024 15:15:46 -0400 Subject: [PATCH 02/61] Add tests for new endpoint (+ JWT corruption) --- src/test/lrsql/admin/route_test.clj | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/lrsql/admin/route_test.clj b/src/test/lrsql/admin/route_test.clj index 8ec278a5b..475f27376 100644 --- a/src/test/lrsql/admin/route_test.clj +++ b/src/test/lrsql/admin/route_test.clj @@ -75,6 +75,11 @@ (curl/get "http://0.0.0.0:8080/admin/me" {:headers headers})) +(defn- verify-me + [headers] + (curl/get "http://0.0.0.0:8080/admin/verify" + {:headers headers})) + (defn- update-account-password [headers body] @@ -133,7 +138,11 @@ headers (merge content-type seed-auth) orig-pass "Swordfish100?" req-body (u/write-json-str {"username" "mylongname" - "password" orig-pass})] + "password" orig-pass}) + ;; Corrupted JWT + bad-jwt (apply str (butlast seed-jwt)) + bad-auth {"Authorization" (str "Bearer " bad-jwt)} + bad-head (merge content-type bad-auth)] (try (testing "seed jwt retrieved" ;; Sanity check that the test credentials are in place @@ -210,6 +219,13 @@ (is (= 200 status)) ;; is the created user (is (= (get edn-body "username") admin-user-default)))) + (testing "verify my admin account" + (let [{:keys [status]} (verify-me headers)] + ;; success + (is (= 204 status)))) + (testing "ensure corrupted JWTs do not pass" + (is-err-code (get-me bad-head) 401) + (is-err-code (verify-me bad-head) 401)) (testing "log into the `myname` account" (let [{:keys [status body]} (login-account content-type req-body) From 57bfee95093ed21d693c9e34d2902cbcdc728d0d Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Tue, 29 Oct 2024 15:17:38 -0400 Subject: [PATCH 03/61] Add endpoint to documentation --- doc/endpoints.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/endpoints.md b/doc/endpoints.md index 29bbcf557..6a504890b 100644 --- a/doc/endpoints.md +++ b/doc/endpoints.md @@ -31,6 +31,7 @@ The response body contains a newly generated JSON Web Token (JWT) on success. A - `DELETE http://example.org/admin/account`: Delete an existing account. The JSON request body must contain a UUID `account-id` value. The endpoint returns a JSON object with the ID of the deleted account on success and returns a `404 NOT FOUND` error if the account does not exist. - `GET http://example.org/admin/account`: Return an array of all admin accounts in the system on success. - `GET http://example.org/admin/me`: Returns the currently authenticated admin accounts on success. +- `GET http://example.org/admin/verify`: Returns a `204 OK` response, without a body, on success (the success conditions are the same as the `/admin/me` endpoint). #### Admin Credential Routes From e16d6e20314237cdcc6b627c38ed9617bc1b74b8 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Tue, 29 Oct 2024 15:27:53 -0400 Subject: [PATCH 04/61] Add /admin/openapi to endpoint docs --- doc/endpoints.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/endpoints.md b/doc/endpoints.md index 6a504890b..d379bc048 100644 --- a/doc/endpoints.md +++ b/doc/endpoints.md @@ -43,6 +43,7 @@ The response body contains a newly generated JSON Web Token (JWT) on success. A #### Misc Admin Routes - `GET http://example.org/admin/env`: Get select environment variables about the configuration which may aid in client-side operations. +- `GET http://example.org/admin/openapi`: Get an OpenAPI JSON spec of the endpoint API, which can then be visualized using an OpenAPI viewer like Swagger. - `DELETE http://example.org/admin/agents`: Runs a *hard delete* of all records of an actor, and associated records (statements, attachments, etc). Intended for privacy purposes like GDPR. Body should be a JSON object of form `{"actor-ifi":}`. Disabled unless the configuration variable enableAdminDeleteActor to be set to `true`. ### Reaction Management Routes From d645cfcba45eb6bece01943e2ee32d6df9a1b916 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Tue, 29 Oct 2024 15:29:43 -0400 Subject: [PATCH 05/61] Add /admin/status to endpoint docs --- doc/endpoints.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/endpoints.md b/doc/endpoints.md index d379bc048..f6f6e121c 100644 --- a/doc/endpoints.md +++ b/doc/endpoints.md @@ -44,6 +44,7 @@ The response body contains a newly generated JSON Web Token (JWT) on success. A - `GET http://example.org/admin/env`: Get select environment variables about the configuration which may aid in client-side operations. - `GET http://example.org/admin/openapi`: Get an OpenAPI JSON spec of the endpoint API, which can then be visualized using an OpenAPI viewer like Swagger. +- `GET http://example.org/admin/status`: Get LRS status information, such as the number of statements in the LRS. - `DELETE http://example.org/admin/agents`: Runs a *hard delete* of all records of an actor, and associated records (statements, attachments, etc). Intended for privacy purposes like GDPR. Body should be a JSON object of form `{"actor-ifi":}`. Disabled unless the configuration variable enableAdminDeleteActor to be set to `true`. ### Reaction Management Routes From 35cfbda14a782d743231ce26618ae3d11e2c8136 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Tue, 29 Oct 2024 15:40:17 -0400 Subject: [PATCH 06/61] Bodyless -> No Content --- doc/endpoints.md | 2 +- src/main/lrsql/admin/interceptors/account.clj | 6 +++--- src/main/lrsql/admin/routes.clj | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/endpoints.md b/doc/endpoints.md index f6f6e121c..30a714074 100644 --- a/doc/endpoints.md +++ b/doc/endpoints.md @@ -31,7 +31,7 @@ The response body contains a newly generated JSON Web Token (JWT) on success. A - `DELETE http://example.org/admin/account`: Delete an existing account. The JSON request body must contain a UUID `account-id` value. The endpoint returns a JSON object with the ID of the deleted account on success and returns a `404 NOT FOUND` error if the account does not exist. - `GET http://example.org/admin/account`: Return an array of all admin accounts in the system on success. - `GET http://example.org/admin/me`: Returns the currently authenticated admin accounts on success. -- `GET http://example.org/admin/verify`: Returns a `204 OK` response, without a body, on success (the success conditions are the same as the `/admin/me` endpoint). +- `GET http://example.org/admin/verify`: Returns a `204 No Content` response, without a body, on success (the success conditions are the same as the `/admin/me` endpoint). #### Admin Credential Routes diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 1c00d7fe1..93e47e11f 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -250,10 +250,10 @@ :response {:status 200 :body result})))})) -(def bodyless - "Return a 204 OK response without a body." +(def no-content + "Return a 204 No Content response, without a body." (interceptor - {:name ::get-bodyless + {:name ::get-no-content :enter (fn get-account [ctx] (assoc ctx :response {:status 204}))})) diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index c7db6cb95..5fd1b3276 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -122,11 +122,11 @@ (ji/validate-jwt jwt-secret jwt-leeway no-val-opts) ji/validate-jwt-account - ai/bodyless)] + ai/no-content)] {:description "Verify that querying account is logged in" :operationId :verify-own-account :security [{:bearerAuth []}] - :responses {204 (g/response "No body") + :responses {204 (g/response "No content body") 401 (g/rref :error-401)}}) ;; Delete account (and associated credentials) (gc/annotate From 55855cd392e7042dcc6f7b607e64223ce2765870 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 31 Oct 2024 16:24:48 -0400 Subject: [PATCH 07/61] Add JWT blocklist table to SQLite --- src/db/sqlite/lrsql/sqlite/sql/ddl.sql | 24 +++++++++++++++++++++-- src/db/sqlite/lrsql/sqlite/sql/delete.sql | 16 +++++++++++++++ src/db/sqlite/lrsql/sqlite/sql/insert.sql | 12 ++++++++++++ src/db/sqlite/lrsql/sqlite/sql/query.sql | 10 ++++++++++ 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/db/sqlite/lrsql/sqlite/sql/ddl.sql b/src/db/sqlite/lrsql/sqlite/sql/ddl.sql index bc53c05ff..d6e9cd4e4 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/ddl.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/ddl.sql @@ -149,8 +149,6 @@ CREATE INDEX IF NOT EXISTS sts_ancestor_id_idx ON statement_to_statement(ancesto -- :doc Create an index on the `statement_to_statement.descendant_id` column. CREATE INDEX IF NOT EXISTS sts_descendant_id_idx ON statement_to_statement(descendant_id) - - /* Document Tables */ -- :name create-state-document-table! @@ -524,3 +522,25 @@ SET sql = 'CREATE TABLE statement_to_actor ( FOREIGN KEY (actor_ifi, actor_type) REFERENCES actor(actor_ifi, actor_type) )' WHERE type = 'table' AND name = 'statement_to_actor' +; + +/* Migration 2024-10-31 - Add JWT Blocklist Table */ + +-- :name create-blocked-jwt-table! +-- :command :execute +-- :doc Create the `blocked_jwt` table if it does not exist yet. +CREATE TABLE IF NOT EXISTS blocked_jwt ( + account_id TEXT NOT NULL REFERENCES admin_account(id) ON DELETE CASCADE, + expiration TIMESTAMP, + PRIMARY KEY (account_id, expiration) +); + +-- :name create-blocked-jwt-account-id-idx! +-- :command :execute +-- :doc Create the `blocked_jwt_account_id_idx` table if it does not exist yet. +CREATE INDEX IF NOT EXISTS blocked_jwt_account_id_idx ON blocked_jwt(account_id); + +-- :name create-blocked-jwt-expiration-idx! +-- :command :execute +-- :doc Create the `blocked_jwt_expiration_idx` table if it does not exist yet. +CREATE INDEX IF NOT EXISTS blocked_jwt_expiration_idx ON blocked_jwt(expiration); diff --git a/src/db/sqlite/lrsql/sqlite/sql/delete.sql b/src/db/sqlite/lrsql/sqlite/sql/delete.sql index 95a4250c9..f8c96bcb0 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/delete.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/delete.sql @@ -123,3 +123,19 @@ DELETE FROM state_document WHERE agent_ifi = :actor-ifi -- :command :execute -- :result :affected DELETE FROM actor WHERE actor_ifi = :actor-ifi + +/* JWT Blocklist */ + +-- :name delete-blocked-jwt-by-account! +-- :command :execute +-- :result :affected +-- :doc Delete all blocked JWTs associated with a given `:account-id`. +DELETE FROM blocked_jwt +WHERE account_id = :account-id; + +-- :name :delete-blocked-jwt-by-time! +-- :command :execute +-- :result :affected +-- :doc Delete all blocked JWTs where `:current-time` is past the expiration time. +DELETE FROM blocked_jwt +WHERE expiration <= :current-time; diff --git a/src/db/sqlite/lrsql/sqlite/sql/insert.sql b/src/db/sqlite/lrsql/sqlite/sql/insert.sql index 9aa1af105..533e20e5f 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/insert.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/insert.sql @@ -159,3 +159,15 @@ INSERT INTO reaction ( ) VALUES ( :primary-key, :title, :ruleset, :active, :created, :modified ); + +/* JWT Blocklist */ + +-- :ame insert-blocked-jwt! +-- :command :insert +-- :result :affected +-- :doc Given a JWT-extracted `:account-id` and `:expiration`, insert a new blocked JWT. +INSERT INTO blocked_jwt ( + account_id, expiration +) VALUES ( + :account-id, :expiration +); diff --git a/src/db/sqlite/lrsql/sqlite/sql/query.sql b/src/db/sqlite/lrsql/sqlite/sql/query.sql index de0e22b3a..0b3dd0855 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/query.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/query.sql @@ -395,3 +395,13 @@ WITH RECURSIVE trigger_history (statement_id, reaction_id, trigger_id) AS ( SELECT reaction_id FROM trigger_history WHERE reaction_id IS NOT NULL; + +/* JWT Blocklist */ + +-- :name query-blocked-jwt-exists +-- :command :query +-- :result :one +-- :doc Query that at least one blocked JWT with `:account_id` and whose expiration is after `:current-time` exists. +SELECT 1 FROM blocked_jwt +WHERE account_id = :account-id +AND :current-time < expiration; From a682c5f3595bd973b28a03f2c2067efe9b312bcf Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 31 Oct 2024 16:25:00 -0400 Subject: [PATCH 08/61] Add JWT blocklist table to Postgres --- src/db/postgres/lrsql/postgres/sql/ddl.sql | 13 +++++++++++++ src/db/postgres/lrsql/postgres/sql/delete.sql | 16 ++++++++++++++++ src/db/postgres/lrsql/postgres/sql/insert.sql | 12 ++++++++++++ src/db/postgres/lrsql/postgres/sql/query.sql | 10 ++++++++++ 4 files changed, 51 insertions(+) diff --git a/src/db/postgres/lrsql/postgres/sql/ddl.sql b/src/db/postgres/lrsql/postgres/sql/ddl.sql index 7a7f5daba..4306c38a8 100644 --- a/src/db/postgres/lrsql/postgres/sql/ddl.sql +++ b/src/db/postgres/lrsql/postgres/sql/ddl.sql @@ -488,3 +488,16 @@ ALTER TABLE lrs_credential ALTER COLUMN secret_key TYPE TEXT; ALTER TABLE credential_to_scope ALTER COLUMN api_key TYPE TEXT; ALTER TABLE credential_to_scope ALTER COLUMN secret_key TYPE TEXT; ALTER TABLE reaction ALTER COLUMN title TYPE TEXT; + +/* Migration 2024-10-31 - Add JWT Blocklist Table */ + +-- :name create-blocked-jwt-table! +-- :command :execute +-- :doc Create the `blocked_jwt` table and associated indexes if they do not exist yet. +CREATE TABLE IF NOT EXISTS blocked_jwt ( + account_id TEXT NOT NULL REFERENCES admin_account(id) ON DELETE CASCADE, + expiration TIMESTAMP, + PRIMARY KEY (account_id, expiration) +); +CREATE INDEX IF NOT EXISTS blocked_jwt_account_id_idx ON blocked_jwt(account_id); +CREATE INDEX IF NOT EXISTS blocked_jwt_expiration_idx ON blocked_jwt(expiration); diff --git a/src/db/postgres/lrsql/postgres/sql/delete.sql b/src/db/postgres/lrsql/postgres/sql/delete.sql index e0ff8a8b7..5d995e26f 100644 --- a/src/db/postgres/lrsql/postgres/sql/delete.sql +++ b/src/db/postgres/lrsql/postgres/sql/delete.sql @@ -108,3 +108,19 @@ DELETE FROM state_document WHERE agent_ifi = :actor-ifi; DELETE FROM actor WHERE actor_ifi = :actor-ifi; ------------------end delete-actor-------------------- + +/* JWT Blocklist */ + +-- :name delete-blocked-jwt-by-account! +-- :command :execute +-- :result :affected +-- :doc Delete all blocked JWTs associated with a given `:account-id`. +DELETE FROM blocked_jwt +WHERE account_id = :account-id; + +-- :name :delete-blocked-jwt-by-time! +-- :command :execute +-- :result :affected +-- :doc Delete all blocked JWTs where `:current-time` is past the expiration time. +DELETE FROM blocked_jwt +WHERE expiration <= :current-time; diff --git a/src/db/postgres/lrsql/postgres/sql/insert.sql b/src/db/postgres/lrsql/postgres/sql/insert.sql index a9822e517..44c6313ee 100644 --- a/src/db/postgres/lrsql/postgres/sql/insert.sql +++ b/src/db/postgres/lrsql/postgres/sql/insert.sql @@ -159,3 +159,15 @@ INSERT INTO reaction ( ) VALUES ( :primary-key, :title, :ruleset, :active, :created, :modified ); + +/* JWT Blocklist */ + +-- :ame insert-blocked-jwt! +-- :command :insert +-- :result :affected +-- :doc Given a JWT-extracted `:account-id` and `:expiration`, insert a new blocked JWT. +INSERT INTO blocked_jwt ( + account_id, expiration +) VALUES ( + :account-id, :expiration +); diff --git a/src/db/postgres/lrsql/postgres/sql/query.sql b/src/db/postgres/lrsql/postgres/sql/query.sql index f1b8f6a71..6c0fa4f5a 100644 --- a/src/db/postgres/lrsql/postgres/sql/query.sql +++ b/src/db/postgres/lrsql/postgres/sql/query.sql @@ -427,3 +427,13 @@ WITH RECURSIVE trigger_history (statement_id, reaction_id, trigger_id) AS ( SELECT reaction_id FROM trigger_history WHERE reaction_id IS NOT NULL; + +/* JWT Blocklist */ + +-- :name query-blocked-jwt-exists +-- :command :query +-- :result :one +-- :doc Query that at least one blocked JWT with `:username` and whose expiration is after `:current-time` exists. +SELECT 1 FROM blocked_jwt +WHERE account_id = :account-id +AND :current-time < expiration; From ce7ee635fb5e897e10823cbded0e3c4793293c52 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 31 Oct 2024 16:27:54 -0400 Subject: [PATCH 09/61] Add and implement backend protocol --- src/db/postgres/lrsql/postgres/record.clj | 13 ++++++++++++- src/db/sqlite/lrsql/sqlite/record.clj | 13 +++++++++++++ src/main/lrsql/backend/protocol.clj | 8 ++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/db/postgres/lrsql/postgres/record.clj b/src/db/postgres/lrsql/postgres/record.clj index 16e8a6f74..33950e12b 100644 --- a/src/db/postgres/lrsql/postgres/record.clj +++ b/src/db/postgres/lrsql/postgres/record.clj @@ -76,7 +76,8 @@ (when (nil? (check-statement-to-actor-cascading-delete tx)) (add-statement-to-actor-cascading-delete! tx)) (when (some? (query-varchar-exists tx)) - (convert-varchars-to-text! tx))) + (convert-varchars-to-text! tx)) + (create-blocked-jwt-table! tx)) bp/BackendUtil (-txn-retry? [_ ex] @@ -197,6 +198,16 @@ (-query-account-count-local [_ tx] (query-account-count-local tx)) + bp/JWTBlocklistBackend + (-insert-blocked-jwt! [_ tx input] + (insert-blocked-jwt! tx input)) + (-delete-blocked-jwt-by-account! [_ tx input] + (delete-blocked-jwt-by-account! tx input)) + (-delete-blocked-jwt-by-time! [_ tx input] + (delete-blocked-jwt-by-time! tx input)) + (-query-blocked-jwt [_ tx input] + (query-blocked-jwt-exists tx input)) + bp/CredentialBackend (-insert-credential! [_ tx input] (insert-credential! tx input)) diff --git a/src/db/sqlite/lrsql/sqlite/record.clj b/src/db/sqlite/lrsql/sqlite/record.clj index 515368559..cfce3d4b0 100644 --- a/src/db/sqlite/lrsql/sqlite/record.clj +++ b/src/db/sqlite/lrsql/sqlite/record.clj @@ -112,6 +112,9 @@ (xapi-statement-add-trigger-id! tx)) (when-not (some? (query-statement-to-actor-has-cascade-delete tx)) (update-schema-simple! tx alter-statement-to-actor-add-cascade-delete!)) + (create-blocked-jwt-table! tx) + (create-blocked-jwt-username-idx! tx) + (create-blocked-jwt-expiration-idx! tx) (log/infof "sqlite schema_version: %d" (:schema_version (query-schema-version tx)))) @@ -237,6 +240,16 @@ (-query-account-count-local [_ tx] (query-account-count-local tx)) + bp/JWTBlocklistBackend + (-insert-blocked-jwt! [_ tx input] + (insert-blocked-jwt! tx input)) + (-delete-blocked-jwt-by-account! [_ tx input] + (delete-blocked-jwt-by-account! tx input)) + (-delete-blocked-jwt-by-time! [_ tx input] + (delete-blocked-jwt-by-time! tx input)) + (-query-blocked-jwt [_ tx input] + (query-blocked-jwt-exists tx input)) + bp/CredentialBackend (-insert-credential! [_ tx input] (insert-credential! tx input)) diff --git a/src/main/lrsql/backend/protocol.clj b/src/main/lrsql/backend/protocol.clj index 1c95aba36..a93c71941 100644 --- a/src/main/lrsql/backend/protocol.clj +++ b/src/main/lrsql/backend/protocol.clj @@ -101,6 +101,14 @@ (-query-all-admin-accounts [this tx]) (-query-account-count-local [this tx])) +(defprotocol JWTBlocklistBackend + ;; Commands + (-insert-blocked-jwt! [this tx input]) + (-delete-blocked-jwt-by-account! [this tx input]) + (-delete-blocked-jwt-by-time! [this tx input]) + ;; Queries + (-query-blocked-jwt [this tx input])) + (defprotocol CredentialBackend ;; Commands (-insert-credential! [this tx input]) From 877ca9d7131121430f23a950d84b47290813fad3 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 31 Oct 2024 17:41:38 -0400 Subject: [PATCH 10/61] Fix typo in HugSQL code --- src/db/postgres/lrsql/postgres/sql/delete.sql | 2 +- src/db/postgres/lrsql/postgres/sql/insert.sql | 2 +- src/db/sqlite/lrsql/sqlite/record.clj | 2 +- src/db/sqlite/lrsql/sqlite/sql/delete.sql | 2 +- src/db/sqlite/lrsql/sqlite/sql/insert.sql | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/db/postgres/lrsql/postgres/sql/delete.sql b/src/db/postgres/lrsql/postgres/sql/delete.sql index 5d995e26f..b7417476d 100644 --- a/src/db/postgres/lrsql/postgres/sql/delete.sql +++ b/src/db/postgres/lrsql/postgres/sql/delete.sql @@ -118,7 +118,7 @@ DELETE FROM actor WHERE actor_ifi = :actor-ifi; DELETE FROM blocked_jwt WHERE account_id = :account-id; --- :name :delete-blocked-jwt-by-time! +-- :name delete-blocked-jwt-by-time! -- :command :execute -- :result :affected -- :doc Delete all blocked JWTs where `:current-time` is past the expiration time. diff --git a/src/db/postgres/lrsql/postgres/sql/insert.sql b/src/db/postgres/lrsql/postgres/sql/insert.sql index 44c6313ee..68dc6ca8c 100644 --- a/src/db/postgres/lrsql/postgres/sql/insert.sql +++ b/src/db/postgres/lrsql/postgres/sql/insert.sql @@ -162,7 +162,7 @@ INSERT INTO reaction ( /* JWT Blocklist */ --- :ame insert-blocked-jwt! +-- :name insert-blocked-jwt! -- :command :insert -- :result :affected -- :doc Given a JWT-extracted `:account-id` and `:expiration`, insert a new blocked JWT. diff --git a/src/db/sqlite/lrsql/sqlite/record.clj b/src/db/sqlite/lrsql/sqlite/record.clj index cfce3d4b0..f7c2823cc 100644 --- a/src/db/sqlite/lrsql/sqlite/record.clj +++ b/src/db/sqlite/lrsql/sqlite/record.clj @@ -113,7 +113,7 @@ (when-not (some? (query-statement-to-actor-has-cascade-delete tx)) (update-schema-simple! tx alter-statement-to-actor-add-cascade-delete!)) (create-blocked-jwt-table! tx) - (create-blocked-jwt-username-idx! tx) + (create-blocked-jwt-account-id-idx! tx) (create-blocked-jwt-expiration-idx! tx) (log/infof "sqlite schema_version: %d" (:schema_version (query-schema-version tx)))) diff --git a/src/db/sqlite/lrsql/sqlite/sql/delete.sql b/src/db/sqlite/lrsql/sqlite/sql/delete.sql index f8c96bcb0..2884e93a3 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/delete.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/delete.sql @@ -133,7 +133,7 @@ DELETE FROM actor WHERE actor_ifi = :actor-ifi DELETE FROM blocked_jwt WHERE account_id = :account-id; --- :name :delete-blocked-jwt-by-time! +-- :name delete-blocked-jwt-by-time! -- :command :execute -- :result :affected -- :doc Delete all blocked JWTs where `:current-time` is past the expiration time. diff --git a/src/db/sqlite/lrsql/sqlite/sql/insert.sql b/src/db/sqlite/lrsql/sqlite/sql/insert.sql index 533e20e5f..edbd40a47 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/insert.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/insert.sql @@ -162,7 +162,7 @@ INSERT INTO reaction ( /* JWT Blocklist */ --- :ame insert-blocked-jwt! +-- :name insert-blocked-jwt! -- :command :insert -- :result :affected -- :doc Given a JWT-extracted `:account-id` and `:expiration`, insert a new blocked JWT. From 01674d0304ea7936ab4ddebcd9422048f151c8b3 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 31 Oct 2024 18:01:14 -0400 Subject: [PATCH 11/61] JWT op specs --- src/main/lrsql/spec/admin/jwt.clj | 46 +++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 src/main/lrsql/spec/admin/jwt.clj diff --git a/src/main/lrsql/spec/admin/jwt.clj b/src/main/lrsql/spec/admin/jwt.clj new file mode 100644 index 000000000..1130e3aac --- /dev/null +++ b/src/main/lrsql/spec/admin/jwt.clj @@ -0,0 +1,46 @@ +(ns lrsql.spec.admin.jwt + (:require [clojure.spec.alpha :as s] + [lrsql.backend.protocol :as bp] + [lrsql.spec.common :as c])) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Interface +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defn admin-jwt-backend? + [bk] + (satisfies? bp/JWTBlocklistBackend bk)) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Inputs +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(s/def ::account-id :lrsql.spec.admin/account-id) +(s/def ::current-time c/instant-spec) +(s/def ::expiration c/instant-spec) + +(def query-blocked-jwt-input-spec + (s/keys :req-un [::account-id + ::current-time])) + +(def insert-blocked-jwt-input-spec + (s/keys :req-un [::account-id + ::expiration])) + +(def delete-blocked-jwt-account-input-spec + (s/keys :req-un [::account-id])) + +(def delete-blocked-jwt-time-input-spec + (s/keys :req-un [::current-time])) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Results +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(s/def :lrsql.spec.admin.jwt.command/result ::account-id) + +(def blocked-jwt-op-result-spec + (s/keys :req-un [:lrsql.spec.admin.jwt.command/result])) + +(def blocked-jwt-query-result-spec + boolean?) From 02f31556d17ae424b8cc4697090b4353e223fb66 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 31 Oct 2024 18:01:22 -0400 Subject: [PATCH 12/61] JWT op inputs --- src/main/lrsql/input/admin/jwt.clj | 35 ++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 src/main/lrsql/input/admin/jwt.clj diff --git a/src/main/lrsql/input/admin/jwt.clj b/src/main/lrsql/input/admin/jwt.clj new file mode 100644 index 000000000..ca5d52371 --- /dev/null +++ b/src/main/lrsql/input/admin/jwt.clj @@ -0,0 +1,35 @@ +(ns lrsql.input.admin.jwt + (:require [clojure.spec.alpha :as s] + [lrsql.spec.admin.jwt :as jwts] + [lrsql.util :as u])) + +(s/fdef query-blocked-jwt-input + :args (s/cat :account-id ::jwts/account-id) + :ret jwts/query-blocked-jwt-input-spec) + +(defn query-blocked-jwt-input + [account-id] + {:account-id account-id + :current-time (u/current-time)}) + +(s/fdef insert-blocked-jwt-input + :args (s/cat :account-id ::jwts/account-id + :expiration ::jwts/expiration) + :ret (s/and jwts/insert-blocked-jwt-input-spec + jwts/delete-blocked-jwt-time-input-spec)) + +(defn insert-blocked-jwt-input + [account-id expiration] + {:account-id account-id + :expiration expiration + :current-time (u/current-time)}) + +(s/fdef delete-blocked-jwts-input + :args (s/cat :account-id ::jwts/account-id) + :ret (s/and jwts/delete-blocked-jwt-account-input-spec + jwts/delete-blocked-jwt-time-input-spec)) + +(defn delete-blocked-jwts-input + [account-id] + {:account-id account-id + :current-time (u/current-time)}) From 95e82d2712889499809677013d6ca478c5490d91 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 31 Oct 2024 18:01:47 -0400 Subject: [PATCH 13/61] JWT ops --- src/main/lrsql/ops/command/admin.clj | 43 +++++++++++++++++++++++++++- src/main/lrsql/ops/query/admin.clj | 20 +++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/main/lrsql/ops/command/admin.clj b/src/main/lrsql/ops/command/admin.clj index 20fa89f4e..538e6f370 100644 --- a/src/main/lrsql/ops/command/admin.clj +++ b/src/main/lrsql/ops/command/admin.clj @@ -3,7 +3,8 @@ [lrsql.backend.protocol :as bp] [lrsql.input.admin :as admin-i] [lrsql.spec.common :refer [transaction?]] - [lrsql.spec.admin :as ads])) + [lrsql.spec.admin :as ads] + [lrsql.spec.admin.jwt :as jwts])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Admin Account Insertion @@ -105,3 +106,43 @@ ensure-input)] (bp/-insert-admin-account-oidc! bk tx insert-input) {:result primary-key}))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Admin JWTs +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(s/fdef purge-blocklist! + :args (s/cat :bk jwts/admin-jwt-backend? + :tx transaction? + :input jwts/delete-blocked-jwt-time-input-spec)) + +(defn purge-blocklist! + "Delete all JWTs from the blocklist that have expired, i.e. whose expirations + are before the `:current-time` in `input`." + [bk tx input] + (bp/-delete-blocked-jwt-by-time! bk tx input)) + +(s/fdef insert-blocked-jwt! + :args (s/cat :bk jwts/admin-jwt-backend? + :tx transaction? + :input jwts/insert-blocked-jwt-input-spec) + :ret jwts/blocked-jwt-op-result-spec) + +(defn insert-blocked-jwt! + "Insert a new JWT `:account-id` and `:expiration` to the blocklist table." + [bk tx input] + (println input) + (println (bp/-insert-blocked-jwt! bk tx input)) + {:result (:account-id input)}) + +(s/fdef delete-blocked-jwts! + :args (s/cat :bk jwts/admin-jwt-backend? + :tx transaction? + :input jwts/delete-blocked-jwt-account-input-spec) + :ret jwts/blocked-jwt-op-result-spec) + +(defn delete-blocked-jwts! + "Delete all JWTs associated with `:account-id` from the blocklist." + [bk tx input] + (bp/-delete-blocked-jwt-by-account! bk tx input) + {:result (:account-id input)}) diff --git a/src/main/lrsql/ops/query/admin.clj b/src/main/lrsql/ops/query/admin.clj index af6078934..c6df82623 100644 --- a/src/main/lrsql/ops/query/admin.clj +++ b/src/main/lrsql/ops/query/admin.clj @@ -3,6 +3,7 @@ [lrsql.backend.protocol :as bp] [lrsql.spec.common :refer [transaction?]] [lrsql.spec.admin :as ads] + [lrsql.spec.admin.jwt :as jwts] [lrsql.spec.admin.status :as ss] [lrsql.util :as u] [lrsql.util.admin :as au])) @@ -70,6 +71,10 @@ :username username}) (bp/-query-all-admin-accounts bk tx))) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Admin LRS Status +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + (s/fdef query-status :args (s/cat :bk ss/admin-status-backend? :tx transaction? @@ -110,3 +115,18 @@ {:stored (u/pad-time-str stored_time) :count scount}) (bp/-query-timeline bk tx (:timeline params)))))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Admin JWTs +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(s/fdef query-blocked-jwt-exists + :args (s/cat :bk jwts/admin-jwt-backend? + :tx transaction? + :input jwts/query-blocked-jwt-input-spec) + :ret jwts/blocked-jwt-query-result-spec) + +(defn query-blocked-jwt-exists + "Query whether an unexpired JWT for the account exists." + [bk tx input] + (boolean (bp/-query-blocked-jwt bk tx input))) From a1ce21049b9e04c58865086f146dcad05a993a18 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 31 Oct 2024 18:02:07 -0400 Subject: [PATCH 14/61] Implement AdminJWTManager protocol --- src/main/lrsql/admin/protocol.clj | 8 ++++++++ src/main/lrsql/system/lrs.clj | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/main/lrsql/admin/protocol.clj b/src/main/lrsql/admin/protocol.clj index 4dbf6766a..3d86ca676 100644 --- a/src/main/lrsql/admin/protocol.clj +++ b/src/main/lrsql/admin/protocol.clj @@ -18,6 +18,14 @@ (-update-admin-password [this account-id old-password new-password] "Update the password for an admin account given old and new passwords.")) +(defprotocol AdminJWTManager + (-block-jwt [this account-id expiration] + "Block the JWT with this account and expiration time. Purge the cache if necessary.") + (-unblock-jwts [this account-id] + "Unblock the JWTs associated with this account.") + (-jwt-blocked? [this account-id] + "Is an unexpired JWT with this account on the blocklist?")) + (defprotocol APIKeyManager (-create-api-keys [this account-id scopes] "Create a new API key pair with the associated scopes.") diff --git a/src/main/lrsql/system/lrs.clj b/src/main/lrsql/system/lrs.clj index e5851824f..5f0185e14 100644 --- a/src/main/lrsql/system/lrs.clj +++ b/src/main/lrsql/system/lrs.clj @@ -12,6 +12,7 @@ [lrsql.input.actor :as agent-input] [lrsql.input.activity :as activity-input] [lrsql.input.admin :as admin-input] + [lrsql.input.admin.jwt :as admin-jwt-input] [lrsql.input.admin.status :as admin-stat-input] [lrsql.input.auth :as auth-input] [lrsql.input.statement :as stmt-input] @@ -304,6 +305,29 @@ account-id new-password)] (admin-cmd/update-admin-password! backend tx input)) {:result result}))))) + + adp/AdminJWTManager + (-block-jwt + [this account-id expiration] + (let [conn (lrs-conn this) + jwt-input (admin-jwt-input/insert-blocked-jwt-input account-id + expiration)] + (jdbc/with-transaction [tx conn] + (admin-cmd/purge-blocklist! backend tx jwt-input) + (admin-cmd/insert-blocked-jwt! backend tx jwt-input)))) + (-unblock-jwts + [this account-id] + (let [conn (lrs-conn this) + jwt-input (admin-jwt-input/delete-blocked-jwts-input account-id)] + (jdbc/with-transaction [tx conn] + (admin-cmd/purge-blocklist! backend tx jwt-input) + (admin-cmd/delete-blocked-jwts! backend tx jwt-input)))) + (-jwt-blocked? + [this account-id] + (let [conn (lrs-conn this) + jwt-input (admin-jwt-input/query-blocked-jwt-input account-id)] + (jdbc/with-transaction [tx conn] + (admin-q/query-blocked-jwt-exists backend tx jwt-input)))) adp/APIKeyManager (-create-api-keys From 0c37dc1aadefff56e95a2e6d1d04f310dfffdd35 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 31 Oct 2024 18:02:23 -0400 Subject: [PATCH 15/61] Add tests for JWT protocol functions --- src/test/lrsql/admin/protocol_test.clj | 16 ++++++++++++++++ src/test/lrsql/input/input_test.clj | 7 +++++++ 2 files changed, 23 insertions(+) diff --git a/src/test/lrsql/admin/protocol_test.clj b/src/test/lrsql/admin/protocol_test.clj index 6ed1bab84..8ad0b467f 100644 --- a/src/test/lrsql/admin/protocol_test.clj +++ b/src/test/lrsql/admin/protocol_test.clj @@ -121,6 +121,22 @@ (is (adp/-existing-account? lrs account-id))) (let [bad-account-id #uuid "00000000-0000-4000-8000-000000000000"] (is (not (adp/-existing-account? lrs bad-account-id))))) + (testing "Admin JWTs" + (let [expiration (u/current-time) + account-id (:result + (adp/-authenticate-account lrs + test-username + test-password))] + (testing "- block" + (is (= account-id + (:result (adp/-block-jwt lrs account-id expiration)))) + (is (true? + (adp/-jwt-blocked? lrs account-id)))) + (testing "- unblock" + (is (= account-id + (:result (adp/-unblock-jwts lrs account-id)))) + (is (false? + (adp/-jwt-blocked? lrs account-id)))))) (testing "Admin password update" (let [account-id (-> (adp/-authenticate-account lrs test-username diff --git a/src/test/lrsql/input/input_test.clj b/src/test/lrsql/input/input_test.clj index 91c43cea5..f8694daed 100644 --- a/src/test/lrsql/input/input_test.clj +++ b/src/test/lrsql/input/input_test.clj @@ -5,6 +5,7 @@ [lrsql.input.activity :as i-av] [lrsql.input.attachment :as i-at] [lrsql.input.admin :as i-admin] + [lrsql.input.admin.jwt :as i-adm-jwt] [lrsql.input.admin.status :as i-adm-stat] [lrsql.input.auth :as i-auth] [lrsql.input.statement :as i-stmt] @@ -57,6 +58,12 @@ (is (nil? (check-validate `i-admin/query-admin-exists-input))) (is (nil? (check-validate `i-admin/delete-admin-input))))) +(deftest test-admin-jwt + (testing "admin JWT inputs" + (is (nil? (check-validate `i-adm-jwt/query-blocked-jwt-input))) + (is (nil? (check-validate `i-adm-jwt/insert-blocked-jwt-input))) + (is (nil? (check-validate `i-adm-jwt/delete-blocked-jwts-input))))) + (deftest test-admin-status (testing "admin status inputs" (is (nil? (check-validate `i-adm-stat/query-timeline-input))) From 8cda065d60f8daf36f45b168c127f43dc414f432 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 09:46:45 -0400 Subject: [PATCH 16/61] Fix protocol tests --- src/main/lrsql/admin/protocol.clj | 4 ++-- src/main/lrsql/ops/command/admin.clj | 3 +-- src/test/lrsql/admin/protocol_test.clj | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/lrsql/admin/protocol.clj b/src/main/lrsql/admin/protocol.clj index 3d86ca676..a581ee5c9 100644 --- a/src/main/lrsql/admin/protocol.clj +++ b/src/main/lrsql/admin/protocol.clj @@ -20,9 +20,9 @@ (defprotocol AdminJWTManager (-block-jwt [this account-id expiration] - "Block the JWT with this account and expiration time. Purge the cache if necessary.") + "Block the JWT with this account and expiration time. Purge expired JWTs from the blocklist if necessary.") (-unblock-jwts [this account-id] - "Unblock the JWTs associated with this account.") + "Unblock the JWTs associated with this account. Purge expired JWTs from the blocklist if necessary.") (-jwt-blocked? [this account-id] "Is an unexpired JWT with this account on the blocklist?")) diff --git a/src/main/lrsql/ops/command/admin.clj b/src/main/lrsql/ops/command/admin.clj index 538e6f370..370f03bd4 100644 --- a/src/main/lrsql/ops/command/admin.clj +++ b/src/main/lrsql/ops/command/admin.clj @@ -131,8 +131,7 @@ (defn insert-blocked-jwt! "Insert a new JWT `:account-id` and `:expiration` to the blocklist table." [bk tx input] - (println input) - (println (bp/-insert-blocked-jwt! bk tx input)) + (bp/-insert-blocked-jwt! bk tx input) {:result (:account-id input)}) (s/fdef delete-blocked-jwts! diff --git a/src/test/lrsql/admin/protocol_test.clj b/src/test/lrsql/admin/protocol_test.clj index 8ad0b467f..6631e2720 100644 --- a/src/test/lrsql/admin/protocol_test.clj +++ b/src/test/lrsql/admin/protocol_test.clj @@ -122,7 +122,7 @@ (let [bad-account-id #uuid "00000000-0000-4000-8000-000000000000"] (is (not (adp/-existing-account? lrs bad-account-id))))) (testing "Admin JWTs" - (let [expiration (u/current-time) + (let [expiration (.plusSeconds (u/current-time) 3600) account-id (:result (adp/-authenticate-account lrs test-username From 2e29d196ba24e8def292099eacc685cea2daa148 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 09:53:00 -0400 Subject: [PATCH 17/61] TEXT -> UUID in Postgres --- src/db/postgres/lrsql/postgres/sql/ddl.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/postgres/lrsql/postgres/sql/ddl.sql b/src/db/postgres/lrsql/postgres/sql/ddl.sql index 4306c38a8..408344f69 100644 --- a/src/db/postgres/lrsql/postgres/sql/ddl.sql +++ b/src/db/postgres/lrsql/postgres/sql/ddl.sql @@ -495,7 +495,7 @@ ALTER TABLE reaction ALTER COLUMN title TYPE TEXT; -- :command :execute -- :doc Create the `blocked_jwt` table and associated indexes if they do not exist yet. CREATE TABLE IF NOT EXISTS blocked_jwt ( - account_id TEXT NOT NULL REFERENCES admin_account(id) ON DELETE CASCADE, + account_id UUID NOT NULL REFERENCES admin_account(id) ON DELETE CASCADE, expiration TIMESTAMP, PRIMARY KEY (account_id, expiration) ); From d01f6fd1bd20552f8242fe0dd729f346670eb2e5 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 10:53:47 -0400 Subject: [PATCH 18/61] Surround all admin route tests with try-finally blocks --- src/test/lrsql/admin/route_test.clj | 265 ++++++++++++++-------------- 1 file changed, 136 insertions(+), 129 deletions(-) diff --git a/src/test/lrsql/admin/route_test.clj b/src/test/lrsql/admin/route_test.clj index 8ec278a5b..943bf1f6a 100644 --- a/src/test/lrsql/admin/route_test.clj +++ b/src/test/lrsql/admin/route_test.clj @@ -497,13 +497,15 @@ sys (support/test-system :conf-overrides hdr-conf) sys' (component/start sys)] - (testing "Custom Sec Headers" - ;; Run a basic admin routes call and verify success - (let [{:keys [headers]} (get-env content-type)] - ;; equals the same combination of custom and default hdr values - (is (= custom-sec-header-expected - (select-keys headers sec-header-names))))) - (component/stop sys'))) + (try + (testing "Custom Sec Headers" + ;; Run a basic admin routes call and verify success + (let [{:keys [headers]} (get-env content-type)] + ;; equals the same combination of custom and default hdr values + (is (= custom-sec-header-expected + (select-keys headers sec-header-names))))) + (finally + (component/stop sys'))))) (def proxy-jwt-body {"usercertificate" "unique.user.1234" @@ -529,24 +531,27 @@ sys' (component/start sys) ;; proxy jwt auth proxy-auth {"Authorization" (str "Bearer " (proxy-jwt proxy-jwt-body))} - headers (merge content-type proxy-auth)] - (testing "Proxy JWT authentication" - ;; Run a basic admin routes call and verify success - (let [{:keys [status body]} (get-account headers) - edn-body (u/parse-json body :object? false)] - ;; 200 response - (is (= status 200)) - ;; not only is there a body but it should now contain our jwt user - (is (some #(= (get % "username") (get proxy-jwt-body "usercertificate")) - edn-body)))) - (testing "Bad Proxy JWT role" - ;; Remove the matching role from jwt role-key field and rerun admin call - (let [bad-jwt-bdy (assoc proxy-jwt-body "group-full" ["NOTADMIN"]) - bad-auth {"Authorization" (str "Bearer " (proxy-jwt bad-jwt-bdy))} - bad-headers (merge content-type bad-auth)] - ;; Bad Auth because nonmatching role - (is-err-code (get-account bad-headers) 401))) - (component/stop sys'))) + headers (merge content-type proxy-auth)] + (try + (testing "Proxy JWT authentication" + ;; Run a basic admin routes call and verify success + (let [{:keys [status body]} (get-account headers) + edn-body (u/parse-json body :object? false)] + ;; 200 response + (is (= status 200)) + ;; not only is there a body but it should now contain our jwt user + (is (some #(= (get % "username") + (get proxy-jwt-body "usercertificate")) + edn-body)))) + (testing "Bad Proxy JWT role" + ;; Remove the matching role from jwt role-key field and rerun admin call + (let [bad-jwt-bdy (assoc proxy-jwt-body "group-full" ["NOTADMIN"]) + bad-auth {"Authorization" (str "Bearer " (proxy-jwt bad-jwt-bdy))} + bad-headers (merge content-type bad-auth)] + ;; Bad Auth because nonmatching role + (is-err-code (get-account bad-headers) 401))) + (finally + (component/stop sys'))))) (deftest auth-routes-test (let [sys (support/test-system) @@ -561,107 +566,109 @@ (get "json-web-token")) auth {"Authorization" (str "Bearer " jwt)} hdr (merge content-type auth)] - (testing "credential creation" - (let [{:keys [status body]} - (curl/post "http://0.0.0.0:8080/admin/creds" - {:headers hdr - :body (u/write-json-str - {"scopes" ["all" "all/read"]})}) - {:strs [api-key secret-key scopes]} - (u/parse-json body)] - (is (= 200 status)) - (is (re-matches Base64RegEx api-key)) - (is (re-matches Base64RegEx secret-key)) - (is (= #{"all" "all/read"} (set scopes))) - (testing "and reading" - (let [{:keys [status body]} - (curl/get - "http://0.0.0.0:8080/admin/creds" - {:headers hdr})] - (is (= 200 status)) - (is (= {"api-key" api-key - "secret-key" secret-key - "scopes" scopes} - (-> body - (u/parse-json :object? false) - (#(filter (fn [cred] (= (get cred "api-key") api-key)) - %)) - first))))) - (testing "and updating" - (let [req-scopes - ["all/read" "statements/read" "statements/read/mine"] - {:keys [status body]} - (curl/put - "http://0.0.0.0:8080/admin/creds" - {:headers hdr - :body (u/write-json-str {"api-key" api-key - "secret-key" secret-key - "scopes" req-scopes})}) - {:strs [scopes]} - (u/parse-json body)] - (is (= 200 status)) - (is (= #{"all/read" "statements/read" "statements/read/mine"} - (set scopes))))) - (testing "and reading after updating" - (let [{:keys [status body]} - (curl/get - "http://0.0.0.0:8080/admin/creds" - {:headers hdr}) - {api-key' "api-key" - secret-key' "secret-key" - scopes' "scopes"} - (-> body - (u/parse-json :object? false) - (#(filter (fn [cred] (= (get cred "api-key") api-key)) %)) - first)] - (is (= 200 status)) - (is (= api-key api-key')) - (is (= secret-key secret-key')) - (is (= #{"all/read" "statements/read" "statements/read/mine"} - (set scopes'))))) - (testing "and no-op scope update" - (let [req-scopes - ["all/read" "statements/read" "statements/read/mine"] - {:keys [status body]} - (curl/put - "http://0.0.0.0:8080/admin/creds" - {:headers hdr - :body (u/write-json-str {"api-key" api-key - "secret-key" secret-key - "scopes" req-scopes})}) - {:strs [scopes]} - (u/parse-json body)] - (is (= 200 status)) - (is (= #{"all/read" "statements/read" "statements/read/mine"} - (set scopes))))) - (testing "and deleting all scopes" - (let [{:keys [status body]} - (curl/put - "http://0.0.0.0:8080/admin/creds" - {:headers hdr - :body (u/write-json-str {"api-key" api-key - "secret-key" secret-key - "scopes" []})}) - {:strs [scopes]} - (u/parse-json body)] - (is (= 200 status)) - (is (= #{} (set scopes))))) - (testing "and deletion" - (let [{:keys [status]} - (curl/delete - "http://0.0.0.0:8080/admin/creds" - {:headers hdr - :body (u/write-json-str {"api-key" api-key - "secret-key" secret-key})})] - (is (= 200 status)))) - (testing "and reading after deletion" - (let [{:keys [status body]} - (curl/get - "http://0.0.0.0:8080/admin/creds" - {:headers hdr}) - edn-res - (u/parse-json body :object? false)] - (is (= 200 status)) - (is (not (some (fn [cred] (= (get cred "api-key") api-key)) - edn-res))))))) - (component/stop sys'))) + (try + (testing "credential creation" + (let [{:keys [status body]} + (curl/post "http://0.0.0.0:8080/admin/creds" + {:headers hdr + :body (u/write-json-str + {"scopes" ["all" "all/read"]})}) + {:strs [api-key secret-key scopes]} + (u/parse-json body)] + (is (= 200 status)) + (is (re-matches Base64RegEx api-key)) + (is (re-matches Base64RegEx secret-key)) + (is (= #{"all" "all/read"} (set scopes))) + (testing "and reading" + (let [{:keys [status body]} + (curl/get + "http://0.0.0.0:8080/admin/creds" + {:headers hdr})] + (is (= 200 status)) + (is (= {"api-key" api-key + "secret-key" secret-key + "scopes" scopes} + (-> body + (u/parse-json :object? false) + (#(filter (fn [cred] (= (get cred "api-key") api-key)) + %)) + first))))) + (testing "and updating" + (let [req-scopes + ["all/read" "statements/read" "statements/read/mine"] + {:keys [status body]} + (curl/put + "http://0.0.0.0:8080/admin/creds" + {:headers hdr + :body (u/write-json-str {"api-key" api-key + "secret-key" secret-key + "scopes" req-scopes})}) + {:strs [scopes]} + (u/parse-json body)] + (is (= 200 status)) + (is (= #{"all/read" "statements/read" "statements/read/mine"} + (set scopes))))) + (testing "and reading after updating" + (let [{:keys [status body]} + (curl/get + "http://0.0.0.0:8080/admin/creds" + {:headers hdr}) + {api-key' "api-key" + secret-key' "secret-key" + scopes' "scopes"} + (-> body + (u/parse-json :object? false) + (#(filter (fn [cred] (= (get cred "api-key") api-key)) %)) + first)] + (is (= 200 status)) + (is (= api-key api-key')) + (is (= secret-key secret-key')) + (is (= #{"all/read" "statements/read" "statements/read/mine"} + (set scopes'))))) + (testing "and no-op scope update" + (let [req-scopes + ["all/read" "statements/read" "statements/read/mine"] + {:keys [status body]} + (curl/put + "http://0.0.0.0:8080/admin/creds" + {:headers hdr + :body (u/write-json-str {"api-key" api-key + "secret-key" secret-key + "scopes" req-scopes})}) + {:strs [scopes]} + (u/parse-json body)] + (is (= 200 status)) + (is (= #{"all/read" "statements/read" "statements/read/mine"} + (set scopes))))) + (testing "and deleting all scopes" + (let [{:keys [status body]} + (curl/put + "http://0.0.0.0:8080/admin/creds" + {:headers hdr + :body (u/write-json-str {"api-key" api-key + "secret-key" secret-key + "scopes" []})}) + {:strs [scopes]} + (u/parse-json body)] + (is (= 200 status)) + (is (= #{} (set scopes))))) + (testing "and deletion" + (let [{:keys [status]} + (curl/delete + "http://0.0.0.0:8080/admin/creds" + {:headers hdr + :body (u/write-json-str {"api-key" api-key + "secret-key" secret-key})})] + (is (= 200 status)))) + (testing "and reading after deletion" + (let [{:keys [status body]} + (curl/get + "http://0.0.0.0:8080/admin/creds" + {:headers hdr}) + edn-res + (u/parse-json body :object? false)] + (is (= 200 status)) + (is (not (some (fn [cred] (= (get cred "api-key") api-key)) + edn-res))))))) + (finally + (component/stop sys'))))) From d58999dac8ebfa93523d48baba46dbd6bd436c85 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 11:40:40 -0400 Subject: [PATCH 19/61] Refactor JWT validation + add blocklist check --- src/main/lrsql/admin/interceptors/jwt.clj | 52 +++++++++++++++-------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/jwt.clj b/src/main/lrsql/admin/interceptors/jwt.clj index 849ff00e7..7b0f4d882 100644 --- a/src/main/lrsql/admin/interceptors/jwt.clj +++ b/src/main/lrsql/admin/interceptors/jwt.clj @@ -8,12 +8,43 @@ ;; NOTE: These interceptors are specifically for JWT validation. ;; For JWT generation see `account/generate-jwt`. +(defn- validate-no-val-jwt + "Proxy JWT, decode JWT without validation and ensure the account is valid." + [lrs token {:keys [no-val-uname no-val-issuer no-val-role-key + no-val-role]}] + (try + (let [{:keys [issuer username] :as result} + (admin-u/proxy-jwt->username-and-issuer + token no-val-uname no-val-issuer no-val-role-key + no-val-role)] + (if (some? username) + (:result (adp/-ensure-account-oidc lrs username issuer)) + result)) + (catch Exception ex + ;; We want any error here to return a 401, but we log + (log/warnf ex "No-val JWT Error: %s" (ex-message ex)) + :lrsql.admin/unauthorized-token-error))) + +(defn- validate-jwt* + "Normal JWT, normal signature verification and blocklist check." + [lrs token secret leeway] + (try + (let [account-id-or-err (admin-u/jwt->account-id token secret leeway)] + (if (uuid? account-id-or-err) + (if-not (adp/-jwt-blocked? lrs account-id-or-err) + account-id-or-err + :lrsql.admin/unauthorized-token-error) + account-id-or-err)) + (catch Exception ex + ;; We want any error here to return a 401, but we log + (log/warnf ex "Unexpected JWT Error: %s" (ex-message ex)) + :lrsql.admin/unauthorized-token-error))) + (defn validate-jwt "Validate that the header JWT is valid (e.g. not expired and signed properly). If no-val? is true run an entirely separate decoding that gets the username and issuer claims, verifies a role and ensures the account if necessary." - [secret leeway {:keys [no-val? no-val-uname no-val-issuer no-val-role-key - no-val-role]}] + [secret leeway {:keys [no-val?] :as no-val-opts}] (interceptor {:name ::validate-jwt :enter @@ -23,21 +54,8 @@ (get-in [:request :headers "authorization"]) admin-u/header->jwt) result (if no-val? - ;; decode jwt w/o validation and ensure account - (try - (let [{:keys [issuer username] :as result} - (admin-u/proxy-jwt->username-and-issuer - token no-val-uname no-val-issuer no-val-role-key - no-val-role)] - (if (some? username) - (:result (adp/-ensure-account-oidc lrs username issuer)) - result)) - (catch Exception ex - ;; We want any error here to return a 401, but we log - (log/warnf ex "No-val JWT Error: %s" (ex-message ex)) - :lrsql.admin/unauthorized-token-error)) - ;; normal jwt, check signature etc - (admin-u/jwt->account-id token secret leeway))] + (validate-no-val-jwt lrs token no-val-opts) + (validate-jwt* lrs token secret leeway))] (cond ;; Success - assoc the account ID as an intermediate value (uuid? result) From 970732c74afab4292974f11dcbda97c931cce1c7 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 11:59:01 -0400 Subject: [PATCH 20/61] Implement loguout route + interceptors --- src/main/lrsql/admin/interceptors/account.clj | 37 +++++++++++++++++++ src/main/lrsql/admin/routes.clj | 14 +++++++ 2 files changed, 51 insertions(+) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 71e821dd2..1729a9abf 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -123,6 +123,17 @@ {:status 401 :body {:error "Invalid Account Credentials"}}))))})) +(def unblock-admin-jwts + "Remove all JWTs associated with the user account from the blocklist." + (interceptor + {:name ::remove-jwt-from-blocklist + :enter + (fn remove-jwt-from-blocklist [ctx] + (let [{lrs :com.yetanalytics/lrs + {:keys [account-id]} ::data} ctx] + (adp/-unblock-jwts lrs account-id) + ctx))})) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Terminal Interceptors ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -250,6 +261,8 @@ :response {:status 200 :body result})))})) +;; JWT interceptors for admin + (defn generate-jwt "Upon account login, generate a new JSON web token." [secret exp] @@ -266,3 +279,27 @@ {:status 200 :body {:account-id account-id :json-web-token json-web-token}})))})) + +(def ^:private block-admin-jwt-error-msg + "This operation is unsupported when `LRSQL_JWT_NO_VAL` is set to `true`.") + +(defn block-admin-jwt + "Add the current JWT to the blocklist. Return an error if we are in + no-val mode." + [no-val?] + (interceptor + {:name ::add-jwt-to-blocklist + :enter + (fn add-jwt-to-blocklist [ctx] + (if-not no-val? + (let [{lrs :com.yetanalytics/lrs + {:keys [account-id expiration]} ::data} ctx] + (adp/-block-jwt lrs account-id expiration) + (assoc (chain/terminate ctx) + :response + {:status 200 + :body {:account-id account-id}})) + (assoc (chain/terminate ctx) + :response + {:status 400 + :body {:error block-admin-jwt-error-msg}})))})) diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 170d9022e..d86689ffe 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -40,6 +40,7 @@ (ai/validate-params :strict? false) ai/authenticate-admin + ai/unblock-admin-jwts (ai/generate-jwt jwt-secret jwt-exp)) :route-name :lrsql.admin.account/login] {:description "Log into an existing account" @@ -51,6 +52,19 @@ :json-web-token :t#string})) 400 (g/rref :error-400) 401 (g/rref :error-401)}}) + (gc/annotate + ["/admin/account/logout" :post (conj common-interceptors + (ji/validate-jwt + jwt-secret jwt-leeway no-val-opts) + ji/validate-jwt-account + (ai/block-admin-jwt (:no-val? no-val-opts))) + :route-name :lrsql.admin.account/logout] + {:description "Log out of this account" + :operationId :logout + :responses {200 (g/response "Account ID" + (gs/o {:account-id :t#string})) + 400 (g/rref :error-400) + 401 (g/rref :error-401)}}) ;; Create new account (gc/annotate ["/admin/account/create" :post (conj common-interceptors From 5f04099485af7f3c8c32d2f31006971bdd43b556 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 11:59:07 -0400 Subject: [PATCH 21/61] Add logout tests --- src/test/lrsql/admin/route_test.clj | 27 ++++++++++++++++++++++++++- src/test/lrsql/util/admin_test.clj | 10 +++++----- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/test/lrsql/admin/route_test.clj b/src/test/lrsql/admin/route_test.clj index 943bf1f6a..76e52c3b0 100644 --- a/src/test/lrsql/admin/route_test.clj +++ b/src/test/lrsql/admin/route_test.clj @@ -52,6 +52,11 @@ {:headers headers :body body})) +(defn- logout-account + [headers] + (curl/post "http://0.0.0.0:8080/admin/account/logout" + {:headers headers})) + (defn- create-account [headers body & {:keys [throw] :or {throw true}}] @@ -470,6 +475,22 @@ (delete-actor headers (u/write-json-str {"actor-ifi" ifi})) (is (zero? (count-by-id (stmt-0 "id")))))) + ;; Need to do this last since this overrides `headers` + (testing "log out then log back into the admin account" + ;; Logout + (let [{:keys [status]} (logout-account headers)] + (is (= 200 status))) + (let [{:keys [status]} (get-me headers)] + (is (= 401 status))) + ;; Login + (let [{:keys [status body]} (login-account content-type seed-body) + new-jwt (-> body + u/parse-json + (get "json-web-token")) + new-auth {"Authorization" (str "Bearer " new-jwt)} + headers (merge content-type new-auth)] + (is (= 200 status)) + (is (= 200 (:status (get-me headers)))))) (finally (component/stop sys'))))) @@ -542,7 +563,11 @@ ;; not only is there a body but it should now contain our jwt user (is (some #(= (get % "username") (get proxy-jwt-body "usercertificate")) - edn-body)))) + edn-body)) + ;; get-me route still works + (is (= 200 (:status (get-me headers)))) + ;; logout fails in this mode + (is-err-code (logout-account headers) 400))) (testing "Bad Proxy JWT role" ;; Remove the matching role from jwt role-key field and rerun admin call (let [bad-jwt-bdy (assoc proxy-jwt-body "group-full" ["NOTADMIN"]) diff --git a/src/test/lrsql/util/admin_test.clj b/src/test/lrsql/util/admin_test.clj index fa394abe0..486fe9937 100644 --- a/src/test/lrsql/util/admin_test.clj +++ b/src/test/lrsql/util/admin_test.clj @@ -16,16 +16,16 @@ (is (= test-id (-> test-id (ua/account-id->jwt "secret" 3600) - (ua/jwt->account-id "secret" 1)))) + (ua/jwt->payload "secret" 1)))) (is (= :lrsql.admin/unauthorized-token-error - (ua/jwt->account-id nil "secret" 3600))) + (ua/jwt->payload nil "secret" 3600))) (is (= :lrsql.admin/unauthorized-token-error - (ua/jwt->account-id "not-a-jwt" "secret" 3600))) + (ua/jwt->payload "not-a-jwt" "secret" 3600))) (is (= :lrsql.admin/unauthorized-token-error (-> test-id (ua/account-id->jwt "secret" 3600) - (ua/jwt->account-id "different-secret" 1)))) + (ua/jwt->payload "different-secret" 1)))) (is (= :lrsql.admin/unauthorized-token-error (let [tok (ua/account-id->jwt test-id "secret" 1) _ (Thread/sleep 1001)] - (ua/jwt->account-id tok "secret" 0))))))) + (ua/jwt->payload tok "secret" 0))))))) From db96757bfdf3f8e4970aa59aea88f4c9309259e1 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 12:09:15 -0400 Subject: [PATCH 22/61] Undo anomalous fn renaming --- src/test/lrsql/util/admin_test.clj | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/lrsql/util/admin_test.clj b/src/test/lrsql/util/admin_test.clj index 486fe9937..fa394abe0 100644 --- a/src/test/lrsql/util/admin_test.clj +++ b/src/test/lrsql/util/admin_test.clj @@ -16,16 +16,16 @@ (is (= test-id (-> test-id (ua/account-id->jwt "secret" 3600) - (ua/jwt->payload "secret" 1)))) + (ua/jwt->account-id "secret" 1)))) (is (= :lrsql.admin/unauthorized-token-error - (ua/jwt->payload nil "secret" 3600))) + (ua/jwt->account-id nil "secret" 3600))) (is (= :lrsql.admin/unauthorized-token-error - (ua/jwt->payload "not-a-jwt" "secret" 3600))) + (ua/jwt->account-id "not-a-jwt" "secret" 3600))) (is (= :lrsql.admin/unauthorized-token-error (-> test-id (ua/account-id->jwt "secret" 3600) - (ua/jwt->payload "different-secret" 1)))) + (ua/jwt->account-id "different-secret" 1)))) (is (= :lrsql.admin/unauthorized-token-error (let [tok (ua/account-id->jwt test-id "secret" 1) _ (Thread/sleep 1001)] - (ua/jwt->payload tok "secret" 0))))))) + (ua/jwt->account-id tok "secret" 0))))))) From 16553915b1511721a1afe7510b5ac107c1a2aab5 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 12:46:59 -0400 Subject: [PATCH 23/61] Add expiration to ctx jwt data --- src/main/lrsql/admin/interceptors/account.clj | 4 ++- src/main/lrsql/admin/interceptors/jwt.clj | 36 +++++++++++-------- src/main/lrsql/util.clj | 10 ++++++ src/main/lrsql/util/admin.clj | 36 ++++++++++--------- src/test/lrsql/admin/route_test.clj | 3 +- src/test/lrsql/util/admin_test.clj | 16 ++++++--- 6 files changed, 66 insertions(+), 39 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 1729a9abf..e5b034f13 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -293,7 +293,9 @@ (fn add-jwt-to-blocklist [ctx] (if-not no-val? (let [{lrs :com.yetanalytics/lrs - {:keys [account-id expiration]} ::data} ctx] + {:keys [account-id expiration]} + :lrsql.admin.interceptors.jwt/data} + ctx] (adp/-block-jwt lrs account-id expiration) (assoc (chain/terminate ctx) :response diff --git a/src/main/lrsql/admin/interceptors/jwt.clj b/src/main/lrsql/admin/interceptors/jwt.clj index 7b0f4d882..79b799cfa 100644 --- a/src/main/lrsql/admin/interceptors/jwt.clj +++ b/src/main/lrsql/admin/interceptors/jwt.clj @@ -14,12 +14,17 @@ no-val-role]}] (try (let [{:keys [issuer username] :as result} - (admin-u/proxy-jwt->username-and-issuer - token no-val-uname no-val-issuer no-val-role-key - no-val-role)] - (if (some? username) - (:result (adp/-ensure-account-oidc lrs username issuer)) - result)) + (admin-u/proxy-jwt->payload token + no-val-uname + no-val-issuer + no-val-role-key + no-val-role)] + (if (keyword? result) + result + (let [{result* :result} (adp/-ensure-account-oidc lrs username issuer)] + (if (keyword? result*) + result* + (assoc result :account-id result*))))) (catch Exception ex ;; We want any error here to return a 401, but we log (log/warnf ex "No-val JWT Error: %s" (ex-message ex)) @@ -29,12 +34,13 @@ "Normal JWT, normal signature verification and blocklist check." [lrs token secret leeway] (try - (let [account-id-or-err (admin-u/jwt->account-id token secret leeway)] - (if (uuid? account-id-or-err) - (if-not (adp/-jwt-blocked? lrs account-id-or-err) - account-id-or-err - :lrsql.admin/unauthorized-token-error) - account-id-or-err)) + (let [{:keys [account-id] :as result} + (admin-u/jwt->payload token secret leeway)] + (if (keyword? result) + result + (if-not (adp/-jwt-blocked? lrs account-id) + result + :lrsql.admin/unauthorized-token-error))) (catch Exception ex ;; We want any error here to return a 401, but we log (log/warnf ex "Unexpected JWT Error: %s" (ex-message ex)) @@ -58,10 +64,10 @@ (validate-jwt* lrs token secret leeway))] (cond ;; Success - assoc the account ID as an intermediate value - (uuid? result) + (map? result) (-> ctx - (assoc-in [::data :account-id] result) - (assoc-in [:request :session ::data :account-id] result)) + (assoc-in [::data] result) + (assoc-in [:request :session ::data] result)) ;; Problem with the non-validated account ensure (= :lrsql.admin/oidc-issuer-mismatch-error result) (assoc (chain/terminate ctx) diff --git a/src/main/lrsql/util.clj b/src/main/lrsql/util.clj index b3e27aa7e..25e874ffc 100644 --- a/src/main/lrsql/util.clj +++ b/src/main/lrsql/util.clj @@ -85,6 +85,16 @@ jt/offset-date-time jt/instant))) +(s/fdef millis->time + :args (s/cat :ts-millis nat-int?) + :ret instant-spec) + +(defn millis->time + "Convert a number representing the number of milliseconds since January 1, + 1970 to a java.util.Instant timestamp." + [ts-millis] + (jt/instant ts-millis)) + (s/fdef time->str :args (s/cat :ts instant-spec) :ret ::xs/timestamp) diff --git a/src/main/lrsql/util/admin.clj b/src/main/lrsql/util/admin.clj index 1cdb86084..1e258afcc 100644 --- a/src/main/lrsql/util/admin.clj +++ b/src/main/lrsql/util/admin.clj @@ -46,38 +46,42 @@ (when auth-header (second (re-matches #"Bearer\s+(.*)" auth-header)))) -(defn jwt->account-id - "Given the JSON Web Token `tok`, return the account ID if valid. - Otherwise return `:lrsql.admin/unauthorized-token-error`. +(defn jwt->payload + "Given the JSON Web Token `tok`, unsign and verify the token using `secret`. + Return a map of `:account-id` and `:expiration` if valid, otherwise return + `:lrsql.admin/unauthorized-token-error`. `leeway` is a time amount (in seconds) provided to compensate for clock drift." [tok secret leeway] (if tok ; Avoid encountering a null pointer exception (try - (-> tok (bj/unsign secret {:leeway leeway}) :acc u/str->uuid) + (let [{:keys [acc exp]} (bj/unsign tok secret {:leeway leeway})] + {:account-id (u/str->uuid acc) + :expiration (u/millis->time (* 1000 exp))}) (catch clojure.lang.ExceptionInfo _ :lrsql.admin/unauthorized-token-error)) :lrsql.admin/unauthorized-token-error)) -(defn proxy-jwt->username-and-issuer - "Decode (without validating!) a JWT claim and get the username, issuer, - and verify that the role-key on the claim contains the expected role." +(defn proxy-jwt->payload + "Decode (without validating!) a JWT claim and verify that the role-key on + the claim contains the expected role. Return a map containing `:username` + and `:issuer` iv valid, otherwise return + `:lrsql.admin/unauthorized-token-error`." [tok uname-key issuer-key role-key role] (if tok (try - (let [body - (-> tok - (split #"\.") - second - u/base64encoded-str->str - u/parse-json) - roles (get body role-key) + (let [payload (-> tok + (split #"\.") + second + u/base64encoded-str->str + u/parse-json) + roles (get payload role-key) has-role? (if (coll? roles) (some? ((set roles) role)) (= roles role))] (if has-role? - {:username (get body uname-key) - :issuer (get body issuer-key)} + {:username (get payload uname-key) + :issuer (get payload issuer-key)} :lrsql.admin/unauthorized-token-error)) (catch clojure.lang.ExceptionInfo _ :lrsql.admin/unauthorized-token-error)) diff --git a/src/test/lrsql/admin/route_test.clj b/src/test/lrsql/admin/route_test.clj index 76e52c3b0..f89e77007 100644 --- a/src/test/lrsql/admin/route_test.clj +++ b/src/test/lrsql/admin/route_test.clj @@ -480,8 +480,7 @@ ;; Logout (let [{:keys [status]} (logout-account headers)] (is (= 200 status))) - (let [{:keys [status]} (get-me headers)] - (is (= 401 status))) + (is-err-code (get-me headers) 401) ;; Login (let [{:keys [status body]} (login-account content-type seed-body) new-jwt (-> body diff --git a/src/test/lrsql/util/admin_test.clj b/src/test/lrsql/util/admin_test.clj index fa394abe0..54b5e216c 100644 --- a/src/test/lrsql/util/admin_test.clj +++ b/src/test/lrsql/util/admin_test.clj @@ -16,16 +16,22 @@ (is (= test-id (-> test-id (ua/account-id->jwt "secret" 3600) - (ua/jwt->account-id "secret" 1)))) + (ua/jwt->payload "secret" 1) + :account-id))) + (is (inst? + (-> test-id + (ua/account-id->jwt "secret" 3600) + (ua/jwt->payload "secret" 1) + :expiration))) (is (= :lrsql.admin/unauthorized-token-error - (ua/jwt->account-id nil "secret" 3600))) + (ua/jwt->payload nil "secret" 3600))) (is (= :lrsql.admin/unauthorized-token-error - (ua/jwt->account-id "not-a-jwt" "secret" 3600))) + (ua/jwt->payload "not-a-jwt" "secret" 3600))) (is (= :lrsql.admin/unauthorized-token-error (-> test-id (ua/account-id->jwt "secret" 3600) - (ua/jwt->account-id "different-secret" 1)))) + (ua/jwt->payload "different-secret" 1)))) (is (= :lrsql.admin/unauthorized-token-error (let [tok (ua/account-id->jwt test-id "secret" 1) _ (Thread/sleep 1001)] - (ua/jwt->account-id tok "secret" 0))))))) + (ua/jwt->payload tok "secret" 0))))))) From 3d28d00225223976368cc1989264fa3657de55ec Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 12:51:51 -0400 Subject: [PATCH 24/61] Add logout endpoint to docs --- doc/endpoints.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/endpoints.md b/doc/endpoints.md index 29bbcf557..49f1192d7 100644 --- a/doc/endpoints.md +++ b/doc/endpoints.md @@ -27,6 +27,7 @@ The following examples use `http://example.org` as the URL body. All methods ret * `password` must contain at least one of the following special characters: `!@#$%^&*_-+=?`. The response body contains a newly generated JSON Web Token (JWT) on success. A `401 UNAUTHORIZED` status code is returned if the credentials are incorrect. +- `POST http://example.org/admin/account/logout`: Log out of the current account. This will revoke any unexpired JWTs associated with the user. (NOTE: This endpoint will return a `400 BAD REQUEST` error if `LRSQL_JWT_NO_VAL` is set to `true`.) - `POST http://example.org/admin/account/create`: Create a new admin account. The request body must be a JSON object that contains `username` and `password` strings. The endpoint returns a JSON object with the ID (UUID) of the newly created user on success, and returns a `409 CONFLICT` if the account already exists. - `DELETE http://example.org/admin/account`: Delete an existing account. The JSON request body must contain a UUID `account-id` value. The endpoint returns a JSON object with the ID of the deleted account on success and returns a `404 NOT FOUND` error if the account does not exist. - `GET http://example.org/admin/account`: Return an array of all admin accounts in the system on success. From 170680fb52b33f115d263c94ba34bbd05e3307ac Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 14:29:10 -0400 Subject: [PATCH 25/61] Add comment --- src/main/lrsql/admin/routes.clj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index d86689ffe..0b56dbcf3 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -52,6 +52,7 @@ :json-web-token :t#string})) 400 (g/rref :error-400) 401 (g/rref :error-401)}}) + ;; Log out of current account (gc/annotate ["/admin/account/logout" :post (conj common-interceptors (ji/validate-jwt From f78b912788ce8599e98acf13626d0010e8984d34 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 14:58:29 -0400 Subject: [PATCH 26/61] Add second account to test blocking expired JWTs --- src/test/lrsql/admin/protocol_test.clj | 54 +++++++++++++++++++------- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/src/test/lrsql/admin/protocol_test.clj b/src/test/lrsql/admin/protocol_test.clj index 6631e2720..b28bdcbbd 100644 --- a/src/test/lrsql/admin/protocol_test.clj +++ b/src/test/lrsql/admin/protocol_test.clj @@ -26,6 +26,8 @@ (def test-username "DonaldChamberlin123") ; co-inventor of SQL +(def test-username-2 "MichaelBJones456") ; co-inventor of JWTs + (def test-password "iLoveSqlS0!") ;; Some statement data for status test @@ -98,7 +100,10 @@ uuid?)) (is (-> (adp/-create-account lrs test-username test-password) :result - (= :lrsql.admin/existing-account-error)))) + (= :lrsql.admin/existing-account-error))) + (is (-> (adp/-create-account lrs test-username-2 test-password) + :result + uuid?))) (testing "Admin account get" (let [accounts (adp/-get-accounts lrs)] (is (vector? accounts)) @@ -122,21 +127,36 @@ (let [bad-account-id #uuid "00000000-0000-4000-8000-000000000000"] (is (not (adp/-existing-account? lrs bad-account-id))))) (testing "Admin JWTs" - (let [expiration (.plusSeconds (u/current-time) 3600) - account-id (:result - (adp/-authenticate-account lrs - test-username - test-password))] + (let [expiration-2 (u/current-time) + expiration (u/offset-time expiration-2 3600 :seconds) + account-id (:result + (adp/-authenticate-account lrs + test-username + test-password)) + account-id-2 (:result + (adp/-authenticate-account lrs + test-username-2 + test-password))] (testing "- block" - (is (= account-id + (is (= account-id ; Current JWT (:result (adp/-block-jwt lrs account-id expiration)))) + (is (= account-id-2 ; Expired JWT (need to add after to avoid purge) + (:result (adp/-block-jwt lrs account-id-2 expiration-2)))) (is (true? - (adp/-jwt-blocked? lrs account-id)))) + (adp/-jwt-blocked? lrs account-id))) + (is (false? ; Expired JWT doesn't count as blocked + (adp/-jwt-blocked? lrs account-id-2)))) (testing "- unblock" (is (= account-id (:result (adp/-unblock-jwts lrs account-id)))) (is (false? - (adp/-jwt-blocked? lrs account-id)))))) + (adp/-jwt-blocked? lrs account-id))) + (is (false? + (adp/-jwt-blocked? lrs account-id-2))) + (is (= account-id-2 + (:result (adp/-unblock-jwts lrs account-id-2)))) + (is (false? + (adp/-jwt-blocked? lrs account-id-2)))))) (testing "Admin password update" (let [account-id (-> (adp/-authenticate-account lrs test-username @@ -159,15 +179,23 @@ (adp/-update-admin-password lrs account-id new-password test-password))) (testing "Admin account deletion" - (let [account-id (-> (adp/-authenticate-account lrs - test-username - test-password) - :result)] + (let [account-id (-> (adp/-authenticate-account lrs + test-username + test-password) + :result) + account-id-2 (-> (adp/-authenticate-account lrs + test-username-2 + test-password) + :result)] (testing "When OIDC is off" (let [oidc-enabled? false] (testing "Succeeds if there is more than one account" (adp/-delete-account lrs account-id oidc-enabled?) + (adp/-delete-account lrs account-id-2 oidc-enabled?) (is (-> (adp/-authenticate-account lrs test-username test-password) + :result + (= :lrsql.admin/missing-account-error))) + (is (-> (adp/-authenticate-account lrs test-username-2 test-password) :result (= :lrsql.admin/missing-account-error)))) (testing "Fails if there is only one account" From b05e1ab8eb4037599f2fa5dec299da543b9e2a6e Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 15:23:42 -0400 Subject: [PATCH 27/61] Add new renew endpoint --- src/main/lrsql/admin/routes.clj | 12 ++++++++++++ src/test/lrsql/admin/route_test.clj | 13 +++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 0b56dbcf3..dc3bbca6f 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -66,6 +66,18 @@ (gs/o {:account-id :t#string})) 400 (g/rref :error-400) 401 (g/rref :error-401)}}) + ;; Renew current account JWT to maintain login + (gc/annotate + ["/admin/account/renew" :get (conj common-interceptors + (ji/validate-jwt + jwt-secret jwt-leeway no-val-opts) + ji/validate-jwt-account + (ai/generate-jwt jwt-secret jwt-exp)) + :route-name :lrsql.admin.account/renew] + {:description "Renew current account login" + :operationId :renew + :responses {200 (g/response "Account ID and JWT") + 401 (g/rref :error-401)}}) ;; Create new account (gc/annotate ["/admin/account/create" :post (conj common-interceptors diff --git a/src/test/lrsql/admin/route_test.clj b/src/test/lrsql/admin/route_test.clj index f89e77007..432c026b8 100644 --- a/src/test/lrsql/admin/route_test.clj +++ b/src/test/lrsql/admin/route_test.clj @@ -57,6 +57,11 @@ (curl/post "http://0.0.0.0:8080/admin/account/logout" {:headers headers})) +(defn- renew-login + [headers] + (curl/get "http://0.0.0.0:8080/admin/account/renew" + {:headers headers})) + (defn- create-account [headers body & {:keys [throw] :or {throw true}}] @@ -215,6 +220,14 @@ (is (= 200 status)) ;; is the created user (is (= (get edn-body "username") admin-user-default)))) + (testing "renew my admin account's JWT" + (let [{:keys [status body]} (renew-login headers) + edn-body (u/parse-json body) + new-jwt (get edn-body "json-web-token")] + ;; success + (is (= 200 status)) + ;; body is a JWT (TODO: Check/test the actual contents of the JWT) + (is (string? new-jwt)))) (testing "log into the `myname` account" (let [{:keys [status body]} (login-account content-type req-body) From 912797dcd119530258aa25b5cbeff47a8ae02993 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 15:36:49 -0400 Subject: [PATCH 28/61] Add ult property to JWTs --- src/main/lrsql/util/admin.clj | 22 ++++++++++++--------- src/test/lrsql/util/admin_test.clj | 31 +++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/main/lrsql/util/admin.clj b/src/main/lrsql/util/admin.clj index 1e258afcc..cfdee51b4 100644 --- a/src/main/lrsql/util/admin.clj +++ b/src/main/lrsql/util/admin.clj @@ -25,18 +25,21 @@ (defn account-id->jwt "Generate a new signed JSON Web Token with `account-id` in the claim - as a custom `:acc` field. The issued-at and expiration time are given as - `:iat` and `:exp`, respectively; the expiration time offset is given by - `exp` in seconds." - [account-id secret exp] + as a custom `:acc` field. The issued-at, expiration, and ultimate expiration + times are given as `:iat`, `:exp`, and `:ult`, respectively. The expiration + and ultimate expiration time offsets are given by `exp` and `ult`, + respectively, in seconds." + [account-id secret exp ult] (let [ctime (u/current-time) etime (u/offset-time ctime exp :seconds) + utime (u/offset-time ctime ult :seconds) claim {:acc account-id ;; Time values MUST be a number containing a NumericDate value ;; ie. a JSON numeric value representing the number of seconds ;; (not milliseconds!) from the 1970 UTC start. :iat (quot (u/time->millis ctime) 1000) - :exp (quot (u/time->millis etime) 1000)}] + :exp (quot (u/time->millis etime) 1000) + :ult (quot (u/time->millis utime) 1000)}] (bj/sign claim secret))) (defn header->jwt @@ -48,16 +51,17 @@ (defn jwt->payload "Given the JSON Web Token `tok`, unsign and verify the token using `secret`. - Return a map of `:account-id` and `:expiration` if valid, otherwise return - `:lrsql.admin/unauthorized-token-error`. + Return a map of `:account-id`, `:expiration`, and `:ultimate` if valid, + otherwise return `:lrsql.admin/unauthorized-token-error`. `leeway` is a time amount (in seconds) provided to compensate for clock drift." [tok secret leeway] (if tok ; Avoid encountering a null pointer exception (try - (let [{:keys [acc exp]} (bj/unsign tok secret {:leeway leeway})] + (let [{:keys [acc exp ult]} (bj/unsign tok secret {:leeway leeway})] {:account-id (u/str->uuid acc) - :expiration (u/millis->time (* 1000 exp))}) + :expiration (u/millis->time (* 1000 exp)) + :ultimate (u/millis->time (* 1000 ult))}) (catch clojure.lang.ExceptionInfo _ :lrsql.admin/unauthorized-token-error)) :lrsql.admin/unauthorized-token-error)) diff --git a/src/test/lrsql/util/admin_test.clj b/src/test/lrsql/util/admin_test.clj index 54b5e216c..a3198f54b 100644 --- a/src/test/lrsql/util/admin_test.clj +++ b/src/test/lrsql/util/admin_test.clj @@ -9,29 +9,42 @@ (is (ua/valid-password? "foo" pass-hash-m)) (is (not (ua/valid-password? "pass" pass-hash-m)))))) +(defn- account-id->jwt + [test-id] + (ua/account-id->jwt test-id "secret" 3600 86400)) + +(defn- jwt->payload + [jwt] + (ua/jwt->payload jwt "secret" 1)) + (deftest jwt-test (let [test-id (u/str->uuid "00000000-0000-1000-0000-000000000001")] (testing "JSON web tokens" - (is (re-matches #".*\..*\..*" (ua/account-id->jwt test-id "secret" 3600))) + (is (re-matches #".*\..*\..*" (account-id->jwt test-id))) (is (= test-id (-> test-id - (ua/account-id->jwt "secret" 3600) - (ua/jwt->payload "secret" 1) + account-id->jwt + jwt->payload :account-id))) (is (inst? (-> test-id - (ua/account-id->jwt "secret" 3600) - (ua/jwt->payload "secret" 1) + account-id->jwt + jwt->payload :expiration))) + (is (inst? + (-> test-id + account-id->jwt + jwt->payload + :ultimate))) (is (= :lrsql.admin/unauthorized-token-error - (ua/jwt->payload nil "secret" 3600))) + (jwt->payload nil))) (is (= :lrsql.admin/unauthorized-token-error - (ua/jwt->payload "not-a-jwt" "secret" 3600))) + (jwt->payload "not-a-jwt"))) (is (= :lrsql.admin/unauthorized-token-error (-> test-id - (ua/account-id->jwt "secret" 3600) + account-id->jwt (ua/jwt->payload "different-secret" 1)))) (is (= :lrsql.admin/unauthorized-token-error - (let [tok (ua/account-id->jwt test-id "secret" 1) + (let [tok (ua/account-id->jwt test-id "secret" 1 100) _ (Thread/sleep 1001)] (ua/jwt->payload tok "secret" 0))))))) From c4f5582b2843bcbebd3e48d4e2c4f943f29162d3 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 15:38:42 -0400 Subject: [PATCH 29/61] Add LRSQL_JWT_ULTIMATE_EXP_TIME var and pass it to admin routes --- .../lrsql/config/prod/default/webserver.edn | 1 + .../lrsql/config/test/default/webserver.edn | 1 + src/main/lrsql/admin/interceptors/account.clj | 20 +++++++++++++++++-- src/main/lrsql/admin/routes.clj | 9 +++++---- src/main/lrsql/system/webserver.clj | 8 +++++--- 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/resources/lrsql/config/prod/default/webserver.edn b/resources/lrsql/config/prod/default/webserver.edn index 6b6e4eb7f..3510d9893 100644 --- a/resources/lrsql/config/prod/default/webserver.edn +++ b/resources/lrsql/config/prod/default/webserver.edn @@ -6,6 +6,7 @@ :key-enable-selfie #boolean #or [#env LRSQL_KEY_ENABLE_SELFIE true] :jwt-exp-time #long #or [#env LRSQL_JWT_EXP_TIME 3600] :jwt-exp-leeway #long #or [#env LRSQL_JWT_EXP_LEEWAY 1] + :jwt-ultimate-exp-time #long #or [#env LRSQL_JWT_ULTIMATE_EXP_TIME 86400] :jwt-no-val #boolean #or [#env LRSQL_JWT_NO_VAL false] :jwt-no-val-uname #or [#env LRSQL_JWT_NO_VAL_UNAME nil] :jwt-no-val-issuer #or [#env LRSQL_JWT_NO_VAL_ISSUER nil] diff --git a/resources/lrsql/config/test/default/webserver.edn b/resources/lrsql/config/test/default/webserver.edn index 8f8badccd..653f990f6 100644 --- a/resources/lrsql/config/test/default/webserver.edn +++ b/resources/lrsql/config/test/default/webserver.edn @@ -4,6 +4,7 @@ :key-enable-selfie true :jwt-exp-time 3600 :jwt-exp-leeway 1 + :jwt-ultimate-exp-time 86400 :jwt-no-val false :jwt-no-val-uname nil :jwt-no-val-issuer nil diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index e5b034f13..573e5ce4a 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -265,7 +265,7 @@ (defn generate-jwt "Upon account login, generate a new JSON web token." - [secret exp] + [secret exp ult] (interceptor {:name ::generate-jwt :enter @@ -273,7 +273,7 @@ (let [{{:keys [account-id]} ::data} ctx json-web-token - (admin-u/account-id->jwt account-id secret exp)] + (admin-u/account-id->jwt account-id secret exp ult)] (assoc ctx :response {:status 200 @@ -305,3 +305,19 @@ :response {:status 400 :body {:error block-admin-jwt-error-msg}})))})) + +(defn renew-admin-jwt + [secret exp] + (interceptor + {:name ::renew-jwt + :enter + (fn renew-jwt [ctx] + (let [{{:keys [account-id]} ::data} + ctx + json-web-token + (admin-u/account-id->jwt account-id secret exp 86400)] + (assoc ctx + :response + {:status 200 + :body {:account-id account-id + :json-web-token json-web-token}})))})) diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index dc3bbca6f..4e014a240 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -33,7 +33,7 @@ (i/lrs-interceptor lrs)]) (defn admin-account-routes - [common-interceptors jwt-secret jwt-exp jwt-leeway no-val-opts] + [common-interceptors jwt-secret jwt-exp jwt-ult jwt-leeway no-val-opts] #{;; Log into an existing account (gc/annotate ["/admin/account/login" :post (conj common-interceptors @@ -41,7 +41,7 @@ :strict? false) ai/authenticate-admin ai/unblock-admin-jwts - (ai/generate-jwt jwt-secret jwt-exp)) + (ai/generate-jwt jwt-secret jwt-exp jwt-ult)) :route-name :lrsql.admin.account/login] {:description "Log into an existing account" :requestBody (g/request (gs/o {:username :t#string @@ -72,7 +72,7 @@ (ji/validate-jwt jwt-secret jwt-leeway no-val-opts) ji/validate-jwt-account - (ai/generate-jwt jwt-secret jwt-exp)) + (ai/generate-jwt jwt-secret jwt-exp jwt-ult)) :route-name :lrsql.admin.account/renew] {:description "Renew current account login" :operationId :renew @@ -303,6 +303,7 @@ accounts." [{:keys [lrs exp + ult leeway secret no-val? @@ -336,7 +337,7 @@ (cset/union routes (when enable-account-routes (admin-account-routes - common-interceptors-oidc secret exp leeway no-val-opts)) + common-interceptors-oidc secret exp ult leeway no-val-opts)) (admin-cred-routes common-interceptors-oidc secret leeway no-val-opts) (when enable-admin-ui diff --git a/src/main/lrsql/system/webserver.clj b/src/main/lrsql/system/webserver.clj index 6996acd97..650dbaf13 100644 --- a/src/main/lrsql/system/webserver.clj +++ b/src/main/lrsql/system/webserver.clj @@ -50,8 +50,9 @@ enable-clamav clamav-host clamav-port] - jwt-exp :jwt-exp-time - jwt-lwy :jwt-exp-leeway} + jwt-exp :jwt-exp-time + jwt-lwy :jwt-exp-leeway + jwt-ult :jwt-ultimate-exp-time} config ;; Keystore and private key ;; The private key is used as the JWT symmetric secret @@ -91,6 +92,7 @@ (add-admin-routes {:lrs lrs :exp jwt-exp + :ult jwt-ult :leeway jwt-lwy :no-val? jwt-no-val :no-val-issuer jwt-no-val-issuer @@ -109,7 +111,7 @@ :enable-reaction-routes enable-reactions :oidc-interceptors oidc-admin-interceptors :oidc-ui-interceptors oidc-admin-ui-interceptors - :head-opts head-opts}) + :head-opts head-opts}) (add-openapi-route {:lrs lrs :head-opts head-opts From 2625a6a0580901d6ccb831d1a2c91b5e1189b17f Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 16:09:08 -0400 Subject: [PATCH 30/61] Add separate jwt creation fn that reuses ult exp time --- src/main/lrsql/util/admin.clj | 31 +++++++++++++++++++++++------- src/test/lrsql/util/admin_test.clj | 12 ++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/main/lrsql/util/admin.clj b/src/main/lrsql/util/admin.clj index cfdee51b4..1704b78e3 100644 --- a/src/main/lrsql/util/admin.clj +++ b/src/main/lrsql/util/admin.clj @@ -23,6 +23,29 @@ ;; JSON Web Tokens ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(defn- jwt-claim + "Create the JWT claim, i.e. payload, containing the account ID `:acc`, the + issued-at time `:iat`, the expiration time `:exp`, and the ultimate + expiration time `:ult`. + + Time values MUST be a number containing a NumericDate value ie. a JSON + numeric value representing the number of seconds (not milliseconds!) from + the 1970 UTC start." + [account-id ctime etime utime] + {:acc account-id + :iat (quot (u/time->millis ctime) 1000) + :exp (quot (u/time->millis etime) 1000) + :ult (quot (u/time->millis utime) 1000)}) + +(defn account-id->jwt* + "Same as `account-id->jwt`, but uses a pre-existing `utime` timestamp instead + of an `ult` offset." + [account-id secret exp utime] + (let [ctime (u/current-time) + etime (u/offset-time ctime exp :seconds) + claim (jwt-claim account-id ctime etime utime)] + (bj/sign claim secret))) + (defn account-id->jwt "Generate a new signed JSON Web Token with `account-id` in the claim as a custom `:acc` field. The issued-at, expiration, and ultimate expiration @@ -33,13 +56,7 @@ (let [ctime (u/current-time) etime (u/offset-time ctime exp :seconds) utime (u/offset-time ctime ult :seconds) - claim {:acc account-id - ;; Time values MUST be a number containing a NumericDate value - ;; ie. a JSON numeric value representing the number of seconds - ;; (not milliseconds!) from the 1970 UTC start. - :iat (quot (u/time->millis ctime) 1000) - :exp (quot (u/time->millis etime) 1000) - :ult (quot (u/time->millis utime) 1000)}] + claim (jwt-claim account-id ctime etime utime)] (bj/sign claim secret))) (defn header->jwt diff --git a/src/test/lrsql/util/admin_test.clj b/src/test/lrsql/util/admin_test.clj index a3198f54b..2857f6a72 100644 --- a/src/test/lrsql/util/admin_test.clj +++ b/src/test/lrsql/util/admin_test.clj @@ -13,6 +13,10 @@ [test-id] (ua/account-id->jwt test-id "secret" 3600 86400)) +(defn- account-id->jwt* + [test-id ult-time] + (ua/account-id->jwt* test-id "secret" 3600 ult-time)) + (defn- jwt->payload [jwt] (ua/jwt->payload jwt "secret" 1)) @@ -36,6 +40,14 @@ account-id->jwt jwt->payload :ultimate))) + (let [utime (-> test-id + account-id->jwt + jwt->payload + :ultimate)] + (is (= utime + (-> (account-id->jwt* test-id utime) + jwt->payload + :ultimate)))) (is (= :lrsql.admin/unauthorized-token-error (jwt->payload nil))) (is (= :lrsql.admin/unauthorized-token-error From 2af38c8634b2c9ade2d3e5bd9c63a5e3a250842f Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 16:09:18 -0400 Subject: [PATCH 31/61] Add renew-admin-jwt interceptor --- src/main/lrsql/admin/interceptors/account.clj | 40 +++++++++++-------- src/main/lrsql/admin/routes.clj | 2 +- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 573e5ce4a..91e7953df 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -1,5 +1,6 @@ (ns lrsql.admin.interceptors.account (:require [clojure.spec.alpha :as s] + [java-time.api :as jt] [io.pedestal.interceptor :refer [interceptor]] [io.pedestal.interceptor.chain :as chain] [lrsql.admin.protocol :as adp] @@ -280,6 +281,29 @@ :body {:account-id account-id :json-web-token json-web-token}})))})) +(defn renew-admin-jwt + [secret exp] + (interceptor + {:name ::renew-jwt + :enter + (fn renew-jwt [ctx] + (let [{{:keys [account-id ultimate]} ::jwt/data} ctx + curr-time (u/current-time)] + (if (jt/before? curr-time ultimate) + (let [json-web-token (admin-u/account-id->jwt* account-id + secret + exp + ultimate)] + (assoc ctx + :response + {:status 200 + :body {:account-id account-id + :json-web-token json-web-token}})) + (assoc (chain/terminate ctx) + :response + {:status 401 + :body {:error "Attempting JWT login after ultimate expiry."}}))))})) + (def ^:private block-admin-jwt-error-msg "This operation is unsupported when `LRSQL_JWT_NO_VAL` is set to `true`.") @@ -305,19 +329,3 @@ :response {:status 400 :body {:error block-admin-jwt-error-msg}})))})) - -(defn renew-admin-jwt - [secret exp] - (interceptor - {:name ::renew-jwt - :enter - (fn renew-jwt [ctx] - (let [{{:keys [account-id]} ::data} - ctx - json-web-token - (admin-u/account-id->jwt account-id secret exp 86400)] - (assoc ctx - :response - {:status 200 - :body {:account-id account-id - :json-web-token json-web-token}})))})) diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 4e014a240..cf96574e0 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -72,7 +72,7 @@ (ji/validate-jwt jwt-secret jwt-leeway no-val-opts) ji/validate-jwt-account - (ai/generate-jwt jwt-secret jwt-exp jwt-ult)) + (ai/renew-admin-jwt jwt-secret jwt-exp)) :route-name :lrsql.admin.account/renew] {:description "Renew current account login" :operationId :renew From b72e933d93ba1aec065327d9e323d2449966ec29 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 17:01:20 -0400 Subject: [PATCH 32/61] Add jwt-expiry test block --- src/test/lrsql/admin/route_test.clj | 47 +++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/src/test/lrsql/admin/route_test.clj b/src/test/lrsql/admin/route_test.clj index 432c026b8..e42304874 100644 --- a/src/test/lrsql/admin/route_test.clj +++ b/src/test/lrsql/admin/route_test.clj @@ -221,13 +221,15 @@ ;; is the created user (is (= (get edn-body "username") admin-user-default)))) (testing "renew my admin account's JWT" + ;; NOTE: More JWT renewal tests in `jwt-expiry` below (let [{:keys [status body]} (renew-login headers) edn-body (u/parse-json body) new-jwt (get edn-body "json-web-token")] ;; success (is (= 200 status)) - ;; body is a JWT (TODO: Check/test the actual contents of the JWT) - (is (string? new-jwt)))) + ;; body is a new JWT + (is (string? new-jwt)) + (is (not= seed-jwt new-jwt)))) (testing "log into the `myname` account" (let [{:keys [status body]} (login-account content-type req-body) @@ -540,6 +542,47 @@ (finally (component/stop sys'))))) +(deftest jwt-expiry + (let [sys (support/test-system + :conf-overrides + {[:webserver :jwt-exp-time] 3 + [:webserver :jwt-ultimate-exp-time] 4}) + sys' (component/start sys) + ;; Seed info + {:keys [admin-user-default + admin-pass-default]} + (get-in sys' [:lrs :config]) + seed-body (u/write-json-str + {"username" admin-user-default + "password" admin-pass-default}) + seed-jwt (-> (login-account content-type seed-body) + :body + u/parse-json + (get "json-web-token")) + seed-auth {"Authorization" (str "Bearer " seed-jwt)} + headers (merge content-type seed-auth)] + (try + (testing "Original JWT works" + (is (= 200 (:status (get-me headers))))) + ;; Refresh JWT + (Thread/sleep 2000) + (let [{:keys [body]} (renew-login headers) + new-jwt (-> body u/parse-json (get "json-web-token")) + new-auth {"Authorization" (str "Bearer " new-jwt)} + headers* (merge content-type new-auth)] + (testing "Only new JWT no longer works after expiration" + (Thread/sleep 2000) + (is-err-code (get-me headers) 401) + (is (= 200 (:status (get-me headers*))))) + (testing "JWT can no longer refresh after ultimate expiration" + (Thread/sleep 1000) + (is-err-code (renew-login headers) 401)) + (testing "JWT no longer works after expiration" + (Thread/sleep 1500) + (is-err-code (get-me headers*) 401))) + (finally + (component/stop sys'))))) + (def proxy-jwt-body {"usercertificate" "unique.user.1234" "iss" "https://idp.domain.com/auth" From 10fbe36ce6481adf8dca58d17a3f1a984052255b Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 1 Nov 2024 17:14:39 -0400 Subject: [PATCH 33/61] Add to docs --- doc/endpoints.md | 1 + doc/env_vars.md | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/endpoints.md b/doc/endpoints.md index 49f1192d7..d08fb2dc7 100644 --- a/doc/endpoints.md +++ b/doc/endpoints.md @@ -28,6 +28,7 @@ The following examples use `http://example.org` as the URL body. All methods ret The response body contains a newly generated JSON Web Token (JWT) on success. A `401 UNAUTHORIZED` status code is returned if the credentials are incorrect. - `POST http://example.org/admin/account/logout`: Log out of the current account. This will revoke any unexpired JWTs associated with the user. (NOTE: This endpoint will return a `400 BAD REQUEST` error if `LRSQL_JWT_NO_VAL` is set to `true`.) +- `GET http://example.org/admin/account/renew`: Renew the current account's login session by issuing a new JWT. For a given JWT, the renewal is only granted if the current time is less than the `ult` timestamp (which is determined by `LRSQL_JWT_ULTIMATE_EXP_TIME`). - `POST http://example.org/admin/account/create`: Create a new admin account. The request body must be a JSON object that contains `username` and `password` strings. The endpoint returns a JSON object with the ID (UUID) of the newly created user on success, and returns a `409 CONFLICT` if the account already exists. - `DELETE http://example.org/admin/account`: Delete an existing account. The JSON request body must contain a UUID `account-id` value. The endpoint returns a JSON object with the ID of the deleted account on success and returns a `404 NOT FOUND` error if the account does not exist. - `GET http://example.org/admin/account`: Return an array of all admin accounts in the system on success. diff --git a/doc/env_vars.md b/doc/env_vars.md index 9973c859c..ce5c3d07b 100644 --- a/doc/env_vars.md +++ b/doc/env_vars.md @@ -141,7 +141,8 @@ _NOTE:_ `LRSQL_STMT_RETRY_LIMIT` and `LRSQL_STMT_RETRY_BUDGET` are used to mitig | Env Var | Config | Description | Default | | --------------------------------- | ------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------- | -| `LRSQL_JWT_EXP_TIME` | `jwtExpTime` | The amount of time, in seconds, after a JWT is created when it expires. Since JWTs are not revocable, **this this time should be short** (i.e. one hour or less). | `3600` (one hour) | +| `LRSQL_JWT_EXP_TIME` | `jwtExpTime` | The amount of time, in seconds, after a JWT is created when it expires. **This this time should be short** (i.e. one hour or less). | `3600` (one hour) | +| `LRSQL_JWT_ULTIMATE_EXP_TIME` | `jwtUltimateExpTime` | The amount of time, in seconds, after a JWT is issued upon an initial login (_not_ a login renewal), after which login renewal can no longer be performed. | `86400` (one day) | | `LRSQL_JWT_EXP_LEEWAY` | `jwtExpLeeway` | The amount of time, in seconds, before or after the expiration instant when a JWT should still count as un-expired. Used to compensate for clock desync. Applied to both LRS and OIDC tokens. | `1` (one second) | | `LRSQL_JWT_NO_VAL` | `jwtNoVal` | (**DANGEROUS!**) This flag removes JWT Token Validation and simply accepts token claims as configured by the associated variables below. It is extremely unlikely that you need this as it is for very specific proxy-overwrite authentication scenarios, and it poses a serious threat to system security if enabled. | `false` | | `LRSQL_JWT_NO_VAL_UNAME` | `jwtNoValUname` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the username when token validation is turned off. | Not Set | From ab7894e43574ae92c0ba0d84d417a8be48ba7fa1 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 4 Nov 2024 10:00:11 -0500 Subject: [PATCH 34/61] Apply leeway to JWT blocklist calculations --- src/main/lrsql/admin/interceptors/account.clj | 9 +++--- src/main/lrsql/admin/interceptors/jwt.clj | 2 +- src/main/lrsql/admin/protocol.clj | 8 ++--- src/main/lrsql/admin/routes.clj | 6 ++-- src/main/lrsql/input/admin/jwt.clj | 29 +++++++++++++------ src/main/lrsql/spec/admin/jwt.clj | 4 ++- src/main/lrsql/system/lrs.clj | 15 ++++++---- src/test/lrsql/admin/protocol_test.clj | 24 ++++++++------- 8 files changed, 59 insertions(+), 38 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index e5b034f13..9a22749ae 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -123,15 +123,16 @@ {:status 401 :body {:error "Invalid Account Credentials"}}))))})) -(def unblock-admin-jwts +(defn unblock-admin-jwts "Remove all JWTs associated with the user account from the blocklist." + [leeway] (interceptor {:name ::remove-jwt-from-blocklist :enter (fn remove-jwt-from-blocklist [ctx] (let [{lrs :com.yetanalytics/lrs {:keys [account-id]} ::data} ctx] - (adp/-unblock-jwts lrs account-id) + (adp/-unblock-jwts lrs account-id leeway) ctx))})) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -286,7 +287,7 @@ (defn block-admin-jwt "Add the current JWT to the blocklist. Return an error if we are in no-val mode." - [no-val?] + [leeway no-val?] (interceptor {:name ::add-jwt-to-blocklist :enter @@ -296,7 +297,7 @@ {:keys [account-id expiration]} :lrsql.admin.interceptors.jwt/data} ctx] - (adp/-block-jwt lrs account-id expiration) + (adp/-block-jwt lrs account-id expiration leeway) (assoc (chain/terminate ctx) :response {:status 200 diff --git a/src/main/lrsql/admin/interceptors/jwt.clj b/src/main/lrsql/admin/interceptors/jwt.clj index 79b799cfa..977ef93b0 100644 --- a/src/main/lrsql/admin/interceptors/jwt.clj +++ b/src/main/lrsql/admin/interceptors/jwt.clj @@ -38,7 +38,7 @@ (admin-u/jwt->payload token secret leeway)] (if (keyword? result) result - (if-not (adp/-jwt-blocked? lrs account-id) + (if-not (adp/-jwt-blocked? lrs account-id leeway) result :lrsql.admin/unauthorized-token-error))) (catch Exception ex diff --git a/src/main/lrsql/admin/protocol.clj b/src/main/lrsql/admin/protocol.clj index a581ee5c9..fe6e35782 100644 --- a/src/main/lrsql/admin/protocol.clj +++ b/src/main/lrsql/admin/protocol.clj @@ -19,11 +19,11 @@ "Update the password for an admin account given old and new passwords.")) (defprotocol AdminJWTManager - (-block-jwt [this account-id expiration] - "Block the JWT with this account and expiration time. Purge expired JWTs from the blocklist if necessary.") - (-unblock-jwts [this account-id] + (-block-jwt [this account-id expiration leeway] + "Block the JWT with this account, expiration time, and leeway. Purge expired JWTs from the blocklist if necessary.") + (-unblock-jwts [this account-id leeway] "Unblock the JWTs associated with this account. Purge expired JWTs from the blocklist if necessary.") - (-jwt-blocked? [this account-id] + (-jwt-blocked? [this account-id leeway] "Is an unexpired JWT with this account on the blocklist?")) (defprotocol APIKeyManager diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 0b56dbcf3..2883a3e2c 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -33,14 +33,14 @@ (i/lrs-interceptor lrs)]) (defn admin-account-routes - [common-interceptors jwt-secret jwt-exp jwt-leeway no-val-opts] + [common-interceptors jwt-secret jwt-exp jwt-leeway {:keys [no-val?] :as no-val-opts}] #{;; Log into an existing account (gc/annotate ["/admin/account/login" :post (conj common-interceptors (ai/validate-params :strict? false) ai/authenticate-admin - ai/unblock-admin-jwts + (ai/unblock-admin-jwts jwt-leeway) (ai/generate-jwt jwt-secret jwt-exp)) :route-name :lrsql.admin.account/login] {:description "Log into an existing account" @@ -58,7 +58,7 @@ (ji/validate-jwt jwt-secret jwt-leeway no-val-opts) ji/validate-jwt-account - (ai/block-admin-jwt (:no-val? no-val-opts))) + (ai/block-admin-jwt jwt-leeway no-val?)) :route-name :lrsql.admin.account/logout] {:description "Log out of this account" :operationId :logout diff --git a/src/main/lrsql/input/admin/jwt.clj b/src/main/lrsql/input/admin/jwt.clj index ca5d52371..4c1a484af 100644 --- a/src/main/lrsql/input/admin/jwt.clj +++ b/src/main/lrsql/input/admin/jwt.clj @@ -3,33 +3,44 @@ [lrsql.spec.admin.jwt :as jwts] [lrsql.util :as u])) +(defn- current-time + "Generate the current time, offset by `leeway` number of seconds earlier. + + See: `buddy.sign.jwt/validate-claims`" + [leeway] + (-> (u/current-time) + (u/offset-time (* -1 leeway) :seconds))) + (s/fdef query-blocked-jwt-input - :args (s/cat :account-id ::jwts/account-id) + :args (s/cat :account-id ::jwts/account-id + :leeway ::jwts/leeway) :ret jwts/query-blocked-jwt-input-spec) (defn query-blocked-jwt-input - [account-id] + [account-id leeway] {:account-id account-id - :current-time (u/current-time)}) + :current-time (current-time leeway)}) (s/fdef insert-blocked-jwt-input :args (s/cat :account-id ::jwts/account-id - :expiration ::jwts/expiration) + :expiration ::jwts/expiration + :leeway ::jwts/leeway) :ret (s/and jwts/insert-blocked-jwt-input-spec jwts/delete-blocked-jwt-time-input-spec)) (defn insert-blocked-jwt-input - [account-id expiration] + [account-id expiration leeway] {:account-id account-id :expiration expiration - :current-time (u/current-time)}) + :current-time (current-time leeway)}) (s/fdef delete-blocked-jwts-input - :args (s/cat :account-id ::jwts/account-id) + :args (s/cat :account-id ::jwts/account-id + :leeway ::jwts/leeway) :ret (s/and jwts/delete-blocked-jwt-account-input-spec jwts/delete-blocked-jwt-time-input-spec)) (defn delete-blocked-jwts-input - [account-id] + [account-id leeway] {:account-id account-id - :current-time (u/current-time)}) + :current-time (current-time leeway)}) diff --git a/src/main/lrsql/spec/admin/jwt.clj b/src/main/lrsql/spec/admin/jwt.clj index 1130e3aac..edcf73c49 100644 --- a/src/main/lrsql/spec/admin/jwt.clj +++ b/src/main/lrsql/spec/admin/jwt.clj @@ -1,7 +1,8 @@ (ns lrsql.spec.admin.jwt (:require [clojure.spec.alpha :as s] [lrsql.backend.protocol :as bp] - [lrsql.spec.common :as c])) + [lrsql.spec.common :as c] + [lrsql.spec.config :as config])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Interface @@ -18,6 +19,7 @@ (s/def ::account-id :lrsql.spec.admin/account-id) (s/def ::current-time c/instant-spec) (s/def ::expiration c/instant-spec) +(s/def ::leeway ::config/jwt-exp-leeway) (def query-blocked-jwt-input-spec (s/keys :req-un [::account-id diff --git a/src/main/lrsql/system/lrs.clj b/src/main/lrsql/system/lrs.clj index 5f0185e14..e618e7b02 100644 --- a/src/main/lrsql/system/lrs.clj +++ b/src/main/lrsql/system/lrs.clj @@ -308,24 +308,27 @@ adp/AdminJWTManager (-block-jwt - [this account-id expiration] + [this account-id expiration leeway] (let [conn (lrs-conn this) jwt-input (admin-jwt-input/insert-blocked-jwt-input account-id - expiration)] + expiration + leeway)] (jdbc/with-transaction [tx conn] (admin-cmd/purge-blocklist! backend tx jwt-input) (admin-cmd/insert-blocked-jwt! backend tx jwt-input)))) (-unblock-jwts - [this account-id] + [this account-id leeway] (let [conn (lrs-conn this) - jwt-input (admin-jwt-input/delete-blocked-jwts-input account-id)] + jwt-input (admin-jwt-input/delete-blocked-jwts-input account-id + leeway)] (jdbc/with-transaction [tx conn] (admin-cmd/purge-blocklist! backend tx jwt-input) (admin-cmd/delete-blocked-jwts! backend tx jwt-input)))) (-jwt-blocked? - [this account-id] + [this account-id leeway] (let [conn (lrs-conn this) - jwt-input (admin-jwt-input/query-blocked-jwt-input account-id)] + jwt-input (admin-jwt-input/query-blocked-jwt-input account-id + leeway)] (jdbc/with-transaction [tx conn] (admin-q/query-blocked-jwt-exists backend tx jwt-input)))) diff --git a/src/test/lrsql/admin/protocol_test.clj b/src/test/lrsql/admin/protocol_test.clj index b28bdcbbd..54ed617f4 100644 --- a/src/test/lrsql/admin/protocol_test.clj +++ b/src/test/lrsql/admin/protocol_test.clj @@ -136,27 +136,31 @@ account-id-2 (:result (adp/-authenticate-account lrs test-username-2 - test-password))] + test-password)) + leeway 2] (testing "- block" (is (= account-id ; Current JWT - (:result (adp/-block-jwt lrs account-id expiration)))) + (:result (adp/-block-jwt lrs account-id expiration leeway)))) (is (= account-id-2 ; Expired JWT (need to add after to avoid purge) - (:result (adp/-block-jwt lrs account-id-2 expiration-2)))) + (:result (adp/-block-jwt lrs account-id-2 expiration-2 leeway)))) (is (true? - (adp/-jwt-blocked? lrs account-id))) + (adp/-jwt-blocked? lrs account-id leeway))) + (is (true? ; Not expired thanks to leeway + (adp/-jwt-blocked? lrs account-id leeway))) + (Thread/sleep 2000) ; Wait 2 seconds to accommodate leeway (is (false? ; Expired JWT doesn't count as blocked - (adp/-jwt-blocked? lrs account-id-2)))) + (adp/-jwt-blocked? lrs account-id-2 leeway)))) (testing "- unblock" (is (= account-id - (:result (adp/-unblock-jwts lrs account-id)))) + (:result (adp/-unblock-jwts lrs account-id leeway)))) (is (false? - (adp/-jwt-blocked? lrs account-id))) + (adp/-jwt-blocked? lrs account-id leeway))) (is (false? - (adp/-jwt-blocked? lrs account-id-2))) + (adp/-jwt-blocked? lrs account-id-2 leeway))) (is (= account-id-2 - (:result (adp/-unblock-jwts lrs account-id-2)))) + (:result (adp/-unblock-jwts lrs account-id-2 leeway)))) (is (false? - (adp/-jwt-blocked? lrs account-id-2)))))) + (adp/-jwt-blocked? lrs account-id-2 leeway)))))) (testing "Admin password update" (let [account-id (-> (adp/-authenticate-account lrs test-username From aa8d44e666693d02616386838590648ec5855b30 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 4 Nov 2024 10:03:36 -0500 Subject: [PATCH 35/61] Add jwt-ultimate-exp-time to config spec --- src/main/lrsql/spec/config.clj | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/lrsql/spec/config.clj b/src/main/lrsql/spec/config.clj index 23645e92c..e73e1b32c 100644 --- a/src/main/lrsql/spec/config.clj +++ b/src/main/lrsql/spec/config.clj @@ -152,6 +152,7 @@ (s/def ::allowed-origins (s/nilable (s/coll-of string?))) (s/def ::jwt-exp-time pos-int?) +(s/def ::jwt-ultimate-exp-time pos-int?) (s/def ::jwt-exp-leeway nat-int?) (s/def ::jwt-no-val boolean?) (s/def ::jwt-no-val-uname (s/nilable string?)) @@ -209,6 +210,7 @@ ::key-password ::key-enable-selfie ::jwt-exp-time + ::jwt-ultimate-exp-time ::jwt-exp-leeway ::jwt-no-val ::enable-admin-ui From 40d39111c64883a93af19a929ac11a9b5c25cb1d Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 4 Nov 2024 10:16:11 -0500 Subject: [PATCH 36/61] Change "ultimate exp" to "refresh exp" --- doc/endpoints.md | 2 +- doc/env_vars.md | 2 +- .../lrsql/config/prod/default/webserver.edn | 2 +- .../lrsql/config/test/default/webserver.edn | 2 +- src/main/lrsql/admin/interceptors/account.clj | 12 +++--- src/main/lrsql/admin/routes.clj | 8 ++-- src/main/lrsql/spec/config.clj | 4 +- src/main/lrsql/system/webserver.clj | 4 +- src/main/lrsql/util/admin.clj | 40 +++++++++---------- src/test/lrsql/admin/route_test.clj | 4 +- src/test/lrsql/util/admin_test.clj | 10 ++--- 11 files changed, 45 insertions(+), 45 deletions(-) diff --git a/doc/endpoints.md b/doc/endpoints.md index d08fb2dc7..91435091c 100644 --- a/doc/endpoints.md +++ b/doc/endpoints.md @@ -28,7 +28,7 @@ The following examples use `http://example.org` as the URL body. All methods ret The response body contains a newly generated JSON Web Token (JWT) on success. A `401 UNAUTHORIZED` status code is returned if the credentials are incorrect. - `POST http://example.org/admin/account/logout`: Log out of the current account. This will revoke any unexpired JWTs associated with the user. (NOTE: This endpoint will return a `400 BAD REQUEST` error if `LRSQL_JWT_NO_VAL` is set to `true`.) -- `GET http://example.org/admin/account/renew`: Renew the current account's login session by issuing a new JWT. For a given JWT, the renewal is only granted if the current time is less than the `ult` timestamp (which is determined by `LRSQL_JWT_ULTIMATE_EXP_TIME`). +- `GET http://example.org/admin/account/renew`: Renew the current account's login session by issuing a new JWT. For a given JWT, the renewal is only granted if the current time is less than the `ref` timestamp (which is determined by `LRSQL_JWT_REFRESH_EXP_TIME`). - `POST http://example.org/admin/account/create`: Create a new admin account. The request body must be a JSON object that contains `username` and `password` strings. The endpoint returns a JSON object with the ID (UUID) of the newly created user on success, and returns a `409 CONFLICT` if the account already exists. - `DELETE http://example.org/admin/account`: Delete an existing account. The JSON request body must contain a UUID `account-id` value. The endpoint returns a JSON object with the ID of the deleted account on success and returns a `404 NOT FOUND` error if the account does not exist. - `GET http://example.org/admin/account`: Return an array of all admin accounts in the system on success. diff --git a/doc/env_vars.md b/doc/env_vars.md index ce5c3d07b..b5d5e8e39 100644 --- a/doc/env_vars.md +++ b/doc/env_vars.md @@ -142,7 +142,7 @@ _NOTE:_ `LRSQL_STMT_RETRY_LIMIT` and `LRSQL_STMT_RETRY_BUDGET` are used to mitig | Env Var | Config | Description | Default | | --------------------------------- | ------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------- | | `LRSQL_JWT_EXP_TIME` | `jwtExpTime` | The amount of time, in seconds, after a JWT is created when it expires. **This this time should be short** (i.e. one hour or less). | `3600` (one hour) | -| `LRSQL_JWT_ULTIMATE_EXP_TIME` | `jwtUltimateExpTime` | The amount of time, in seconds, after a JWT is issued upon an initial login (_not_ a login renewal), after which login renewal can no longer be performed. | `86400` (one day) | +| `LRSQL_JWT_REFRESH_EXP_TIME` | `jwtRefreshExpTime` | The amount of time, in seconds, after a JWT is issued upon an initial login (_not_ a login renewal), after which login renewal can no longer be performed. Note: this is unaffected by expiration leeway. | `86400` (one day) | | `LRSQL_JWT_EXP_LEEWAY` | `jwtExpLeeway` | The amount of time, in seconds, before or after the expiration instant when a JWT should still count as un-expired. Used to compensate for clock desync. Applied to both LRS and OIDC tokens. | `1` (one second) | | `LRSQL_JWT_NO_VAL` | `jwtNoVal` | (**DANGEROUS!**) This flag removes JWT Token Validation and simply accepts token claims as configured by the associated variables below. It is extremely unlikely that you need this as it is for very specific proxy-overwrite authentication scenarios, and it poses a serious threat to system security if enabled. | `false` | | `LRSQL_JWT_NO_VAL_UNAME` | `jwtNoValUname` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the username when token validation is turned off. | Not Set | diff --git a/resources/lrsql/config/prod/default/webserver.edn b/resources/lrsql/config/prod/default/webserver.edn index 3510d9893..e4c279e4b 100644 --- a/resources/lrsql/config/prod/default/webserver.edn +++ b/resources/lrsql/config/prod/default/webserver.edn @@ -6,7 +6,7 @@ :key-enable-selfie #boolean #or [#env LRSQL_KEY_ENABLE_SELFIE true] :jwt-exp-time #long #or [#env LRSQL_JWT_EXP_TIME 3600] :jwt-exp-leeway #long #or [#env LRSQL_JWT_EXP_LEEWAY 1] - :jwt-ultimate-exp-time #long #or [#env LRSQL_JWT_ULTIMATE_EXP_TIME 86400] + :jwt-refresh-exp-time #long #or [#env LRSQL_JWT_REFRESH_EXP_TIME 86400] :jwt-no-val #boolean #or [#env LRSQL_JWT_NO_VAL false] :jwt-no-val-uname #or [#env LRSQL_JWT_NO_VAL_UNAME nil] :jwt-no-val-issuer #or [#env LRSQL_JWT_NO_VAL_ISSUER nil] diff --git a/resources/lrsql/config/test/default/webserver.edn b/resources/lrsql/config/test/default/webserver.edn index 653f990f6..105be1aae 100644 --- a/resources/lrsql/config/test/default/webserver.edn +++ b/resources/lrsql/config/test/default/webserver.edn @@ -4,7 +4,7 @@ :key-enable-selfie true :jwt-exp-time 3600 :jwt-exp-leeway 1 - :jwt-ultimate-exp-time 86400 + :jwt-refresh-exp-time 86400 :jwt-no-val false :jwt-no-val-uname nil :jwt-no-val-issuer nil diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 1fad76a7d..99c2c6f18 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -267,7 +267,7 @@ (defn generate-jwt "Upon account login, generate a new JSON web token." - [secret exp ult] + [secret exp ref] (interceptor {:name ::generate-jwt :enter @@ -275,7 +275,7 @@ (let [{{:keys [account-id]} ::data} ctx json-web-token - (admin-u/account-id->jwt account-id secret exp ult)] + (admin-u/account-id->jwt account-id secret exp ref)] (assoc ctx :response {:status 200 @@ -288,13 +288,13 @@ {:name ::renew-jwt :enter (fn renew-jwt [ctx] - (let [{{:keys [account-id ultimate]} ::jwt/data} ctx + (let [{{:keys [account-id refresh-exp]} ::jwt/data} ctx curr-time (u/current-time)] - (if (jt/before? curr-time ultimate) + (if (jt/before? curr-time refresh-exp) (let [json-web-token (admin-u/account-id->jwt* account-id secret exp - ultimate)] + refresh-exp)] (assoc ctx :response {:status 200 @@ -303,7 +303,7 @@ (assoc (chain/terminate ctx) :response {:status 401 - :body {:error "Attempting JWT login after ultimate expiry."}}))))})) + :body {:error "Attempting JWT login after refresh expiry."}}))))})) (def ^:private block-admin-jwt-error-msg "This operation is unsupported when `LRSQL_JWT_NO_VAL` is set to `true`.") diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 93daec4b8..562ad2084 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -33,7 +33,7 @@ (i/lrs-interceptor lrs)]) (defn admin-account-routes - [common-interceptors jwt-secret jwt-exp jwt-ult jwt-leeway {:keys [no-val?] :as no-val-opts}] + [common-interceptors jwt-secret jwt-exp jwt-ref jwt-leeway {:keys [no-val?] :as no-val-opts}] #{;; Log into an existing account (gc/annotate ["/admin/account/login" :post (conj common-interceptors @@ -41,7 +41,7 @@ :strict? false) ai/authenticate-admin (ai/unblock-admin-jwts jwt-leeway) - (ai/generate-jwt jwt-secret jwt-exp jwt-ult)) + (ai/generate-jwt jwt-secret jwt-exp jwt-ref)) :route-name :lrsql.admin.account/login] {:description "Log into an existing account" :requestBody (g/request (gs/o {:username :t#string @@ -303,7 +303,7 @@ accounts." [{:keys [lrs exp - ult + ref leeway secret no-val? @@ -337,7 +337,7 @@ (cset/union routes (when enable-account-routes (admin-account-routes - common-interceptors-oidc secret exp ult leeway no-val-opts)) + common-interceptors-oidc secret exp ref leeway no-val-opts)) (admin-cred-routes common-interceptors-oidc secret leeway no-val-opts) (when enable-admin-ui diff --git a/src/main/lrsql/spec/config.clj b/src/main/lrsql/spec/config.clj index e73e1b32c..feeb92960 100644 --- a/src/main/lrsql/spec/config.clj +++ b/src/main/lrsql/spec/config.clj @@ -152,7 +152,7 @@ (s/def ::allowed-origins (s/nilable (s/coll-of string?))) (s/def ::jwt-exp-time pos-int?) -(s/def ::jwt-ultimate-exp-time pos-int?) +(s/def ::jwt-refresh-exp-time pos-int?) (s/def ::jwt-exp-leeway nat-int?) (s/def ::jwt-no-val boolean?) (s/def ::jwt-no-val-uname (s/nilable string?)) @@ -210,7 +210,7 @@ ::key-password ::key-enable-selfie ::jwt-exp-time - ::jwt-ultimate-exp-time + ::jwt-refresh-exp-time ::jwt-exp-leeway ::jwt-no-val ::enable-admin-ui diff --git a/src/main/lrsql/system/webserver.clj b/src/main/lrsql/system/webserver.clj index 650dbaf13..bdecb45c4 100644 --- a/src/main/lrsql/system/webserver.clj +++ b/src/main/lrsql/system/webserver.clj @@ -52,7 +52,7 @@ clamav-port] jwt-exp :jwt-exp-time jwt-lwy :jwt-exp-leeway - jwt-ult :jwt-ultimate-exp-time} + jwt-ref :jwt-refresh-exp-time} config ;; Keystore and private key ;; The private key is used as the JWT symmetric secret @@ -92,7 +92,7 @@ (add-admin-routes {:lrs lrs :exp jwt-exp - :ult jwt-ult + :ref jwt-ref :leeway jwt-lwy :no-val? jwt-no-val :no-val-issuer jwt-no-val-issuer diff --git a/src/main/lrsql/util/admin.clj b/src/main/lrsql/util/admin.clj index 1704b78e3..3bb5cd95d 100644 --- a/src/main/lrsql/util/admin.clj +++ b/src/main/lrsql/util/admin.clj @@ -25,38 +25,38 @@ (defn- jwt-claim "Create the JWT claim, i.e. payload, containing the account ID `:acc`, the - issued-at time `:iat`, the expiration time `:exp`, and the ultimate - expiration time `:ult`. + issued-at time `:iat`, the expiration time `:exp`, and the refresh + expiration time `:ref`. Time values MUST be a number containing a NumericDate value ie. a JSON numeric value representing the number of seconds (not milliseconds!) from the 1970 UTC start." - [account-id ctime etime utime] + [account-id ctime etime rtime] {:acc account-id :iat (quot (u/time->millis ctime) 1000) :exp (quot (u/time->millis etime) 1000) - :ult (quot (u/time->millis utime) 1000)}) + :ref (quot (u/time->millis rtime) 1000)}) (defn account-id->jwt* - "Same as `account-id->jwt`, but uses a pre-existing `utime` timestamp instead - of an `ult` offset." - [account-id secret exp utime] + "Same as `account-id->jwt`, but uses a pre-existing `rtime` timestamp instead + of an `ref` offset." + [account-id secret exp rtime] (let [ctime (u/current-time) etime (u/offset-time ctime exp :seconds) - claim (jwt-claim account-id ctime etime utime)] + claim (jwt-claim account-id ctime etime rtime)] (bj/sign claim secret))) (defn account-id->jwt "Generate a new signed JSON Web Token with `account-id` in the claim - as a custom `:acc` field. The issued-at, expiration, and ultimate expiration - times are given as `:iat`, `:exp`, and `:ult`, respectively. The expiration - and ultimate expiration time offsets are given by `exp` and `ult`, - respectively, in seconds." - [account-id secret exp ult] + as a custom `:acc` field. The issued-at, expiration, and refresh expiration + times are given as `:iat`, `:exp`, and `:ref`, respectively. The token and + refresh expiration time offsets are given by `exp` and `ref`, respectively, + in seconds." + [account-id secret exp ref] (let [ctime (u/current-time) etime (u/offset-time ctime exp :seconds) - utime (u/offset-time ctime ult :seconds) - claim (jwt-claim account-id ctime etime utime)] + rtime (u/offset-time ctime ref :seconds) + claim (jwt-claim account-id ctime etime rtime)] (bj/sign claim secret))) (defn header->jwt @@ -68,17 +68,17 @@ (defn jwt->payload "Given the JSON Web Token `tok`, unsign and verify the token using `secret`. - Return a map of `:account-id`, `:expiration`, and `:ultimate` if valid, + Return a map of `:account-id`, `:expiration`, and `:refresh-exp` if valid, otherwise return `:lrsql.admin/unauthorized-token-error`. `leeway` is a time amount (in seconds) provided to compensate for clock drift." [tok secret leeway] (if tok ; Avoid encountering a null pointer exception (try - (let [{:keys [acc exp ult]} (bj/unsign tok secret {:leeway leeway})] - {:account-id (u/str->uuid acc) - :expiration (u/millis->time (* 1000 exp)) - :ultimate (u/millis->time (* 1000 ult))}) + (let [{:keys [acc exp ref]} (bj/unsign tok secret {:leeway leeway})] + {:account-id (u/str->uuid acc) + :expiration (u/millis->time (* 1000 exp)) + :refresh-exp (u/millis->time (* 1000 ref))}) (catch clojure.lang.ExceptionInfo _ :lrsql.admin/unauthorized-token-error)) :lrsql.admin/unauthorized-token-error)) diff --git a/src/test/lrsql/admin/route_test.clj b/src/test/lrsql/admin/route_test.clj index e42304874..6f594be2e 100644 --- a/src/test/lrsql/admin/route_test.clj +++ b/src/test/lrsql/admin/route_test.clj @@ -546,7 +546,7 @@ (let [sys (support/test-system :conf-overrides {[:webserver :jwt-exp-time] 3 - [:webserver :jwt-ultimate-exp-time] 4}) + [:webserver :jwt-refresh-exp-time] 4}) sys' (component/start sys) ;; Seed info {:keys [admin-user-default @@ -574,7 +574,7 @@ (Thread/sleep 2000) (is-err-code (get-me headers) 401) (is (= 200 (:status (get-me headers*))))) - (testing "JWT can no longer refresh after ultimate expiration" + (testing "JWT can no longer refresh after refresh expiration" (Thread/sleep 1000) (is-err-code (renew-login headers) 401)) (testing "JWT no longer works after expiration" diff --git a/src/test/lrsql/util/admin_test.clj b/src/test/lrsql/util/admin_test.clj index 2857f6a72..5851a0f11 100644 --- a/src/test/lrsql/util/admin_test.clj +++ b/src/test/lrsql/util/admin_test.clj @@ -14,8 +14,8 @@ (ua/account-id->jwt test-id "secret" 3600 86400)) (defn- account-id->jwt* - [test-id ult-time] - (ua/account-id->jwt* test-id "secret" 3600 ult-time)) + [test-id ref-time] + (ua/account-id->jwt* test-id "secret" 3600 ref-time)) (defn- jwt->payload [jwt] @@ -39,15 +39,15 @@ (-> test-id account-id->jwt jwt->payload - :ultimate))) + :refresh-exp))) (let [utime (-> test-id account-id->jwt jwt->payload - :ultimate)] + :refresh-exp)] (is (= utime (-> (account-id->jwt* test-id utime) jwt->payload - :ultimate)))) + :refresh-exp)))) (is (= :lrsql.admin/unauthorized-token-error (jwt->payload nil))) (is (= :lrsql.admin/unauthorized-token-error From 71163c32aef6fac4b1130aaabb7872e65221b33b Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 4 Nov 2024 16:14:20 -0500 Subject: [PATCH 37/61] Redo schema by directly storing JWTs --- src/db/postgres/lrsql/postgres/record.clj | 2 - src/db/postgres/lrsql/postgres/sql/ddl.sql | 8 ++-- src/db/postgres/lrsql/postgres/sql/delete.sql | 11 +---- src/db/postgres/lrsql/postgres/sql/insert.sql | 6 +-- src/db/postgres/lrsql/postgres/sql/query.sql | 5 +-- src/db/sqlite/lrsql/sqlite/record.clj | 7 +-- src/db/sqlite/lrsql/sqlite/sql/ddl.sql | 16 +++---- src/db/sqlite/lrsql/sqlite/sql/delete.sql | 11 +---- src/db/sqlite/lrsql/sqlite/sql/insert.sql | 6 +-- src/db/sqlite/lrsql/sqlite/sql/query.sql | 5 +-- src/main/lrsql/admin/interceptors/account.clj | 19 ++------ src/main/lrsql/admin/interceptors/jwt.clj | 9 ++-- src/main/lrsql/admin/protocol.clj | 10 ++--- src/main/lrsql/admin/routes.clj | 3 +- src/main/lrsql/backend/protocol.clj | 1 - src/main/lrsql/input/admin/jwt.clj | 44 ++++++------------- src/main/lrsql/ops/command/admin.clj | 16 +------ src/main/lrsql/spec/admin/jwt.clj | 20 +++------ src/main/lrsql/system/lrs.clj | 19 ++------ src/test/lrsql/admin/protocol_test.clj | 40 ++++------------- src/test/lrsql/input/input_test.clj | 3 +- 21 files changed, 73 insertions(+), 188 deletions(-) diff --git a/src/db/postgres/lrsql/postgres/record.clj b/src/db/postgres/lrsql/postgres/record.clj index 33950e12b..53f6c6355 100644 --- a/src/db/postgres/lrsql/postgres/record.clj +++ b/src/db/postgres/lrsql/postgres/record.clj @@ -201,8 +201,6 @@ bp/JWTBlocklistBackend (-insert-blocked-jwt! [_ tx input] (insert-blocked-jwt! tx input)) - (-delete-blocked-jwt-by-account! [_ tx input] - (delete-blocked-jwt-by-account! tx input)) (-delete-blocked-jwt-by-time! [_ tx input] (delete-blocked-jwt-by-time! tx input)) (-query-blocked-jwt [_ tx input] diff --git a/src/db/postgres/lrsql/postgres/sql/ddl.sql b/src/db/postgres/lrsql/postgres/sql/ddl.sql index 408344f69..de1a12e8a 100644 --- a/src/db/postgres/lrsql/postgres/sql/ddl.sql +++ b/src/db/postgres/lrsql/postgres/sql/ddl.sql @@ -495,9 +495,7 @@ ALTER TABLE reaction ALTER COLUMN title TYPE TEXT; -- :command :execute -- :doc Create the `blocked_jwt` table and associated indexes if they do not exist yet. CREATE TABLE IF NOT EXISTS blocked_jwt ( - account_id UUID NOT NULL REFERENCES admin_account(id) ON DELETE CASCADE, - expiration TIMESTAMP, - PRIMARY KEY (account_id, expiration) + jwt TEXT PRIMARY KEY, + evict_time TIMESTAMP ); -CREATE INDEX IF NOT EXISTS blocked_jwt_account_id_idx ON blocked_jwt(account_id); -CREATE INDEX IF NOT EXISTS blocked_jwt_expiration_idx ON blocked_jwt(expiration); +CREATE INDEX IF NOT EXISTS blocked_jwt_evict_time_idx ON blocked_jwt(evict_time); diff --git a/src/db/postgres/lrsql/postgres/sql/delete.sql b/src/db/postgres/lrsql/postgres/sql/delete.sql index b7417476d..e1dcf6e3c 100644 --- a/src/db/postgres/lrsql/postgres/sql/delete.sql +++ b/src/db/postgres/lrsql/postgres/sql/delete.sql @@ -111,16 +111,9 @@ DELETE FROM actor WHERE actor_ifi = :actor-ifi; /* JWT Blocklist */ --- :name delete-blocked-jwt-by-account! --- :command :execute --- :result :affected --- :doc Delete all blocked JWTs associated with a given `:account-id`. -DELETE FROM blocked_jwt -WHERE account_id = :account-id; - -- :name delete-blocked-jwt-by-time! -- :command :execute -- :result :affected --- :doc Delete all blocked JWTs where `:current-time` is past the expiration time. +-- :doc Delete all blocked JWTs where `:current-time` is past the eviction time. DELETE FROM blocked_jwt -WHERE expiration <= :current-time; +WHERE evict_time <= :current-time; diff --git a/src/db/postgres/lrsql/postgres/sql/insert.sql b/src/db/postgres/lrsql/postgres/sql/insert.sql index 68dc6ca8c..b3e41d802 100644 --- a/src/db/postgres/lrsql/postgres/sql/insert.sql +++ b/src/db/postgres/lrsql/postgres/sql/insert.sql @@ -165,9 +165,9 @@ INSERT INTO reaction ( -- :name insert-blocked-jwt! -- :command :insert -- :result :affected --- :doc Given a JWT-extracted `:account-id` and `:expiration`, insert a new blocked JWT. +-- :doc Insert a `:jwt` and a `:eviction-time` into the blocklist. INSERT INTO blocked_jwt ( - account_id, expiration + jwt, evict_time ) VALUES ( - :account-id, :expiration + :jwt, :eviction-time ); diff --git a/src/db/postgres/lrsql/postgres/sql/query.sql b/src/db/postgres/lrsql/postgres/sql/query.sql index 6c0fa4f5a..19eb95c03 100644 --- a/src/db/postgres/lrsql/postgres/sql/query.sql +++ b/src/db/postgres/lrsql/postgres/sql/query.sql @@ -433,7 +433,6 @@ WHERE reaction_id IS NOT NULL; -- :name query-blocked-jwt-exists -- :command :query -- :result :one --- :doc Query that at least one blocked JWT with `:username` and whose expiration is after `:current-time` exists. +-- :doc Query that `:jwt` is in the blocklist. SELECT 1 FROM blocked_jwt -WHERE account_id = :account-id -AND :current-time < expiration; +WHERE jwt = :jwt; diff --git a/src/db/sqlite/lrsql/sqlite/record.clj b/src/db/sqlite/lrsql/sqlite/record.clj index f7c2823cc..276ab96d3 100644 --- a/src/db/sqlite/lrsql/sqlite/record.clj +++ b/src/db/sqlite/lrsql/sqlite/record.clj @@ -113,8 +113,7 @@ (when-not (some? (query-statement-to-actor-has-cascade-delete tx)) (update-schema-simple! tx alter-statement-to-actor-add-cascade-delete!)) (create-blocked-jwt-table! tx) - (create-blocked-jwt-account-id-idx! tx) - (create-blocked-jwt-expiration-idx! tx) + (create-blocked-jwt-evict-time-idx! tx) (log/infof "sqlite schema_version: %d" (:schema_version (query-schema-version tx)))) @@ -243,13 +242,11 @@ bp/JWTBlocklistBackend (-insert-blocked-jwt! [_ tx input] (insert-blocked-jwt! tx input)) - (-delete-blocked-jwt-by-account! [_ tx input] - (delete-blocked-jwt-by-account! tx input)) (-delete-blocked-jwt-by-time! [_ tx input] (delete-blocked-jwt-by-time! tx input)) (-query-blocked-jwt [_ tx input] (query-blocked-jwt-exists tx input)) - + bp/CredentialBackend (-insert-credential! [_ tx input] (insert-credential! tx input)) diff --git a/src/db/sqlite/lrsql/sqlite/sql/ddl.sql b/src/db/sqlite/lrsql/sqlite/sql/ddl.sql index d6e9cd4e4..0e33fd5db 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/ddl.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/ddl.sql @@ -530,17 +530,11 @@ WHERE type = 'table' AND name = 'statement_to_actor' -- :command :execute -- :doc Create the `blocked_jwt` table if it does not exist yet. CREATE TABLE IF NOT EXISTS blocked_jwt ( - account_id TEXT NOT NULL REFERENCES admin_account(id) ON DELETE CASCADE, - expiration TIMESTAMP, - PRIMARY KEY (account_id, expiration) + jwt TEXT PRIMARY KEY, + evict_time TIMESTAMP ); --- :name create-blocked-jwt-account-id-idx! +-- :name create-blocked-jwt-evict-time-idx! -- :command :execute --- :doc Create the `blocked_jwt_account_id_idx` table if it does not exist yet. -CREATE INDEX IF NOT EXISTS blocked_jwt_account_id_idx ON blocked_jwt(account_id); - --- :name create-blocked-jwt-expiration-idx! --- :command :execute --- :doc Create the `blocked_jwt_expiration_idx` table if it does not exist yet. -CREATE INDEX IF NOT EXISTS blocked_jwt_expiration_idx ON blocked_jwt(expiration); +-- :doc Create the `blocked_jwt_evict_time_idx` table if it does not exist yet. +CREATE INDEX IF NOT EXISTS blocked_jwt_evict_time_idx ON blocked_jwt(evict_time); diff --git a/src/db/sqlite/lrsql/sqlite/sql/delete.sql b/src/db/sqlite/lrsql/sqlite/sql/delete.sql index 2884e93a3..571fee7fa 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/delete.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/delete.sql @@ -126,16 +126,9 @@ DELETE FROM actor WHERE actor_ifi = :actor-ifi /* JWT Blocklist */ --- :name delete-blocked-jwt-by-account! --- :command :execute --- :result :affected --- :doc Delete all blocked JWTs associated with a given `:account-id`. -DELETE FROM blocked_jwt -WHERE account_id = :account-id; - -- :name delete-blocked-jwt-by-time! -- :command :execute -- :result :affected --- :doc Delete all blocked JWTs where `:current-time` is past the expiration time. +-- :doc Delete all blocked JWTs where `:current-time` is past the eviction time. DELETE FROM blocked_jwt -WHERE expiration <= :current-time; +WHERE evict_time <= :current-time; diff --git a/src/db/sqlite/lrsql/sqlite/sql/insert.sql b/src/db/sqlite/lrsql/sqlite/sql/insert.sql index edbd40a47..f900ec1ee 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/insert.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/insert.sql @@ -165,9 +165,9 @@ INSERT INTO reaction ( -- :name insert-blocked-jwt! -- :command :insert -- :result :affected --- :doc Given a JWT-extracted `:account-id` and `:expiration`, insert a new blocked JWT. +-- :doc Insert a `:jwt` and a `:eviction-time` into the blocklist. INSERT INTO blocked_jwt ( - account_id, expiration + jwt, evict_time ) VALUES ( - :account-id, :expiration + :jwt, :eviction-time ); diff --git a/src/db/sqlite/lrsql/sqlite/sql/query.sql b/src/db/sqlite/lrsql/sqlite/sql/query.sql index 0b3dd0855..800fa8da9 100644 --- a/src/db/sqlite/lrsql/sqlite/sql/query.sql +++ b/src/db/sqlite/lrsql/sqlite/sql/query.sql @@ -401,7 +401,6 @@ WHERE reaction_id IS NOT NULL; -- :name query-blocked-jwt-exists -- :command :query -- :result :one --- :doc Query that at least one blocked JWT with `:account_id` and whose expiration is after `:current-time` exists. +-- :doc Query that `:jwt` is in the blocklist. SELECT 1 FROM blocked_jwt -WHERE account_id = :account-id -AND :current-time < expiration; +WHERE jwt = :jwt diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 9a22749ae..6eaee428e 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -123,18 +123,6 @@ {:status 401 :body {:error "Invalid Account Credentials"}}))))})) -(defn unblock-admin-jwts - "Remove all JWTs associated with the user account from the blocklist." - [leeway] - (interceptor - {:name ::remove-jwt-from-blocklist - :enter - (fn remove-jwt-from-blocklist [ctx] - (let [{lrs :com.yetanalytics/lrs - {:keys [account-id]} ::data} ctx] - (adp/-unblock-jwts lrs account-id leeway) - ctx))})) - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Terminal Interceptors ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -287,17 +275,16 @@ (defn block-admin-jwt "Add the current JWT to the blocklist. Return an error if we are in no-val mode." - [leeway no-val?] + [exp no-val?] (interceptor {:name ::add-jwt-to-blocklist :enter (fn add-jwt-to-blocklist [ctx] (if-not no-val? (let [{lrs :com.yetanalytics/lrs - {:keys [account-id expiration]} - :lrsql.admin.interceptors.jwt/data} + {:keys [jwt account-id]} :lrsql.admin.interceptors.jwt/data} ctx] - (adp/-block-jwt lrs account-id expiration leeway) + (adp/-block-jwt lrs jwt exp) (assoc (chain/terminate ctx) :response {:status 200 diff --git a/src/main/lrsql/admin/interceptors/jwt.clj b/src/main/lrsql/admin/interceptors/jwt.clj index 977ef93b0..7a2329a44 100644 --- a/src/main/lrsql/admin/interceptors/jwt.clj +++ b/src/main/lrsql/admin/interceptors/jwt.clj @@ -24,7 +24,7 @@ (let [{result* :result} (adp/-ensure-account-oidc lrs username issuer)] (if (keyword? result*) result* - (assoc result :account-id result*))))) + (assoc result :account-id result* :jwt token))))) (catch Exception ex ;; We want any error here to return a 401, but we log (log/warnf ex "No-val JWT Error: %s" (ex-message ex)) @@ -34,12 +34,11 @@ "Normal JWT, normal signature verification and blocklist check." [lrs token secret leeway] (try - (let [{:keys [account-id] :as result} - (admin-u/jwt->payload token secret leeway)] + (let [result (admin-u/jwt->payload token secret leeway)] (if (keyword? result) result - (if-not (adp/-jwt-blocked? lrs account-id leeway) - result + (if-not (adp/-jwt-blocked? lrs token) + (assoc result :jwt token) :lrsql.admin/unauthorized-token-error))) (catch Exception ex ;; We want any error here to return a 401, but we log diff --git a/src/main/lrsql/admin/protocol.clj b/src/main/lrsql/admin/protocol.clj index fe6e35782..452e64c0b 100644 --- a/src/main/lrsql/admin/protocol.clj +++ b/src/main/lrsql/admin/protocol.clj @@ -19,12 +19,10 @@ "Update the password for an admin account given old and new passwords.")) (defprotocol AdminJWTManager - (-block-jwt [this account-id expiration leeway] - "Block the JWT with this account, expiration time, and leeway. Purge expired JWTs from the blocklist if necessary.") - (-unblock-jwts [this account-id leeway] - "Unblock the JWTs associated with this account. Purge expired JWTs from the blocklist if necessary.") - (-jwt-blocked? [this account-id leeway] - "Is an unexpired JWT with this account on the blocklist?")) + (-block-jwt [this jwt expiration] + "Block `jwt` and apply an associated `expiration` number of seconds. Purge expired JWTs from the blocklist if necessary.") + (-jwt-blocked? [this jwt] + "Is `jwt` on the blocklist?")) (defprotocol APIKeyManager (-create-api-keys [this account-id scopes] diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 2883a3e2c..cdd976a50 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -40,7 +40,6 @@ (ai/validate-params :strict? false) ai/authenticate-admin - (ai/unblock-admin-jwts jwt-leeway) (ai/generate-jwt jwt-secret jwt-exp)) :route-name :lrsql.admin.account/login] {:description "Log into an existing account" @@ -58,7 +57,7 @@ (ji/validate-jwt jwt-secret jwt-leeway no-val-opts) ji/validate-jwt-account - (ai/block-admin-jwt jwt-leeway no-val?)) + (ai/block-admin-jwt jwt-exp no-val?)) :route-name :lrsql.admin.account/logout] {:description "Log out of this account" :operationId :logout diff --git a/src/main/lrsql/backend/protocol.clj b/src/main/lrsql/backend/protocol.clj index a93c71941..c8478e8fd 100644 --- a/src/main/lrsql/backend/protocol.clj +++ b/src/main/lrsql/backend/protocol.clj @@ -104,7 +104,6 @@ (defprotocol JWTBlocklistBackend ;; Commands (-insert-blocked-jwt! [this tx input]) - (-delete-blocked-jwt-by-account! [this tx input]) (-delete-blocked-jwt-by-time! [this tx input]) ;; Queries (-query-blocked-jwt [this tx input])) diff --git a/src/main/lrsql/input/admin/jwt.clj b/src/main/lrsql/input/admin/jwt.clj index 4c1a484af..8876d023d 100644 --- a/src/main/lrsql/input/admin/jwt.clj +++ b/src/main/lrsql/input/admin/jwt.clj @@ -3,44 +3,28 @@ [lrsql.spec.admin.jwt :as jwts] [lrsql.util :as u])) -(defn- current-time - "Generate the current time, offset by `leeway` number of seconds earlier. - - See: `buddy.sign.jwt/validate-claims`" - [leeway] +(defn- eviction-time + "Generate the current time, offset by `exp` number of seconds later." + [exp] (-> (u/current-time) - (u/offset-time (* -1 leeway) :seconds))) + (u/offset-time exp :seconds))) (s/fdef query-blocked-jwt-input - :args (s/cat :account-id ::jwts/account-id - :leeway ::jwts/leeway) + :args (s/cat :jwt ::jwts/jwt) :ret jwts/query-blocked-jwt-input-spec) (defn query-blocked-jwt-input - [account-id leeway] - {:account-id account-id - :current-time (current-time leeway)}) + [jwt] + {:jwt jwt}) (s/fdef insert-blocked-jwt-input - :args (s/cat :account-id ::jwts/account-id - :expiration ::jwts/expiration - :leeway ::jwts/leeway) + :args (s/cat :jwt ::jwts/jwt + :exp ::jwts/exp) :ret (s/and jwts/insert-blocked-jwt-input-spec - jwts/delete-blocked-jwt-time-input-spec)) + #_jwts/delete-blocked-jwt-time-input-spec)) (defn insert-blocked-jwt-input - [account-id expiration leeway] - {:account-id account-id - :expiration expiration - :current-time (current-time leeway)}) - -(s/fdef delete-blocked-jwts-input - :args (s/cat :account-id ::jwts/account-id - :leeway ::jwts/leeway) - :ret (s/and jwts/delete-blocked-jwt-account-input-spec - jwts/delete-blocked-jwt-time-input-spec)) - -(defn delete-blocked-jwts-input - [account-id leeway] - {:account-id account-id - :current-time (current-time leeway)}) + [jwt exp] + {:jwt jwt + :eviction-time (eviction-time exp) + :current-time (u/current-time)}) diff --git a/src/main/lrsql/ops/command/admin.clj b/src/main/lrsql/ops/command/admin.clj index 370f03bd4..85500a7ad 100644 --- a/src/main/lrsql/ops/command/admin.clj +++ b/src/main/lrsql/ops/command/admin.clj @@ -114,7 +114,7 @@ (s/fdef purge-blocklist! :args (s/cat :bk jwts/admin-jwt-backend? :tx transaction? - :input jwts/delete-blocked-jwt-time-input-spec)) + :input any? #_jwts/delete-blocked-jwt-time-input-spec)) (defn purge-blocklist! "Delete all JWTs from the blocklist that have expired, i.e. whose expirations @@ -132,16 +132,4 @@ "Insert a new JWT `:account-id` and `:expiration` to the blocklist table." [bk tx input] (bp/-insert-blocked-jwt! bk tx input) - {:result (:account-id input)}) - -(s/fdef delete-blocked-jwts! - :args (s/cat :bk jwts/admin-jwt-backend? - :tx transaction? - :input jwts/delete-blocked-jwt-account-input-spec) - :ret jwts/blocked-jwt-op-result-spec) - -(defn delete-blocked-jwts! - "Delete all JWTs associated with `:account-id` from the blocklist." - [bk tx input] - (bp/-delete-blocked-jwt-by-account! bk tx input) - {:result (:account-id input)}) + {:result (:jwt input)}) diff --git a/src/main/lrsql/spec/admin/jwt.clj b/src/main/lrsql/spec/admin/jwt.clj index edcf73c49..06f919bf9 100644 --- a/src/main/lrsql/spec/admin/jwt.clj +++ b/src/main/lrsql/spec/admin/jwt.clj @@ -16,30 +16,24 @@ ;; Inputs ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(s/def ::account-id :lrsql.spec.admin/account-id) -(s/def ::current-time c/instant-spec) -(s/def ::expiration c/instant-spec) -(s/def ::leeway ::config/jwt-exp-leeway) +(s/def ::jwt string?) +(s/def ::exp ::config/jwt-exp-time) +(s/def ::eviction-time c/instant-spec) (def query-blocked-jwt-input-spec - (s/keys :req-un [::account-id - ::current-time])) + (s/keys :req-un [::jwt])) (def insert-blocked-jwt-input-spec - (s/keys :req-un [::account-id - ::expiration])) + (s/keys :req-un [::jwt ::eviction-time])) -(def delete-blocked-jwt-account-input-spec - (s/keys :req-un [::account-id])) - -(def delete-blocked-jwt-time-input-spec +#_(def delete-blocked-jwt-time-input-spec (s/keys :req-un [::current-time])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Results ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(s/def :lrsql.spec.admin.jwt.command/result ::account-id) +(s/def :lrsql.spec.admin.jwt.command/result any? #_::account-id) (def blocked-jwt-op-result-spec (s/keys :req-un [:lrsql.spec.admin.jwt.command/result])) diff --git a/src/main/lrsql/system/lrs.clj b/src/main/lrsql/system/lrs.clj index e618e7b02..b265d6286 100644 --- a/src/main/lrsql/system/lrs.clj +++ b/src/main/lrsql/system/lrs.clj @@ -308,27 +308,16 @@ adp/AdminJWTManager (-block-jwt - [this account-id expiration leeway] + [this jwt exp] (let [conn (lrs-conn this) - jwt-input (admin-jwt-input/insert-blocked-jwt-input account-id - expiration - leeway)] + jwt-input (admin-jwt-input/insert-blocked-jwt-input jwt exp)] (jdbc/with-transaction [tx conn] (admin-cmd/purge-blocklist! backend tx jwt-input) (admin-cmd/insert-blocked-jwt! backend tx jwt-input)))) - (-unblock-jwts - [this account-id leeway] - (let [conn (lrs-conn this) - jwt-input (admin-jwt-input/delete-blocked-jwts-input account-id - leeway)] - (jdbc/with-transaction [tx conn] - (admin-cmd/purge-blocklist! backend tx jwt-input) - (admin-cmd/delete-blocked-jwts! backend tx jwt-input)))) (-jwt-blocked? - [this account-id leeway] + [this jwt] (let [conn (lrs-conn this) - jwt-input (admin-jwt-input/query-blocked-jwt-input account-id - leeway)] + jwt-input (admin-jwt-input/query-blocked-jwt-input jwt)] (jdbc/with-transaction [tx conn] (admin-q/query-blocked-jwt-exists backend tx jwt-input)))) diff --git a/src/test/lrsql/admin/protocol_test.clj b/src/test/lrsql/admin/protocol_test.clj index 54ed617f4..f7efefe18 100644 --- a/src/test/lrsql/admin/protocol_test.clj +++ b/src/test/lrsql/admin/protocol_test.clj @@ -127,40 +127,18 @@ (let [bad-account-id #uuid "00000000-0000-4000-8000-000000000000"] (is (not (adp/-existing-account? lrs bad-account-id))))) (testing "Admin JWTs" - (let [expiration-2 (u/current-time) - expiration (u/offset-time expiration-2 3600 :seconds) - account-id (:result - (adp/-authenticate-account lrs - test-username - test-password)) - account-id-2 (:result - (adp/-authenticate-account lrs - test-username-2 - test-password)) - leeway 2] + (let [expiration 1000 + jwt-1 "Foo" + jwt-2 "Bar"] (testing "- block" - (is (= account-id ; Current JWT - (:result (adp/-block-jwt lrs account-id expiration leeway)))) - (is (= account-id-2 ; Expired JWT (need to add after to avoid purge) - (:result (adp/-block-jwt lrs account-id-2 expiration-2 leeway)))) + (is (= jwt-1 ; Current JWT + (:result (adp/-block-jwt lrs jwt-1 expiration)))) + (is (= jwt-2 ; Expired JWT (need to add after to avoid purge) + (:result (adp/-block-jwt lrs jwt-2 expiration)))) (is (true? - (adp/-jwt-blocked? lrs account-id leeway))) - (is (true? ; Not expired thanks to leeway - (adp/-jwt-blocked? lrs account-id leeway))) - (Thread/sleep 2000) ; Wait 2 seconds to accommodate leeway + (adp/-jwt-blocked? lrs jwt-1))) (is (false? ; Expired JWT doesn't count as blocked - (adp/-jwt-blocked? lrs account-id-2 leeway)))) - (testing "- unblock" - (is (= account-id - (:result (adp/-unblock-jwts lrs account-id leeway)))) - (is (false? - (adp/-jwt-blocked? lrs account-id leeway))) - (is (false? - (adp/-jwt-blocked? lrs account-id-2 leeway))) - (is (= account-id-2 - (:result (adp/-unblock-jwts lrs account-id-2 leeway)))) - (is (false? - (adp/-jwt-blocked? lrs account-id-2 leeway)))))) + (adp/-jwt-blocked? lrs jwt-2)))))) (testing "Admin password update" (let [account-id (-> (adp/-authenticate-account lrs test-username diff --git a/src/test/lrsql/input/input_test.clj b/src/test/lrsql/input/input_test.clj index f8694daed..b63768c5e 100644 --- a/src/test/lrsql/input/input_test.clj +++ b/src/test/lrsql/input/input_test.clj @@ -61,8 +61,7 @@ (deftest test-admin-jwt (testing "admin JWT inputs" (is (nil? (check-validate `i-adm-jwt/query-blocked-jwt-input))) - (is (nil? (check-validate `i-adm-jwt/insert-blocked-jwt-input))) - (is (nil? (check-validate `i-adm-jwt/delete-blocked-jwts-input))))) + (is (nil? (check-validate `i-adm-jwt/insert-blocked-jwt-input))))) (deftest test-admin-status (testing "admin status inputs" From d53744d592bca602fd326b263d0c6b737f055fb7 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 4 Nov 2024 16:20:36 -0500 Subject: [PATCH 38/61] Fix admin JWT protocol test --- src/test/lrsql/admin/protocol_test.clj | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/lrsql/admin/protocol_test.clj b/src/test/lrsql/admin/protocol_test.clj index f7efefe18..c5851045c 100644 --- a/src/test/lrsql/admin/protocol_test.clj +++ b/src/test/lrsql/admin/protocol_test.clj @@ -130,7 +130,13 @@ (let [expiration 1000 jwt-1 "Foo" jwt-2 "Bar"] - (testing "- block" + (is (false? + (adp/-jwt-blocked? lrs jwt-1))) + (is (= jwt-1 + (:result (adp/-block-jwt lrs jwt-1 expiration)))) + (is (true? + (adp/-jwt-blocked? lrs jwt-1))) + #_(testing "- block" (is (= jwt-1 ; Current JWT (:result (adp/-block-jwt lrs jwt-1 expiration)))) (is (= jwt-2 ; Expired JWT (need to add after to avoid purge) From 68d2c503ae87514a5ae6c7012f6faebf82d7637a Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 4 Nov 2024 16:34:34 -0500 Subject: [PATCH 39/61] Make blocklist purging a separate protocol fn --- src/main/lrsql/admin/interceptors/account.clj | 5 +++- src/main/lrsql/admin/protocol.clj | 4 ++- src/main/lrsql/input/admin/jwt.clj | 14 ++++++--- src/main/lrsql/ops/command/admin.clj | 6 ++-- src/main/lrsql/spec/admin/jwt.clj | 5 ++-- src/main/lrsql/system/lrs.clj | 7 ++++- src/test/lrsql/admin/protocol_test.clj | 29 ++++++++++++------- 7 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 6eaee428e..1d9e0f8c3 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -259,10 +259,12 @@ {:name ::generate-jwt :enter (fn generate-jwt [ctx] - (let [{{:keys [account-id]} ::data} + (let [{lrs :com.yetanalytics/lrs + {:keys [account-id]} ::data} ctx json-web-token (admin-u/account-id->jwt account-id secret exp)] + (adp/-purge-blocklist lrs) ; Update blocklist upon login (assoc ctx :response {:status 200 @@ -284,6 +286,7 @@ (let [{lrs :com.yetanalytics/lrs {:keys [jwt account-id]} :lrsql.admin.interceptors.jwt/data} ctx] + (adp/-purge-blocklist lrs) ; Update blocklist upon logout (adp/-block-jwt lrs jwt exp) (assoc (chain/terminate ctx) :response diff --git a/src/main/lrsql/admin/protocol.clj b/src/main/lrsql/admin/protocol.clj index 452e64c0b..51e727579 100644 --- a/src/main/lrsql/admin/protocol.clj +++ b/src/main/lrsql/admin/protocol.clj @@ -19,8 +19,10 @@ "Update the password for an admin account given old and new passwords.")) (defprotocol AdminJWTManager + (-purge-blocklist [this] + "Purge the blocklist of any JWTs that have expired since they were added.") (-block-jwt [this jwt expiration] - "Block `jwt` and apply an associated `expiration` number of seconds. Purge expired JWTs from the blocklist if necessary.") + "Block `jwt` and apply an associated `expiration` number of seconds.") (-jwt-blocked? [this jwt] "Is `jwt` on the blocklist?")) diff --git a/src/main/lrsql/input/admin/jwt.clj b/src/main/lrsql/input/admin/jwt.clj index 8876d023d..f829f5812 100644 --- a/src/main/lrsql/input/admin/jwt.clj +++ b/src/main/lrsql/input/admin/jwt.clj @@ -20,11 +20,17 @@ (s/fdef insert-blocked-jwt-input :args (s/cat :jwt ::jwts/jwt :exp ::jwts/exp) - :ret (s/and jwts/insert-blocked-jwt-input-spec - #_jwts/delete-blocked-jwt-time-input-spec)) + :ret jwts/insert-blocked-jwt-input-spec) (defn insert-blocked-jwt-input [jwt exp] {:jwt jwt - :eviction-time (eviction-time exp) - :current-time (u/current-time)}) + :eviction-time (eviction-time exp)}) + +(s/fdef purge-blocklist-input + :args (s/cat) + :ret jwts/purge-blocklist-input-spec) + +(defn purge-blocklist-input + [] + {:current-time (u/current-time)}) diff --git a/src/main/lrsql/ops/command/admin.clj b/src/main/lrsql/ops/command/admin.clj index 85500a7ad..8f1b09d40 100644 --- a/src/main/lrsql/ops/command/admin.clj +++ b/src/main/lrsql/ops/command/admin.clj @@ -114,13 +114,15 @@ (s/fdef purge-blocklist! :args (s/cat :bk jwts/admin-jwt-backend? :tx transaction? - :input any? #_jwts/delete-blocked-jwt-time-input-spec)) + :input jwts/purge-blocklist-input-spec) + :ret nil?) (defn purge-blocklist! "Delete all JWTs from the blocklist that have expired, i.e. whose expirations are before the `:current-time` in `input`." [bk tx input] - (bp/-delete-blocked-jwt-by-time! bk tx input)) + (bp/-delete-blocked-jwt-by-time! bk tx input) + nil) (s/fdef insert-blocked-jwt! :args (s/cat :bk jwts/admin-jwt-backend? diff --git a/src/main/lrsql/spec/admin/jwt.clj b/src/main/lrsql/spec/admin/jwt.clj index 06f919bf9..e9fc5f72f 100644 --- a/src/main/lrsql/spec/admin/jwt.clj +++ b/src/main/lrsql/spec/admin/jwt.clj @@ -19,6 +19,7 @@ (s/def ::jwt string?) (s/def ::exp ::config/jwt-exp-time) (s/def ::eviction-time c/instant-spec) +(s/def ::current-time c/instant-spec) (def query-blocked-jwt-input-spec (s/keys :req-un [::jwt])) @@ -26,14 +27,14 @@ (def insert-blocked-jwt-input-spec (s/keys :req-un [::jwt ::eviction-time])) -#_(def delete-blocked-jwt-time-input-spec +(def purge-blocklist-input-spec (s/keys :req-un [::current-time])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Results ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(s/def :lrsql.spec.admin.jwt.command/result any? #_::account-id) +(s/def :lrsql.spec.admin.jwt.command/result ::jwt) (def blocked-jwt-op-result-spec (s/keys :req-un [:lrsql.spec.admin.jwt.command/result])) diff --git a/src/main/lrsql/system/lrs.clj b/src/main/lrsql/system/lrs.clj index b265d6286..665ca78be 100644 --- a/src/main/lrsql/system/lrs.clj +++ b/src/main/lrsql/system/lrs.clj @@ -307,12 +307,17 @@ {:result result}))))) adp/AdminJWTManager + (-purge-blocklist + [this] + (let [conn (lrs-conn this) + input (admin-jwt-input/purge-blocklist-input)] + (jdbc/with-transaction [tx conn] + (admin-cmd/purge-blocklist! backend tx input)))) (-block-jwt [this jwt exp] (let [conn (lrs-conn this) jwt-input (admin-jwt-input/insert-blocked-jwt-input jwt exp)] (jdbc/with-transaction [tx conn] - (admin-cmd/purge-blocklist! backend tx jwt-input) (admin-cmd/insert-blocked-jwt! backend tx jwt-input)))) (-jwt-blocked? [this jwt] diff --git a/src/test/lrsql/admin/protocol_test.clj b/src/test/lrsql/admin/protocol_test.clj index c5851045c..cede77732 100644 --- a/src/test/lrsql/admin/protocol_test.clj +++ b/src/test/lrsql/admin/protocol_test.clj @@ -127,23 +127,30 @@ (let [bad-account-id #uuid "00000000-0000-4000-8000-000000000000"] (is (not (adp/-existing-account? lrs bad-account-id))))) (testing "Admin JWTs" - (let [expiration 1000 + (let [expiration 2 jwt-1 "Foo" jwt-2 "Bar"] - (is (false? - (adp/-jwt-blocked? lrs jwt-1))) - (is (= jwt-1 - (:result (adp/-block-jwt lrs jwt-1 expiration)))) - (is (true? - (adp/-jwt-blocked? lrs jwt-1))) - #_(testing "- block" - (is (= jwt-1 ; Current JWT + (testing "- are unblocked by default" + (is (false? + (adp/-jwt-blocked? lrs jwt-1))) + (is (false? + (adp/-jwt-blocked? lrs jwt-2)))) + (testing "- can be blocked" + (is (= jwt-1 (:result (adp/-block-jwt lrs jwt-1 expiration)))) - (is (= jwt-2 ; Expired JWT (need to add after to avoid purge) + (Thread/sleep 2000) + (is (= jwt-2 (:result (adp/-block-jwt lrs jwt-2 expiration)))) (is (true? (adp/-jwt-blocked? lrs jwt-1))) - (is (false? ; Expired JWT doesn't count as blocked + (is (true? + (adp/-jwt-blocked? lrs jwt-2)))) + (testing "- can be purged from blocklist only when expired" + (is (= nil + (adp/-purge-blocklist lrs))) + (is (false? + (adp/-jwt-blocked? lrs jwt-1))) + (is (true? (adp/-jwt-blocked? lrs jwt-2)))))) (testing "Admin password update" (let [account-id (-> (adp/-authenticate-account lrs From 7bb60a466c34089c0362ff35b4007d3ca8f42962 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 4 Nov 2024 16:49:21 -0500 Subject: [PATCH 40/61] Handle JWT conflict errors --- src/main/lrsql/admin/interceptors/account.clj | 15 ++++++++++----- src/main/lrsql/admin/protocol.clj | 2 +- src/main/lrsql/ops/command/admin.clj | 14 ++++++++++---- src/test/lrsql/admin/protocol_test.clj | 4 +++- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 1d9e0f8c3..7a61ed59b 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -287,11 +287,16 @@ {:keys [jwt account-id]} :lrsql.admin.interceptors.jwt/data} ctx] (adp/-purge-blocklist lrs) ; Update blocklist upon logout - (adp/-block-jwt lrs jwt exp) - (assoc (chain/terminate ctx) - :response - {:status 200 - :body {:account-id account-id}})) + (let [result (adp/-block-jwt lrs jwt exp)] + (if-not (contains? result :error) + (assoc (chain/terminate ctx) + :response + {:status 200 + :body {:account-id account-id}}) + (assoc (chain/terminate ctx) + :response + {:status 409 + :body {:error "JWT has already been revoked."}})))) (assoc (chain/terminate ctx) :response {:status 400 diff --git a/src/main/lrsql/admin/protocol.clj b/src/main/lrsql/admin/protocol.clj index 51e727579..e4ed12aea 100644 --- a/src/main/lrsql/admin/protocol.clj +++ b/src/main/lrsql/admin/protocol.clj @@ -22,7 +22,7 @@ (-purge-blocklist [this] "Purge the blocklist of any JWTs that have expired since they were added.") (-block-jwt [this jwt expiration] - "Block `jwt` and apply an associated `expiration` number of seconds.") + "Block `jwt` and apply an associated `expiration` number of seconds. Returns an error if `jwt` is already in the blocklist.") (-jwt-blocked? [this jwt] "Is `jwt` on the blocklist?")) diff --git a/src/main/lrsql/ops/command/admin.clj b/src/main/lrsql/ops/command/admin.clj index 8f1b09d40..4e511d5a9 100644 --- a/src/main/lrsql/ops/command/admin.clj +++ b/src/main/lrsql/ops/command/admin.clj @@ -2,7 +2,7 @@ (:require [clojure.spec.alpha :as s] [lrsql.backend.protocol :as bp] [lrsql.input.admin :as admin-i] - [lrsql.spec.common :refer [transaction?]] + [lrsql.spec.common :as cs :refer [transaction?]] [lrsql.spec.admin :as ads] [lrsql.spec.admin.jwt :as jwts])) @@ -128,10 +128,16 @@ :args (s/cat :bk jwts/admin-jwt-backend? :tx transaction? :input jwts/insert-blocked-jwt-input-spec) - :ret jwts/blocked-jwt-op-result-spec) + :ret (s/or :success jwts/blocked-jwt-op-result-spec + :error (s/keys :req-un [::cs/error]))) (defn insert-blocked-jwt! "Insert a new JWT `:account-id` and `:expiration` to the blocklist table." [bk tx input] - (bp/-insert-blocked-jwt! bk tx input) - {:result (:jwt input)}) + (if (some? (bp/-query-blocked-jwt bk tx input)) + {:error (ex-info "Cannot have identical JWTs in the blocklist" + {:type ::jwt-conflict-error + :jwt (:jwt input)})} + (do + (bp/-insert-blocked-jwt! bk tx input) + {:result (:jwt input)}))) diff --git a/src/test/lrsql/admin/protocol_test.clj b/src/test/lrsql/admin/protocol_test.clj index cede77732..d6acb9bf2 100644 --- a/src/test/lrsql/admin/protocol_test.clj +++ b/src/test/lrsql/admin/protocol_test.clj @@ -151,7 +151,9 @@ (is (false? (adp/-jwt-blocked? lrs jwt-1))) (is (true? - (adp/-jwt-blocked? lrs jwt-2)))))) + (adp/-jwt-blocked? lrs jwt-2)))) + (testing "- cannot insert duplicates into blocklist" + (is (some? (:error (adp/-block-jwt lrs jwt-2 expiration))))))) (testing "Admin password update" (let [account-id (-> (adp/-authenticate-account lrs test-username From c37066d16eb3f4494de13e36b27eaf060e84a2fb Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 4 Nov 2024 17:11:20 -0500 Subject: [PATCH 41/61] Re-apply leeway to JWT eviction calcs --- src/main/lrsql/admin/interceptors/account.clj | 8 ++-- src/main/lrsql/admin/protocol.clj | 2 +- src/main/lrsql/admin/routes.clj | 6 ++- src/main/lrsql/input/admin/jwt.clj | 14 ++++-- src/main/lrsql/spec/admin/jwt.clj | 1 + src/main/lrsql/system/lrs.clj | 6 +-- src/test/lrsql/admin/protocol_test.clj | 43 ++++++++++--------- 7 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 7a61ed59b..7c7f4da74 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -254,7 +254,7 @@ (defn generate-jwt "Upon account login, generate a new JSON web token." - [secret exp] + [secret exp leeway] (interceptor {:name ::generate-jwt :enter @@ -264,7 +264,7 @@ ctx json-web-token (admin-u/account-id->jwt account-id secret exp)] - (adp/-purge-blocklist lrs) ; Update blocklist upon login + (adp/-purge-blocklist lrs leeway) ; Update blocklist upon login (assoc ctx :response {:status 200 @@ -277,7 +277,7 @@ (defn block-admin-jwt "Add the current JWT to the blocklist. Return an error if we are in no-val mode." - [exp no-val?] + [exp leeway no-val?] (interceptor {:name ::add-jwt-to-blocklist :enter @@ -286,7 +286,7 @@ (let [{lrs :com.yetanalytics/lrs {:keys [jwt account-id]} :lrsql.admin.interceptors.jwt/data} ctx] - (adp/-purge-blocklist lrs) ; Update blocklist upon logout + (adp/-purge-blocklist lrs leeway) ; Update blocklist upon logout (let [result (adp/-block-jwt lrs jwt exp)] (if-not (contains? result :error) (assoc (chain/terminate ctx) diff --git a/src/main/lrsql/admin/protocol.clj b/src/main/lrsql/admin/protocol.clj index e4ed12aea..dc6e52bdb 100644 --- a/src/main/lrsql/admin/protocol.clj +++ b/src/main/lrsql/admin/protocol.clj @@ -19,7 +19,7 @@ "Update the password for an admin account given old and new passwords.")) (defprotocol AdminJWTManager - (-purge-blocklist [this] + (-purge-blocklist [this leeway] "Purge the blocklist of any JWTs that have expired since they were added.") (-block-jwt [this jwt expiration] "Block `jwt` and apply an associated `expiration` number of seconds. Returns an error if `jwt` is already in the blocklist.") diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index cdd976a50..5ede19ba9 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -40,7 +40,8 @@ (ai/validate-params :strict? false) ai/authenticate-admin - (ai/generate-jwt jwt-secret jwt-exp)) + (ai/generate-jwt + jwt-secret jwt-exp jwt-leeway)) :route-name :lrsql.admin.account/login] {:description "Log into an existing account" :requestBody (g/request (gs/o {:username :t#string @@ -57,7 +58,8 @@ (ji/validate-jwt jwt-secret jwt-leeway no-val-opts) ji/validate-jwt-account - (ai/block-admin-jwt jwt-exp no-val?)) + (ai/block-admin-jwt + jwt-exp jwt-leeway no-val?)) :route-name :lrsql.admin.account/logout] {:description "Log out of this account" :operationId :logout diff --git a/src/main/lrsql/input/admin/jwt.clj b/src/main/lrsql/input/admin/jwt.clj index f829f5812..63d3f7bdc 100644 --- a/src/main/lrsql/input/admin/jwt.clj +++ b/src/main/lrsql/input/admin/jwt.clj @@ -9,6 +9,14 @@ (-> (u/current-time) (u/offset-time exp :seconds))) +(defn- current-time + "Generate the current time, offset by `leeway` number of seconds earlier. + + See: `buddy.sign.jwt/validate-claims`" + [leeway] + (-> (u/current-time) + (u/offset-time (* -1 leeway) :seconds))) + (s/fdef query-blocked-jwt-input :args (s/cat :jwt ::jwts/jwt) :ret jwts/query-blocked-jwt-input-spec) @@ -28,9 +36,9 @@ :eviction-time (eviction-time exp)}) (s/fdef purge-blocklist-input - :args (s/cat) + :args (s/cat :leeway ::jwts/leeway) :ret jwts/purge-blocklist-input-spec) (defn purge-blocklist-input - [] - {:current-time (u/current-time)}) + [leeway] + {:current-time (current-time leeway)}) diff --git a/src/main/lrsql/spec/admin/jwt.clj b/src/main/lrsql/spec/admin/jwt.clj index e9fc5f72f..c402d3d42 100644 --- a/src/main/lrsql/spec/admin/jwt.clj +++ b/src/main/lrsql/spec/admin/jwt.clj @@ -18,6 +18,7 @@ (s/def ::jwt string?) (s/def ::exp ::config/jwt-exp-time) +(s/def ::leeway ::config/jwt-exp-leeway) (s/def ::eviction-time c/instant-spec) (s/def ::current-time c/instant-spec) diff --git a/src/main/lrsql/system/lrs.clj b/src/main/lrsql/system/lrs.clj index 665ca78be..cb12aca53 100644 --- a/src/main/lrsql/system/lrs.clj +++ b/src/main/lrsql/system/lrs.clj @@ -308,9 +308,9 @@ adp/AdminJWTManager (-purge-blocklist - [this] - (let [conn (lrs-conn this) - input (admin-jwt-input/purge-blocklist-input)] + [this leeway] + (let [conn (lrs-conn this) + input (admin-jwt-input/purge-blocklist-input leeway)] (jdbc/with-transaction [tx conn] (admin-cmd/purge-blocklist! backend tx input)))) (-block-jwt diff --git a/src/test/lrsql/admin/protocol_test.clj b/src/test/lrsql/admin/protocol_test.clj index d6acb9bf2..bd4f460fa 100644 --- a/src/test/lrsql/admin/protocol_test.clj +++ b/src/test/lrsql/admin/protocol_test.clj @@ -127,33 +127,36 @@ (let [bad-account-id #uuid "00000000-0000-4000-8000-000000000000"] (is (not (adp/-existing-account? lrs bad-account-id))))) (testing "Admin JWTs" - (let [expiration 2 - jwt-1 "Foo" - jwt-2 "Bar"] + (let [exp 2 + leeway 1 + jwt "Foo"] (testing "- are unblocked by default" (is (false? - (adp/-jwt-blocked? lrs jwt-1))) - (is (false? - (adp/-jwt-blocked? lrs jwt-2)))) + (adp/-jwt-blocked? lrs jwt)))) (testing "- can be blocked" - (is (= jwt-1 - (:result (adp/-block-jwt lrs jwt-1 expiration)))) - (Thread/sleep 2000) - (is (= jwt-2 - (:result (adp/-block-jwt lrs jwt-2 expiration)))) + (is (= jwt + (:result (adp/-block-jwt lrs jwt exp)))) (is (true? - (adp/-jwt-blocked? lrs jwt-1))) + (adp/-jwt-blocked? lrs jwt)))) + (testing "- cannot insert duplicates into blocklist" + (is (some? (:error (adp/-block-jwt lrs jwt exp))))) + (testing "- cannot be purged from blocklist if not expired" + (is (= nil + (adp/-purge-blocklist lrs leeway))) (is (true? - (adp/-jwt-blocked? lrs jwt-2)))) - (testing "- can be purged from blocklist only when expired" + (adp/-jwt-blocked? lrs jwt)))) + (testing "- not counted as expired in blocklist due to leeway" + (Thread/sleep 2000) (is (= nil - (adp/-purge-blocklist lrs))) - (is (false? - (adp/-jwt-blocked? lrs jwt-1))) + (adp/-purge-blocklist lrs leeway))) (is (true? - (adp/-jwt-blocked? lrs jwt-2)))) - (testing "- cannot insert duplicates into blocklist" - (is (some? (:error (adp/-block-jwt lrs jwt-2 expiration))))))) + (adp/-jwt-blocked? lrs jwt)))) + (testing "- can be purged from blocklist when expired" + (Thread/sleep 1000) + (is (= nil + (adp/-purge-blocklist lrs leeway))) + (is (false? + (adp/-jwt-blocked? lrs jwt)))))) (testing "Admin password update" (let [account-id (-> (adp/-authenticate-account lrs test-username From 00b75103034bb587980d2007b52342939b83090e Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Tue, 5 Nov 2024 17:09:44 -0500 Subject: [PATCH 42/61] Add jwt-exp-time to admin UI route response --- src/main/lrsql/admin/interceptors/ui.clj | 6 ++++-- src/main/lrsql/admin/routes.clj | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/ui.clj b/src/main/lrsql/admin/interceptors/ui.clj index 565dcf238..067564d06 100644 --- a/src/main/lrsql/admin/interceptors/ui.clj +++ b/src/main/lrsql/admin/interceptors/ui.clj @@ -16,7 +16,8 @@ config to inject: :enable-admin-status - boolean, determines if the admin status endpoint is enabled." - [{:keys [enable-admin-delete-actor + [{:keys [jwt-exp-time + enable-admin-delete-actor enable-admin-status admin-language-code enable-reactions @@ -39,7 +40,8 @@ {:status 200 :body (merge - (cond-> {:url-prefix url-prefix + (cond-> {:jwt-exp-time jwt-exp-time + :url-prefix url-prefix :proxy-path proxy-path :enable-admin-delete-actor enable-admin-delete-actor :enable-admin-status enable-admin-status diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 562ad2084..4df294640 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -344,7 +344,8 @@ (admin-ui-routes (into common-interceptors oidc-ui-interceptors) - {:enable-admin-status enable-admin-status + {:jwt-exp-time exp + :enable-admin-status enable-admin-status :enable-reactions enable-reaction-routes :no-val? no-val? :no-val-logout-url no-val-logout-url From 7a2c8740fbe9706bc047e4216e2c77576ab5f7da Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 11 Nov 2024 11:06:53 -0500 Subject: [PATCH 43/61] Add jwtRefreshInterval and jwtInteractionWindow config vars --- doc/env_vars.md | 4 +++- resources/lrsql/config/prod/default/webserver.edn | 2 ++ resources/lrsql/config/test/default/webserver.edn | 2 ++ src/main/lrsql/admin/interceptors/ui.clj | 4 ++++ src/main/lrsql/spec/config.clj | 13 ++++++++++--- 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/doc/env_vars.md b/doc/env_vars.md index b5d5e8e39..0d3aa0be0 100644 --- a/doc/env_vars.md +++ b/doc/env_vars.md @@ -142,8 +142,10 @@ _NOTE:_ `LRSQL_STMT_RETRY_LIMIT` and `LRSQL_STMT_RETRY_BUDGET` are used to mitig | Env Var | Config | Description | Default | | --------------------------------- | ------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------- | | `LRSQL_JWT_EXP_TIME` | `jwtExpTime` | The amount of time, in seconds, after a JWT is created when it expires. **This this time should be short** (i.e. one hour or less). | `3600` (one hour) | -| `LRSQL_JWT_REFRESH_EXP_TIME` | `jwtRefreshExpTime` | The amount of time, in seconds, after a JWT is issued upon an initial login (_not_ a login renewal), after which login renewal can no longer be performed. Note: this is unaffected by expiration leeway. | `86400` (one day) | | `LRSQL_JWT_EXP_LEEWAY` | `jwtExpLeeway` | The amount of time, in seconds, before or after the expiration instant when a JWT should still count as un-expired. Used to compensate for clock desync. Applied to both LRS and OIDC tokens. | `1` (one second) | +| `LRSQL_JWT_REFRESH_EXP_TIME` | `jwtRefreshExpTime` | The amount of time, in seconds, after a JWT is issued upon an initial login (_not_ a login renewal), after which login renewal can no longer be performed. Note: this is unaffected by expiration leeway. | `86400` (one day) | +| `LRSQL_JWT_REFRESH_INTERVAL` | `jwtRefreshInterval` | The amount of time that the client should poll the server in order to refresh the JWT. This **must** be less than `LRSQL_JWT_EXP_TIME`. | `3540` (59 minutes) | +| `LRSQL_JWT_INTERACTION_WINDOW` | `jwtInteractionWindow` | The amount of time before a potential JWT refresh that the client checks for interaction. This **must** be less than `LRSQL_JWT_REFRESH_INTERVAL`. | `600` (10 minutes) | | `LRSQL_JWT_NO_VAL` | `jwtNoVal` | (**DANGEROUS!**) This flag removes JWT Token Validation and simply accepts token claims as configured by the associated variables below. It is extremely unlikely that you need this as it is for very specific proxy-overwrite authentication scenarios, and it poses a serious threat to system security if enabled. | `false` | | `LRSQL_JWT_NO_VAL_UNAME` | `jwtNoValUname` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the username when token validation is turned off. | Not Set | | `LRSQL_JWT_NO_VAL_ISSUER` | `jwtNoValIssuer` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the issuer when token validation is turned off. | Not Set | diff --git a/resources/lrsql/config/prod/default/webserver.edn b/resources/lrsql/config/prod/default/webserver.edn index e4c279e4b..9a330e29e 100644 --- a/resources/lrsql/config/prod/default/webserver.edn +++ b/resources/lrsql/config/prod/default/webserver.edn @@ -7,6 +7,8 @@ :jwt-exp-time #long #or [#env LRSQL_JWT_EXP_TIME 3600] :jwt-exp-leeway #long #or [#env LRSQL_JWT_EXP_LEEWAY 1] :jwt-refresh-exp-time #long #or [#env LRSQL_JWT_REFRESH_EXP_TIME 86400] + :jwt-refresh-interval #long #or [#env LRSQL_JWT_REFRESH_INTERVAL 3540] + :jwt-interaction-window #long #or [#env LRSQL_JWT_INTERACTION_WINDOW 600] :jwt-no-val #boolean #or [#env LRSQL_JWT_NO_VAL false] :jwt-no-val-uname #or [#env LRSQL_JWT_NO_VAL_UNAME nil] :jwt-no-val-issuer #or [#env LRSQL_JWT_NO_VAL_ISSUER nil] diff --git a/resources/lrsql/config/test/default/webserver.edn b/resources/lrsql/config/test/default/webserver.edn index 105be1aae..16e6dac7f 100644 --- a/resources/lrsql/config/test/default/webserver.edn +++ b/resources/lrsql/config/test/default/webserver.edn @@ -5,6 +5,8 @@ :jwt-exp-time 3600 :jwt-exp-leeway 1 :jwt-refresh-exp-time 86400 + :jwt-refresh-interval 3540 + :jwt-interaction-window 600 :jwt-no-val false :jwt-no-val-uname nil :jwt-no-val-issuer nil diff --git a/src/main/lrsql/admin/interceptors/ui.clj b/src/main/lrsql/admin/interceptors/ui.clj index 067564d06..c86380a2d 100644 --- a/src/main/lrsql/admin/interceptors/ui.clj +++ b/src/main/lrsql/admin/interceptors/ui.clj @@ -17,6 +17,8 @@ :enable-admin-status - boolean, determines if the admin status endpoint is enabled." [{:keys [jwt-exp-time + jwt-refresh-interval + jwt-interaction-window enable-admin-delete-actor enable-admin-status admin-language-code @@ -41,6 +43,8 @@ :body (merge (cond-> {:jwt-exp-time jwt-exp-time + :jwt-refresh-interval jwt-refresh-interval + :jwt-interaction-window jwt-interaction-window :url-prefix url-prefix :proxy-path proxy-path :enable-admin-delete-actor enable-admin-delete-actor diff --git a/src/main/lrsql/spec/config.clj b/src/main/lrsql/spec/config.clj index feeb92960..2a62456a6 100644 --- a/src/main/lrsql/spec/config.clj +++ b/src/main/lrsql/spec/config.clj @@ -152,8 +152,10 @@ (s/def ::allowed-origins (s/nilable (s/coll-of string?))) (s/def ::jwt-exp-time pos-int?) -(s/def ::jwt-refresh-exp-time pos-int?) (s/def ::jwt-exp-leeway nat-int?) +(s/def ::jwt-refresh-exp-time pos-int?) +(s/def ::jwt-refresh-interval pos-int?) +(s/def ::jwt-interaction-window pos-int?) (s/def ::jwt-no-val boolean?) (s/def ::jwt-no-val-uname (s/nilable string?)) (s/def ::jwt-no-val-issuer (s/nilable string?)) @@ -210,8 +212,10 @@ ::key-password ::key-enable-selfie ::jwt-exp-time - ::jwt-refresh-exp-time ::jwt-exp-leeway + ::jwt-refresh-exp-time + ::jwt-refresh-interval + ::jwt-interaction-window ::jwt-no-val ::enable-admin-ui ::enable-admin-status @@ -248,7 +252,10 @@ (if jwt-no-val (and jwt-no-val-uname jwt-no-val-issuer jwt-no-val-role-key jwt-no-val-role) - true)))) + true)) + ;; validation for JWT temporal intervals + (fn [{:keys [jwt-exp-time jwt-refresh-interval jwt-interaction-window]}] + (< jwt-interaction-window jwt-refresh-interval jwt-exp-time)))) (s/def ::tuning (s/keys :opt-un [::enable-jsonb])) From a5ee968279f552ebbec3fe4d23415de6ba9832e6 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 11 Nov 2024 11:07:18 -0500 Subject: [PATCH 44/61] Remove jwt-exp-time from get-env return --- src/main/lrsql/admin/interceptors/ui.clj | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/ui.clj b/src/main/lrsql/admin/interceptors/ui.clj index c86380a2d..d6a37bc9b 100644 --- a/src/main/lrsql/admin/interceptors/ui.clj +++ b/src/main/lrsql/admin/interceptors/ui.clj @@ -16,8 +16,7 @@ config to inject: :enable-admin-status - boolean, determines if the admin status endpoint is enabled." - [{:keys [jwt-exp-time - jwt-refresh-interval + [{:keys [jwt-refresh-interval jwt-interaction-window enable-admin-delete-actor enable-admin-status @@ -42,8 +41,7 @@ {:status 200 :body (merge - (cond-> {:jwt-exp-time jwt-exp-time - :jwt-refresh-interval jwt-refresh-interval + (cond-> {:jwt-refresh-interval jwt-refresh-interval :jwt-interaction-window jwt-interaction-window :url-prefix url-prefix :proxy-path proxy-path From b8247b48b0d51944779758b3ae579f12ffd4ea25 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 11 Nov 2024 11:18:13 -0500 Subject: [PATCH 45/61] Pass new JWT config to interceptor --- src/main/lrsql/admin/routes.clj | 5 ++++- src/main/lrsql/system/webserver.clj | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 4df294640..3b47ae759 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -312,6 +312,8 @@ no-val-role-key no-val-role no-val-logout-url + refresh-interval + interaction-window enable-admin-delete-actor enable-admin-ui enable-admin-status @@ -344,7 +346,8 @@ (admin-ui-routes (into common-interceptors oidc-ui-interceptors) - {:jwt-exp-time exp + {:jwt-refresh-interval refresh-interval + :jwt-interaction-window interaction-window :enable-admin-status enable-admin-status :enable-reactions enable-reaction-routes :no-val? no-val? diff --git a/src/main/lrsql/system/webserver.clj b/src/main/lrsql/system/webserver.clj index bdecb45c4..1d8b13e8b 100644 --- a/src/main/lrsql/system/webserver.clj +++ b/src/main/lrsql/system/webserver.clj @@ -40,6 +40,8 @@ sec-head-content allow-all-origins allowed-origins + jwt-refresh-interval + jwt-interaction-window jwt-no-val jwt-no-val-uname jwt-no-val-issuer @@ -94,6 +96,8 @@ :exp jwt-exp :ref jwt-ref :leeway jwt-lwy + :refresh-interval jwt-refresh-interval + :interaction-window jwt-interaction-window :no-val? jwt-no-val :no-val-issuer jwt-no-val-issuer :no-val-uname jwt-no-val-uname From 4e40b175edbf1b08455438cbcc0731b648b991b6 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 11 Nov 2024 12:34:44 -0500 Subject: [PATCH 46/61] Change config vars to less than or equal to --- doc/env_vars.md | 2 +- src/main/lrsql/spec/config.clj | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/env_vars.md b/doc/env_vars.md index 0d3aa0be0..a6c66881b 100644 --- a/doc/env_vars.md +++ b/doc/env_vars.md @@ -145,7 +145,7 @@ _NOTE:_ `LRSQL_STMT_RETRY_LIMIT` and `LRSQL_STMT_RETRY_BUDGET` are used to mitig | `LRSQL_JWT_EXP_LEEWAY` | `jwtExpLeeway` | The amount of time, in seconds, before or after the expiration instant when a JWT should still count as un-expired. Used to compensate for clock desync. Applied to both LRS and OIDC tokens. | `1` (one second) | | `LRSQL_JWT_REFRESH_EXP_TIME` | `jwtRefreshExpTime` | The amount of time, in seconds, after a JWT is issued upon an initial login (_not_ a login renewal), after which login renewal can no longer be performed. Note: this is unaffected by expiration leeway. | `86400` (one day) | | `LRSQL_JWT_REFRESH_INTERVAL` | `jwtRefreshInterval` | The amount of time that the client should poll the server in order to refresh the JWT. This **must** be less than `LRSQL_JWT_EXP_TIME`. | `3540` (59 minutes) | -| `LRSQL_JWT_INTERACTION_WINDOW` | `jwtInteractionWindow` | The amount of time before a potential JWT refresh that the client checks for interaction. This **must** be less than `LRSQL_JWT_REFRESH_INTERVAL`. | `600` (10 minutes) | +| `LRSQL_JWT_INTERACTION_WINDOW` | `jwtInteractionWindow` | The amount of time before a potential JWT refresh that the client checks for interaction. This **must** be less than or equal to `LRSQL_JWT_REFRESH_INTERVAL`. | `600` (10 minutes) | | `LRSQL_JWT_NO_VAL` | `jwtNoVal` | (**DANGEROUS!**) This flag removes JWT Token Validation and simply accepts token claims as configured by the associated variables below. It is extremely unlikely that you need this as it is for very specific proxy-overwrite authentication scenarios, and it poses a serious threat to system security if enabled. | `false` | | `LRSQL_JWT_NO_VAL_UNAME` | `jwtNoValUname` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the username when token validation is turned off. | Not Set | | `LRSQL_JWT_NO_VAL_ISSUER` | `jwtNoValIssuer` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the issuer when token validation is turned off. | Not Set | diff --git a/src/main/lrsql/spec/config.clj b/src/main/lrsql/spec/config.clj index 2a62456a6..d5da1825c 100644 --- a/src/main/lrsql/spec/config.clj +++ b/src/main/lrsql/spec/config.clj @@ -255,7 +255,8 @@ true)) ;; validation for JWT temporal intervals (fn [{:keys [jwt-exp-time jwt-refresh-interval jwt-interaction-window]}] - (< jwt-interaction-window jwt-refresh-interval jwt-exp-time)))) + (and (<= jwt-interaction-window jwt-refresh-interval) + (< jwt-refresh-interval jwt-exp-time))))) (s/def ::tuning (s/keys :opt-un [::enable-jsonb])) From ab80835bcf048f469e75148d1f321ce96f1dd3c7 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 11 Nov 2024 16:36:16 -0500 Subject: [PATCH 47/61] Make route tests pass with new config numbers --- src/test/lrsql/admin/route_test.clj | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/lrsql/admin/route_test.clj b/src/test/lrsql/admin/route_test.clj index 6f594be2e..2ef95923c 100644 --- a/src/test/lrsql/admin/route_test.clj +++ b/src/test/lrsql/admin/route_test.clj @@ -546,7 +546,9 @@ (let [sys (support/test-system :conf-overrides {[:webserver :jwt-exp-time] 3 - [:webserver :jwt-refresh-exp-time] 4}) + [:webserver :jwt-refresh-exp-time] 4 + [:webserver :jwt-refresh-interval] 2 + [:webserver :jwt-interaction-window] 2}) sys' (component/start sys) ;; Seed info {:keys [admin-user-default From 07b912d3a0db3c4ab81f0a2c028d8c0a3840e5e8 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Wed, 13 Nov 2024 15:38:12 -0500 Subject: [PATCH 48/61] Use TIMESTAMP WITH TIME ZONE for evict_time --- src/db/postgres/lrsql/postgres/sql/ddl.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/postgres/lrsql/postgres/sql/ddl.sql b/src/db/postgres/lrsql/postgres/sql/ddl.sql index de1a12e8a..ce6686a97 100644 --- a/src/db/postgres/lrsql/postgres/sql/ddl.sql +++ b/src/db/postgres/lrsql/postgres/sql/ddl.sql @@ -496,6 +496,6 @@ ALTER TABLE reaction ALTER COLUMN title TYPE TEXT; -- :doc Create the `blocked_jwt` table and associated indexes if they do not exist yet. CREATE TABLE IF NOT EXISTS blocked_jwt ( jwt TEXT PRIMARY KEY, - evict_time TIMESTAMP + evict_time TIMESTAMP WITH TIME ZONE ); CREATE INDEX IF NOT EXISTS blocked_jwt_evict_time_idx ON blocked_jwt(evict_time); From 1b8a96f28a695a935e08a5851ae11b7e4c58d91e Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Wed, 13 Nov 2024 15:39:24 -0500 Subject: [PATCH 49/61] Only purge blocklist on logout --- src/main/lrsql/admin/interceptors/account.clj | 6 ++---- src/main/lrsql/admin/routes.clj | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 882da683c..812cb76d4 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -262,17 +262,15 @@ (defn generate-jwt "Upon account login, generate a new JSON web token." - [secret exp leeway] + [secret exp] (interceptor {:name ::generate-jwt :enter (fn generate-jwt [ctx] - (let [{lrs :com.yetanalytics/lrs - {:keys [account-id]} ::data} + (let [{{:keys [account-id]} ::data} ctx json-web-token (admin-u/account-id->jwt account-id secret exp)] - (adp/-purge-blocklist lrs leeway) ; Update blocklist upon login (assoc ctx :response {:status 200 diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index dd463f187..fb81b305a 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -41,7 +41,7 @@ :strict? false) ai/authenticate-admin (ai/generate-jwt - jwt-secret jwt-exp jwt-leeway)) + jwt-secret jwt-exp)) :route-name :lrsql.admin.account/login] {:description "Log into an existing account" :requestBody (g/request (gs/o {:username :t#string From b115a41d4b94a66f6938facb2f96b489cf28bab3 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Wed, 13 Nov 2024 16:08:14 -0500 Subject: [PATCH 50/61] Fail silently upon double logout --- src/main/lrsql/admin/interceptors/account.clj | 15 +++++---------- src/test/lrsql/admin/route_test.clj | 2 ++ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/account.clj b/src/main/lrsql/admin/interceptors/account.clj index 812cb76d4..93d1ef2df 100644 --- a/src/main/lrsql/admin/interceptors/account.clj +++ b/src/main/lrsql/admin/interceptors/account.clj @@ -293,16 +293,11 @@ {:keys [jwt account-id]} :lrsql.admin.interceptors.jwt/data} ctx] (adp/-purge-blocklist lrs leeway) ; Update blocklist upon logout - (let [result (adp/-block-jwt lrs jwt exp)] - (if-not (contains? result :error) - (assoc (chain/terminate ctx) - :response - {:status 200 - :body {:account-id account-id}}) - (assoc (chain/terminate ctx) - :response - {:status 409 - :body {:error "JWT has already been revoked."}})))) + (adp/-block-jwt lrs jwt exp) + (assoc (chain/terminate ctx) + :response + {:status 200 + :body {:account-id account-id}})) (assoc (chain/terminate ctx) :response {:status 400 diff --git a/src/test/lrsql/admin/route_test.clj b/src/test/lrsql/admin/route_test.clj index d0e76e903..bba402a89 100644 --- a/src/test/lrsql/admin/route_test.clj +++ b/src/test/lrsql/admin/route_test.clj @@ -497,6 +497,8 @@ (let [{:keys [status]} (logout-account headers)] (is (= 200 status))) (is-err-code (get-me headers) 401) + ;; Logout again, should be unauthenticated + (is-err-code (logout-account headers) 401) ;; Login (let [{:keys [status body]} (login-account content-type seed-body) new-jwt (-> body From ec85d87c259b11a9cd30c33c16d41e33c647d4fb Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 15 Nov 2024 15:19:49 -0500 Subject: [PATCH 51/61] Implement get-spa and use that for /admin --- src/main/lrsql/admin/interceptors/ui.clj | 8 ++++++- src/main/lrsql/admin/routes.clj | 27 +++++++++++++++++------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/ui.clj b/src/main/lrsql/admin/interceptors/ui.clj index 565dcf238..8c0b4ba0d 100644 --- a/src/main/lrsql/admin/interceptors/ui.clj +++ b/src/main/lrsql/admin/interceptors/ui.clj @@ -5,11 +5,17 @@ [lrsql.admin.interceptors.oidc :as oidc-i] [lrsql.init.localization :refer [custom-language-map]])) +(defn get-spa + [path-prefix] + (fn [_] + (-> (resp/resource-response (str path-prefix "public/admin/index.html")) + (assoc-in [:headers "Content-Type"] "text/html")))) + (defn admin-ui-redirect "Handler function to redirect to the admin ui" [path-prefix] (fn [_] - (resp/redirect (str path-prefix "/admin/index.html")))) + (resp/redirect (str path-prefix "/admin")))) (defn get-env "Provide select config data to client upon request. Takes a map with static diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 170d9022e..9e3ef654a 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -215,18 +215,29 @@ (defn admin-ui-routes [common-interceptors {:keys [proxy-path] :as inject-config}] - #{;; Redirect root to admin UI + #{["/admin/env" :get (conj common-interceptors + (ui/get-env inject-config)) + :route-name :lrsql.admin.ui/get-env] + ;; SPA index.html retirevals + redirects + ["/admin" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/path-redirect] ["/" :get (ui/admin-ui-redirect proxy-path) :route-name :lrsql.admin.ui/root-redirect] - ;; Redirect admin w/o slash to admin UI - ["/admin" :get (ui/admin-ui-redirect proxy-path) - :route-name :lrsql.admin.ui/path-redirect] - ;; Redirect admin with slash to admin UI ["/admin/" :get (ui/admin-ui-redirect proxy-path) :route-name :lrsql.admin.ui/slash-redirect] - ["/admin/env" :get (conj common-interceptors - (ui/get-env inject-config)) - :route-name :lrsql.admin.ui/get-env]}) + ;; re-route routes + ["/admin/credentials" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/admin-credentials-redirect] + ["/admin/accounts" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/admin-accounts-redirect] + ["/admin/accounts/password" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/admin-accounts-password-redirect] + ["/admin/data-management" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/admin-data-management-redirect] + ["/admin/status" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/admin-status-redirect] + ["/admin/reactions" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/reactions-redirect]}) (defn admin-reaction-routes [common-interceptors jwt-secret jwt-leeway no-val-opts] From 29c3922315041ada96d1a9007b856ebd73bf97b0 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 15 Nov 2024 15:45:04 -0500 Subject: [PATCH 52/61] Add /ui for UI routes --- src/main/lrsql/admin/interceptors/ui.clj | 2 +- src/main/lrsql/admin/routes.clj | 28 ++++++++++-------------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/ui.clj b/src/main/lrsql/admin/interceptors/ui.clj index 8c0b4ba0d..ac35a605f 100644 --- a/src/main/lrsql/admin/interceptors/ui.clj +++ b/src/main/lrsql/admin/interceptors/ui.clj @@ -15,7 +15,7 @@ "Handler function to redirect to the admin ui" [path-prefix] (fn [_] - (resp/redirect (str path-prefix "/admin")))) + (resp/redirect (str path-prefix "/admin/ui")))) (defn get-env "Provide select config data to client upon request. Takes a map with static diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 9e3ef654a..c439400a5 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -218,26 +218,20 @@ #{["/admin/env" :get (conj common-interceptors (ui/get-env inject-config)) :route-name :lrsql.admin.ui/get-env] - ;; SPA index.html retirevals + redirects - ["/admin" :get (ui/get-spa proxy-path) - :route-name :lrsql.admin.ui/path-redirect] + ;; SPA routes + ["/admin/ui" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/main-path] + ["/admin/ui/" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/slash-path] + ["/admin/ui/*path" :get (ui/get-spa proxy-path) + :route-name :lrsql.admin.ui/sub-paths] + ;; SPA redirects ["/" :get (ui/admin-ui-redirect proxy-path) :route-name :lrsql.admin.ui/root-redirect] - ["/admin/" :get (ui/admin-ui-redirect proxy-path) + ["/admin" :get (ui/admin-ui-redirect proxy-path) :route-name :lrsql.admin.ui/slash-redirect] - ;; re-route routes - ["/admin/credentials" :get (ui/get-spa proxy-path) - :route-name :lrsql.admin.ui/admin-credentials-redirect] - ["/admin/accounts" :get (ui/get-spa proxy-path) - :route-name :lrsql.admin.ui/admin-accounts-redirect] - ["/admin/accounts/password" :get (ui/get-spa proxy-path) - :route-name :lrsql.admin.ui/admin-accounts-password-redirect] - ["/admin/data-management" :get (ui/get-spa proxy-path) - :route-name :lrsql.admin.ui/admin-data-management-redirect] - ["/admin/status" :get (ui/get-spa proxy-path) - :route-name :lrsql.admin.ui/admin-status-redirect] - ["/admin/reactions" :get (ui/get-spa proxy-path) - :route-name :lrsql.admin.ui/reactions-redirect]}) + ["/admin/" :get (ui/admin-ui-redirect proxy-path) + :route-name :lrsql.admin.ui/slash-redirect-2]}) (defn admin-reaction-routes [common-interceptors jwt-secret jwt-leeway no-val-opts] From c4959bd11fff0de337747d2c302292704e7d1d44 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 21 Nov 2024 16:18:07 -0500 Subject: [PATCH 53/61] Remove proxy-path from get-spa --- src/main/lrsql/admin/interceptors/ui.clj | 10 +++++----- src/main/lrsql/admin/routes.clj | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/ui.clj b/src/main/lrsql/admin/interceptors/ui.clj index ac35a605f..ab968785a 100644 --- a/src/main/lrsql/admin/interceptors/ui.clj +++ b/src/main/lrsql/admin/interceptors/ui.clj @@ -6,13 +6,13 @@ [lrsql.init.localization :refer [custom-language-map]])) (defn get-spa - [path-prefix] - (fn [_] - (-> (resp/resource-response (str path-prefix "public/admin/index.html")) - (assoc-in [:headers "Content-Type"] "text/html")))) + "Handler function that returns the index.html file." + [_] + (-> (resp/resource-response "public/admin/index.html") + (assoc-in [:headers "Content-Type"] "text/html"))) (defn admin-ui-redirect - "Handler function to redirect to the admin ui" + "Handler function to redirect to the admin UI." [path-prefix] (fn [_] (resp/redirect (str path-prefix "/admin/ui")))) diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index c439400a5..8875e9d62 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -219,11 +219,11 @@ (ui/get-env inject-config)) :route-name :lrsql.admin.ui/get-env] ;; SPA routes - ["/admin/ui" :get (ui/get-spa proxy-path) + ["/admin/ui" :get ui/get-spa :route-name :lrsql.admin.ui/main-path] - ["/admin/ui/" :get (ui/get-spa proxy-path) + ["/admin/ui/" :get ui/get-spa :route-name :lrsql.admin.ui/slash-path] - ["/admin/ui/*path" :get (ui/get-spa proxy-path) + ["/admin/ui/*path" :get ui/get-spa :route-name :lrsql.admin.ui/sub-paths] ;; SPA redirects ["/" :get (ui/admin-ui-redirect proxy-path) From 740f803663eb4f7524524302cc5c515b42166595 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 21 Nov 2024 18:07:28 -0500 Subject: [PATCH 54/61] Use Selmer templates to inject proxy path prefix --- src/main/lrsql/admin/interceptors/ui.clj | 10 +++++++--- src/main/lrsql/admin/routes.clj | 6 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/main/lrsql/admin/interceptors/ui.clj b/src/main/lrsql/admin/interceptors/ui.clj index ab968785a..a00e1497f 100644 --- a/src/main/lrsql/admin/interceptors/ui.clj +++ b/src/main/lrsql/admin/interceptors/ui.clj @@ -1,5 +1,6 @@ (ns lrsql.admin.interceptors.ui (:require [ring.util.response :as resp] + [selmer.parser :as selm-parser] [io.pedestal.interceptor :refer [interceptor]] [com.yetanalytics.lrs.pedestal.interceptor :as i] [lrsql.admin.interceptors.oidc :as oidc-i] @@ -7,9 +8,12 @@ (defn get-spa "Handler function that returns the index.html file." - [_] - (-> (resp/resource-response "public/admin/index.html") - (assoc-in [:headers "Content-Type"] "text/html"))) + [path-prefix] + (fn [_] + (-> (selm-parser/render-file "public/admin/index.html" + {:prefix path-prefix}) + resp/response + (resp/content-type "text/html")))) (defn admin-ui-redirect "Handler function to redirect to the admin UI." diff --git a/src/main/lrsql/admin/routes.clj b/src/main/lrsql/admin/routes.clj index 8875e9d62..c439400a5 100644 --- a/src/main/lrsql/admin/routes.clj +++ b/src/main/lrsql/admin/routes.clj @@ -219,11 +219,11 @@ (ui/get-env inject-config)) :route-name :lrsql.admin.ui/get-env] ;; SPA routes - ["/admin/ui" :get ui/get-spa + ["/admin/ui" :get (ui/get-spa proxy-path) :route-name :lrsql.admin.ui/main-path] - ["/admin/ui/" :get ui/get-spa + ["/admin/ui/" :get (ui/get-spa proxy-path) :route-name :lrsql.admin.ui/slash-path] - ["/admin/ui/*path" :get ui/get-spa + ["/admin/ui/*path" :get (ui/get-spa proxy-path) :route-name :lrsql.admin.ui/sub-paths] ;; SPA redirects ["/" :get (ui/admin-ui-redirect proxy-path) From 7046115c0eaa2c80716e7c83352f1508e3f08107 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Fri, 22 Nov 2024 11:20:01 -0500 Subject: [PATCH 55/61] Add a testing checklist in the dev docs --- doc/dev.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/doc/dev.md b/doc/dev.md index 2facd7676..14eb68b77 100644 --- a/doc/dev.md +++ b/doc/dev.md @@ -4,6 +4,30 @@ The SQL LRS is a Clojure Web Application built on the Pedestal Framework. +### Testing + +Development is primary test-driven, which an exhaustive suite of unit tests. To run them locally, run a `make test-[database]` command. In addition, all tests are run for all versions in GitHub Actions CI. + +However, in some situations, mainly UI development, relying on the unit tests may be inadequate. In these cases, there are multiple scenarios that SQL LRS can be used in that should be tested: + +#### Regular login + +This is by far the most common scenario. The user logs into the admin console, which then keeps the user logged in via JWTs. + +#### OIDC login + +The user logs in via OIDC using a Keycloak server. Steps to set up a Keycloak server and activate OIDC login can be found [here](oidc.md#keycloak-demo). + +#### Proxy JWT override + +The browser overrides SQL LRS's usual JWT issuing flow with an external JWT, which they can inject into request headers using tools such as the [Header Editor](https://chromewebstore.google.com/detail/header-editor/eningockdidmgiojffjmkdblpjocbhgh) Chrome extension. + +#### Proxy paths + +Users using a proxy server may prepend all HTTP endpoints using a prefix, e.g. append `/foo` to get `/foo/admin/account/login`. Steps to set up a proxy path demo can be found [here](other_demos.md#proxied-lrs-demo). + +Going through all the above scenarios is not necessary if the changes purely affect frontend features. However, they will be necessary if the changes affect login or the interface between the frontend and backend. + ### Build The SQL LRS can be built or run with the following Makefile targets. They can be executed with `make [target]`. From 839aee7118abefa3ac9774bdad1cea74261188f5 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Wed, 27 Nov 2024 12:25:53 -0500 Subject: [PATCH 56/61] Shorten list --- doc/dev.md | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/doc/dev.md b/doc/dev.md index 14eb68b77..22b1fce4e 100644 --- a/doc/dev.md +++ b/doc/dev.md @@ -8,25 +8,10 @@ The SQL LRS is a Clojure Web Application built on the Pedestal Framework. Development is primary test-driven, which an exhaustive suite of unit tests. To run them locally, run a `make test-[database]` command. In addition, all tests are run for all versions in GitHub Actions CI. -However, in some situations, mainly UI development, relying on the unit tests may be inadequate. In these cases, there are multiple scenarios that SQL LRS can be used in that should be tested: - -#### Regular login - -This is by far the most common scenario. The user logs into the admin console, which then keeps the user logged in via JWTs. - -#### OIDC login - -The user logs in via OIDC using a Keycloak server. Steps to set up a Keycloak server and activate OIDC login can be found [here](oidc.md#keycloak-demo). - -#### Proxy JWT override - -The browser overrides SQL LRS's usual JWT issuing flow with an external JWT, which they can inject into request headers using tools such as the [Header Editor](https://chromewebstore.google.com/detail/header-editor/eningockdidmgiojffjmkdblpjocbhgh) Chrome extension. - -#### Proxy paths - -Users using a proxy server may prepend all HTTP endpoints using a prefix, e.g. append `/foo` to get `/foo/admin/account/login`. Steps to set up a proxy path demo can be found [here](other_demos.md#proxied-lrs-demo). - -Going through all the above scenarios is not necessary if the changes purely affect frontend features. However, they will be necessary if the changes affect login or the interface between the frontend and backend. +However, in some situations, such as UI development, relying on the unit tests may be inadequate. In these cases, in addition to performing visual tests on the UI, one may need to test these specific scenarios: +- Login with OIDC ([demo](oidc.md#keycloak-demo)) +- Proxy paths ([demo](other_demos.md#proxied-lrs-demo)) +- JWT override ([JWT config vars](env_vars.md#jwt-config)) ### Build From 1dbe44a3de757c5da762220cc314648a21c4453e Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Wed, 27 Nov 2024 12:31:50 -0500 Subject: [PATCH 57/61] Grammar properly in the first sentence --- doc/dev.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/dev.md b/doc/dev.md index 22b1fce4e..e87d1620d 100644 --- a/doc/dev.md +++ b/doc/dev.md @@ -6,7 +6,7 @@ The SQL LRS is a Clojure Web Application built on the Pedestal Framework. ### Testing -Development is primary test-driven, which an exhaustive suite of unit tests. To run them locally, run a `make test-[database]` command. In addition, all tests are run for all versions in GitHub Actions CI. +Development is primarily test-driven, which uses an exhaustive suite of unit tests. To run them locally, run a `make test-[database]` command. In addition, all tests are run for all versions in GitHub Actions CI. However, in some situations, such as UI development, relying on the unit tests may be inadequate. In these cases, in addition to performing visual tests on the UI, one may need to test these specific scenarios: - Login with OIDC ([demo](oidc.md#keycloak-demo)) From 52a0c7e560035b25c1727daecade550c0d93dd62 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Tue, 17 Dec 2024 15:48:02 -0500 Subject: [PATCH 58/61] Add url-prefix restrictions to config spec --- src/main/lrsql/spec/config.clj | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/lrsql/spec/config.clj b/src/main/lrsql/spec/config.clj index 23645e92c..fb41baf07 100644 --- a/src/main/lrsql/spec/config.clj +++ b/src/main/lrsql/spec/config.clj @@ -1,5 +1,6 @@ (ns lrsql.spec.config (:require [clojure.spec.alpha :as s] + [clojure.string :as cstr] [xapi-schema.spec :as xs] [lrsql.spec.util :as u])) @@ -107,6 +108,14 @@ [{:keys [pool-validation-timeout pool-connection-timeout]}] (< pool-validation-timeout pool-connection-timeout)))) +(defn- prefix? [s] + (cstr/starts-with? s "/")) + +(defn- not-admin-prefix? [s] + (not (cstr/starts-with? s "/admin"))) + +(s/def ::stmt-url-prefix (s/and string? prefix? not-admin-prefix?)) + (s/def ::admin-user-default string?) (s/def ::admin-pass-default string?) @@ -147,6 +156,7 @@ (s/def ::http-host string?) (s/def ::http-port nat-int?) (s/def ::ssl-port nat-int?) +(s/def ::url-prefix ::stmt-url-prefix) (s/def ::allow-all-origins boolean?) (s/def ::allowed-origins (s/nilable (s/coll-of string?))) From 99d02f12f52a6325c8046b46dc0d22573be57c71 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Tue, 17 Dec 2024 15:48:08 -0500 Subject: [PATCH 59/61] Mention changes in docs --- doc/env_vars.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/env_vars.md b/doc/env_vars.md index 9973c859c..ddc12241f 100644 --- a/doc/env_vars.md +++ b/doc/env_vars.md @@ -123,7 +123,7 @@ _NOTE:_ `LRSQL_STMT_RETRY_LIMIT` and `LRSQL_STMT_RETRY_BUDGET` are used to mitig | `LRSQL_HTTP_HOST` | `httpHost` | The host that the webserver will run on. | `0.0.0.0` | | `LRSQL_HTTP_PORT` | `httpPort` | The HTTP port that the webserver will be open on. | `8080` | | `LRSQL_SSL_PORT` | `sslPort` | The HTTPS port that the webserver will be open on. | `8443` | -| `LRSQL_URL_PREFIX` | `urlPrefix` | The prefix of the webserver URL path, e.g. the prefix in `http://0.0.0.0:8080/xapi` is `/xapi`. Used when constructing the `more` value for multi-statement queries. *(Note: Only applies to LRS xapi endpoints, not admin/ui endpoints)* | `/xapi` | +| `LRSQL_URL_PREFIX` | `urlPrefix` | The prefix of the webserver URL path, e.g. the prefix in `http://0.0.0.0:8080/xapi` is `/xapi`. Used when constructing the `more` value for multi-statement queries. Cannot start with `/admin`. *(Note: Only applies to LRS xapi endpoints, not admin/ui endpoints)* | `/xapi` | | `LRSQL_PROXY_PATH` | `proxyPath` | This path modification is exclusively for use with a proxy, such as apache or nginx or a load balancer, where a path is added to prefix the entire application (such as `https://www.mysystem.com/mylrs/xapi/statements`). This does not actually change the routes of the application, it informs the admin frontend where to look for the server endpoints based on the proxied setup, and thus must be used in conjunction with a third party proxy. If used, the value must start with a leading `/` but not end with one (e.g. `/mylrs` is valid, as is `/mylrs/b` but `/mylrs/` is not). Use with caution. | Not Set | #### TLS/SSL Certificate From 42a19a3c3953adc46fbc00a4465c1368b0f7e53d Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 3 Mar 2025 12:38:21 -0500 Subject: [PATCH 60/61] Fix keycloak redirect url error in demo realm file --- dev-resources/keycloak_demo/test-realm.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-resources/keycloak_demo/test-realm.json b/dev-resources/keycloak_demo/test-realm.json index 7611be5e8..61cfe3f04 100644 --- a/dev-resources/keycloak_demo/test-realm.json +++ b/dev-resources/keycloak_demo/test-realm.json @@ -602,7 +602,7 @@ "enabled" : true, "alwaysDisplayInConsole" : false, "clientAuthenticatorType" : "client-secret", - "redirectUris" : [ "http://localhost:9500/", "http://localhost:8080/admin/index.html", "http://localhost:9500/*" ], + "redirectUris" : [ "http://localhost:9500/", "http://localhost:8080/admin/ui", "http://localhost:9500/admin/ui" ], "webOrigins" : [ "http://localhost:8080", "http://localhost:9500" ], "notBefore" : 0, "bearerOnly" : false, From 48ccbbf6a362ccd179760215ec6106ac15b24ba2 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Mon, 3 Mar 2025 12:54:22 -0500 Subject: [PATCH 61/61] Better explain env vars --- doc/env_vars.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/env_vars.md b/doc/env_vars.md index 219d9e54a..3b45bc152 100644 --- a/doc/env_vars.md +++ b/doc/env_vars.md @@ -144,8 +144,8 @@ _NOTE:_ `LRSQL_STMT_RETRY_LIMIT` and `LRSQL_STMT_RETRY_BUDGET` are used to mitig | `LRSQL_JWT_EXP_TIME` | `jwtExpTime` | The amount of time, in seconds, after a JWT is created when it expires. **This this time should be short** (i.e. one hour or less). | `3600` (one hour) | | `LRSQL_JWT_EXP_LEEWAY` | `jwtExpLeeway` | The amount of time, in seconds, before or after the expiration instant when a JWT should still count as un-expired. Used to compensate for clock desync. Applied to both LRS and OIDC tokens. | `1` (one second) | | `LRSQL_JWT_REFRESH_EXP_TIME` | `jwtRefreshExpTime` | The amount of time, in seconds, after a JWT is issued upon an initial login (_not_ a login renewal), after which login renewal can no longer be performed. Note: this is unaffected by expiration leeway. | `86400` (one day) | -| `LRSQL_JWT_REFRESH_INTERVAL` | `jwtRefreshInterval` | The amount of time that the client should poll the server in order to refresh the JWT. This **must** be less than `LRSQL_JWT_EXP_TIME`. | `3540` (59 minutes) | -| `LRSQL_JWT_INTERACTION_WINDOW` | `jwtInteractionWindow` | The amount of time before a potential JWT refresh that the client checks for interaction. This **must** be less than or equal to `LRSQL_JWT_REFRESH_INTERVAL`. | `600` (10 minutes) | +| `LRSQL_JWT_REFRESH_INTERVAL` | `jwtRefreshInterval` | The amount of time after either a login or a previous JWT refresh that the client should poll the server in order to refresh the JWT. This **must** be less than `LRSQL_JWT_EXP_TIME`. | `3540` (59 minutes) | +| `LRSQL_JWT_INTERACTION_WINDOW` | `jwtInteractionWindow` | The amount of time before a potential JWT refresh that the client checks for interaction. If an interaction happens during this window, a JWT refresh will be performed the next time the client polls for one; no refresh happens if there is no interaction (or if an interaction happens before this window). This **must** be less than or equal to `LRSQL_JWT_REFRESH_INTERVAL`. | `600` (10 minutes) | | `LRSQL_JWT_NO_VAL` | `jwtNoVal` | (**DANGEROUS!**) This flag removes JWT Token Validation and simply accepts token claims as configured by the associated variables below. It is extremely unlikely that you need this as it is for very specific proxy-overwrite authentication scenarios, and it poses a serious threat to system security if enabled. | `false` | | `LRSQL_JWT_NO_VAL_UNAME` | `jwtNoValUname` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the username when token validation is turned off. | Not Set | | `LRSQL_JWT_NO_VAL_ISSUER` | `jwtNoValIssuer` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the issuer when token validation is turned off. | Not Set |