Handle basic credentials with colon character in the password.

[rudism]

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.

      user-pass   = userid ":" password
      userid      = *<TEXT excluding ":">
      password    = *TEXT

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

• I have read the CONTRIBUTING guidelines.
• I have added test cases which verify the correct operation of this feature or patch.
• I have added documentation pertaining to this feature or patch.
• The automated test suite ($ make test) executes successfully.
• The automated code linting ($ make lint) executes successfully.

[rudism]

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.

[neemah]

Is this ever get merged?

[bodo22]

@jaredhanson ist there anything I can help with to get this MR merged and released to npm?

[MatthiasKunnen]

@bodo22, I've created a fork in which I fixed some behavior including this issue. See: https://www.npmjs.com/package/passport-http-2.

[bodo22]

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.

[la-bibe]

Can we know when will this be merged please ?

[gkTim]

Please merge this fix!

[kvanrobbroeck]

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!