stormpath / express-stormpath

Build simple, secure web applications with Stormpath and Express!
http://docs.stormpath.com/nodejs/express/
Apache License 2.0
325 stars 111 forks source link

groupsRequired Middleware iterator not firing #625

Open mrosenberg opened 7 years ago

mrosenberg commented 7 years ago

lib/middleware/groups-required.js

        userGroups.each(
          function (group, iterateNext) {
            if (groups.indexOf(group.name) > -1) {
              if (!all || --done === 0) {
                isInGroup = true;
              }
            }
            iterateNext();
          },
          function () {
            callback(null, isInGroup);
          }
        );

The first function never fires, it looks like the underlying collection's items property is empty even though the groups are expanded on the user object.

nbarbettini commented 7 years ago

I've reproduced this and definitely think it's a bug.

Steps:

router.get('/profile', stormpath.groupsRequired(['Everyone'], false), function(req, res) {
  res.render('profile');
});

Looks like it's because of this code: https://github.com/stormpath/stormpath-sdk-node/blob/okta/lib/resource/Group.js#L129-L134 This skips any groups that don't start with group: which means any custom groups created after the import can't be used with groupsRequired middleware.

gvdhorst commented 7 years ago

express-sprtmpath skipping groups that don't start with group: indeed seems to be the problem.

As a workaround you can prefixing all group names with group: (in both stormpath.groupsRequired() and the Okta control panel).

mrosenberg commented 7 years ago

I wrote a custom middleware to handle this use case for us so I'm fine with closing this. I'll revisit this once the Okta Node SDK is stable.