h
h copied to clipboard
hypothesis
Navbar missing in admin group edit page
Steps to reproduce
- Log in to Hypothesis as an administrator
- Go to the details view for an existing group in the admin pages (at
/admin/groups/{id}, eg. https://hypothes.is/admin/groups/world)
Expected behaviour
Navbar is visible at top of page
Actual behaviour
Navbar is present but contains no links:
Additional details
The navbar rendering logic omits links if the user does not have permission to use a particular section of the admin pages. My initial guess is that the request.has_permission check is incorrectly failing for all of these links.
CC @lyzadanger - I think you know best how the permissions logic works?
My not-terribly-useful update: I see what's broke but don't have a path to fix it yet. This has to do with ACL inheritance in traversal.
For admin-area permissions control, oauthclients and organizations are able to use traversal/Roots and inherit from h.traversal.roots.Root's ACL. This is because there is no __acl__ defined on the underlying models. Pyramid will look up the tree and use the first ACL it finds. Thus with oauthclients and organizations, Pyramid won't find an ACL on the models and will fall back on the ACL defined by the Root. That's where the admin-related permissions are set.
However, GroupRoot, used here for traversal, deals in Group models, which have an ACL already defined. Because Pyramid finds that ACL first, it doesn't have any interest in looking at the ACLs of any of the parents. So Root's ACL is ignored. Put another way, I can explicitly set the GroupRoot's __parent__ to Root, as oauthclients and organizations do, but the Root ACL will be ignored because h.models.group.Group has an ACL that supersedes it.
There are a few other complicating factors here that I won't go into, but this is going to need us to put our heads together...it's related to some of the other h refactoring we've been wanting to do for a while.
/cc @seanh
Note: I'm tossing this into the engineering backlog so it doesn't get lost, but my hunch is that because this is such a minor problem and the fix is (potentially) complex, we might not get to it soon unless there's an easy way to fix this.