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

Method "Remove" from Authentication Service gives Internal Server Error when using JWT Authentication with Cookies. #606

Closed
uriannrima opened this Issue Nov 17, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@uriannrima
Copy link

uriannrima commented Nov 17, 2017

Steps to reproduce

  • Generate new application
  • Generate authentication service (can be InMemory)
  • Configure to use JWT with Cookie:
    => Setup cookie in default.json, Enabled, HTTP Only, no HTTPs
    => Setup cookie-parser
    => Setup JWT CookieExtractor (like this one, but change to req.cookies[config.cookie.name])
  • Login should work normally
  • Requests should work normally.
  • Try to call "Remove" from Authentication Service (delete to /authentication)

Expected behavior

Cookie should be removed from client, and return an success result.

Actual behavior

Cookie IS REMOVED from client, but returns an Internal Server Error:

Server Error:
info: error: authentication - Method: remove: token must provided
error: Error: token must provided

System configuration

Tell us about the applicable parts of your setup.

NodeJS version: 8.9.1

Operating System: Windows

Browser Version: Chrome / Postman

Module Loader: Webpack

Some Insight

After some research and debugging, pin pointed the problem to service.js from Feathers-Authentication (inside lib folder), and found out this piece of code:

{
    key: 'remove',
    value: function remove(id, params) {
      var defaults = this.app.get('authentication') || this.app.get('auth');
      var authHeader = params.headers && params.headers[defaults.header.toLowerCase()];
      var authParams = authHeader && authHeader.match(/(\S+)\s+(\S+)/);
      var accessToken = id !== null ? id : authParams && authParams[2] || authHeader;

      // TODO (EK): return error if token is missing?
      return this.passport.verifyJWT(accessToken, (0, _lodash2.default)(defaults, params)).then(function (payload) {
        return { accessToken: accessToken };
      });
    }
  }

The problem seems to occur because it tries to find the accessToken on request header 'Authorization', but since I'm using cookies, it can't find and gives the error. After making a small tweak, the problem goes away, but I don't know if it is the right fix:

{
    key: 'remove',
    value: function remove(id, params) {
      var defaults = this.app.get('authentication') || this.app.get('auth');
      var authHeader = params.headers && params.headers[defaults.header.toLowerCase()];
      var authParams = authHeader && authHeader.match(/(\S+)\s+(\S+)/);
      var accessToken = id !== null ? id : authParams && authParams[2] || authHeader;
      // Get accessToken from cookies instead of header if not found.
      accessToken = accessToken ? accessToken : params.cookies && defaults.cookie ? params.cookies[defaults.cookie.name] : undefined;

      // TODO (EK): return error if token is missing?
      return this.passport.verifyJWT(accessToken, (0, _lodash2.default)(defaults, params)).then(function (payload) {
        return { accessToken: accessToken };
      });
    }
  }
@uriannrima

This comment has been minimized.

Copy link
Author

uriannrima commented Nov 17, 2017

Well, found another (and at the moment better) solution. I created a hook to run before the remove event from Authentication:

const extractAuthHeaderFromCookies = function (hook) {
    let { app, params } = hook;
    let defaults = app.get('authentication') || app.get('auth');
    let authHeader = params.headers && params.headers[defaults.header.toLowerCase()];
    if (!authHeader && params.cookies && defaults.cookie.name) {
      params.headers[defaults.header.toLowerCase()] = params.cookies[defaults.cookie.name];
    }
  };

Then register it:

app.service('authentication').hooks({
    before: {
      create: [
        authentication.hooks.authenticate(config.strategies)
      ],
      remove: [
        authentication.hooks.authenticate('jwt'),
        extractAuthHeaderFromCookies
      ]
    }
  });

It checks if there isn't a authHeader, and if cookies seems enabled, if it is, extract it and set authHeader. Error is gone, and I can commit it in my project while I don't know if I'm doing something wrong. (:

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Nov 18, 2017

The cookie - although possible - is not really intended as an authentication strategy to avoid CSRF issues. Your solution works. Another way would be to use the cookieParser and exposeCookies Express middleware.

@daffl daffl closed this Nov 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.