search

ForbesLindesay / connect-roles

Provides dynamic roles based authorisation for node.js connect and express servers.
749 stars 62 forks source link

replace req.path with req.url to be mountpoint agnostic #32

Open ArdentZeal opened 10 years ago

ArdentZeal commented 10 years ago

Hello,

with the new routers of express 4, I redid my app layout andended up having several mount points.

e.g.

self.express.designer.use('/api/v1', self.express.api);

These mountpoints get stripped in the req.url attribute, so everything below is agnostic to it.

e.g.

req.url: /users/7/apps/12/versions/12/hasAppInstance req.path: /api/v1/users/7/apps/12/versions/12/hasAppInstance

Your module though is using req.path instead of req.url ind use3. I "fixed" it locally and I could also provide a pull request, although the change is really pretty minor.

I just wanted to know if I am missing sth when implementing this proposed bugfix.

Kind Regards

ForbesLindesay commented 10 years ago

The issue is that I often define connect-roles at the top level of my application, and then use them in many different sub apps at different mount points. e.g.

roles.use('access page', '/project/:id/:page', function (req) {
  return req.user.projects.contains(req.params.id) &&
         req.user.projectPages.contains(req.params.page);
});

roles.use('access page', '/settings/:page', function (req) {
  return req.user.settingsPages.contains(req.params.page);
});

app.use('/project', require('./apps/project'));
app.use('/settings', require('./apps/settings'));

We could make it an option. Something like useRelativePaths (feel free to bike-shed here).

dskrvk commented 7 years ago

Implemented in https://github.com/ForbesLindesay/connect-roles/commit/dc250ece48b0dc97491f1d22e8bce0c5991ed98a ?