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

Bad cookie value even with clearInvalid true #34

Closed
jdarling opened this issue Nov 8, 2014 · 12 comments
Closed

Bad cookie value even with clearInvalid true #34

jdarling opened this issue Nov 8, 2014 · 12 comments
Assignees
Labels
bug
Milestone

Comments

@jdarling
Copy link

@jdarling jdarling commented Nov 8, 2014

Trying to use hapi-auth-cookie with a custom cookie name, validationFunc, and random password on each app restart seems to lead to "Bad cookie value" response anytime the app restarts. I thought clearInvalid would automatically clear the cookie, but it doesn't appear to.

Dirty sample below that shows basically what I'm doing without all the login/logout/etc... for clarity.

server.pack.register(Cookie, function (err){
  server.auth.strategy('auth', 'cookie', {
    password: config.password||uuid(),
    cookie: 'myapp',
    redirectTo: false,
    validateFunc: validate,
    isSecure: config.isSecure||false,
    clearInvalid: true
  });

  server.route([
    {
      method: 'GET',
      path: 'api/v1/test/auth',
      handler: function(req, reply){
        reply('Ok :)');
      },
      auth: 'auth'
    }
  ]);
});

Any pointers on where I'm going wrong?

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Nov 12, 2014

Is the password the only thing that's changing between restarts?

@jdarling

This comment has been minimized.

Copy link
Author

@jdarling jdarling commented Nov 12, 2014

Only thing that changes is the password.

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Nov 12, 2014

What's the exact behavior?

This is how it should work as you an see from the tests:

  1. bring up app first time
  2. user logs in and gets a cookie
  3. bring up app second time with new password
  4. user logs in with old cookie and gets error along with cookie removed
  5. user logs in again and gets new cookie
@hueniverse hueniverse added the question label Nov 12, 2014
@hueniverse hueniverse self-assigned this Nov 12, 2014
@jdarling

This comment has been minimized.

Copy link
Author

@jdarling jdarling commented Nov 14, 2014

Sorry it took me so long to get back to this. What happens when I try it is:

  1. Start app for first time
  2. Log in and get cookie
  3. Restart app, gets a new password
  4. Refresh page, see "Bad cookie value" response.
  5. Open developer tools and see that the cookie is still there.
  6. Refresh page, go to step 4 :(

In the end, the cookie never gets removed so the user never gets a new cookie.

This is the easiest example:

var Hapi = require('hapi');

var users = {
    john: {
        id: 'john',
        password: 'password',
        name: 'John Doe'
    }
};

var home = function (request, reply) {

    reply('<html><head><title>Login page</title></head><body><h3>Welcome '
      + request.auth.credentials.name
      + '!</h3><br/><form method="get" action="/logout">'
      + '<input type="submit" value="Logout">'
      + '</form></body></html>');
};

var login = function (request, reply) {

    if (request.auth.isAuthenticated) {
        return reply.redirect('/');
    }

    var message = '';
    var account = null;

    if (request.method === 'post') {

        if (!request.payload.username ||
            !request.payload.password) {

            message = 'Missing username or password';
        }
        else {
            account = users[request.payload.username];
            if (!account ||
                account.password !== request.payload.password) {

                message = 'Invalid username or password';
            }
        }
    }

    if (request.method === 'get' ||
        message) {

        return reply('<html><head><title>Login page</title></head><body>'
            + (message ? '<h3>' + message + '</h3><br/>' : '')
            + '<form method="post" action="/login">'
            + 'Username: <input type="text" name="username"><br>'
            + 'Password: <input type="password" name="password"><br/>'
            + '<input type="submit" value="Login"></form></body></html>');
    }

    request.auth.session.set(account);
    return reply.redirect('/');
};

var logout = function (request, reply) {

    request.auth.session.clear();
    return reply.redirect('/');
};

var server = new Hapi.Server('localhost', 8000);

var uuid = function(){
  var rand = function(count){
    var out = '', i=0;
    for (; i<count; i++) {
      out += (((1+Math.random())*0x10000)|0).toString(16).substring(1);
    }
    return out;
  }
  return rand(2)+'-'+rand(1)+'-'+rand(1)+'-'+rand(1)+'-'+rand(3);
};

server.pack.register(require('hapi-auth-cookie'), function (err) {

    server.auth.strategy('session', 'cookie', {
        password: uuid(),
        cookie: 'sid-example',
        redirectTo: '/login',
        isSecure: false
    });

    server.route([
        {
            method: 'GET',
            path: '/',
            config: {
                handler: home,
                auth: 'session'
            }
        },
        {
            method: ['GET', 'POST'],
            path: '/login',
            config: {
                handler: login,
                auth: {
                    mode: 'try',
                    strategy: 'session'
                },
                plugins: {
                    'hapi-auth-cookie': {
                        redirectTo: false
                    }
                }
            }
        },
        {
            method: 'GET',
            path: '/logout',
            config: {
                handler: logout,
                auth: 'session'
            }
        }
    ]);

    server.start();
});

Start it, log in. Refresh the page see that it works. Restart the server. Refresh the page. Get error no matter what:

{
statusCode: 400,
error: "Bad Request",
message: "Bad cookie value: sid-example"
}
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Nov 20, 2014

Your example doesn't actually set clearInvalid...

@jdarling

This comment has been minimized.

Copy link
Author

@jdarling jdarling commented Nov 20, 2014

Sorry, minor typo when I was copying from the doc's page to here, should have been:

    server.auth.strategy('session', 'cookie', {
        password: uuid(),
        cookie: 'sid-example',
        redirectTo: '/login',
        isSecure: false,
        clearInvalid: true
    });

Following the same pattern get the same results. Cookie never cleared.

I plan on looking into this but it isn't actually causing me any harm at this point.

@jdarling

This comment has been minimized.

Copy link
Author

@jdarling jdarling commented Nov 20, 2014

Actually placing some logging messages in scheme.authenticate it never gets called with the invalid cookie. So I'm guessing before scheme.authenticate gets called something internal to Hapi is trying to decode the cookie, seeing it as invalid and returning the error:

{
statusCode: 400,
error: "Bad Request",
message: "Bad cookie value: sid-example"
}

Found it, in lib/index.js on line 34 you copy the configuration object:

    var cookieOptions = {
        encoding: 'iron',
        password: settings.password,
        isSecure: settings.isSecure !== false,                  // Defaults to true
        path: '/',
        isHttpOnly: settings.isHttpOnly !== false               // Defaults to true
    };

but never copy the clearInvalid flag. Changing the above code to:

    var cookieOptions = {
        encoding: 'iron',
        password: settings.password,
        isSecure: settings.isSecure !== false,                  // Defaults to true
        path: '/',
        isHttpOnly: settings.isHttpOnly !== false,               // Defaults to true
        clearInvalid: settings.clearInvalid
    };

Resolves the issue. I'll put together a PR and test case tomorrow if I have some time.

@hueniverse hueniverse added bug and removed question labels Nov 20, 2014
@jdarling

This comment has been minimized.

Copy link
Author

@jdarling jdarling commented Nov 20, 2014

PR #37 37 fixes this

@jdarling

This comment has been minimized.

Copy link
Author

@jdarling jdarling commented Nov 20, 2014

I really need to learn to use Github better :(

hueniverse added a commit that referenced this issue Nov 20, 2014
Fix clearInvalid (for issue #34)
@hueniverse hueniverse closed this Nov 20, 2014
@hueniverse hueniverse added this to the 1.4.1 milestone Nov 20, 2014
@bsiddiqui

This comment has been minimized.

Copy link

@bsiddiqui bsiddiqui commented Nov 25, 2014

@hueniverse if the auth mode is try why do you still get the Bad Cookie Value error? Docs state that with "'try' invalid authentication is accepted, and the user will still reach the route handler," but I've found that the server responds with the error still.

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Nov 26, 2014

@bsiddiqui while this can go either way, I agree with you that the better response is a 401 not 400. This is fixed in #40

@bsiddiqui

This comment has been minimized.

Copy link

@bsiddiqui bsiddiqui commented Nov 26, 2014

@hueniverse awesome

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