jshttp / basic-auth

Generic basic auth Authorization header field parser
MIT License
702 stars 86 forks source link

Does not work when missing colon #33

Closed edgracilla closed 7 years ago

edgracilla commented 7 years ago

it returns undefined

dougwilson commented 7 years ago

Hi @edgracilla I just tried running the EXAMPLE in the README over HTTPS on Google Chrome 59 and it worked just fine. Can you provide more information on how to reproduce your issue? If you're not sure where to start:

  1. The version of this module.
  2. The version of Node.js
  3. The web browser you're using
  4. If your web browser is indicating any kind of SSL certificate error
  5. Full source code showing the issue (ideally just use the example from the README).
  6. If you're not using the README example, provide instructions for how to reproduce.

Thanks!

edgracilla commented 7 years ago

Our app requires to capture 3 variations in basic auth (no user, user only, user and pass). Your regex fail to capture user only and returns undefined.

var userPassRegExp = /^([^:]*):(.*)$/

user:pass@localhost // Ok
user@localhost // Fail
dougwilson commented 7 years ago

Hi @edgracilla all three of those are required to have the : in the content, as per the specification for basic auth (RFC 2617, since June 1999). You can even see that the web browsers follow this, for example here is what Chrome does when I enter only a username using the README example:

1) Run the following app:

$ node -e 'require("http").createServer(function(req,res){console.dir(req.headers);res.writeHead(401,{"WWW-Authenticate":"Basic realm=\"example\""});res.end()}).listen(3000)'

Then navigate to http://localhost:3000 and enter just a username.

screen shot 2017-07-14 at 10 07 12

Then go back to the console to see the headers the web browser sent:

{ host: 'localhost:3000',
  connection: 'keep-alive',
  'cache-control': 'max-age=0',
  authorization: 'Basic dXNlckBsb2NhbGhvc3Q6',
  'upgrade-insecure-requests': '1',
  'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36',
  accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8',
  'accept-encoding': 'gzip, deflate, br',
  'accept-language': 'en-US,en;q=0.8' }

You can see the header sent by Chrome: Basic dXNlckBsb2NhbGhvc3Q6

If you un-base64-encode the basic header, you see the colon at the end:

$ node -pe 'new Buffer("dXNlckBsb2NhbGhvc3Q6","base64").toString()'
user@localhost:

What web browser re you using that is violating the basic auth specification?

edgracilla commented 7 years ago

We are not using any browser, our users (which is developers too) will utilize the https module and will do the https.request(). In this case, appending the ':' to comply with the RFC might be an issue because not all of them are aware of the standard.

I already solved my issue using below basic POC codes to handle non-RFC compliant.

let decodeb64 = (str) => {
    if (!str) return

    let arr = str.split(' ')

    if (arr.length === 2) {
      str = Buffer.from(arr[1], 'base64').toString()
      arr = str = str.split(':')

      if (arr.length === 1) {
        return {name: arr[0]}
      } else if (arr.length === 2) {
        return {name: arr[0], pass: arr[1]}
      }
    }
  }

If your regex is RFC compliant then that would be fine and you may close this issue. But in the wild if newbie developer will do the basic auth by generating base64 manually and they are not aware of the RFC, no colons will be appended if it is user only.

{
  'Authorization': `Basic ${new Buffer(conf.username).toString('base64')}`
}
dougwilson commented 7 years ago

Gotcha, I will close the issue, then. It sounds like the bug is in the client not this module. Standards exist as a point of agreement made between parties. This module complies with the standard, just the same as all software. If people are building a new implementation, that's great! Just build them to the standard the industry has been in agreement on for almost 20 years :)

edgracilla commented 7 years ago

If people are building a new implementation, that's great!

Yup, standard are standards. But not all devs are aware of every details of it. In my case, I just need to handle the fault of people who will not follow the standard. I just don't want to be bombarded with email/issue report that it is my apps fault. Saving time and energy by simply supporting the missing/forgotten colon 😁