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 <jreeves@weavejester.com>

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

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)]

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"})