Get rid of enforcing absolute URLs for redirects.

CHANGES.md

@@ -1,5 +1,14 @@
 ## [Friend](http://github.com/cemerick/friend) changelog
 
+### `0.3.x`
+
+* HTTP redirect responses now don't enforce absolute URLs - this is a revert of the change made in 0.1.2.
+  Note that if you are using ring-defaults, you need to 
+  [set `:absolute-redirects` to `false`](https://github.com/ring-clojure/ring-defaults#customizing).
+  (a new [issue submitted](https://github.com/ring-clojure/ring-defaults/issues/39) for ring-defaults)
+  See https://github.com/clj-commons/friend/issues/4.
+
+
 ### `0.3.0`
 
 * The "classic" OpenId (i.e. OpenId 1.1/2.0) workflow provided by `cemerick.friend.openid` has been

project.clj

@@ -21,7 +21,9 @@
                                   [compojure "1.1.5"]
                                   [ring "1.2.0"]
                                   [robert/hooke "1.3.0"]
-                                  [clj-http "0.3.6"]]}
+                                  [clj-http "0.3.6"]
+                                  ;; test/.../functional.clj uses the jetty adapter
+                                  [ring/ring-jetty-adapter "1.9.5"]]}
              :sanity-check {:aot :all
                             :warn-on-reflection true
                             :compile-path "target/sanity-check-aot"}

src/cemerick/friend.clj

@@ -1,8 +1,7 @@
 (ns cemerick.friend
   (:require [cemerick.friend.util :as util]
-            [clojure.set :as set])
-  (:use (ring.util [response :as response :only (redirect)])
-        [slingshot.slingshot :only (throw+ try+)])
+            [ring.util.response :as response]
+            [slingshot.slingshot :refer [throw+ try+]])
   (:refer-clojure :exclude (identity)))
 
 (defn merge-authentication
@@ -105,10 +104,9 @@ Equivalent to (complement current-authentication)."}
     ::auth-config
     :login-uri
     (#(str (:context request) %))
-    (util/resolve-absolute-uri request)
-    ring.util.response/redirect
+    response/redirect
     (assoc :session (:session request))
-    (assoc-in [:session ::unauthorized-uri] (util/original-url request))))
+    (assoc-in [:session ::unauthorized-uri] (util/relative-url request))))
 
 (defn authenticate-response
   "Adds to the response's :session for responses with a :friend/ensure-identity-request key."

src/cemerick/friend/util.clj

@@ -12,9 +12,26 @@
 ; this was never _really_ part of the API, was implemented (badly) before req/request-url was available
 (def ^:deprecated original-url req/request-url)
 
-(defn resolve-absolute-uri
+(defn ^:deprecated resolve-absolute-uri
+  "DEPRECATED. Use `relative-url`.
+
+  This function is only kept for backward compatibility.
+  It was meant to conform to an old HTTP spec that required absolute URLs for redirects
+  but that's no longer needed."
   [^String uri request]
   (-> (original-url request)
-    java.net.URI.
-    (.resolve uri)
-    str))
+      java.net.URI.
+      (.resolve uri)
+      str))
+
+
+(defn relative-url
+  "Similar to `util/original-url` (and thus `ring.util.request/request-url`)
+  except that it only uses `:uri` and `:query-string` parts of the request
+  to build the URL and ignores `:scheme` and the 'host' header.
+  This is to avoid issues with redirects using absolute URLs when the app is running behind an SSL proxy."
+  [request]
+  (str
+   (:uri request)
+   (when-let [query (:query-string request)]
+     (str "?" query))))

src/cemerick/friend/workflows.clj

@@ -1,10 +1,10 @@
 (ns cemerick.friend.workflows
   (:require [cemerick.friend :as friend]
-            [cemerick.friend.util :as util]
-            [ring.util.request :as req])
-  (:use [clojure.string :only (trim)]
-        [cemerick.friend.util :only (gets)])
-  (:import org.apache.commons.codec.binary.Base64))
+            [cemerick.friend.util :as util :refer [gets]]
+            [ring.util.request :as req]
+            [ring.util.response :as response])
+  (:import java.net.URLEncoder
+           org.apache.commons.codec.binary.Base64))
 
 (defn http-basic-deny
   [realm request]
@@ -68,14 +68,12 @@
 
 (defn interactive-login-redirect
   [{:keys [form-params params] :as request}]
-  (ring.util.response/redirect
+  (response/redirect
     (let [param (str "&login_failed=Y&username="
-                     (java.net.URLEncoder/encode (username form-params params)))
+                     (URLEncoder/encode (username form-params params)))
           ^String login-uri (-> request ::friend/auth-config :login-uri (#(str (:context request) %)))]
-      (util/resolve-absolute-uri
-        (str (if (.contains login-uri "?") login-uri (str login-uri "?"))
-          param)
-        request))))
+      (str (if (.contains login-uri "?") login-uri (str login-uri "?"))
+           param))))
 
 (defn interactive-form
   [& {:keys [login-uri credential-fn login-failure-handler redirect-on-auth?] :as form-config

test/test_friend/functional.clj

@@ -34,7 +34,7 @@
           :let [resp (http/get url)]]
     (is (http/success? resp))
     (is (= (page-bodies uri) (:body resp))))
-  
+
   (let [api-resp (http/get (url "/free-api") {:as :json})]
     (is (http/success? api-resp))
     (is (= {:data 99} (:body api-resp)))))
@@ -69,17 +69,18 @@
       ; clj-http *should* redirect us, but isn't yet; working on it: 
       ; https://github.com/dakrone/clj-http/issues/57
       (is (http/redirect? resp))
-      (is (= (url "/user/account?query-string=test") (-> resp :headers (get "location")))))
+      (is (= "/user/account?query-string=test"
+             (-> resp :headers (get "location")))))
     (check-user-role-access)
     (is (= {:roles ["test-friend.mock-app/user"]} (:body (http/get (url "/echo-roles") {:as :json}))))
-    
+
     ; deny on admin role
     (try+
       (http/get (url "/admin"))
       (assert false) ; should never get here
       (catch [:status 403] _
         (is true)))
-    
+
     (testing "logout blocks access to privileged routes"
       (is (= (page-bodies "/") (:body (http/get (url "/logout")))))
       (is (= (page-bodies "/login") (:body (http/get (url "/user/account"))))))))
@@ -100,13 +101,12 @@
         (is (= "auth-data" (post-session-data "auth-data")))
         (is (= "auth-data" (get-session-data)))
         (check-user-role-access)
-        
+
         (http/get (url "/logout"))
         (let [should-be-login-redirect (http/get (url "/user/account")
                                                  {:follow-redirects false})]
           (is (= 302 (:status should-be-login-redirect)))
-          (is (re-matches #"http://localhost:\d+/login"
-                (-> should-be-login-redirect :headers (get "location")))))
+          (is (=  "/login" (-> should-be-login-redirect :headers (get "location")))))
         ; TODO should logout blow away the session completely?
         (is (= "auth-data" (get-session-data)))))))
 
@@ -118,7 +118,7 @@
       (assert false) ; should never get here
       (catch [:status 403] resp
         (is (= "Sorry, you do not have access to this resource." (:body resp)))))
-    
+
     (http/post (url "/login") {:form-params {:username "root" :password "admin_password"}})
     (is (= (page-bodies "/hook-admin") (:body (http/get (url "/hook-admin")))))))
 
@@ -135,7 +135,7 @@
 (deftest admin-login
   (binding [clj-http.core/*cookie-store* (clj-http.cookies/cookie-store)]
     (is (= (page-bodies "/login") (:body (http/get (url "/admin")))))
-    
+
     (http/post (url "/login") {:form-params {:username "root" :password "admin_password"}})
     (is (= (page-bodies "/admin") (:body (http/get (url "/admin")))))
     (check-user-role-access)
@@ -151,13 +151,13 @@
   (binding [clj-http.core/*cookie-store* (clj-http.cookies/cookie-store)]
     (is (= (page-bodies "/login") (:body (http/get (url "/admin")))))
     (http/post (url "/login") {:form-params {:username "root" :password "admin_password"}})
-    
+
     (try+
       (http/get (url "/wat"))
       (assert false)
       (catch [:status 404] e))
     (is (= (page-bodies "/admin") (:body (http/get (url "/admin")))))
-    
+
     (is (= (page-bodies "/") (:body (http/get (url "/logout")))))
     (is (= (page-bodies "/login") (:body (http/get (url "/admin")))))))
 

test/test_friend/interactive_form.clj

@@ -1,8 +1,8 @@
 (ns test-friend.interactive-form
-  (:require [cemerick.friend :as friend])
-  (:use clojure.test
-        ring.mock.request
-        [cemerick.friend.workflows :only (interactive-form)]))
+  (:require [cemerick.friend :as friend]
+            [clojure.test :refer :all]
+            [ring.mock.request :as mock]
+            [cemerick.friend.workflows :refer (interactive-form)]))
 
 (deftest form-workflow
   (let [got-creds (atom false)
@@ -14,28 +14,28 @@
                                                         (when (and (= "open sesame" password)
                                                                    (= "Aladdin" username))
                                                           {:identity username})))]
-    (is (nil? (form-handler (request :get login-uri))))
+    (is (nil? (form-handler (mock/request :get login-uri))))
 
     (is (= {:status 302
-            :headers {"Location" "http://localhost/my_login?&login_failed=Y&username="}
+            :headers {"Location" "/my_login?&login_failed=Y&username="}
             :body ""}
-           (form-handler (request :post login-uri))))
+           (form-handler (mock/request :post login-uri))))
 
     (is (= {:status 302
-            :headers {"Location" "http://localhost/my_login?&login_failed=Y&username=foo"}
+            :headers {"Location" "/my_login?&login_failed=Y&username=foo"}
             :body ""}
-           (form-handler (assoc (request :post login-uri)
+           (form-handler (assoc (mock/request :post login-uri)
                                 :params {:username "foo"}
                                 :form-params {"username" "foo"}))))
 
     (is (= {:status 302
-            :headers {"Location" "http://localhost/my_login?&login_failed=Y&username=foobar"}
+            :headers {"Location" "/my_login?&login_failed=Y&username=foobar"}
             :body ""}
-           (form-handler (assoc (request :post login-uri)
+           (form-handler (assoc (mock/request :post login-uri)
                                 :params {:username "foo"}
                                 :form-params {"username" "foobar"}))))
 
-    (let [auth (form-handler (assoc (request :post login-uri)
+    (let [auth (form-handler (assoc (mock/request :post login-uri)
                                     :params {:username "foo"
                                              :password "open sesame"}
                                     :form-params {"username" "Aladdin"