Properly handle empty credsExtra fields

Marks "location" as Maybe in GitHub responses. Without this, users could
experience an InvalidProfileResponse error when missing.

Also fixes cases where fields were Maybe, but the (.:?) combinator was
not being used in the parser.
This commit is contained in:
Andrew Darqui 2016-06-20 02:14:11 -04:00 committed by patrick brisbin
parent 3f204a9ae3
commit 64b65ca4c6
No known key found for this signature in database
GPG Key ID: ADB6812F871D4478
4 changed files with 17 additions and 19 deletions

View File

@ -15,6 +15,7 @@ module Yesod.Auth.OAuth2
, oauth2Url , oauth2Url
, fromProfileURL , fromProfileURL
, YesodOAuth2Exception(..) , YesodOAuth2Exception(..)
, maybeExtra
, module Network.OAuth.OAuth2 , module Network.OAuth.OAuth2
) where ) where
@ -149,3 +150,9 @@ appendQuery url query =
if '?' `C8.elem` url if '?' `C8.elem` url
then url <> "&" <> query then url <> "&" <> query
else url <> "?" <> query else url <> "?" <> query
-- | A helper for providing an optional value to credsExtra
--
maybeExtra :: Text -> Maybe Text -> [(Text, Text)]
maybeExtra k (Just v) = [(k, v)]
maybeExtra _ Nothing = []

View File

@ -37,7 +37,7 @@ data GithubUser = GithubUser
, githubUserName :: Maybe Text , githubUserName :: Maybe Text
, githubUserLogin :: Text , githubUserLogin :: Text
, githubUserAvatarUrl :: Text , githubUserAvatarUrl :: Text
, githubUserLocation :: Text , githubUserLocation :: Maybe Text
, githubUserPublicEmail :: Maybe Text , githubUserPublicEmail :: Maybe Text
} }
@ -47,8 +47,8 @@ instance FromJSON GithubUser where
<*> o .:? "name" <*> o .:? "name"
<*> o .: "login" <*> o .: "login"
<*> o .: "avatar_url" <*> o .: "avatar_url"
<*> o .: "location" <*> o .:? "location"
<*> o .: "email" <*> o .:? "email"
parseJSON _ = mzero parseJSON _ = mzero
@ -104,18 +104,12 @@ toCreds user userMails token = Creds
[ ("email", githubUserEmailAddress email) [ ("email", githubUserEmailAddress email)
, ("login", githubUserLogin user) , ("login", githubUserLogin user)
, ("avatar_url", githubUserAvatarUrl user) , ("avatar_url", githubUserAvatarUrl user)
, ("location", githubUserLocation user)
, ("access_token", decodeUtf8 $ accessToken token) , ("access_token", decodeUtf8 $ accessToken token)
] ]
++ maybeName (githubUserName user) ++ maybeExtra "name" (githubUserName user)
++ maybePublicEmail (githubUserPublicEmail user) ++ maybeExtra "email" (githubUserPublicEmail user)
++ maybeExtra "location" (githubUserLocation user)
} }
where where
email = fromMaybe (head userMails) $ find githubUserEmailPrimary userMails email = fromMaybe (head userMails) $ find githubUserEmailPrimary userMails
maybeName Nothing = []
maybeName (Just name) = [("name", name)]
maybePublicEmail Nothing = []
maybePublicEmail (Just e) = [("public_email", e)]

View File

@ -29,7 +29,6 @@ import Control.Exception.Lifted
import Control.Monad (mzero) import Control.Monad (mzero)
import Data.Aeson import Data.Aeson
import Data.Monoid ((<>)) import Data.Monoid ((<>))
import Data.Maybe (maybeToList)
import Data.Text (Text) import Data.Text (Text)
import Data.Text.Encoding (encodeUtf8, decodeUtf8) import Data.Text.Encoding (encodeUtf8, decodeUtf8)
import Network.HTTP.Conduit (Manager) import Network.HTTP.Conduit (Manager)
@ -135,8 +134,6 @@ uidBuilder f user token = Creds
, ("family_name", googleUserFamilyName user) , ("family_name", googleUserFamilyName user)
, ("avatar_url", googleUserPicture user) , ("avatar_url", googleUserPicture user)
, ("access_token", decodeUtf8 $ accessToken token) , ("access_token", decodeUtf8 $ accessToken token)
] ++ maybeHostedDomain ]
++ maybeExtra "hosted_domain" (googleUserHostedDomain user)
} }
where
maybeHostedDomain = maybeToList $ (,) "hosted_domain" <$> googleUserHostedDomain user

View File

@ -33,8 +33,8 @@ data SpotifyUserImage = SpotifyUserImage
instance FromJSON SpotifyUserImage where instance FromJSON SpotifyUserImage where
parseJSON (Object v) = SpotifyUserImage parseJSON (Object v) = SpotifyUserImage
<$> v .: "height" <$> v .:? "height"
<*> v .: "width" <*> v .:? "width"
<*> v .: "url" <*> v .: "url"
parseJSON _ = mzero parseJSON _ = mzero