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.

Sign up for GitHub

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

Jump to bottom

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.
This was referenced Jan 21, 2014
Closed
Open
@OliverJAsh OliverJAsh mentioned this issue Jan 25, 2014
Closed
@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 FlorianSW mentioned this issue Jun 12, 2017
Closed
@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
@FlorianSW
Basic: Fix auth when password contains colon
202931f 
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
Open
@saborrie
Copy link

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

@rudism rudism mentioned this issue Dec 4, 2018
Open
5 tasks
MatthiasKunnen added a commit to MatthiasKunnen/passport-http-2 that referenced this issue Dec 20, 2018
@MatthiasKunnen
Corrected handling of HTTP Basic edge cases
4cc025a 
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
@MatthiasKunnen
Corrected handling of HTTP Basic edge cases
da58b3a 
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.
@pofider pofider mentioned this issue Jan 29, 2020
Open
@ebaynaud ebaynaud mentioned this issue Feb 26, 2020
Open
AaronDewes pushed a commit to AaronDewes/modern-passport-http that referenced this issue Feb 6, 2021
@MatthiasKunnen @AaronDewes
Changes from http-passport-2
76779d6 
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.
@lukechilds lukechilds mentioned this issue Feb 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@blevine @OliverJAsh @saborrie @FlorianSW