Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Express middleware and params #192

Open
dottodot opened this issue Jun 6, 2016 · 11 comments
Open

Express middleware and params #192

dottodot opened this issue Jun 6, 2016 · 11 comments

Comments

@dottodot
Copy link

dottodot commented Jun 6, 2016

I'm a bit confused as to how to get the express middleware to work with params.

The documentation suggests you can add the middleware to route with params i.e.

app.put('/blogs/:id', acl.middleware(), function(req, res, next){…}

however if I add a resource as /blogs/:id I get a insufficient permissions response.

@LukevdPalen
Copy link

LukevdPalen commented Jun 17, 2016

@dottodot Think you're right and a bit confusing because the description suggest otherwise I'm having the same issues, can't find any reference in the code for such functionality.

The code (https://github.com/OptimalBits/node_acl/blob/master/lib/acl.js#L883) will do a lookup for a bucket called allows_/blogs/1234 and won't find such bucket because it's stored as allows_/blogs/:id

req.route.path contains the pattern but when using sub routers the reference will only contain the part defined in the router.

req.baseUrl + req.route.path instead of req.originalUrl should fix it.

@gundalar
Copy link

@dottodot , were you able to get it to work. I am also facing the same issue. I don't understand the solution given by @LukevdPalen .

@dottodot
Copy link
Author

@gundalar I just created my own middleware to get round the problem.

@cookie-ag
Copy link

@dottodot Can you share the logic for handling multi-level params such as /feature/:feature/sub-feature/:sub-feature?

@dottodot
Copy link
Author

dottodot commented Sep 5, 2016

@serganus I don't actually have anything for multi-level params as I don't have any routes that have them but I think my middleware would work on all routes as I'm checking using req.route.path

function aclCheck(req, res, next) {
    let id = 0;
  if (req.user) {
        id = req.user._id;
  }
    global.acl.isAllowed(id.toString(), '/api' + req.route.path, req.method.toLowerCase(), function(err, allowed) {
        if (allowed) {
      return next();
        }
    res.status(403).json({
      error: 'You are not permitted to perform this action.'
    });
    });
}

@dimsolution
Copy link

@dottodot, @serganus An another example to make your own acl middleware ;)

import errors from 'feathers-errors'
import Acl from '../acl'

/**
 * @description Check if user has rights on url
 * @param req
 * @param res
 * @param next
 */
export default function (req, res, next) {
        // Make your own fuction to retrieve the user_id (I'm using feathers + jwt + an authentication middleware)
        const user_id = req.feathers.user ? req.feathers.user.id : 'anonymous'
        // Customize your req request to match with your acl resource (I'm using Swagger in this example)
        Acl.isAllowed(user_id, req.swagger.apiPath, req.method.toLowerCase(), (err, result) => {
            if (result) return next()           
            return next(new errors.Forbidden(`NOT_AUTHORIZED`))
        })
}

@wilk
Copy link

wilk commented Feb 16, 2017

I solved it with a custom middleware as well for Express 4:

const acl = require('../util/acl.util')

module.exports = resource => (req, res, next) => {
  acl.isAllowed(req.session.userId, resource, req.method.toLowerCase(), (err, allowed) => {
    if (err) return next(err)

    if (!allowed) return res.status(403).send('Insufficient permissions to access resource')
  
    next()
  })
}

Then used in this way:

app.use('/api/users', authMiddleware, aclMiddleware('users'), userRouter)

@saurabhh
Copy link

saurabhh commented Mar 29, 2017

@wilk :)

Hey, can you please elaborate what is authMiddleware & aclMiddleware? because i'm facing problem when i pass these two names.

@wilk
Copy link

wilk commented Mar 29, 2017

@saurabhh authMiddleware is just an authentication middleware, where the request is being authenticated, while aclMiddleware is the custom middleware explained in the example above.

@Prozi
Copy link

Prozi commented Dec 3, 2017

this is what I use for middleware function

  (req, res, next) => {
      const route = req.path.substr(1)
      const userId = req.user ? req.user.id : 'guest'
      acl.isAllowed(userId, route, req.method.toLowerCase(), (err, result) => {
        if (result) return next()
        return next(err)
      })
    }

@ChaitanyaBabar
Copy link

@dottodot Think you're right and a bit confusing because the description suggest otherwise I'm having the same issues, can't find any reference in the code for such functionality.

The code (https://github.com/OptimalBits/node_acl/blob/master/lib/acl.js#L883) will do a lookup for a bucket called allows_/blogs/1234 and won't find such bucket because it's stored as allows_/blogs/:id

~req.route.path contains the pattern but when using sub routers the reference will only contain the part defined in the router. ~

req.baseUrl + req.route.path instead of req.originalUrl should fix it.

For me addition of url = req.baseUrl + req.route.path; here i.e. https://github.com/OptimalBits/node_acl/blob/master/lib/acl.js#L682 in the acl.js solved the issue for the urls with parameters.

@LukevdPalen ,
I guess this should go into library also as it would handle urls with params.
What do you think ?

Thanks,
Chaitanya

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants