Open rudism opened 5 years ago
I'll note that there are older PRs #67 and #69 to address this same issue, but this patch doesn't involve re-assembling the split password or adding new failure conditions, I think it's the least impactful way to address it.
Is this ever get merged?
@jaredhanson ist there anything I can help with to get this MR merged and released to npm?
@bodo22, I've created a fork in which I fixed some behavior including this issue. See: https://www.npmjs.com/package/passport-http-2.
Thanks! I've created a fork myself already :). As this project has 50k npm downloads a week it would be really nice to see the changes be applied here.
Can we know when will this be merged please ?
Please merge this fix!
Would be awesome to see this merged too, we're going to get a fork for this as well, but thanks to @rudism for having done the work already!
Per RFC 2617 Section 2: Basic Authentication Scheme, after decoding the basic authentication base64 credentials value, the
userid
is comprised of everything up to but not including the first:
character, and the password is everything after that first:
character. It seems that a strict reading would imply that passwords could themselves contain:
characters as they would be part of the text that follows the first:
character.This patch (which closes issue #20) changes the behavior of the basic authentication strategy to more closely adhere to the RFC quoted above.
Previously it was splitting the base64 decoded string on the
:
character and using the 0th element as the username, the 1st element as the password, and dropping everything else, including part of the password if it contained another:
character. After this patch it will only split the decoded value on the first:
character and include everything afterwards in the password even if it contains additional:
characters.I have added a new test to verify that passwords with
:
characters now authenticate correctly. I have not added any new documentation as this is a pretty minor code change to fix a small problem and does not significantly add to or alter any existing functionality.Checklist
$ make test
) executes successfully.$ make lint
) executes successfully.