krakenjs / kraken-example-with-passport

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

Bypass secure routes #23

Open rodmoreno opened 8 years ago

rodmoreno commented 8 years ago

In this example using the 'auth' library routes that require authentication are specified, in this case they: /admin & /profile, but if I have the role of 'user' and there the route: /admin/whatever I can get on that route.

Sorry for my english

grawk commented 8 years ago

Hi @rodmoreno. Thanks for taking the time to file this issue. I'll get back to you once I'm able to begin looking into it.

quarterpi commented 7 years ago

Hi, I have been dealing with this bug myself and the only solution that I have found to date is to specify every possible route to be blocked. Personally, I don't think that specifying each route is the best solution, especially when you generating pages based on some id from a database (for example, '/admin/profiles/5bd2c68c9b3062a301790e3'). I have been trying to get something working with regex patterns, with no luck (this regex : /\/admin.*/ matches all /admin routes, but does not work in this context). Perhaps someone could shed a little more light on how the 'auth' object gets used? It does not seem to follow the express routing pattern. Any insights would be appreciated.

quarterpi commented 7 years ago

Forgive my ignorance, but I can't find a reference that explains this line of code if (!auth[route]). To be more specific, the !auth[route] part has me scratching my head. Can someone please point me to where I can learn the syntax rules for this operation? (I have been able to gather from running this code in the debugger that it is essentially comparing the strings stored in auth against route, and returning either true or false based on the presence or absence of a match (true if no match is found, otherwise false.). I would like to understand this line of code better as I have never encountered it before.) Thanks in advance.

grawk commented 7 years ago

@quarterpi It's a logic switch and you've defined the behavior perfectly. There is nothing extraordinary about this pattern.

grawk commented 7 years ago

The existing whitelist/blacklist approach to auth in this app is very rudimentary and doesn't really scale to the examples mentioned by you. You might instead go with a regex pattern in the middleware configuration itself. Please see: https://github.com/krakenjs/kraken-js/wiki/Using-krakenjs-middleware-config-for-whitelisting-and-blacklisting-routes#whitelisting and https://github.com/krakenjs/kraken-js/wiki/Using-krakenjs-middleware-config-for-whitelisting-and-blacklisting-routes#blacklisting

Someone can file a PR to upgrade from the naive approach currently taken. It is not a high priority for the maintaining team.

quarterpi commented 7 years ago

To correct myself and answer my own question, the line of code that reads !auth[route] compares all of the keys in auth with the route requested. That also explains why my regex wasn't working, since this method does not use the usual express regex routing pattern and does a string string comparison. I am currently working on a solution. I will post it if I find one that I feel is suitable.

grawk commented 7 years ago

We'd be grateful if you either post your solution or open a PR to bring it into this example!

quarterpi commented 7 years ago

@grawk thanks for your response. I finally realized the obvious. Thanks for the links you provided.

quarterpi commented 7 years ago

@grawk I will if I get something satisfactory.