From 1a76e62d5615cee9b5682bb4dec3d3797d992dad Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Fri, 10 Jan 2025 14:35:09 +0100 Subject: [PATCH 01/30] Refresh expired access tokens Adds an additional flow to the existing middleware to refresh expired tokens. The implementation handles multiple active grants. Expired grants that failed to refresh are removed. The refresh workflow is unconditionally activated. Since a token refresh may occur for any request, access tokens are now always added to the response's `:session`. This may break code which previously relied on the `:session` only being set during the initial grant workflow. I do not think this can be avoided. If a refresh occurs, the tokens in `(:session request)` are left as-is, the updated access tokens are accessibly via the existing `:oauth2/access-token` key. This allows downstream handlers to observe that a token refresh occurred. There is a potential bug, where concurrent requests with expired tokens may race. For example, consider a page containing a `css` and a `js` resource. If a user's access token was to expire exactly as the `index.html` finishes loading, their browser may concurrently fetch both the `css` and `js` resources, triggering two concurrent token refresh attempts with the same token, one of which may fail. I do not see a way to address this without introducing considerable complexity. I have added a timeout of 60 seconds to the refresh http request, which means a slow oauth backend will cause users to become logged out. I think this is more informative for users than hanging forever. Fixes #40 --- src/ring/middleware/oauth2.clj | 144 ++++++++++++++++++++++++--- test/ring/middleware/oauth2_test.clj | 105 +++++++++++++++++-- 2 files changed, 227 insertions(+), 22 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index f4a52f9..8539353 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -114,26 +114,29 @@ (defn- access-token-http-options [{:keys [access-token-uri client-id client-secret basic-auth?] - :or {basic-auth? false} :as profile} - request] + :or {basic-auth? false}} + form-params] (let [opts {:method :post :url access-token-uri :accept :json :as :json - :form-params (request-params profile request)}] + :form-params form-params}] (if basic-auth? (add-header-credentials opts client-id client-secret) (add-form-credentials opts client-id client-secret)))) (defn- get-access-token ([profile request] - (-> (http/request (access-token-http-options profile request)) + (-> (access-token-http-options profile (request-params profile request)) + http/request (format-access-token))) ([profile request respond raise] - (http/request (-> (access-token-http-options profile request) - (assoc :async? true)) - (comp respond format-access-token) - raise))) + (http/request + (-> (access-token-http-options profile + (request-params profile request)) + (assoc :async? true)) + (comp respond format-access-token) + raise))) (defn state-mismatch-handler ([_] @@ -188,33 +191,142 @@ (respond (redirect-response profile session token))) raise))))) -(defn- assoc-access-tokens [request] - (if-let [tokens (-> request :session ::access-tokens)] +(defn- get-expired + "Returns expired profile keys and refresh tokens in `access-tokens`." + [access-tokens] + (let [now (new Date)] + (for [[profile-key {:keys [expires refresh-token]}] access-tokens + :when (and expires refresh-token (.before expires now))] + {:profile-key profile-key :refresh-token refresh-token}))) + +(defn- update-tokens + "If `maybe-grant` is nil, removes `profile-key` from `access-token; otherwise + merges `profile-key` with `maybe-grant`." + [access-tokens [profile-key maybe-grant]] + (if maybe-grant + ;; `update ... merge` to properly handle case where authorization server + ;; does not update the refresh token after use and we should re-use the + ;; existing refresh token + (update access-tokens profile-key merge maybe-grant) + (dissoc access-tokens profile-key))) + +(def socket-timeout 60000) + +(defn- refresh-one-token + ([profile refresh-token] + (-> (access-token-http-options + profile + {:grant_type "refresh_token" :refresh_token refresh-token}) + (assoc :socket-timeout socket-timeout) + http/request + format-access-token)) + ([profile refresh-token respond raise] + (-> (access-token-http-options + profile + {:grant_type "refresh_token" + :refresh_token refresh-token}) + (assoc :async? true + :socket-timeout socket-timeout) + (http/request (comp respond format-access-token) raise)))) + +(defn- valid-token? [token] + (and token (string? token) (not (str/blank? token)))) + +(defn- refresh-all-tokens + "Refreshes all expired tokens, yielding an updated map of tokens" + ([profiles access-tokens] + (let [refresh-results + (for [{:keys [profile-key refresh-token]} (get-expired access-tokens) + :let [profile (profile-key profiles)] + :when (and profile (valid-token? refresh-token))] + [profile-key + (try (refresh-one-token profile refresh-token) + (catch clojure.lang.ExceptionInfo _ + nil))])] + (reduce update-tokens access-tokens refresh-results))) + ([profiles access-tokens respond] + ;; strategy: launch all requests concurrently, keeping track of completed + ;; requests in `results`. When all requests have finished, respond. + (let [expired (get-expired access-tokens) + total (count expired) + results (atom {}) ;; map from profile-key to result + respond-when-done! #(when (= (count @results) total) + (respond (reduce update-tokens access-tokens @results)))] + (if (zero? total) + (respond access-tokens) + (doseq [{:keys [profile-key refresh-token]} expired + :let [profile (profile-key profiles)] + :when (and profile (valid-token? refresh-token))] + (refresh-one-token profile refresh-token + (fn [refresh-result] + (swap! results assoc profile-key refresh-result) + (respond-when-done!)) + (fn [_] + (swap! results assoc profile-key nil) + (respond-when-done!)))))))) + +(defn- assoc-access-tokens-in-request [request tokens] + (if tokens (assoc request :oauth2/access-tokens tokens) request)) +(defn- assoc-access-tokens-in-response + "If any tokens are present, adds to them the `:session` key of `response`." + [response tokens] + (if tokens + (assoc-in response [:session ::access-tokens] tokens) + response)) + (defn- parse-redirect-url [{:keys [redirect-uri]}] (.getPath (java.net.URI. redirect-uri))) (defn- valid-profile? [{:keys [client-id client-secret]}] (and (some? client-id) (some? client-secret))) -(defn wrap-oauth2 [handler profiles] +(defn wrap-oauth2 + "Middleware that handles OAuth2 authentication flows. + + Parameters: + * `handler`: The downstream ring handler + * `profiles`: A map of profiles + + Each request URI is matched against the profiles to determine the appropriate + OAuth2 flow handler. If no match is found, the request is passed to the + downstream handler with existing access tokens added to the request under the + `:oauth2/access-tokens` key. + + Expired tokens are refreshed using their refresh-token if possible. If refresh + fails, the access token is removed." + [handler profiles] {:pre [(every? valid-profile? (vals profiles))]} - (let [profiles (for [[k v] profiles] (assoc v :id k)) - launches (into {} (map (juxt :launch-uri identity)) profiles) - redirects (into {} (map (juxt parse-redirect-url identity)) profiles)] + (let [id-profiles (for [[k v] profiles] (assoc v :id k)) + launches (into {} (map (juxt :launch-uri identity)) id-profiles) + redirects (into {} (map (juxt parse-redirect-url identity)) id-profiles)] (fn ([{:keys [uri] :as request}] (if-let [profile (launches uri)] ((make-launch-handler profile) request) (if-let [profile (redirects uri)] ((:redirect-handler profile (make-redirect-handler profile)) request) - (handler (assoc-access-tokens request))))) + (let [access-tokens (get-in request [:session ::access-tokens]) + refreshed-tokens (refresh-all-tokens profiles access-tokens)] + (-> request + (assoc-access-tokens-in-request refreshed-tokens) + handler + (assoc-access-tokens-in-response refreshed-tokens)))))) ([{:keys [uri] :as request} respond raise] (if-let [profile (launches uri)] ((make-launch-handler profile) request respond raise) (if-let [profile (redirects uri)] ((:redirect-handler profile (make-redirect-handler profile)) request respond raise) - (handler (assoc-access-tokens request) respond raise))))))) + (let [access-tokens (get-in request [:session ::access-tokens]) + respond (fn [refreshed-tokens] + (handler + (assoc-access-tokens-in-request + request refreshed-tokens) + (comp respond + #(assoc-access-tokens-in-response + % refreshed-tokens)) + raise))] + (refresh-all-tokens profiles access-tokens respond)))))))) diff --git a/test/ring/middleware/oauth2_test.clj b/test/ring/middleware/oauth2_test.clj index 92151f1..652e72c 100644 --- a/test/ring/middleware/oauth2_test.clj +++ b/test/ring/middleware/oauth2_test.clj @@ -109,8 +109,9 @@ b-ms (.getTime b)] (< (- a-ms 1000) b-ms (+ a-ms 1000)))) -(defn- seconds-from-now-to-date [secs] - (-> (Instant/now) (.plusSeconds secs) (Date/from))) +(defn- seconds-from-now-to-date + ([now secs] (-> now (.plusSeconds secs) (Date/from))) + ([secs] (seconds-from-now-to-date (Instant/now) secs))) (deftest test-redirect-uri (fake/with-fake-routes @@ -203,7 +204,10 @@ (deftest test-access-tokens-key (let [tokens {:test {:token "defdef", :expires 3600}}] - (is (= {:status 200, :headers {}, :body tokens} + (is (= {:status 200, + :headers {}, + :body tokens, + :session {::oauth2/access-tokens tokens}} (-> (mock/request :get "/") (assoc :session {::oauth2/access-tokens tokens}) (test-handler)))))) @@ -376,10 +380,11 @@ (deftest test-handler-passthrough (let [tokens {:test "tttkkkk"} + session {::oauth2/access-tokens tokens} request (-> (mock/request :get "/example") - (assoc :session {::oauth2/access-tokens tokens}))] + (assoc :session session))] (testing "sync handler" - (is (= {:status 200, :headers {}, :body tokens} + (is (= {:status 200, :headers {}, :body tokens :session session} (test-handler request)))) (testing "async handler" @@ -388,5 +393,93 @@ (test-handler request respond raise) (is (= :empty (deref raise 100 :empty))) - (is (= {:status 200, :headers {}, :body tokens} + (is (= {:status 200, :headers {}, :body tokens :session session} (deref respond 100 :empty))))))) + +(def refresh-token-response + {:status 200 + :headers {"Content-Type" "application/json"} + :body "{\"access_token\":\"newtoken\",\"expires_in\":3600, + \"refresh_token\":\"newrefresh\",\"foo\":\"bar\"}"}) + +(deftest test-token-refresh-success + (fake/with-fake-routes + {"https://example.com/oauth2/access-token" + (fn [req] + (let [params (codec/form-decode (slurp (:body req)))] + (is (= "refresh_token" (get params "grant_type"))) + (is (= "oldrefresh" (get params "refresh_token"))) + refresh-token-response))} + + (let [now (Instant/now) + old-expires (seconds-from-now-to-date now -60) + new-expires (seconds-from-now-to-date now 3600) + new-token {:token "newtoken" + :refresh-token "newrefresh" + :extra-data {:foo "bar"}} + request (-> (mock/request :get "/") + (assoc :session + {::oauth2/access-tokens + {:test {:token "oldtoken" + :refresh-token "oldrefresh" + :expires old-expires}}}))] + (testing "sync refresh" + (let [response (test-handler request)] + (is (= 200 (:status response))) + ;; then handler has new token + (is (= new-token (dissoc (get-in response [:body :test]) :expires))) + (is (approx-eq new-expires (get-in response [:body :test :expires]))) + ;; and the user's session is updated + (is (= new-token (dissoc (get-in response [:session ::oauth2/access-tokens :test]) + :expires))))) + (testing "async refresh" + (let [respond (promise) + raise (promise)] + (test-handler request respond raise) + (is (= :empty (deref raise 100 :empty))) + (let [response (deref respond 100 :empty)] + ;; then handler has new token + (is (not= response :empty)) + (is (= new-token (dissoc (get-in response [:body :test]) :expires))) + ;; user session is updated + (is (= new-token + (dissoc (get-in response [:session ::oauth2/access-tokens + :test]) + :expires))))))))) + +(def refresh-token-error-response + {:headers {"content-type" "application/json"}, + :status 400, + :body "{\"error\": \"invalid_grant\"}"}) + +(deftest test-token-refresh-failure + (fake/with-fake-routes + {"https://example.com/oauth2/access-token" + (constantly refresh-token-error-response)} + + ;; setup a session with two grants, where one grant is expired and which + ;; will error on refresh + (let [profiles {:test-0 test-profile :test-1 test-profile} + handler (wrap-oauth2 token-handler profiles) + good-grant {:token "good-token" + :refresh-token "refresh-token" + :expires (seconds-from-now-to-date 3600)} + expired-grant {:token "expired-token" + :refresh-token "invalid" + :expires (seconds-from-now-to-date -60)} + request (-> (mock/request :get "/") + (assoc :session + {::oauth2/access-tokens + {:test-0 expired-grant + :test-1 good-grant}}))] + (testing "sync handler" + (let [response (handler request)] + (is (= {:test-1 good-grant} + (:body response))))) + (testing "async refresh" + (let [respond (promise) + raise (promise)] + (handler request respond raise) + (is (= :empty (deref raise 100 :empty))) + (let [response (deref respond 100 :empty)] + (is (= {:test-1 good-grant} (:body response))))))))) From 9f0c3f411475b7ff5f112814b454fd68f33b0461 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Sun, 12 Jan 2025 18:13:04 +0100 Subject: [PATCH 02/30] reflow to 80 characters per line --- src/ring/middleware/oauth2.clj | 3 ++- test/ring/middleware/oauth2_test.clj | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 8539353..38f2a03 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -251,7 +251,8 @@ total (count expired) results (atom {}) ;; map from profile-key to result respond-when-done! #(when (= (count @results) total) - (respond (reduce update-tokens access-tokens @results)))] + (respond (reduce update-tokens + access-tokens @results)))] (if (zero? total) (respond access-tokens) (doseq [{:keys [profile-key refresh-token]} expired diff --git a/test/ring/middleware/oauth2_test.clj b/test/ring/middleware/oauth2_test.clj index 652e72c..c86fe3e 100644 --- a/test/ring/middleware/oauth2_test.clj +++ b/test/ring/middleware/oauth2_test.clj @@ -430,8 +430,10 @@ (is (= new-token (dissoc (get-in response [:body :test]) :expires))) (is (approx-eq new-expires (get-in response [:body :test :expires]))) ;; and the user's session is updated - (is (= new-token (dissoc (get-in response [:session ::oauth2/access-tokens :test]) - :expires))))) + (is (= new-token + (dissoc (get-in response + [:session ::oauth2/access-tokens :test]) + :expires))))) (testing "async refresh" (let [respond (promise) raise (promise)] From 0a0b10622d96534018e4e668da5c3af928c83dc8 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Sun, 12 Jan 2025 18:21:53 +0100 Subject: [PATCH 03/30] =?UTF-8?q?rename=20get-expired=20=E2=86=92=20expire?= =?UTF-8?q?d-access-tokens?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/ring/middleware/oauth2.clj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 38f2a03..2140188 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -191,7 +191,7 @@ (respond (redirect-response profile session token))) raise))))) -(defn- get-expired +(defn- expired-access-tokens "Returns expired profile keys and refresh tokens in `access-tokens`." [access-tokens] (let [now (new Date)] @@ -236,7 +236,7 @@ "Refreshes all expired tokens, yielding an updated map of tokens" ([profiles access-tokens] (let [refresh-results - (for [{:keys [profile-key refresh-token]} (get-expired access-tokens) + (for [{:keys [profile-key refresh-token]} (expired-access-tokens access-tokens) :let [profile (profile-key profiles)] :when (and profile (valid-token? refresh-token))] [profile-key @@ -247,7 +247,7 @@ ([profiles access-tokens respond] ;; strategy: launch all requests concurrently, keeping track of completed ;; requests in `results`. When all requests have finished, respond. - (let [expired (get-expired access-tokens) + (let [expired (expired-access-tokens access-tokens) total (count expired) results (atom {}) ;; map from profile-key to result respond-when-done! #(when (= (count @results) total) From cbfa368228b7a597f72f57e0c8f0c26dbbd6ae93 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Sun, 12 Jan 2025 18:23:14 +0100 Subject: [PATCH 04/30] remove docstrings on non-public functions; remove unrelated changes --- src/ring/middleware/oauth2.clj | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 2140188..3d25c39 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -192,7 +192,6 @@ raise))))) (defn- expired-access-tokens - "Returns expired profile keys and refresh tokens in `access-tokens`." [access-tokens] (let [now (new Date)] (for [[profile-key {:keys [expires refresh-token]}] access-tokens @@ -200,8 +199,6 @@ {:profile-key profile-key :refresh-token refresh-token}))) (defn- update-tokens - "If `maybe-grant` is nil, removes `profile-key` from `access-token; otherwise - merges `profile-key` with `maybe-grant`." [access-tokens [profile-key maybe-grant]] (if maybe-grant ;; `update ... merge` to properly handle case where authorization server @@ -233,7 +230,6 @@ (and token (string? token) (not (str/blank? token)))) (defn- refresh-all-tokens - "Refreshes all expired tokens, yielding an updated map of tokens" ([profiles access-tokens] (let [refresh-results (for [{:keys [profile-key refresh-token]} (expired-access-tokens access-tokens) @@ -272,7 +268,6 @@ request)) (defn- assoc-access-tokens-in-response - "If any tokens are present, adds to them the `:session` key of `response`." [response tokens] (if tokens (assoc-in response [:session ::access-tokens] tokens) @@ -285,19 +280,6 @@ (and (some? client-id) (some? client-secret))) (defn wrap-oauth2 - "Middleware that handles OAuth2 authentication flows. - - Parameters: - * `handler`: The downstream ring handler - * `profiles`: A map of profiles - - Each request URI is matched against the profiles to determine the appropriate - OAuth2 flow handler. If no match is found, the request is passed to the - downstream handler with existing access tokens added to the request under the - `:oauth2/access-tokens` key. - - Expired tokens are refreshed using their refresh-token if possible. If refresh - fails, the access token is removed." [handler profiles] {:pre [(every? valid-profile? (vals profiles))]} (let [id-profiles (for [[k v] profiles] (assoc v :id k)) From ba38ad2c557b062fbe945876de76eefb3d16ce98 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Sun, 12 Jan 2025 18:24:33 +0100 Subject: [PATCH 05/30] prefer `(Date.)` over `(new Date)` --- src/ring/middleware/oauth2.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 3d25c39..f77224e 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -193,7 +193,7 @@ (defn- expired-access-tokens [access-tokens] - (let [now (new Date)] + (let [now (Date.)] (for [[profile-key {:keys [expires refresh-token]}] access-tokens :when (and expires refresh-token (.before expires now))] {:profile-key profile-key :refresh-token refresh-token}))) From 65e81fcbcde1ccc74d3bda3da79efe0095058d95 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Sun, 12 Jan 2025 18:25:39 +0100 Subject: [PATCH 06/30] remove token type check --- src/ring/middleware/oauth2.clj | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index f77224e..71c4856 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -226,15 +226,12 @@ :socket-timeout socket-timeout) (http/request (comp respond format-access-token) raise)))) -(defn- valid-token? [token] - (and token (string? token) (not (str/blank? token)))) - (defn- refresh-all-tokens ([profiles access-tokens] (let [refresh-results (for [{:keys [profile-key refresh-token]} (expired-access-tokens access-tokens) :let [profile (profile-key profiles)] - :when (and profile (valid-token? refresh-token))] + :when (and profile refresh-token)] [profile-key (try (refresh-one-token profile refresh-token) (catch clojure.lang.ExceptionInfo _ @@ -253,7 +250,7 @@ (respond access-tokens) (doseq [{:keys [profile-key refresh-token]} expired :let [profile (profile-key profiles)] - :when (and profile (valid-token? refresh-token))] + :when (and profile refresh-token)] (refresh-one-token profile refresh-token (fn [refresh-result] (swap! results assoc profile-key refresh-result) From 5327380c40619d85c9cb5588cce1ea1ef594e528 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Sun, 12 Jan 2025 18:39:17 +0100 Subject: [PATCH 07/30] refactor `expired-access-tokens` as filter --- src/ring/middleware/oauth2.clj | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 71c4856..752e56c 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -193,10 +193,11 @@ (defn- expired-access-tokens [access-tokens] - (let [now (Date.)] - (for [[profile-key {:keys [expires refresh-token]}] access-tokens - :when (and expires refresh-token (.before expires now))] - {:profile-key profile-key :refresh-token refresh-token}))) + (let [now (Date.) + expired-access-token? (fn [[_ {:keys [expires refresh-token]}]] + (and refresh-token expires + (.before expires now)))] + (->> access-tokens (filter expired-access-token?) (into {})))) (defn- update-tokens [access-tokens [profile-key maybe-grant]] @@ -229,7 +230,8 @@ (defn- refresh-all-tokens ([profiles access-tokens] (let [refresh-results - (for [{:keys [profile-key refresh-token]} (expired-access-tokens access-tokens) + (for [[profile-key {:keys [refresh-token]}] (expired-access-tokens + access-tokens) :let [profile (profile-key profiles)] :when (and profile refresh-token)] [profile-key @@ -248,7 +250,7 @@ access-tokens @results)))] (if (zero? total) (respond access-tokens) - (doseq [{:keys [profile-key refresh-token]} expired + (doseq [[profile-key {:keys [refresh-token]}] expired :let [profile (profile-key profiles)] :when (and profile refresh-token)] (refresh-one-token profile refresh-token From 448624ea998ba123aabf38aa2a41c1132c13b48f Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Sun, 12 Jan 2025 19:00:04 +0100 Subject: [PATCH 08/30] fix confusing naming --- src/ring/middleware/oauth2.clj | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 752e56c..d1f00b2 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -290,12 +290,12 @@ ((make-launch-handler profile) request) (if-let [profile (redirects uri)] ((:redirect-handler profile (make-redirect-handler profile)) request) - (let [access-tokens (get-in request [:session ::access-tokens]) - refreshed-tokens (refresh-all-tokens profiles access-tokens)] + (let [access-tokens (->> (get-in request [:session ::access-tokens]) + (refresh-all-tokens profiles))] (-> request - (assoc-access-tokens-in-request refreshed-tokens) + (assoc-access-tokens-in-request access-tokens) handler - (assoc-access-tokens-in-response refreshed-tokens)))))) + (assoc-access-tokens-in-response access-tokens)))))) ([{:keys [uri] :as request} respond raise] (if-let [profile (launches uri)] ((make-launch-handler profile) request respond raise) @@ -303,12 +303,12 @@ ((:redirect-handler profile (make-redirect-handler profile)) request respond raise) (let [access-tokens (get-in request [:session ::access-tokens]) - respond (fn [refreshed-tokens] + respond (fn [access-tokens] (handler (assoc-access-tokens-in-request - request refreshed-tokens) + request access-tokens) (comp respond #(assoc-access-tokens-in-response - % refreshed-tokens)) + % access-tokens)) raise))] (refresh-all-tokens profiles access-tokens respond)))))))) From 1c0b548544514702f03d37685fa4cbc1559f7379 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Sun, 12 Jan 2025 19:03:18 +0100 Subject: [PATCH 09/30] rename socket-timeout constant to be more specific --- src/ring/middleware/oauth2.clj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index d1f00b2..c683545 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -208,14 +208,14 @@ (update access-tokens profile-key merge maybe-grant) (dissoc access-tokens profile-key))) -(def socket-timeout 60000) +(def refresh-socket-timeout 60000) (defn- refresh-one-token ([profile refresh-token] (-> (access-token-http-options profile {:grant_type "refresh_token" :refresh_token refresh-token}) - (assoc :socket-timeout socket-timeout) + (assoc :socket-timeout refresh-socket-timeout) http/request format-access-token)) ([profile refresh-token respond raise] @@ -224,7 +224,7 @@ {:grant_type "refresh_token" :refresh_token refresh-token}) (assoc :async? true - :socket-timeout socket-timeout) + :socket-timeout refresh-socket-timeout) (http/request (comp respond format-access-token) raise)))) (defn- refresh-all-tokens From 94ad054ef35a07ee95215098ea344ae334e99767 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 15 Jan 2025 11:36:02 +0100 Subject: [PATCH 10/30] refactor `refresh-all-tokens` --- src/ring/middleware/oauth2.clj | 73 ++++++++++++++-------------- test/ring/middleware/oauth2_test.clj | 1 + 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index c683545..ba16395 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -227,39 +227,39 @@ :socket-timeout refresh-socket-timeout) (http/request (comp respond format-access-token) raise)))) +(defn- refresh-tasks [profiles access-tokens] + (->> (expired-access-tokens access-tokens) + (keep (fn [[profile-key {:keys [refresh-token]}]] + (when (and (get profiles profile-key) refresh-token) + [profile-key [(get profiles profile-key) refresh-token]]))))) + +(defn- async-map-values [f respond m] + (let [total (count m) + results (atom {}) + respond-when-done #(when (= (count %) total) (respond %))] + (if (zero? total) + (respond {}) + (doseq [[k v] m + :let [respond #(respond-when-done (swap! results assoc k %)) + raise (fn [_] (respond nil))]] + (f v respond raise))))) + (defn- refresh-all-tokens ([profiles access-tokens] - (let [refresh-results - (for [[profile-key {:keys [refresh-token]}] (expired-access-tokens - access-tokens) - :let [profile (profile-key profiles)] - :when (and profile refresh-token)] - [profile-key - (try (refresh-one-token profile refresh-token) - (catch clojure.lang.ExceptionInfo _ - nil))])] - (reduce update-tokens access-tokens refresh-results))) + (->> (refresh-tasks profiles access-tokens) + (map (fn [[profile-key [profile refresh-token]]] + [profile-key + (try (refresh-one-token profile refresh-token) + (catch clojure.lang.ExceptionInfo _ nil))])) + (reduce update-tokens access-tokens))) ([profiles access-tokens respond] - ;; strategy: launch all requests concurrently, keeping track of completed - ;; requests in `results`. When all requests have finished, respond. - (let [expired (expired-access-tokens access-tokens) - total (count expired) - results (atom {}) ;; map from profile-key to result - respond-when-done! #(when (= (count @results) total) - (respond (reduce update-tokens - access-tokens @results)))] - (if (zero? total) - (respond access-tokens) - (doseq [[profile-key {:keys [refresh-token]}] expired - :let [profile (profile-key profiles)] - :when (and profile refresh-token)] - (refresh-one-token profile refresh-token - (fn [refresh-result] - (swap! results assoc profile-key refresh-result) - (respond-when-done!)) - (fn [_] - (swap! results assoc profile-key nil) - (respond-when-done!)))))))) + (async-map-values + (fn [[profile refresh-token] respond raise] + (refresh-one-token profile refresh-token respond raise)) + (fn [refreshed-tokens] + (respond + (reduce update-tokens access-tokens refreshed-tokens))) + (refresh-tasks profiles access-tokens)))) (defn- assoc-access-tokens-in-request [request tokens] (if tokens @@ -304,11 +304,10 @@ request respond raise) (let [access-tokens (get-in request [:session ::access-tokens]) respond (fn [access-tokens] - (handler - (assoc-access-tokens-in-request - request access-tokens) - (comp respond - #(assoc-access-tokens-in-response - % access-tokens)) - raise))] + (let [request (assoc-access-tokens-in-request + request access-tokens) + respond (comp respond + #(assoc-access-tokens-in-response + % access-tokens))] + (handler request respond raise)))] (refresh-all-tokens profiles access-tokens respond)))))))) diff --git a/test/ring/middleware/oauth2_test.clj b/test/ring/middleware/oauth2_test.clj index c86fe3e..35899a4 100644 --- a/test/ring/middleware/oauth2_test.clj +++ b/test/ring/middleware/oauth2_test.clj @@ -484,4 +484,5 @@ (handler request respond raise) (is (= :empty (deref raise 100 :empty))) (let [response (deref respond 100 :empty)] + (is (not= response :empty)) (is (= {:test-1 good-grant} (:body response))))))))) From 9339b82784322ef002e94de5900ac719df9ce58f Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 15 Jan 2025 19:57:47 +0100 Subject: [PATCH 11/30] refactor http param options --- src/ring/middleware/oauth2.clj | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index ba16395..82aa86d 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -98,7 +98,7 @@ (defn- get-code-verifier [request] (get-in request [:session ::code-verifier])) -(defn- request-params [{:keys [pkce?] :as profile} request] +(defn- access-token-request-params [{:keys [pkce?] :as profile} request] (-> {:grant_type "authorization_code" :code (get-authorization-code request) :redirect_uri (redirect-uri profile request)} @@ -112,7 +112,7 @@ (merge {:client_id id :client_secret secret})))) -(defn- access-token-http-options +(defn- token-http-options [{:keys [access-token-uri client-id client-secret basic-auth?] :or {basic-auth? false}} form-params] @@ -125,15 +125,18 @@ (add-header-credentials opts client-id client-secret) (add-form-credentials opts client-id client-secret)))) +(defn- access-token-http-options + [profile request] + (token-http-options profile (access-token-request-params profile request))) + (defn- get-access-token ([profile request] - (-> (access-token-http-options profile (request-params profile request)) + (-> (access-token-http-options profile request) http/request (format-access-token))) ([profile request respond raise] (http/request - (-> (access-token-http-options profile - (request-params profile request)) + (-> (access-token-http-options profile request) (assoc :async? true)) (comp respond format-access-token) raise))) @@ -210,19 +213,18 @@ (def refresh-socket-timeout 60000) +(defn- refresh-token-request-options [profile refresh-token] + (token-http-options profile {:grant_type "refresh_token" + :refresh_token refresh-token})) + (defn- refresh-one-token ([profile refresh-token] - (-> (access-token-http-options - profile - {:grant_type "refresh_token" :refresh_token refresh-token}) + (-> (refresh-token-request-options profile refresh-token) (assoc :socket-timeout refresh-socket-timeout) http/request format-access-token)) ([profile refresh-token respond raise] - (-> (access-token-http-options - profile - {:grant_type "refresh_token" - :refresh_token refresh-token}) + (-> (refresh-token-request-options profile refresh-token) (assoc :async? true :socket-timeout refresh-socket-timeout) (http/request (comp respond format-access-token) raise)))) From e8d25b938918eccf6f25e9cbbe9e369117f0d5e1 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 15 Jan 2025 20:01:00 +0100 Subject: [PATCH 12/30] remove spurious newlines in `defn` --- src/ring/middleware/oauth2.clj | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 82aa86d..e8346ce 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -125,8 +125,7 @@ (add-header-credentials opts client-id client-secret) (add-form-credentials opts client-id client-secret)))) -(defn- access-token-http-options - [profile request] +(defn- access-token-http-options [profile request] (token-http-options profile (access-token-request-params profile request))) (defn- get-access-token @@ -194,16 +193,14 @@ (respond (redirect-response profile session token))) raise))))) -(defn- expired-access-tokens - [access-tokens] +(defn- expired-access-tokens [access-tokens] (let [now (Date.) expired-access-token? (fn [[_ {:keys [expires refresh-token]}]] (and refresh-token expires (.before expires now)))] (->> access-tokens (filter expired-access-token?) (into {})))) -(defn- update-tokens - [access-tokens [profile-key maybe-grant]] +(defn- update-tokens [access-tokens [profile-key maybe-grant]] (if maybe-grant ;; `update ... merge` to properly handle case where authorization server ;; does not update the refresh token after use and we should re-use the @@ -268,8 +265,7 @@ (assoc request :oauth2/access-tokens tokens) request)) -(defn- assoc-access-tokens-in-response - [response tokens] +(defn- assoc-access-tokens-in-response [response tokens] (if tokens (assoc-in response [:session ::access-tokens] tokens) response)) @@ -280,8 +276,7 @@ (defn- valid-profile? [{:keys [client-id client-secret]}] (and (some? client-id) (some? client-secret))) -(defn wrap-oauth2 - [handler profiles] +(defn wrap-oauth2 [handler profiles] {:pre [(every? valid-profile? (vals profiles))]} (let [id-profiles (for [[k v] profiles] (assoc v :id k)) launches (into {} (map (juxt :launch-uri identity)) id-profiles) From 67c85439ee54f2022b6223c914a754559d00a69a Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 15 Jan 2025 20:07:34 +0100 Subject: [PATCH 13/30] inline `now` --- src/ring/middleware/oauth2.clj | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index e8346ce..e19ec9d 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -194,10 +194,9 @@ raise))))) (defn- expired-access-tokens [access-tokens] - (let [now (Date.) - expired-access-token? (fn [[_ {:keys [expires refresh-token]}]] + (let [expired-access-token? (fn [[_ {:keys [expires refresh-token]}]] (and refresh-token expires - (.before expires now)))] + (.before expires (Date.))))] (->> access-tokens (filter expired-access-token?) (into {})))) (defn- update-tokens [access-tokens [profile-key maybe-grant]] From b0c3f83ff0bbc6c06cc1a4d5f79933e250b81166 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 15 Jan 2025 20:12:50 +0100 Subject: [PATCH 14/30] slightly refactor `refresh-one-token` for clarity --- src/ring/middleware/oauth2.clj | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index e19ec9d..f1083a3 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -210,20 +210,18 @@ (def refresh-socket-timeout 60000) (defn- refresh-token-request-options [profile refresh-token] - (token-http-options profile {:grant_type "refresh_token" - :refresh_token refresh-token})) + (-> (token-http-options profile {:grant_type "refresh_token" + :refresh_token refresh-token}) + (assoc :socket-timeout refresh-socket-timeout))) (defn- refresh-one-token ([profile refresh-token] - (-> (refresh-token-request-options profile refresh-token) - (assoc :socket-timeout refresh-socket-timeout) - http/request + (-> (http/request (refresh-token-request-options profile refresh-token)) format-access-token)) ([profile refresh-token respond raise] - (-> (refresh-token-request-options profile refresh-token) - (assoc :async? true - :socket-timeout refresh-socket-timeout) - (http/request (comp respond format-access-token) raise)))) + (let [opts (-> (refresh-token-request-options profile refresh-token) + (assoc :async? true))] + (http/request opts (comp respond format-access-token) raise)))) (defn- refresh-tasks [profiles access-tokens] (->> (expired-access-tokens access-tokens) From a4f5e2ccba914067e9fa78d7c46bc82de27aa5b3 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Tue, 21 Jan 2025 11:43:16 +0100 Subject: [PATCH 15/30] rework token refresh into a middle function - extract `wrap-refresh-acceess-tokens` - only set response `:session` if changed and if handler did not set it already - add a test for the above --- src/ring/middleware/oauth2.clj | 48 +++++++++++++++++----------- test/ring/middleware/oauth2_test.clj | 48 +++++++++++++++++++++++----- 2 files changed, 69 insertions(+), 27 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index f1083a3..eb8173e 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -262,11 +262,33 @@ (assoc request :oauth2/access-tokens tokens) request)) -(defn- assoc-access-tokens-in-response [response tokens] - (if tokens - (assoc-in response [:session ::access-tokens] tokens) +(defn- assoc-access-tokens-in-response + [original-tokens updated-tokens response] + (if (and (not (contains? response :session)) + (not= original-tokens updated-tokens)) + (assoc-in response [:session ::access-tokens] updated-tokens) response)) +(defn- wrap-refresh-access-tokens [handler profiles] + (fn ([request] + (let [access-tokens (get-in request [:session ::access-tokens]) + updated-access-tokens (refresh-all-tokens profiles access-tokens) + response (handler (assoc-access-tokens-in-request request + updated-access-tokens))] + (assoc-access-tokens-in-response access-tokens updated-access-tokens + response))) + ([request respond raise] + (let [access-tokens (get-in request [:session ::access-tokens]) + respond + (fn [updated-access-tokens] + (let [request (assoc-access-tokens-in-request + request updated-access-tokens) + update-session (partial assoc-access-tokens-in-response + access-tokens updated-access-tokens) + respond (comp respond update-session)] + (handler request respond raise)))] + (refresh-all-tokens profiles access-tokens respond))))) + (defn- parse-redirect-url [{:keys [redirect-uri]}] (.getPath (java.net.URI. redirect-uri))) @@ -277,31 +299,19 @@ {:pre [(every? valid-profile? (vals profiles))]} (let [id-profiles (for [[k v] profiles] (assoc v :id k)) launches (into {} (map (juxt :launch-uri identity)) id-profiles) - redirects (into {} (map (juxt parse-redirect-url identity)) id-profiles)] + redirects (into {} (map (juxt parse-redirect-url identity)) id-profiles) + handler (wrap-refresh-access-tokens handler profiles)] (fn ([{:keys [uri] :as request}] (if-let [profile (launches uri)] ((make-launch-handler profile) request) (if-let [profile (redirects uri)] ((:redirect-handler profile (make-redirect-handler profile)) request) - (let [access-tokens (->> (get-in request [:session ::access-tokens]) - (refresh-all-tokens profiles))] - (-> request - (assoc-access-tokens-in-request access-tokens) - handler - (assoc-access-tokens-in-response access-tokens)))))) + (handler request)))) ([{:keys [uri] :as request} respond raise] (if-let [profile (launches uri)] ((make-launch-handler profile) request respond raise) (if-let [profile (redirects uri)] ((:redirect-handler profile (make-redirect-handler profile)) request respond raise) - (let [access-tokens (get-in request [:session ::access-tokens]) - respond (fn [access-tokens] - (let [request (assoc-access-tokens-in-request - request access-tokens) - respond (comp respond - #(assoc-access-tokens-in-response - % access-tokens))] - (handler request respond raise)))] - (refresh-all-tokens profiles access-tokens respond)))))))) + (handler request respond raise))))))) diff --git a/test/ring/middleware/oauth2_test.clj b/test/ring/middleware/oauth2_test.clj index 35899a4..6083b38 100644 --- a/test/ring/middleware/oauth2_test.clj +++ b/test/ring/middleware/oauth2_test.clj @@ -204,10 +204,7 @@ (deftest test-access-tokens-key (let [tokens {:test {:token "defdef", :expires 3600}}] - (is (= {:status 200, - :headers {}, - :body tokens, - :session {::oauth2/access-tokens tokens}} + (is (= {:status 200, :headers {}, :body tokens} (-> (mock/request :get "/") (assoc :session {::oauth2/access-tokens tokens}) (test-handler)))))) @@ -380,11 +377,10 @@ (deftest test-handler-passthrough (let [tokens {:test "tttkkkk"} - session {::oauth2/access-tokens tokens} request (-> (mock/request :get "/example") - (assoc :session session))] + (assoc :session {::oauth2/access-tokens tokens}))] (testing "sync handler" - (is (= {:status 200, :headers {}, :body tokens :session session} + (is (= {:status 200, :headers {}, :body tokens} (test-handler request)))) (testing "async handler" @@ -393,7 +389,7 @@ (test-handler request respond raise) (is (= :empty (deref raise 100 :empty))) - (is (= {:status 200, :headers {}, :body tokens :session session} + (is (= {:status 200, :headers {}, :body tokens} (deref respond 100 :empty))))))) (def refresh-token-response @@ -486,3 +482,39 @@ (let [response (deref respond 100 :empty)] (is (not= response :empty)) (is (= {:test-1 good-grant} (:body response))))))))) + +(deftest test-token-refresh-clear-session + (fake/with-fake-routes + {"https://example.com/oauth2/access-token" + (constantly refresh-token-response)} + + (let [clear-response {:status 200 :headers {} :body nil :session nil} + session-clear-handler (fn + ([_request] clear-response) + ([_request respond _raise] + (respond clear-response))) + handler (wrap-oauth2 session-clear-handler {:test test-profile}) + now (Instant/now) + old-expires (seconds-from-now-to-date now -60) + request (-> (mock/request :get "/") + (assoc :session + {::oauth2/access-tokens + {:test {:token "oldtoken" + :refresh-token "oldrefresh" + :expires old-expires}}}))] + + (testing "sync handler" + (let [response (handler request)] + (is (= 200 (:status response))) + (is (nil? (:session response))))) + + (testing "async handler" + (let [respond (promise) + raise (promise)] + (handler request respond raise) + (let [response (deref respond 100 :empty) + error (deref raise 100 :empty)] + (is (not= :empty response)) + (is (= :empty error)) + (is (= 200 (:status response))) + (is (nil? (:session response))))))))) From 069e4b5ec6c9d93a47d5da2ee3d066bd36adc946 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Tue, 21 Jan 2025 12:37:45 +0100 Subject: [PATCH 16/30] minor refactor --- src/ring/middleware/oauth2.clj | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index eb8173e..d37f6f9 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -130,15 +130,13 @@ (defn- get-access-token ([profile request] - (-> (access-token-http-options profile request) - http/request + (-> (http/request (access-token-http-options profile request)) (format-access-token))) ([profile request respond raise] - (http/request - (-> (access-token-http-options profile request) - (assoc :async? true)) - (comp respond format-access-token) - raise))) + (http/request (-> (access-token-http-options profile request) + (assoc :async? true)) + (comp respond format-access-token) + raise))) (defn state-mismatch-handler ([_] From 26f2b34d1176b5ef33b6b2814f4cbaa90bf1e5bf Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Fri, 24 Jan 2025 09:38:20 +0100 Subject: [PATCH 17/30] update response session unless nil or tokens unchanged --- src/ring/middleware/oauth2.clj | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index d37f6f9..5b71b47 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -260,12 +260,14 @@ (assoc request :oauth2/access-tokens tokens) request)) +(defn- nil-session? [response] + (and (contains? response :session) (nil? (:session response)))) + (defn- assoc-access-tokens-in-response [original-tokens updated-tokens response] - (if (and (not (contains? response :session)) - (not= original-tokens updated-tokens)) - (assoc-in response [:session ::access-tokens] updated-tokens) - response)) + (if (or (nil-session? response) (= original-tokens updated-tokens)) + response + (assoc-in response [:session ::access-tokens] updated-tokens))) (defn- wrap-refresh-access-tokens [handler profiles] (fn ([request] From ec2989adb00321efeeb722e46aa6699159e65830 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Sun, 13 Apr 2025 23:46:12 +0200 Subject: [PATCH 18/30] cosmetic change --- src/ring/middleware/oauth2.clj | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 5b71b47..ba48a4a 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -263,8 +263,7 @@ (defn- nil-session? [response] (and (contains? response :session) (nil? (:session response)))) -(defn- assoc-access-tokens-in-response - [original-tokens updated-tokens response] +(defn- assoc-access-tokens-in-response [original-tokens updated-tokens response] (if (or (nil-session? response) (= original-tokens updated-tokens)) response (assoc-in response [:session ::access-tokens] updated-tokens))) From 257c77e298e3644b1a3247a131775e5381f1824e Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 16 Apr 2025 21:50:38 +0200 Subject: [PATCH 19/30] rename refresh-token-request-options -> refresh-token-http-options --- src/ring/middleware/oauth2.clj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index ba48a4a..49c3669 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -207,17 +207,17 @@ (def refresh-socket-timeout 60000) -(defn- refresh-token-request-options [profile refresh-token] +(defn- refresh-token-http-options [profile refresh-token] (-> (token-http-options profile {:grant_type "refresh_token" :refresh_token refresh-token}) (assoc :socket-timeout refresh-socket-timeout))) (defn- refresh-one-token ([profile refresh-token] - (-> (http/request (refresh-token-request-options profile refresh-token)) + (-> (http/request (refresh-token-http-options profile refresh-token)) format-access-token)) ([profile refresh-token respond raise] - (let [opts (-> (refresh-token-request-options profile refresh-token) + (let [opts (-> (refresh-token-http-options profile refresh-token) (assoc :async? true))] (http/request opts (comp respond format-access-token) raise)))) From 46472f01e5990e69116b50d9a95c8e90f27001a2 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 16 Apr 2025 21:51:30 +0200 Subject: [PATCH 20/30] refactor expired-access-tokens --- src/ring/middleware/oauth2.clj | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 49c3669..852223e 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -191,11 +191,11 @@ (respond (redirect-response profile session token))) raise))))) -(defn- expired-access-tokens [access-tokens] - (let [expired-access-token? (fn [[_ {:keys [expires refresh-token]}]] - (and refresh-token expires - (.before expires (Date.))))] - (->> access-tokens (filter expired-access-token?) (into {})))) +(defn- expired-access-token? [{:keys [expires refresh-token]}] + (and refresh-token expires (.before expires (Date.)))) + +(defn expired-access-tokens [tokens] + (into {} (filter (comp expired-access-token? val)) tokens)) (defn- update-tokens [access-tokens [profile-key maybe-grant]] (if maybe-grant From 84589a15204bdaf29f40b08b87735586e1ea6e59 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 16 Apr 2025 21:53:52 +0200 Subject: [PATCH 21/30] replace `:let` with `let` --- src/ring/middleware/oauth2.clj | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 852223e..2168a9f 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -233,10 +233,10 @@ respond-when-done #(when (= (count %) total) (respond %))] (if (zero? total) (respond {}) - (doseq [[k v] m - :let [respond #(respond-when-done (swap! results assoc k %)) - raise (fn [_] (respond nil))]] - (f v respond raise))))) + (doseq [[k v] m] + (let [respond #(respond-when-done (swap! results assoc k %)) + raise (fn [_] (respond nil))] + (f v respond raise)))))) (defn- refresh-all-tokens ([profiles access-tokens] From 60b950d824f88f34f92110c965243d5a72cb912d Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 16 Apr 2025 22:26:21 +0200 Subject: [PATCH 22/30] remove extra space --- src/ring/middleware/oauth2.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 2168a9f..7637b7e 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -296,7 +296,7 @@ (defn wrap-oauth2 [handler profiles] {:pre [(every? valid-profile? (vals profiles))]} - (let [id-profiles (for [[k v] profiles] (assoc v :id k)) + (let [id-profiles (for [[k v] profiles] (assoc v :id k)) launches (into {} (map (juxt :launch-uri identity)) id-profiles) redirects (into {} (map (juxt parse-redirect-url identity)) id-profiles) handler (wrap-refresh-access-tokens handler profiles)] From b057931a0cc035e626226d81251b608ddeee95b2 Mon Sep 17 00:00:00 2001 From: Lukas Studer Date: Wed, 16 Apr 2025 22:26:34 +0200 Subject: [PATCH 23/30] more concise `wrap-refresh-access-tokens` --- src/ring/middleware/oauth2.clj | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 7637b7e..22cb7fb 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -270,23 +270,20 @@ (defn- wrap-refresh-access-tokens [handler profiles] (fn ([request] - (let [access-tokens (get-in request [:session ::access-tokens]) - updated-access-tokens (refresh-all-tokens profiles access-tokens) - response (handler (assoc-access-tokens-in-request request - updated-access-tokens))] - (assoc-access-tokens-in-response access-tokens updated-access-tokens - response))) + (let [tokens (get-in request [:session ::access-tokens]) + tokens' (refresh-all-tokens profiles tokens) + request (assoc-access-tokens-in-request request tokens') + response (handler request)] + (assoc-access-tokens-in-response tokens tokens' response))) ([request respond raise] - (let [access-tokens (get-in request [:session ::access-tokens]) - respond - (fn [updated-access-tokens] - (let [request (assoc-access-tokens-in-request - request updated-access-tokens) - update-session (partial assoc-access-tokens-in-response - access-tokens updated-access-tokens) - respond (comp respond update-session)] - (handler request respond raise)))] - (refresh-all-tokens profiles access-tokens respond))))) + (let [tokens (get-in request [:session ::access-tokens])] + (refresh-all-tokens + profiles tokens + (fn [tokens'] + (let [request (assoc-access-tokens-in-request request tokens') + respond #(respond (assoc-access-tokens-in-response + tokens tokens' %))] + (handler request respond raise)))))))) (defn- parse-redirect-url [{:keys [redirect-uri]}] (.getPath (java.net.URI. redirect-uri))) From 6a3081ca96aa918d61657aa64df60bf1963ef090 Mon Sep 17 00:00:00 2001 From: "lukas.studer" Date: Fri, 19 Dec 2025 10:23:38 +0100 Subject: [PATCH 24/30] correctly handle extra state from application Correctly handles case where `request` contains `:session` map with extra data. --- src/ring/middleware/oauth2.clj | 22 ++++++++++----- test/ring/middleware/oauth2_test.clj | 40 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 22cb7fb..b6ea255 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -263,10 +263,20 @@ (defn- nil-session? [response] (and (contains? response :session) (nil? (:session response)))) -(defn- assoc-access-tokens-in-response [original-tokens updated-tokens response] - (if (or (nil-session? response) (= original-tokens updated-tokens)) - response - (assoc-in response [:session ::access-tokens] updated-tokens))) +(defn- assoc-access-tokens-in-response + [request original-tokens updated-tokens response] + (cond + ;; handler explicitly cleared the session, we simply forward + (nil-session? response) response + ;; tokens did not update, again forward as-is + (= original-tokens updated-tokens) response + ;; handler is also changing session; add our refresh to the change + (contains? response :session) (assoc-in response [:session ::access-tokens] + updated-tokens) + ;; handler did not change session, update original + :else (let [original-session (get request :session)] + (assoc response :session + (assoc original-session ::access-tokens updated-tokens))))) (defn- wrap-refresh-access-tokens [handler profiles] (fn ([request] @@ -274,7 +284,7 @@ tokens' (refresh-all-tokens profiles tokens) request (assoc-access-tokens-in-request request tokens') response (handler request)] - (assoc-access-tokens-in-response tokens tokens' response))) + (assoc-access-tokens-in-response request tokens tokens' response))) ([request respond raise] (let [tokens (get-in request [:session ::access-tokens])] (refresh-all-tokens @@ -282,7 +292,7 @@ (fn [tokens'] (let [request (assoc-access-tokens-in-request request tokens') respond #(respond (assoc-access-tokens-in-response - tokens tokens' %))] + request tokens tokens' %))] (handler request respond raise)))))))) (defn- parse-redirect-url [{:keys [redirect-uri]}] diff --git a/test/ring/middleware/oauth2_test.clj b/test/ring/middleware/oauth2_test.clj index 6083b38..f90bbc1 100644 --- a/test/ring/middleware/oauth2_test.clj +++ b/test/ring/middleware/oauth2_test.clj @@ -518,3 +518,43 @@ (is (= :empty error)) (is (= 200 (:status response))) (is (nil? (:session response))))))))) +(deftest test-token-refresh-preserves-session-state + (fake/with-fake-routes + {"https://example.com/oauth2/access-token" + (constantly refresh-token-response)} + + (let [now (Instant/now) + old-expires (seconds-from-now-to-date now -60) + request (-> (mock/request :get "/") + (assoc :session + {:user-id 123 ; extra session state + ::oauth2/access-tokens + {:test {:token "oldtoken" + :refresh-token "oldrefresh" + :expires old-expires}}}))] + + (testing "handler sets new session state during refresh" + (let [handler (wrap-oauth2 + (fn + ([_] {:status 200 :body "ok" + :session {:user-id 123 :cart-items 5}}) + ([_ respond _] (respond {:status 200 :body "ok" + :session {:user-id 123 :cart-items 5}}))) + {:test test-profile}) + response (handler request)] + ;; Handler's session changes preserved + (is (= 5 (get-in response [:session :cart-items]))) + ;; Refreshed token added to handler's session + (is (= "newtoken" (get-in response [:session ::oauth2/access-tokens :test :token]))))) + + (testing "handler doesn't change session, extra state preserved" + (let [handler (wrap-oauth2 + (fn + ([_] {:status 200 :body "ok"}) + ([_ respond _] (respond {:status 200 :body "ok"}))) + {:test test-profile}) + response (handler request)] + ;; Original session's extra state preserved + (is (= 123 (get-in response [:session :user-id]))) + ;; Token refreshed + (is (= "newtoken" (get-in response [:session ::oauth2/access-tokens :test :token])))))))) From 1ea3ae47b1b05941ab18b7f4a94925451614ee02 Mon Sep 17 00:00:00 2001 From: "lukas.studer" Date: Fri, 19 Dec 2025 10:29:24 +0100 Subject: [PATCH 25/30] format unit tests to 80 char width --- test/ring/middleware/oauth2_test.clj | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/ring/middleware/oauth2_test.clj b/test/ring/middleware/oauth2_test.clj index f90bbc1..e6b35fe 100644 --- a/test/ring/middleware/oauth2_test.clj +++ b/test/ring/middleware/oauth2_test.clj @@ -539,13 +539,15 @@ ([_] {:status 200 :body "ok" :session {:user-id 123 :cart-items 5}}) ([_ respond _] (respond {:status 200 :body "ok" - :session {:user-id 123 :cart-items 5}}))) + :session {:user-id 123 + :cart-items 5}}))) {:test test-profile}) response (handler request)] ;; Handler's session changes preserved (is (= 5 (get-in response [:session :cart-items]))) ;; Refreshed token added to handler's session - (is (= "newtoken" (get-in response [:session ::oauth2/access-tokens :test :token]))))) + (is (= "newtoken" (get-in response [:session ::oauth2/access-tokens + :test :token]))))) (testing "handler doesn't change session, extra state preserved" (let [handler (wrap-oauth2 @@ -557,4 +559,5 @@ ;; Original session's extra state preserved (is (= 123 (get-in response [:session :user-id]))) ;; Token refreshed - (is (= "newtoken" (get-in response [:session ::oauth2/access-tokens :test :token])))))))) + (is (= "newtoken" (get-in response [:session ::oauth2/access-tokens + :test :token])))))))) From 2efc5fad3caa9a2ca9171f3d5d3683a5fc3e5604 Mon Sep 17 00:00:00 2001 From: "lukas.studer" Date: Fri, 19 Dec 2025 13:45:00 +0100 Subject: [PATCH 26/30] catch generic exception on sync refresh --- src/ring/middleware/oauth2.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index b6ea255..f246712 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -244,7 +244,7 @@ (map (fn [[profile-key [profile refresh-token]]] [profile-key (try (refresh-one-token profile refresh-token) - (catch clojure.lang.ExceptionInfo _ nil))])) + (catch Exception _ nil))])) (reduce update-tokens access-tokens))) ([profiles access-tokens respond] (async-map-values From b13273e1534de62d131b8c969105668f644d4280 Mon Sep 17 00:00:00 2001 From: "lukas.studer" Date: Fri, 19 Dec 2025 13:52:46 +0100 Subject: [PATCH 27/30] refactor `cond` in `assoc-access-tokens-in-response` --- src/ring/middleware/oauth2.clj | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index f246712..dcc47f8 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -263,20 +263,21 @@ (defn- nil-session? [response] (and (contains? response :session) (nil? (:session response)))) +(defn- get-current-session [request response] + (if (contains? response :session) + (:session response) + (:session request))) + (defn- assoc-access-tokens-in-response [request original-tokens updated-tokens response] - (cond - ;; handler explicitly cleared the session, we simply forward - (nil-session? response) response - ;; tokens did not update, again forward as-is - (= original-tokens updated-tokens) response - ;; handler is also changing session; add our refresh to the change - (contains? response :session) (assoc-in response [:session ::access-tokens] - updated-tokens) - ;; handler did not change session, update original - :else (let [original-session (get request :session)] - (assoc response :session - (assoc original-session ::access-tokens updated-tokens))))) + (if (or (nil-session? response) + (= original-tokens updated-tokens)) + ;; either handler explicitly cleared session or no token refresh occurred + response + ;; otherwise add refreshed tokens to current session + (let [session (-> (get-current-session request response) + (assoc ::access-tokens updated-tokens))] + (assoc response :session session)))) (defn- wrap-refresh-access-tokens [handler profiles] (fn ([request] From 690dd3f9bd0c4039d7fd4002654bd2cdb06c71a4 Mon Sep 17 00:00:00 2001 From: "lukas.studer" Date: Fri, 19 Dec 2025 14:00:55 +0100 Subject: [PATCH 28/30] remove timeout on refresh request --- src/ring/middleware/oauth2.clj | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index dcc47f8..4098955 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -205,12 +205,9 @@ (update access-tokens profile-key merge maybe-grant) (dissoc access-tokens profile-key))) -(def refresh-socket-timeout 60000) - (defn- refresh-token-http-options [profile refresh-token] - (-> (token-http-options profile {:grant_type "refresh_token" - :refresh_token refresh-token}) - (assoc :socket-timeout refresh-socket-timeout))) + (token-http-options profile {:grant_type "refresh_token" + :refresh_token refresh-token})) (defn- refresh-one-token ([profile refresh-token] From 265759affbfe35f51d4a4439744836a5ca5a8109 Mon Sep 17 00:00:00 2001 From: "lukas.studer" Date: Fri, 19 Dec 2025 14:41:52 +0100 Subject: [PATCH 29/30] extract error handling form `async-map-values` --- src/ring/middleware/oauth2.clj | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 4098955..780bf8c 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -231,9 +231,8 @@ (if (zero? total) (respond {}) (doseq [[k v] m] - (let [respond #(respond-when-done (swap! results assoc k %)) - raise (fn [_] (respond nil))] - (f v respond raise)))))) + (let [respond #(respond-when-done (swap! results assoc k %))] + (f v respond)))))) (defn- refresh-all-tokens ([profiles access-tokens] @@ -245,8 +244,10 @@ (reduce update-tokens access-tokens))) ([profiles access-tokens respond] (async-map-values - (fn [[profile refresh-token] respond raise] - (refresh-one-token profile refresh-token respond raise)) + (fn [[profile refresh-token] respond] + ;; on failure, yield a result of `nil` as refreshed token to signal error + (let [raise (fn [_] (respond nil))] + (refresh-one-token profile refresh-token respond raise))) (fn [refreshed-tokens] (respond (reduce update-tokens access-tokens refreshed-tokens))) From db75c05284aa2a3458025ebb124efe9d3c14439b Mon Sep 17 00:00:00 2001 From: "lukas.studer" Date: Fri, 19 Dec 2025 14:48:15 +0100 Subject: [PATCH 30/30] add a \n to test --- test/ring/middleware/oauth2_test.clj | 1 + 1 file changed, 1 insertion(+) diff --git a/test/ring/middleware/oauth2_test.clj b/test/ring/middleware/oauth2_test.clj index e6b35fe..18a2812 100644 --- a/test/ring/middleware/oauth2_test.clj +++ b/test/ring/middleware/oauth2_test.clj @@ -518,6 +518,7 @@ (is (= :empty error)) (is (= 200 (:status response))) (is (nil? (:session response))))))))) + (deftest test-token-refresh-preserves-session-state (fake/with-fake-routes {"https://example.com/oauth2/access-token"