mirror of
https://github.com/freckle/yesod-auth-oauth2.git
synced 2026-04-24 03:37:44 +02:00
Exclude + from state tokens
When the state token is sent to an OAuth2 provider, it undergoes %-encoding as a URL parameter. Presumably, the OAuth2 provider decodes it as part of handling things (because it would take work to prevent their own web frameworks from doing so), and then re-%-encodes it coming back to us again as a callback parameter. For us, and all existing providers, + is not a %-encoded character, so it's sent as-is and sent back as-is. So far so good. ClassLink, though, chooses to decode + to space. I'm not aware of the actual spec or if this is a reasonable thing to do, but they do. This results in them sending %20 back to us, which doesn't match and we fail. We can't predict or prescribe what providers do in this area, so our options are: - Look for a match in our Session as-is OR with spaces replaced by + This is harder than it sounds: a token could contain +'s or spaces, and we'd be getting back only spaces. To succeed, we'd actually have to check every permutation of space/+ substitution. - Filter + from our tokens The only downside is we may generate slightly fewer than 30 characters, and so produce slightly less secure tokens. I chose this option. - Generate tokens without + to begin with This would be ideal, but I'm just not familiar enough with Crypto.Random. I would happily accept a PR to use this option.
This commit is contained in:
parent
424b71de5c
commit
73a7e9805c
@ -156,14 +156,25 @@ withCallbackAndState name oauth2 csrf = do
|
|||||||
getParentUrlRender :: MonadHandler m => m (Route (SubHandlerSite m) -> Text)
|
getParentUrlRender :: MonadHandler m => m (Route (SubHandlerSite m) -> Text)
|
||||||
getParentUrlRender = (.) <$> getUrlRender <*> getRouteToParent
|
getParentUrlRender = (.) <$> getUrlRender <*> getRouteToParent
|
||||||
|
|
||||||
-- | Set a random, 30-character value in the session
|
-- | Set a random, ~30-character value in the session
|
||||||
|
--
|
||||||
|
-- Some (but not all) providers decode a @+@ in the state token as a space when
|
||||||
|
-- sending it back to us. We don't expect this and fail. And if we did code for
|
||||||
|
-- it, we'd then fail on the providers that /don't/ do that.
|
||||||
|
--
|
||||||
|
-- Therefore, we just exclude @+@ in our tokens, which means this function may
|
||||||
|
-- return slightly less than 30 characters.
|
||||||
|
--
|
||||||
setSessionCSRF :: MonadHandler m => Text -> m Text
|
setSessionCSRF :: MonadHandler m => Text -> m Text
|
||||||
setSessionCSRF sessionKey = do
|
setSessionCSRF sessionKey = do
|
||||||
csrfToken <- liftIO randomToken
|
csrfToken <- liftIO randomToken
|
||||||
csrfToken <$ setSession sessionKey csrfToken
|
csrfToken <$ setSession sessionKey csrfToken
|
||||||
where
|
where
|
||||||
randomToken =
|
randomToken =
|
||||||
decodeUtf8 . convertToBase @ByteString Base64 <$> getRandomBytes 64
|
T.filter (/= '+')
|
||||||
|
. decodeUtf8
|
||||||
|
. convertToBase @ByteString Base64
|
||||||
|
<$> getRandomBytes 64
|
||||||
|
|
||||||
-- | Verify the callback provided the same CSRF token as in our session
|
-- | Verify the callback provided the same CSRF token as in our session
|
||||||
verifySessionCSRF :: MonadHandler m => Text -> m Text
|
verifySessionCSRF :: MonadHandler m => Text -> m Text
|
||||||
@ -172,8 +183,14 @@ verifySessionCSRF sessionKey = do
|
|||||||
sessionToken <- lookupSession sessionKey
|
sessionToken <- lookupSession sessionKey
|
||||||
deleteSession sessionKey
|
deleteSession sessionKey
|
||||||
|
|
||||||
unless (sessionToken == Just token)
|
unless (sessionToken == Just token) $ do
|
||||||
$ permissionDenied "Invalid OAuth2 state token"
|
$(logError)
|
||||||
|
$ "state token does not match. "
|
||||||
|
<> "Param: "
|
||||||
|
<> tshow token
|
||||||
|
<> "State: "
|
||||||
|
<> tshow sessionToken
|
||||||
|
permissionDenied "Invalid OAuth2 state token"
|
||||||
|
|
||||||
return token
|
return token
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user