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

Add simple authentication helpers #962

Merged
merged 1 commit into from
Mar 26, 2015
Merged

Conversation

s9gf4ult
Copy link
Contributor

I think it must be in yesod-core

@snoyberg
Copy link
Member

Can you clarify why this must be in yesod-core? This looks like it would be a great piece of code to start in a separate library to allow the API to evolve and, once stabilized, consider moving into a more core package.

@s9gf4ult
Copy link
Contributor Author

This is trivial implementation of common standardized methods of authentication. Many users will need this and it looks like core functionality for any web framework. But if you think it must be in separate package I will create some.

@snoyberg
Copy link
Member

I'm not necessarily opposed, I just wanted to understand the "must" comment, which I think I get now. Here's my recommendation:

  • The non-Yesod stuff could likely go into wai-extra instead to be more generally available
  • I'd avoid decodeUtf8, since it's a partial function. I'd instead use decodeUtf8With lenientDecode
  • Instead of putting this in a dedicated module, let's just add it to Yesod.Core.Handler

Does that sound reasonable?

@s9gf4ult
Copy link
Contributor Author

Ok, so:

  • move extractXAuth to wai-extra
  • use decodeUtf8With lenientDecode
  • move lookupXAuth to Yesod.Core.Handler

looks logical for me.

@snoyberg
Copy link
Member

Cool. Would you be able to send a PR?

On Wed, Mar 25, 2015 at 11:09 AM Alexey Uimanov notifications@github.com
wrote:

Ok, so:

  • move extractXAuth to wai-extra
  • use decodeUtf8With lenientDecode
  • move lookupXAuth to Yesod.Core.Handler

looks logical for me.


Reply to this email directly or view it on GitHub
#962 (comment).

@s9gf4ult
Copy link
Contributor Author

Yes, I will prepare PR towards evening.

@s9gf4ult
Copy link
Contributor Author

Ok, ive patched wai-extra here yesodweb/wai#352

@snoyberg snoyberg merged commit 79dc6c3 into yesodweb:master Mar 26, 2015
@snoyberg
Copy link
Member

Looks good, I'm releasing both to Hackage now. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants