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

client-heavy interface to yesod-auth #479

Closed
gregwebs opened this issue Jan 14, 2013 · 10 comments
Closed

client-heavy interface to yesod-auth #479

gregwebs opened this issue Jan 14, 2013 · 10 comments

Comments

@gregwebs
Copy link
Member

When not authenticated, making a request that requires authentication receives a 303 redirect. 401 makes more sense for a client-heavy app.

@snoyberg
Copy link
Member

I can see two approaches here:

  1. Determine whether this is an AJAX called based on request headers, possibly the accept header.
  2. Provide separate functions.

But we need to maintain the 303 for server-side apps.

@gregwebs
Copy link
Member Author

I think we need different JSON responses for most of the yesod-auth handlers. Creating separate handlers means documenting which handler subset should be used. For this handler a 401 along with a json body giving the redirect url is probably best. Of course, the client can also know the redirect url ahead of time.

@gregwebs
Copy link
Member Author

Other improvements besides the first mentioned need to be made. https://groups.google.com/forum/?fromgroups=#!topic/yesodweb/qBd95eDucsU

@tlaitinen
Copy link

Different browsers seem to have different Accept-headers for XMLHTTPRequest. Are there any drawbacks for submitting an additional form-parameter, e.g. "ajax=1" along with the username and password? The client side (at least in my case) does not need anything special - just one definitive bit of information whether the login succeeded or not.

@snoyberg
Copy link
Member

Are there any browsers which don't support setting accept: application/json? IMO, that's the right course of action for indicating you want a JSON response.

@tlaitinen
Copy link

Ah, yes, I guess setting it manually should work. It sounds good to me.

@snoyberg
Copy link
Member

I've implemented this on the yesod1.2 branch. I'll leave this issue open until that branch is merged to master in case someone wants to implement a yesod 1.1 fix as well.

@gregwebs
Copy link
Member Author

Thanks for implementing. This isn't actually working for me right now though, I am trying to track it down.

@gregwebs
Copy link
Member Author

this looks like the issue: https://github.com/yesodweb/yesod/blob/yesod1.2/yesod-core/Yesod/Core/Class/Yesod.hs#L297

A simple fix is not presenting itself to me since the return type is (). There seems to be duplication with yesod-auth, but the core is trying to avoid the yesod-auth dependency.

@gregwebs gregwebs mentioned this issue Mar 29, 2013
@gregwebs
Copy link
Member Author

figured out a pull request for the issue

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