search

krakenjs / kraken-example-with-passport

An example integrating kraken with passport authentication
53 stars 33 forks source link

Incorrect isAuthenticated use #1

Closed grawk closed 9 years ago

grawk commented 9 years ago

This is copied from https://github.com/krakenjs/kraken-examples/issues/42 on the original combined example repository. Will fix it here and close both once fixed. Please track progress here.

Original issue description from @klall

In https://github.com/krakenjs/kraken-examples/blob/master/with.passport/controllers/index.js#L27

this line exists:

    router.get('/admin', auth.isAuthenticated('admin'), auth.injectUser(), function(req, res) {

However, parameters such as 'admin' are not really used in https://github.com/krakenjs/kraken-examples/blob/master/with.passport/lib/auth.js#L57

As well don't we already invoke isAuthenticated for each route?

https://github.com/krakenjs/kraken-examples/blob/master/with.passport/config/config.json#L89

If I change the line to:

    router.get('/admin',  function(req, res) {

the code should work as expected no?

aredridel commented 9 years ago

We should probably finish out the role based access control in the example.

grawk commented 9 years ago

The role based access control does work, but as you (@aredridel) had mentioned in the thread under the original bug, some of the refactoring was left undone. I had refactored the role checking to read from req.user.role but not removed this inline middleware call.

grawk commented 9 years ago

Resolved with https://github.com/krakenjs/kraken-example-with-passport/pull/3