diff --git a/CHANGES.md b/CHANGES.md index 8e90bd3..db43c58 100644 --- a/CHANGES.md +++ b/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 diff --git a/project.clj b/project.clj index 0fdb869..06c2341 100644 --- a/project.clj +++ b/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"} diff --git a/src/cemerick/friend.clj b/src/cemerick/friend.clj index b046daa..211320c 100644 --- a/src/cemerick/friend.clj +++ b/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." diff --git a/src/cemerick/friend/util.clj b/src/cemerick/friend/util.clj index e578a7b..9449996 100644 --- a/src/cemerick/friend/util.clj +++ b/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)))) diff --git a/src/cemerick/friend/workflows.clj b/src/cemerick/friend/workflows.clj index 9e324ae..c4de00c 100644 --- a/src/cemerick/friend/workflows.clj +++ b/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 diff --git a/test/test_friend/functional.clj b/test/test_friend/functional.clj index ae4f438..bc9e420 100644 --- a/test/test_friend/functional.clj +++ b/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"))))))) diff --git a/test/test_friend/interactive_form.clj b/test/test_friend/interactive_form.clj index 5541354..549cfbc 100644 --- a/test/test_friend/interactive_form.clj +++ b/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"