From ad561a09d641fea202bc3f81873af9a543dc1c85 Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Sat, 28 Nov 2015 11:42:05 -0500 Subject: [PATCH] Append the state query parameter correctly If the endpoint URL doesn't yet have any query parameters, this code would construct an invalid URL by appending `&state=` rather than `?state=`. We now correctly append `?state=` or `&state=` depending on the URL already containing a `?` character. Fixes #44 This is a "good enough" solution for the moment, done for ease of implementation and speed of delivery. An eventual better solution would be to use a URL-parsing and constructing library to parse the existing endpoint, append parameter tuples, and re-construct a new URL. Restructuring the data types so we start handling real URL and QueryParam values instead of ByteStrings would also be an option, though we're a little cornered by the Yesod.Auth.OAuth interface. --- Yesod/Auth/OAuth2.hs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Yesod/Auth/OAuth2.hs b/Yesod/Auth/OAuth2.hs index 7a9145d..86c4968 100644 --- a/Yesod/Auth/OAuth2.hs +++ b/Yesod/Auth/OAuth2.hs @@ -38,6 +38,7 @@ import Yesod.Core import Yesod.Form import qualified Data.ByteString.Lazy as BL +import qualified Data.ByteString.Char8 as C8 -- | Provider name and Aeson parse error data YesodOAuth2Exception = InvalidProfileResponse Text BL.ByteString @@ -87,7 +88,7 @@ authOAuth2Widget widget name oauth getCreds = AuthPlugin name dispatch login return oauth { oauthCallback = Just $ encodeUtf8 $ render $ tm url , oauthOAuthorizeEndpoint = oauthOAuthorizeEndpoint oauth - <> "&state=" <> encodeUtf8 csrfToken + `appendQuery` "state=" <> encodeUtf8 csrfToken } dispatch "GET" ["forward"] = do @@ -141,3 +142,9 @@ fromProfileURL name url toCreds manager token = do bsToText :: ByteString -> Text bsToText = decodeUtf8With lenientDecode + +appendQuery :: ByteString -> ByteString -> ByteString +appendQuery url query = + if '?' `C8.elem` url + then url <> "&" <> query + else url <> "?" <> query