Skip to content
This repository was archived by the owner on Mar 22, 2022. It is now read-only.

Server-side header option does not accept capital letters #218

Closed
mmwtsn opened this issue Jun 3, 2016 · 4 comments
Closed

Server-side header option does not accept capital letters #218

mmwtsn opened this issue Jun 3, 2016 · 4 comments

Comments

@mmwtsn
Copy link
Contributor

mmwtsn commented Jun 3, 2016

Issue

RESTful authentication fails if the optional header key in the server-side configuration options includes capital letters.

Background

I brought this up in the Slack channel yesterday and @ekryski asked that I open an issue with steps to reproduce the problem. This happens because internally Node's http module lower cases all headers names to normalize their values. If a user attempts to configure their authentication options with a header that includes capital letters lookup inside src/middleware/express.js fails.

For example, this configuration:

// Application setup
.configure(authentication({ header: 'X-Auth-Token' })

Causes the following situation internally wen a POST request is made with a X-Auth-Token header:

// Express middleware
console.log(options.header);              // => 'X-Auth-Token'
console.log(req.headers['x-auth-token']); // => token
console.log(req.headers['X-Auth-Token']); // => undefined

Steps to reproduce

  1. Clone the repository locally and cd into the project root.

  2. Open example/app.js in your text editor.

  3. Add the optional header value with inside example/app.js:

    diff --git a/example/app.js b/example/app.js
    index 444ad12..9c52a6b 100644
    --- a/example/app.js
    +++ b/example/app.js
    @@ -17,7 +17,8 @@ var app = feathers()
    .use(bodyParser.urlencoded({ extended: true }))
    // Configure feathers-authentication
    .configure(authentication({
    -    idField: 'id'
    +    idField: 'id',
    +    header: 'X-Auth-Token'
    }))
    // Initialize a user service
    .use('/users', memory())
  4. Set up the example server:

    $ npm install
    $ node example/app.js
    Feathers authentication app started on 127.0.0.1:3030
  5. In a new terminal session, create a new token using the default user:

    $ curl -X POST \
    -H 'Content-Type: application/json' \
    -d '{ "email": "admin@feathersjs.com", "password": "admin" }' \
    http://127.0.0.1:3030/auth/local
  6. Use the token to view a restricted resource:

    $ curl -X GET \
    -H 'X-Auth-Token:$YOUR_TOKEN_FROM_ABOVE' \
    http://127.0.0.1:3030/messages

    Despite using the header I configured the app to use this request fails for me.

@mmwtsn
Copy link
Contributor Author

mmwtsn commented Jun 3, 2016

This is a little bit confusing right now because:

  • the server-side config uses a lowercase default (authorization)
  • the client-side config uses an uppercase default (Authorization)

As Eric pointed out in Slack the Node default is lowercase header values. I think it makes sense to move forward with and align both the server- and client-side defaults to that standard.

@mmwtsn mmwtsn mentioned this issue Jun 3, 2016
2 tasks
@marshallswain
Copy link
Member

Stellar issue report, BTW. I think it makes sense to lowercase everywhere.

@mmwtsn
Copy link
Contributor Author

mmwtsn commented Jun 3, 2016

Sweet.

Do you mind if I push it up to #219 as well?

Here's the diff:

diff --git a/src/client/hooks.js b/src/client/hooks.js
index 35a4551..ab0b7ad 100644
--- a/src/client/hooks.js
+++ b/src/client/hooks.js
@@ -13,7 +13,7 @@ export function populateHeader(options = {}) {
   return function(hook) {
     if (hook.params.token) {
       hook.params.headers = Object.assign({}, {
-        [options.header || 'Authorization']: hook.params.token
+        [options.header || 'authorization']: hook.params.token
       }, hook.params.headers);
     }
   };
diff --git a/test/integration/rest.test.js b/test/integration/rest.test.js
index b59a7f6..2ddd748 100644
--- a/test/integration/rest.test.js
+++ b/test/integration/rest.test.js
@@ -259,7 +259,7 @@ describe('REST authentication', function() {
             method: 'GET',
             json: true,
             headers: {
-              Authorization: validToken
+              authorization: validToken
             }
           };
         });

@ekryski
Copy link
Member

ekryski commented Jun 4, 2016

@mmwtsn Yeah man! Thanks for the stellar issue!

Yeah I think that's just fine to push it up.

@daffl daffl closed this as completed in #219 Jun 9, 2016
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants