Skip to content

Commit

Permalink
fix: inability to access /admin if not superadmin
Browse files Browse the repository at this point in the history
There was an odd issue where non-superadmins could not use
the /admin route to access the ACP, even though they had
appropriate access. For whatever reason, it could not
be reliably reproduced on my dev. As it turns out, the
reason was because I was checking the wrong privilege,
and my dev database had this wrong privilege leftover
from the initial development of the ACP admin privileges
feature. Dumb.

Anyhow, that fixes this issue.
  • Loading branch information
julianlam committed Aug 21, 2020
1 parent 29e3ab2 commit 03bd76d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 5 deletions.
14 changes: 11 additions & 3 deletions src/middleware/admin.js
Expand Up @@ -117,9 +117,17 @@ module.exports = function (middleware) {

// Otherwise, check for privilege based on page (if not in mapping, deny access)
const path = req.path.replace(/^(\/api)?\/admin\/?/g, '');
const privilege = privileges.admin.resolve(path);
if (!privilege || !await privileges.admin.can(privilege, req.uid)) {
return controllers.helpers.notAllowed(req, res);
if (path) {
const privilege = privileges.admin.resolve(path);
if (!privilege || !await privileges.admin.can(privilege, req.uid)) {
return controllers.helpers.notAllowed(req, res);
}
} else {
// If accessing /admin, check for any valid admin privs
const privilegeSet = await privileges.admin.get(req.uid);
if (!Object.values(privilegeSet).some(Boolean)) {
return controllers.helpers.notAllowed(req, res);
}
}

return next();
Expand Down
2 changes: 0 additions & 2 deletions src/privileges/admin.js
Expand Up @@ -94,8 +94,6 @@ module.exports = function (privileges) {
privileges.admin.resolve = (path) => {
if (privileges.admin.routeMap[path]) {
return privileges.admin.routeMap[path];
} else if (path === '') {
return 'manage:dashboard';

This comment has been minimized.

Copy link
@julianlam

julianlam Aug 21, 2020

Author Member

manage:dashboard was renamed to admin:dashboard, but this check remained, and of course, my dev db had this value set to true. 🤦

}

let privilege;
Expand Down

0 comments on commit 03bd76d

Please sign in to comment.