express-stormpath icon indicating copy to clipboard operation
express-stormpath copied to clipboard

groupsRequired Middleware iterator not firing

Open mrosenberg opened this issue 6 years ago • 3 comments

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.

mrosenberg avatar Aug 03 '17 20:08 mrosenberg

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

Steps:

  • Cloned https://github.com/stormpath/express-stormpath-sample-project and hooked it up to my Storpmath->Okta imported applicatoin
  • Modified lib/routes.js like so:
router.get('/profile', stormpath.groupsRequired(['Everyone'], false), function(req, res) {
  res.render('profile');
});
  • Log in with a user in the Everyone group (I've tried other custom groups as well) and try to access /profile - the Unauthorized page is displayed

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.

nbarbettini avatar Aug 10 '17 19:08 nbarbettini

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).

gvdhorst avatar Aug 11 '17 09:08 gvdhorst

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.

mrosenberg avatar Aug 11 '17 11:08 mrosenberg