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

BASIC strategy does not support passwords that contain colons #20

Open
blevine opened this issue Jan 21, 2014 · 3 comments · May be fixed by #21 or #69
Open

BASIC strategy does not support passwords that contain colons #20

blevine opened this issue Jan 21, 2014 · 3 comments · May be fixed by #21 or #69

Comments

@blevine
Copy link

blevine commented Jan 21, 2014

Colons are legal characters in passwords. Because of the way the BASIC strategy splits the BASIC username:password header, passwords containing a colon character fail. Per the following code from basic.js:

var scheme = parts[0]
, credentials = new Buffer(parts[1], 'base64').toString().split(':');

if (!/Basic/i.test(scheme)) { return this.fail(this._challenge()); }
if (credentials.length < 2) { return this.fail(400); }

var userid = credentials[0];
var password = credentials[1];

you can see that a split(':') on "myusername:my:password" will result in 3 parts instead of the expected 2. Better to use something like:

.split(':').slice(1).join(':')

or a regexp to get the password. Not sure that I can work up a patch before the new year, but reporting the issue now.

@OliverJAsh
Copy link

To work around this issue, I’m using encodeURIComponent on the user credentials before sending them along in the request. Perhaps it would be a good idea if passport-http would natively decode the credentials as URI components?

@FlorianSW
Copy link

We've this issue, too. Is there a plan to fix this issue? The provided PR doesn't look that bad (I left a comment, though). So probably we've the chance to fix this (already very old) issue? :)

FlorianSW added a commit to FlorianSW/passport-http that referenced this issue Jul 6, 2017
A colon is a valid character in the password, however currently the
chars including and after the colon are stripped of the password which
leads in false-positives (user can't login even if the password is
correct). This commit fixes that.

Fixes jaredhanson#20
@FlorianSW FlorianSW linked a pull request Jul 6, 2017 that will close this issue
@saborrie
Copy link

Any reason that this hasn't been merged/closed yet? This issue is now 4 years old 😮

MatthiasKunnen added a commit to MatthiasKunnen/passport-http-2 that referenced this issue Dec 20, 2018
Previous implementation:
empty user-pass  -> error basic realm
no " " separator -> error 400
no : separator   -> error basic realm
empty password   -> error basic realm
empty username   -> error basic realm

New implementation:
empty user-pass  -> error 400
no " " separator -> error 400
no : separator   -> error 400
empty password   -> success
empty username   -> success

Also fixed passwords containing ':' being truncated.

This fixes:
- jaredhanson/passport-http#20
- jaredhanson/passport-http#41
- jaredhanson/passport-http#42
- jaredhanson/passport-http#63
- jaredhanson/passport-http#78

The new implemementation complies with
https://tools.ietf.org/html/rfc2617#section-2.
MatthiasKunnen added a commit to MatthiasKunnen/passport-http-2 that referenced this issue Dec 20, 2018
Previous implementation:
empty user-pass  -> error basic realm
no " " separator -> error 400
no : separator   -> error basic realm
empty password   -> error basic realm
empty username   -> error basic realm

New implementation:
empty user-pass  -> error 400
no " " separator -> error 400
no : separator   -> error 400
empty password   -> success
empty username   -> success

Also fixed passwords containing ':' being truncated.

This fixes:
- jaredhanson/passport-http#20
- jaredhanson/passport-http#41
- jaredhanson/passport-http#42
- jaredhanson/passport-http#63
- jaredhanson/passport-http#78

The new implemementation complies with
https://tools.ietf.org/html/rfc2617#section-2.
AaronDewes pushed a commit to AaronDewes/modern-passport-http that referenced this issue Feb 6, 2021
This is mostly the following change:

Corrected handling of HTTP Basic edge cases

Previous implementation:
empty user-pass  -> error basic realm
no " " separator -> error 400
no : separator   -> error basic realm
empty password   -> error basic realm
empty username   -> error basic realm

New implementation:
empty user-pass  -> error 400
no " " separator -> error 400
no : separator   -> error 400
empty password   -> success
empty username   -> success

Also fixed passwords containing ':' being truncated.

This fixes:
- jaredhanson#20
- jaredhanson#41
- jaredhanson#42
- jaredhanson#63
- jaredhanson#78

The new implemementation complies with
https://tools.ietf.org/html/rfc2617#section-2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants