[admin-theme] menu doesn't respect assigned permissions #103

[NoumbissiValere]

Fixed issue by making sure users can access just the models
they are permitted to access.

Fixes #103
Closes #104

[coveralls]

Coverage increased (+0.004%) to 99.471% when pulling 0af49a0 on issue-103-menu-not-respect-permissions into b1488f4 on master.

[coveralls]

Coverage decreased (-0.3%) to 99.211% when pulling 971e935 on issue-103-menu-not-respect-permissions into b1488f4 on master.

[nemesifier]

this is fragile, there could be two models using the same name but in different apps.

This would be better and more elegant:

request.user.has_perm(item['model'])

But it generates one query for every request, I think we can do the following: openwisp/openwisp-users#153 (review)

[okraits]

@NoumbissiValere What about #104?

[NoumbissiValere]

@okraits the first commit on this PR is #104. That was the solution I thought of since I couldn't push to your Branch

[okraits]

@okraits the first commit on this PR is #104. That was the solution I thought of since I couldn't push to your Branch

I'm sorry, I didn't see that! Thank you for clarifying!

[nemesifier]

I forgot to send this review

[nemesifier]

superuser must always be allowed, this test is wrong. Keep the superuser test but change it to expect the menu item is shown.
Add another test for a non superuser which does not have the permission.

Use good names please.

test_superuser_always_sees_menu_items

test_operator_cant_see_menu_item

or similar but self explanatory

[NoumbissiValere]

Ok. Even though i thought we are focusing on the fact that if a user have a permission on a module he should see its menu item, otherwise he doesn't see it irrespective of who he is.

[nemesifier]

too complex. BTW, it breaks the UI for superusers.

I tried this which seems to work:

def build_menu(request):
    default_items = getattr(settings, 'OPENWISP_DEFAULT_ADMIN_MENU_ITEMS', [])
    custom_items = getattr(settings, 'OPENWISP_ADMIN_MENU_ITEMS', [])
    items = custom_items or default_items
    menu = []
    # loop over each item to build the menu
    # and check user has permission to see each item
    for item in items:
        app_label, model = item['model'].split('.')
        model_class = registry.apps.get_model(app_label, model)
        model_label = model.lower()
        url = reverse('admin:{}_{}_changelist'.format(app_label, model_label))
        label = item.get('label', model_class._meta.verbose_name_plural)
        view_perm = f'{app_label}.view_{model_label}'
        change_perm = f'{app_label}.change_{model_label}'
        user = request.user
        # use cached helper from openwisp-users if available
        has_permission_method = user.has_permission if hasattr(user, 'has_permission') else user.has_perm
        if has_permission_method(view_perm) or has_permission_method(change_perm):
            menu.append({'url': url, 'label': label, 'class': model_label})
    return menu

Notice that I made the request param mandatory, I don't remember why I made it optional, maybe to ease testing, but better remove that.

[nemesifier]

Great work 👍