From bac5bbce0964ed9c0a77676b13ced92e4433f9a7 Mon Sep 17 00:00:00 2001 From: Matthew Molloy Date: Wed, 8 Mar 2023 05:22:10 +0900 Subject: [PATCH] Add PKCE support Proof Key for Code Exchange (PKCE) is an extension to the Authorization Code flow to prevent CSRF and authorization code injection attacks. This commit adds the :pkce? option, disabled by default. See: https://oauth.net/2/pkce/ Co-authored-by: James Reeves --- README.md | 8 ++++ src/ring/middleware/oauth2.clj | 67 +++++++++++++++++++++------- test/ring/middleware/oauth2_test.clj | 43 ++++++++++++++++++ 3 files changed, 101 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 93b1ad6..c765023 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,14 @@ and complete authentication of the user. [the specification]: https://tools.ietf.org/html/rfc6749#section-2.3.1 +### PKCE + +Some oauth providers require an additional step called *Proof Key for +Code Exchange* ([PKCE][]). Ring-OAuth2 will include a proof key in the +workflow when `:pkce? true`is included in the profile map. + +[pkce]: https://www.oauth.com/oauth2-servers/pkce/authorization-request/ + ## Workflow diagram The following image is a workflow diagram that describes the OAuth2 diff --git a/src/ring/middleware/oauth2.clj b/src/ring/middleware/oauth2.clj index 2a57ff8..a46f813 100644 --- a/src/ring/middleware/oauth2.clj +++ b/src/ring/middleware/oauth2.clj @@ -6,7 +6,10 @@ [ring.util.request :as req] [ring.util.response :as resp]) (:import [java.time Instant] - [java.util Date])) + [java.util Date] + [java.security MessageDigest] + [java.nio.charset StandardCharsets] + [org.apache.commons.codec.binary Base64])) (defn- redirect-uri [profile request] (-> (req/request-url request) @@ -17,23 +20,49 @@ (defn- scopes [profile] (str/join " " (map name (:scopes profile)))) -(defn- authorize-uri [profile request state] +(defn- base64 [^bytes bs] + (String. (Base64/encodeBase64 bs))) + +(defn- str->sha256 [^String s] + (-> (MessageDigest/getInstance "SHA-256") + (.digest (.getBytes s StandardCharsets/UTF_8)))) + +(defn- base64url [base64-str] + (-> base64-str (str/replace "+" "-") (str/replace "/" "_"))) + +(defn- verifier->challenge [^String verifier] + (-> verifier str->sha256 base64 base64url (str/replace "=" ""))) + +(defn- authorize-params [profile request state verifier] + (-> {:response_type "code" + :client_id (:client-id profile) + :redirect_uri (redirect-uri profile request) + :scope (scopes profile) + :state state} + (cond-> (:pkce? profile) + (assoc :code_challenge (verifier->challenge verifier) + :code_challenge_method "S256")))) + +(defn- authorize-uri [profile request state verifier] (str (:authorize-uri profile) (if (.contains ^String (:authorize-uri profile) "?") "&" "?") - (codec/form-encode {:response_type "code" - :client_id (:client-id profile) - :redirect_uri (redirect-uri profile request) - :scope (scopes profile) - :state state}))) + (codec/form-encode (authorize-params profile request state verifier)))) (defn- random-state [] - (-> (random/base64 9) (str/replace "+" "-") (str/replace "/" "_"))) + (base64url (random/base64 9))) + +(defn- random-code-verifier [] + (base64url (random/base64 63))) -(defn- make-launch-handler [profile] +(defn- make-launch-handler [{:keys [pkce?] :as profile}] (fn [{:keys [session] :or {session {}} :as request}] - (let [state (random-state)] - (-> (resp/redirect (authorize-uri profile request state)) - (assoc :session (assoc session ::state state)))))) + (let [state (random-state) + verifier (when pkce? (random-code-verifier)) + session' (-> session + (assoc ::state state) + (cond-> pkce? (assoc ::code-verifier verifier)))] + (-> (resp/redirect (authorize-uri profile request state verifier)) + (assoc :session session'))))) (defn- state-matches? [request] (= (get-in request [:session ::state]) @@ -61,10 +90,14 @@ (defn- get-authorization-code [request] (get-in request [:query-params "code"])) -(defn- request-params [profile request] - {:grant_type "authorization_code" - :code (get-authorization-code request) - :redirect_uri (redirect-uri profile request)}) +(defn- get-code-verifier [request] + (get-in request [:session ::code-verifier])) + +(defn- request-params [{:keys [pkce?] :as profile} request] + (-> {:grant_type "authorization_code" + :code (get-authorization-code request) + :redirect_uri (redirect-uri profile request)} + (cond-> pkce? (assoc :code_verifier (get-code-verifier request))))) (defn- add-header-credentials [opts id secret] (assoc opts :basic-auth [id secret])) @@ -108,7 +141,7 @@ (-> (resp/redirect landing-uri) (assoc :session (-> session (assoc-in [::access-tokens id] access-token) - (dissoc ::state))))))))) + (dissoc ::state ::code-verifier))))))))) (defn- assoc-access-tokens [request] (if-let [tokens (-> request :session ::access-tokens)] diff --git a/test/ring/middleware/oauth2_test.clj b/test/ring/middleware/oauth2_test.clj index bd91f49..7464122 100644 --- a/test/ring/middleware/oauth2_test.clj +++ b/test/ring/middleware/oauth2_test.clj @@ -2,6 +2,7 @@ (:require [clj-http.fake :as fake] [clojure.string :as str] [clojure.test :refer [deftest is testing]] + [cheshire.core :as cheshire] [ring.middleware.oauth2 :as oauth2 :refer [wrap-oauth2]] [ring.mock.request :as mock] [ring.middleware.params :refer [wrap-params]] @@ -19,12 +20,18 @@ :client-id "abcdef" :client-secret "01234567890abcdef"}) +(def test-profile-pkce + (assoc test-profile :pkce? true)) + (defn- token-handler [{:keys [oauth2/access-tokens]}] {:status 200, :headers {}, :body access-tokens}) (def test-handler (wrap-oauth2 token-handler {:test test-profile})) +(def test-handler-pkce + (wrap-oauth2 token-handler {:test test-profile-pkce})) + (deftest test-launch-uri (let [response (test-handler (mock/request :get "/oauth2/test")) location (get-in response [:headers "Location"]) @@ -41,6 +48,14 @@ (is (= {::oauth2/state (params "state")} (:session response))))) +(deftest test-launch-uri-pkce + (let [response (test-handler-pkce (mock/request :get "/oauth2/test")) + location (get-in response [:headers "Location"]) + [_ query] (str/split location #"\?" 2) + params (codec/form-decode query)] + (is (contains? params "code_challenge")) + (is (= "S256" (get params "code_challenge_method"))))) + (deftest test-missing-fields (let [profile (assoc test-profile :client-id nil)] (is (thrown? AssertionError (wrap-oauth2 token-handler {:test profile})))) @@ -259,6 +274,34 @@ (-> response :session ::oauth2/access-tokens :test :expires))))))) +(defn openid-response-with-code-verifier [req] + {:status 200 + :headers {"Content-Type" "application/json"} + :body (cheshire/generate-string + {:access_token "defdef" + :expires_in 3600 + :refresh_token "ghighi" + :id_token "abc.def.ghi" + :code_verifier (-> req :body slurp codec/form-decode + (get "code_verifier"))})}) + +(deftest test-openid-response-with-code-verifier + (fake/with-fake-routes + {"https://example.com/oauth2/access-token" + openid-response-with-code-verifier} + + (testing "verifier in extra data" + (let [request (-> (mock/request :get "/oauth2/test/callback") + (assoc :session {::oauth2/state "xyzxyz" + ::oauth2/code-verifier "jkljkl"}) + (assoc :query-params {"code" "abcabc" + "state" "xyzxyz"})) + response (test-handler-pkce request)] + (is (= "jkljkl" + (-> response + :session ::oauth2/access-tokens :test + :extra-data :code_verifier))))))) + (defn- redirect-handler [_] {:status 200, :headers {}, :body "redirect-handler-response-body"})