Permalink
Browse files

Always send absolute URIs on redirects, fixes gh-42

  • Loading branch information...
1 parent a969539 commit 5b0432385bc86f94ae32f7094a3d096803555e58 @cemerick committed Jan 12, 2013
View
@@ -1,21 +1,11 @@
(ns cemerick.friend
- (:require [clojure.set :as set])
+ (:require [clojure.set :as set]
+ [cemerick.friend.util :as util])
(:use (ring.util [response :as response :only (redirect)])
[slingshot.slingshot :only (throw+ try+)]
[clojure.core.incubator :only (-?>)])
(:refer-clojure :exclude (identity)))
-(defn- original-url
- [{:keys [scheme server-name server-port uri query-string]}]
- (str (name scheme) "://" server-name
- (cond
- (and (= :http scheme) (not= server-port 80)) (str \: server-port)
- (and (= :https scheme) (not= server-port 443)) (str \: server-port)
- :else nil)
- uri
- (when (seq query-string)
- (str \? query-string))))
-
(def ^{:dynamic true} *default-scheme-ports* {:http 80 :https 443})
(defn requires-scheme
@@ -40,7 +30,7 @@
(if (= (:scheme request) scheme)
(handler request)
; TODO should this be permanent 301?
- (redirect (original-url (assoc request
+ (redirect (util/original-url (assoc request
:scheme scheme
:server-port (scheme-mapping scheme))))))))
@@ -58,7 +48,7 @@
(fn [request]
(if (= (get-in request [:headers "x-forwarded-proto"]) (name scheme))
(handler request)
- (redirect (original-url
+ (redirect (util/original-url
(assoc request
:scheme scheme
:server-port (scheme-mapping scheme))))))))
@@ -165,7 +155,8 @@ Equivalent to (complement current-authentication)."}
(defn- redirect-unauthorized
[redirect-uri request]
- (-> (ring.util.response/redirect redirect-uri)
+ (-> (util/resolve-absolute-uri request redirect-uri)
+ ring.util.response/redirect
(assoc :session (:session request))
(assoc-in [:session ::unauthorized-uri] (:uri request))))
@@ -1,6 +1,7 @@
(ns cemerick.friend.openid
(:require [cemerick.friend :as friend]
[cemerick.friend.workflows :as workflows]
+ [cemerick.friend.util :as util]
clojure.walk
ring.util.response
[clojure.core.cache :as cache])
@@ -43,7 +44,7 @@
[^ConsumerManager mgr discovery-cache user-identifier {:keys [session] :as request} realm]
(let [discoveries (.discover mgr user-identifier)
provider-info (.associate mgr discoveries)
- return-url (str (#'friend/original-url request) "?" return-key "=1")
+ return-url (str (util/original-url request) "?" return-key "=1")
auth-req (request-attribute-exchange
(if realm
(.authenticate mgr provider-info return-url realm)
@@ -83,7 +84,7 @@
(defn- handle-return
[^ConsumerManager mgr discovery-cache {:keys [params session] :as req} openid-config]
(let [provider-info (get @discovery-cache (::openid-disc session))
- url (#'friend/original-url req)
+ url (util/original-url req)
plist (ParameterList. params)
credentials (build-credentials (.verify mgr url plist provider-info))]
(swap! discovery-cache cache/evict (::openid-disc session))
@@ -122,4 +123,5 @@
(update-in request [::friend/auth-config] merge openid-config)))
;; TODO correct response code?
- :else (login-failure-handler request)))))))
+ :else ((gets :login-failure-handler openid-config (::friend/auth-config request))
+ request)))))))
@@ -7,4 +7,22 @@
(-?>> (map #(find % key) maps)
(remove nil?)
first
- val))
+ val))
+
+(defn original-url
+ [{:keys [scheme server-name server-port uri query-string]}]
+ (str (name scheme) "://" server-name
+ (cond
+ (and (= :http scheme) (not= server-port 80)) (str \: server-port)
+ (and (= :https scheme) (not= server-port 443)) (str \: server-port)
+ :else nil)
+ uri
+ (when (seq query-string)
+ (str \? query-string))))
+
+(defn resolve-absolute-uri
+ [request uri]
+ (-> (original-url request)
+ java.net.URI.
+ (.resolve uri)
+ str))
@@ -1,5 +1,6 @@
-(ns cemerick.friend.workflows
- (:require [cemerick.friend :as friend])
+1(ns cemerick.friend.workflows
+ (:require [cemerick.friend :as friend]
+ [cemerick.friend.util :as util])
(:use [clojure.string :only (trim)]
[cemerick.friend.util :only (gets)])
(:import org.apache.commons.codec.binary.Base64))
@@ -59,11 +60,13 @@
(defn interactive-login-redirect
[{:keys [params] :as request}]
- (ring.util.response/redirect (let [param (str "&login_failed=Y&username=" (java.net.URLEncoder/encode
- (:username params "")))
- login-uri (-> request ::friend/auth-config :login-uri)]
- (str (if (.contains login-uri "?") login-uri (str login-uri "?"))
- param))))
+ (ring.util.response/redirect
+ (let [param (str "&login_failed=Y&username="
+ (java.net.URLEncoder/encode (:username params "")))
+ login-uri (-> request ::friend/auth-config :login-uri)]
+ (util/resolve-absolute-uri 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
@@ -103,7 +103,8 @@
(let [should-be-login-redirect (http/get (url "/user/account")
{:follow-redirects false})]
(is (= 302 (:status should-be-login-redirect)))
- (is (= "/login" (-> should-be-login-redirect :headers (get "location")))))
+ (is (re-matches #"http://localhost:\d+/login"
+ (-> should-be-login-redirect :headers (get "location")))))
; TODO should logout blow away the session completely?
(is (= "auth-data" (get-session-data)))))))
@@ -17,12 +17,12 @@
(is (nil? (form-handler (request :get login-uri))))
(is (= {:status 302
- :headers {"Location" "/my_login?&login_failed=Y&username="}
+ :headers {"Location" "http://localhost/my_login?&login_failed=Y&username="}
:body ""}
(form-handler (request :post login-uri))))
(is (= {:status 302
- :headers {"Location" "/my_login?&login_failed=Y&username=foo"}
+ :headers {"Location" "http://localhost/my_login?&login_failed=Y&username=foo"}
:body ""}
(form-handler (assoc (request :post login-uri)
:params {:username "foo"}))))

0 comments on commit 5b04323

Please sign in to comment.