Format and add tests #56
Conversation
| unsignedPayload :: Either LBS.ByteString (RequestPayload, Signature) | ||
| unsignedPayload = case extractOn isAuthHeader $ rpHeaders signedPayload of | ||
| (Nothing, _) -> Left "No 'Authentication' header" | ||
| (Just (_, val), headers) -> case BS.stripPrefix "HMAC " val of | ||
| Just sig -> Right | ||
| ( signedPayload { rpHeaders = headers } | ||
| , Signature sig | ||
| ) | ||
| Just sig -> | ||
| Right | ||
| ( signedPayload{rpHeaders = headers} | ||
| , Signature sig | ||
| ) | ||
| Nothing -> Left "Can not strip 'HMAC' prefix in header" | ||
|
|
There was a problem hiding this comment.
I don't know much about the HMAC authentication standard, but I think this code makes the library can not be reused in many cases :
- The signature is specified in a different header name (such as x-sendbird-signature)
- The payload is request body or combination of request body with some header content (such as Stripe payload)
So I think it could be better if we can specify the header name of the signature and the function to map from request to singed payload
There was a problem hiding this comment.
Like you said, our current implementation of the authentication differs from Sendbird's which also differs from Stripe's. While it would be interesting to be customizable to handle as much use case as possible, I fear that there's too many different use cases out there and we would lose the "simple" aspect of the library.
For example, we could expose the requestSignature function and allow the client to change the actual implementation, but the library may lose most of its usefulness as a consequence.
What's everyone else's opinion on that?
There was a problem hiding this comment.
Sorry, missed this earlier.
@thongpv87 Just so that I understand your point better :
- Does it mean that with this change merged into
master, that you would have to re-implement or change some existing code in order to be compatible with a future version ofservant-hmac-auth?
There was a problem hiding this comment.
In anticipation of a new major version with a breaking change (take into account the request's body for the HMAC signature), this PR adds the following: