freckle/yesod-auth-oauth2

Add custom token response parsing

cbeav opened this issue · 4 comments

cbeav commented

Ran into a frustrating issue while building a plugin for AzureAD. As far as I can tell, the token fetching "works", but is failing to parse the response:

127.0.0.1 - - [26/Oct/2018:16:28:57 -0500] "GET /auth/login HTTP/1.1" 200 156 "" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.67 Safari/537.36"
127.0.0.1 - - [26/Oct/2018:16:28:59 -0500] "GET /auth/page/azuread/forward HTTP/1.1" 303 0 "http://localhost:3001/auth/login" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.67 Safari/537.36"
26/Oct/2018:16:29:00 -0500 [Error] OAuth2 error (azuread): ErrorResponse {erName = Unknown "OAuth2Error {error = Left \"Decode error\", errorDescription = Just \"Error: Error in $: key \\\"error\\\" not present\\n Original Response:\\n\\\"{\\\\\\\"token_type\\\\\\\":\\\\\\\"Bearer\\\\\\\",\\\\\\\"expires_in\\\\\\\":\\\\\\\"3600\\\\\\\",\\\\\\\"ext_expires_in\\\\\\\":\\\\\\\"0\\\\\\\",\\\\\\\"expires_on\\\\\\\":\\\\\\\"1540592896\\\\\\\",\\\\\\\"access_token\\\\\\\":...

If I'm understanding this correctly, it looks like it's failing to parse an OAuth2Token, then falling back to trying to find an errors field, then vomiting because it can't do either.

The problem appears to be that Microsoft's API is returning an escaped JSON string as the body, and not pure JSON. All the fields you'd expect to see on the token spec are there, just with hella escaped quotes burying them.

Hoping for 1) confirmation that this theory makes sense and 2) suggestions on how this might best be implemented within yesod-auth-oauth2. Happy to do whatever's needed to get this working, but want to find a solution that doesn't pollute the codebase so much just for one funky API.

For posterity, and to be incorporated into a future PR, here's all I'm doing so far:

{-# LANGUAGE OverloadedStrings #-}
-- |
--
-- OAuth2 plugin for Azure AD.
--
--
module Yesod.Auth.OAuth2.AzureAD
    ( oauth2AzureAD
    , oauth2AzureADScoped
    ) where

import Prelude
import Yesod.Auth.OAuth2.Prelude

import qualified Data.Text as T

newtype User = User Text

instance FromJSON User where
    parseJSON = withObject "User" $ \o -> User
        <$> o .: "mail"

pluginName :: Text
pluginName = "azuread"

defaultScopes :: [Text]
defaultScopes = ["openid", "profile"]

oauth2AzureAD :: YesodAuth m => Text -> Text -> AuthPlugin m
oauth2AzureAD = oauth2AzureADScoped defaultScopes

oauth2AzureADScoped :: YesodAuth m => [Text] -> Text -> Text -> AuthPlugin m
oauth2AzureADScoped scopes clientId clientSecret =
    authOAuth2 pluginName oauth2 $ \manager token -> do
        (User userId, userResponse) <-
            authGetProfile pluginName manager token "https://graph.microsoft.com/v1.0/me"

        pure Creds
            { credsPlugin = pluginName
            , credsIdent = T.pack $ show userId
            , credsExtra = setExtra token userResponse
            }
  where
    oauth2 = OAuth2
        { oauthClientId = clientId
        , oauthClientSecret = clientSecret
        , oauthOAuthorizeEndpoint = "https://login.windows.net/common/oauth2/authorize" `withQuery`
            [ scopeParam "," scopes
            ]
        , oauthAccessTokenEndpoint = "https://login.windows.net/common/oauth2/token"
        , oauthCallback = Nothing
        }
cbeav commented

Learning a bit more about this has plunged me pretty immediately into hoauth. It's fetchAccessToken that's failing via the parseResponseJSON in there. Let me know if you have any suggestions, but this is feeling quickly out-of-scope as an issue for yesod-auth-oauth2.

cbeav commented

Lol, this turned out to be more fun than expected. The Microsoft API returns expiresIn on the token as a string and not an int, and Aeson doesn't like that. Doesn't feel totally insane that the instance FromJSON Int should cover this, will see if there's any case to be made there. In the meantime I forked a lot of this and made my own OAuth2Token that just doesn't use expiresIn, since none of the rest of the library cares about that.

Hey Beavers!

Wow, that sounds rough. I'm glad you ended up sorting it out, but that's sad that MS isn't following spec. I'm sure hoauth2 would be open to a PR to make it a bit more flexible to making that provider work though.

cbeav commented

Hey Patrick! Haha, yeah will try to see if we can move things along there. Hope all's well, will circle back here once I've got a cleaner PR to open.