Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clearCreds support for JSON #991

Closed
meteficha opened this issue May 15, 2015 · 3 comments
Closed

clearCreds support for JSON #991

meteficha opened this issue May 15, 2015 · 3 comments
Assignees

Comments

@meteficha
Copy link
Member

This is the current definition of clearCreds, used on logout:

-- | Clears current user credentials for the session.
--
-- Since 1.1.7
clearCreds :: YesodAuth master
           => Bool -- ^ if HTTP redirect to 'logoutDest' should be done
           -> HandlerT master IO ()
clearCreds doRedirects = do
    y <- getYesod
    onLogout
    deleteSession credsKey
    when doRedirects $ do
        redirectUltDest $ logoutDest y

Note that it always redirects, even if a request was made accepting JSON responses. One could argue that a JSON client should just forget the session cookie themselves, but I think this is a bad idea for two reasons:

  • Other session variables may still be relevant even if the user logged out.
  • There may be some relevant action on onLogout that should be taken.
  • The server could change its session implementation.

I propose changing the above definition to something along the lines of:

-- | Clears current user credentials for the session.
--
-- Since 1.1.7
clearCreds :: YesodAuth master
           => Bool -- ^ if HTTP redirect to 'logoutDest' should be done
           -> HandlerT master IO ()
clearCreds doRedirects = do
    y <- getYesod
    onLogout
    deleteSession credsKey
    when doRedirects $ do
        aj <- acceptsJson
        if aj
            then do
                let msg = "Logged out successfully!" :: Text
                sendResponse $ A.object ["message" A..= msg]
            else
                redirectUltDest $ logoutDest y
@snoyberg
Copy link
Member

I'm OK with that. We should update the documentation to explain that. Also,
selectRep would probably be easier.

On Fri, May 15, 2015, 8:11 AM Felipe Lessa notifications@github.com wrote:

This is the current definition of clearCreds, used on logout:

-- | Clears current user credentials for the session.---- Since 1.1.7clearCreds :: YesodAuth master
=> Bool -- ^ if HTTP redirect to 'logoutDest' should be done
-> HandlerT master IO ()
clearCreds doRedirects = do
y <- getYesod
onLogout
deleteSession credsKey
when doRedirects $ do
redirectUltDest $ logoutDest y

Note that it always redirects, even if a request was made accepting JSON
responses. One could argue that a JSON client should just forget the
session cookie themselves, but I think this is a bad idea for two reasons:

Other session variables may still be relevant even if the user logged
out.

There may be some relevant action on onLogout that should be taken.

The server could change its session implementation.

I propose changing the above definition to something along the lines of:

-- | Clears current user credentials for the session.---- Since 1.1.7clearCreds :: YesodAuth master
=> Bool -- ^ if HTTP redirect to 'logoutDest' should be done
-> HandlerT master IO ()
clearCreds doRedirects = do
y <- getYesod
onLogout
deleteSession credsKey
when doRedirects $ do
aj <- acceptsJson
if aj
then do
let msg = "Logged out successfully!" :: Text
sendResponse $ A.object ["message" A..= msg]
else
redirectUltDest $ logoutDest y


Reply to this email directly or view it on GitHub
#991.

@meteficha
Copy link
Member Author

Adding to my to do list.

@StevenXL
Copy link
Member

See #1598

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants