Permalink
Browse files

Add support for extracting basic authentication from request header

  • Loading branch information...
carlssonia committed Jan 5, 2012
1 parent ee4bfc6 commit 99e2365935b3e92d27b0881bc94c164ff4dda584
Showing with 17 additions and 0 deletions.
  1. +1 −0 snap-core.cabal
  2. +1 −0 src/Snap/Core.hs
  3. +15 −0 src/Snap/Internal/Http/Types.hs
View
@@ -141,6 +141,7 @@ Library
attoparsec-enumerator >= 0.3 && <0.4,
base >= 4 && < 5,
base16-bytestring <= 0.2,
+ base64-bytestring <= 0.2,
blaze-builder >= 0.2.1.4 && <0.4,
blaze-builder-enumerator >= 0.2 && <0.3,
bytestring,
View
@@ -90,6 +90,7 @@ module Snap.Core
, rqContextPath
, rqURI
, rqQueryString
+ , rqBasicAuthentication
, rqParams
, rqParam
, getParam
@@ -21,6 +21,7 @@ module Snap.Internal.Http.Types where
import Blaze.ByteString.Builder
import Control.Monad (liftM)
import Data.ByteString (ByteString)
+import qualified Data.ByteString.Base64 as B64
import qualified Data.ByteString.Char8 as B
import Data.ByteString.Internal (c2w,w2c)
import qualified Data.ByteString as S
@@ -443,6 +444,20 @@ instance HasHeaders Response where
updateHeaders f r = r { rspHeaders = f (rspHeaders r) }
+------------------------------------------------------------------------------
+-- | Returns the authorization userid and password assuming Basic
+-- Authentication Scheme.
+rqBasicAuthentication :: Request -- ^ HTTP request

This comment has been minimized.

Show comment Hide comment
@gregorycollins

gregorycollins Jan 9, 2012

Thanks for tackling this. First of all: I think this code should go into a new module Snap.Util.HttpAuthentication' rather than getting lumped intoSnap.Core`.

Secondly: I'm uncomfortable with providing basic authentication out of the box without any disclaimers/warning labels/restrictions on usage. Could you please change this API to:

withBasicAuthentication :: MonadSnap m => ((ByteString, ByteString) -> m a) -> m a

and check that rqIsSecure is True within the handler. I really don't think we want to make it easy for people to send cleartext passwords out over the internet in our public API. A suitable disclaimer in the comment (that we will only authenticate secure connections using this method).

If you really feel that cleartext basic authentication is something that needs to be in the API, then add the function as withUnsafeBasicAuthentication with a big disclaimer in the docstring. Personally I think we're better off just omitting it.

I think the thing we all would really like here (and I think hvr mentioned it also) is RFC 2617 digest authentication. Would you be interested in tackling that?

@gregorycollins

gregorycollins Jan 9, 2012

Thanks for tackling this. First of all: I think this code should go into a new module Snap.Util.HttpAuthentication' rather than getting lumped intoSnap.Core`.

Secondly: I'm uncomfortable with providing basic authentication out of the box without any disclaimers/warning labels/restrictions on usage. Could you please change this API to:

withBasicAuthentication :: MonadSnap m => ((ByteString, ByteString) -> m a) -> m a

and check that rqIsSecure is True within the handler. I really don't think we want to make it easy for people to send cleartext passwords out over the internet in our public API. A suitable disclaimer in the comment (that we will only authenticate secure connections using this method).

If you really feel that cleartext basic authentication is something that needs to be in the API, then add the function as withUnsafeBasicAuthentication with a big disclaimer in the docstring. Personally I think we're better off just omitting it.

I think the thing we all would really like here (and I think hvr mentioned it also) is RFC 2617 digest authentication. Would you be interested in tackling that?

This comment has been minimized.

Show comment Hide comment
@carlssonia

carlssonia Jan 11, 2012

Owner

Restricting the API to secure connections is a good idea!

Would you be OK with skipping the continuation-passing style and simply have something like

getBasicAuthentication :: MonadSnap m => m (Maybe (ByteString, ByteString))

where the function would return Nothing in case of insecure connection, or missing/invalid header? That would be much easier to use, I think.

@carlssonia

carlssonia Jan 11, 2012

Owner

Restricting the API to secure connections is a good idea!

Would you be OK with skipping the continuation-passing style and simply have something like

getBasicAuthentication :: MonadSnap m => m (Maybe (ByteString, ByteString))

where the function would return Nothing in case of insecure connection, or missing/invalid header? That would be much easier to use, I think.

This comment has been minimized.

Show comment Hide comment
@carlssonia

carlssonia Jan 11, 2012

Owner

Regarding digest authentication: I'm afraid I don't have a need for that at the moment...

@carlssonia

carlssonia Jan 11, 2012

Owner

Regarding digest authentication: I'm afraid I don't have a need for that at the moment...

+ -> Maybe (ByteString, ByteString)
+rqBasicAuthentication rq = do
+ ("Basic ", d) <- B.splitAt 6 `fmap` getHeader "Authorization" rq
+ case B64.decode d of
+ Left _ -> Nothing
+ Right e -> case B.break (==':') e of
+ (u,pw) | B.take 1 pw == ":" -> return (u, B.drop 1 pw)
+ _ -> Nothing
+
+
------------------------------------------------------------------------------
-- | Looks up the value(s) for the given named parameter. Parameters initially
-- come from the request's query string and any decoded POST body (if the

0 comments on commit 99e2365

Please sign in to comment.