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

`appendNext` no longer accepts a string #67

Closed
rockbot opened this issue Apr 30, 2015 · 3 comments · Fixed by #68
Closed

`appendNext` no longer accepts a string #67

rockbot opened this issue Apr 30, 2015 · 3 comments · Fixed by #68
Assignees
Labels
Milestone

Comments

@rockbot
Copy link

@rockbot rockbot commented Apr 30, 2015

Received this error:

/Users/rockbot/npm-inc/newww/node_modules/hoek/lib/index.js:669
        throw arguments[1];
                       ^
ValidationError: child "appendNext" fails because ["appendNext" must be a boolean]
    at Object.exports.process (/Users/rockbot/npm-inc/newww/node_modules/hapi-auth-cookie/node_modules/joi/lib/errors.js:140:17)
    at internals.Any._validateWithOptions (/Users/rockbot/npm-inc/newww/node_modules/hapi-auth-cookie/node_modules/joi/lib/any.js:642:25)
    at root.validate (/Users/rockbot/npm-inc/newww/node_modules/hapi-auth-cookie/node_modules/joi/lib/index.js:102:23)
    at Object.internals.implementation [as cookie] (/Users/rockbot/npm-inc/newww/node_modules/hapi-auth-cookie/lib/index.js:41:23)
    at internals.Auth.strategy (/Users/rockbot/npm-inc/newww/node_modules/hapi/lib/auth.js:47:41)
    at internals.Plugin._applyChild (/Users/rockbot/npm-inc/newww/node_modules/hapi/lib/plugin.js:449:19)
    at Object.auth.strategy (/Users/rockbot/npm-inc/newww/node_modules/hapi/lib/plugin.js:61:65)
    at /Users/rockbot/npm-inc/newww/server.js:90:15
    at done (/Users/rockbot/npm-inc/newww/node_modules/hapi/node_modules/items/lib/index.js:30:25)
    at Object.exports.register (/Users/rockbot/npm-inc/newww/node_modules/hapi-auth-cookie/lib/index.js:15:5)
    at /Users/rockbot/npm-inc/newww/node_modules/hapi/lib/plugin.js:242:14
    at iterate (/Users/rockbot/npm-inc/newww/node_modules/hapi/node_modules/items/lib/index.js:35:13)
    at Object.exports.serial (/Users/rockbot/npm-inc/newww/node_modules/hapi/node_modules/items/lib/index.js:38:9)
    at internals.Plugin.register (/Users/rockbot/npm-inc/newww/node_modules/hapi/lib/plugin.js:224:11)
    at Object.<anonymous> (/Users/rockbot/npm-inc/newww/server.js:80:8)
    at Module._compile (module.js:456:26)

Here's the code that we've written (worked fine with version 2.0.0):

  server.auth.strategy('session', 'cookie', 'required', {
    password: process.env.SESSION_PASSWORD,
    appendNext: 'done',
    redirectTo: '/login',
    cookie: process.env.SESSION_COOKIE,
    clearInvalid: true,
    validateFunc: function (session, cb) {
      cache.get(session.sid, function (err, item, cached) {

        if (err) {
          return cb(err, false);
        }
        if (!cached) {
          return cb(null, false);
        }

        return cb(null, true, item);
      });
    }
  });

Sidenote: if this was a purposeful, breaking change, the community would expect the version to be bumped to 3.0.0, not 2.1.0.

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented May 1, 2015

Reading the readme of the 2.0.0, it already expected a boolean. I think 2.1.0 is more strict about its input, so what was truthy before ('done') is not enough now, I'm not sure I'd qualify this as breaking.

@jaw187 jaw187 added the support label May 4, 2015
@geek

This comment has been minimized.

Copy link
Member

@geek geek commented May 4, 2015

This looks like an unintentional breaking change. The previous readme says Set to a string to use a different parameter name This is a feature that was removed. We should likely make the validation more relaxed to support this previous feature. We also need a test for this condition :)

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented May 4, 2015

My bad, read a bit too fast, sorry.

@hueniverse hueniverse added this to the 2.2.0 milestone May 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.