Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

express.static usage #4

Closed
erhangundogan opened this Issue Dec 25, 2011 · 3 comments

Comments

Projects
None yet
3 participants

Hi jared.

First of all i want to say that passport is a great auth library. It's simple yet powerful one. Keep up your good work.

Let me ask you something about passport in general. In connect/express application i'm using static route handler before your initialize/session handlers (code below) to reduce serialization overhead. Is it good or bad practise?

  app.use(express.logger());
  app.use(express.bodyParser());
  app.use(express.methodOverride());
  app.use(express.cookieParser());
  app.use("/public", express.static(__dirname + "/public")); // static route handler above others
  app.use(express.session({
    secret: "your secret",
    maxAge: new Date(Date.now() + 3600000),
    store: new mongoStore({ db: MongoDbConnection, collection: dbConfig.collection })
  }));
  app.use(passport.initialize());
  app.use(passport.session());
  app.use(app.router);
Owner

jaredhanson commented Dec 26, 2011

Erhan -

It is OK to put static middleware above Passport, and as you note it will cut down on the serialization overhead.

Shortly, I'm going to implement an option to the session middleware that allows paths to be excluded. For example:

app.use(passport.session({ exclude: '/about' }));

or, for multiple excludes:

app.use(passport.session({ exclude: ['/products', '/about'] }));

This will allow for a little more flexibility in defining the middleware chain, while also keeping serialization overhead low for routes that don't require authenticated sessions. But it sounds like this option isn't necessary for you, so feel free to continue using it as you are. I'll close this ticket once the above option is implemented.

Thanks for the kind words. I'm happy that people are using and benefiting from Passport.

@ghost ghost assigned jaredhanson Dec 26, 2011

@erhangundogan erhangundogan reopened this Dec 26, 2011

Owner

jaredhanson commented Apr 1, 2012

I've been mulling over the exclude option for a while, and have yet to find a compelling use case for it. Meanwhile, Connect and Express have started including static middleware before session middleware (you are just ahead of the curve!)

Since that seems to be the latest best practice in the community, that's my recommendation for Passport too, and I'm closing this ticket. See #14 for further discussion.

@jaredhanson jaredhanson closed this Apr 1, 2012

Dear Jared,

Excellent work on passport. The exclude function would be a great addition to allow for no serialization on some routes, such as:

app.get('/find/:word', routes.findAll);
app.get('/user/:userId', user.getUserName);

Would you please consider implementing this exclude functionality as suggested; or recommend another way of allowing for no deserialization on these two (and other) routes?

Thanks in advance,
Ruan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment