rkusa / koa-passport

Passport middleware for Koa
MIT License
774 stars 55 forks source link

req.flash is not a function #35

Closed jpetitcolas closed 8 years ago

jpetitcolas commented 9 years ago

When I configure my login route as following:

router.post('/login', passport.authenticate('local', {
    successRedirect: '/user/logout',
    failureRedirect: '/user/login?wrong',
    failureFlash: true
}));

I got a req.flash is not a function error. And indeed, req.flash is undefined.

PS: don't take care about URL, it's just for testing purposes. ;)

rkusa commented 9 years ago

Koa has no build-in flash functionality. You have to add it using a module or simply something like:

app.use(function* (next) {
  this.flash = function(type, msg) {
    this.session.flash = { type: type, message: msg }
  }

  yield next
})
cymen commented 7 years ago

I adapted the above to Koa 2:

app.use(function(ctx, next) {
  ctx.flash = function(type, msg) {
    ctx.session.flash = { type: type, message: msg };
  }

  return next();
});
elquimista commented 7 years ago

another alternative is to use koa-connect-flash

DJviolin commented 7 years ago

@cymen Thank You for your solution! I upgraded a little bit, because when passport throwing a success or error, the flash message doesn't disappear when you refresh the site, so somehow we should delete ctx.session.flash object on site refresh. My solution is to place this code under the passport initializations:

// authentication
require('./include/auth');
app.use(passport.initialize());
app.use(passport.session());

// clear flash after if it was actually set (so on the next request)
app.use(async (ctx, next) => {
  /*if (ctx.session.flash !== undefined) {
    ctx.session.flash = undefined;
  }*/
  ctx.session.flash = this || undefined; // shorthand for if
  await next();
});

This is used together with your code.

cymen commented 7 years ago

@DJviolin Ah! That is a great idea -- I was clearing it in other places but I'm going to do the same as you.

Have you thought about moving flash to cookies? I want to enable caching on some of my public pages however I can't right now due to flash. But if the flash is pushed to cookies and JavaScript (my requirements make that okay)...

DJviolin commented 7 years ago

So because on server restart, the sessions are lost (because stored in memory) and maybe better to separate the flash logic from ctx.session?

I found this implementation regarding this, but I still analyzing it and trying to understand how it's working:

https://github.com/danneu/koa-skeleton/blob/92be44ce6c87ba6a40443d584d4744743a43ec68/src/middleware.js#L35

Also, this boilerplate using database storing for sessions, so at server restart, user's not will be logged out like with Passport's local store (server memory) and you can log the logged in/out times, ip address, user_agent, etc into a database table, which is nice. More security many way when you can know who/what/when does on your site. One thing to note that this implementation is broken with passport.js, because passport is tied to ctx.session.flash. But the upper boilerplate completely ditching any session or authentication module, pure native solution. Right now I struggle a lot to move my sessions into postgres, but I don't want to use Redis (which has examples for passport.js), because thinking in the long term, memory can be expensive.

cymen commented 7 years ago

For the sessions, I'm storing in redis with koa-redis. This is working well for me -- can gracefully restart cluster in production without downtime.

Oh, I read more -- so far, my sessions are small and not super big usage so no problems with memory.

DJviolin commented 7 years ago

I managed to separate flash messages into cookies and exposing under ctx.flash. I used the example boilerplate from my previous post. Because passport.js targeting ctx.session.flash by default, you have to write your own authentication redirects in your router:

// Passport.js custom callback
// Official implementation is here:
// http://passportjs.org/docs#custom-callback
router.post('/auth', async (ctx, next) => {
  await passport.authenticate('local', (err, user, info) => {
    //console.log(`${err}\n${JSON.stringify(user, null, 4)}\n${info}`);
    if (err) { return next(err); }
    if (!user) {
      ctx.flash = {
        type: 'error',
        message: 'Login error!',
      };
      return ctx.redirect('/login');
    }
    ctx.login(user, () => {
      if (err) { return next(err); }
      ctx.flash = {
        type: 'success',
        message: 'Login was succesful!',
      };
      return ctx.redirect('/admin');
    });
  })(ctx, next);
});

This stores a json cookie in a similar object what passport creating by default. The cookies is always deleted after 10 seconds or on the next request. 10 second is defined in the linked implementation, meaning if a request takes longer, than the user can miss the flash, but you can define longer period.