Skip to content

Commit

Permalink
Re-apply leeway to JWT eviction calcs
Browse files Browse the repository at this point in the history
  • Loading branch information
kelvinqian00 committed Nov 4, 2024
1 parent 7bb60a4 commit c37066d
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 33 deletions.
8 changes: 4 additions & 4 deletions src/main/lrsql/admin/interceptors/account.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/main/lrsql/admin/protocol.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
6 changes: 4 additions & 2 deletions src/main/lrsql/admin/routes.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
14 changes: 11 additions & 3 deletions src/main/lrsql/input/admin/jwt.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)})
1 change: 1 addition & 0 deletions src/main/lrsql/spec/admin/jwt.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
6 changes: 3 additions & 3 deletions src/main/lrsql/system/lrs.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 23 additions & 20 deletions src/test/lrsql/admin/protocol_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c37066d

Please sign in to comment.