The documentation mentions the following:

Organic group members with the right permission can also manage menus at node/[nid]/og_menu.

I have not been able to get this to work under D7. Which permission is needed? Even for the admin with uid 1 I get 'Permission denied' when I try to access this path.

CommentFileSizeAuthor
#3 1307238-1-og_menu-permission.patch1.17 KBpfrenssen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rv0’s picture

rv0 CreditAttribution: rv0 commented
Status: Active » Postponed (maintainer needs more info)

"Administer OG menus"
on admin/people/permissions

rv0’s picture

rv0 CreditAttribution: rv0 commented
Status: Postponed (maintainer needs more info) » Closed (fixed)

No feedback, so this should be marked fixed

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Category: support » task
Status: Closed (fixed) » Needs review
FileSize
1307238-1-og_menu-permission.patch1.17 KB

Your reply in #1 adequately answered this question for me so I did not reply ;)
Looking back at it it would perhaps be nice if this was explained properly in the documentation. The permission is mentioned elsewhere in the Readme but not for this particular section.

Proposed fix attached.

The readme can also use some formatting and spelling improvements, but that's another issue.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented

Looking at og_menu_access() and reading my original request I think I know what caused my confusion.

og_menu_access() does more than just check access rights. It also checks if the given node exists and if it is a group type. If this is not the case it returns a 403 Access Denied. This is not the right response, it should be a 404 Page Not Found.

The problem is that this function is trying to do two things at once: it is checking access AND it is validating input. This is a Code Smell® :) From my original request I can tell that I thought I stumbled on a bug since I was logged in as UID1 and got an access denied. UID1 should have access to everything and should never be served any access denied responses.

Shall I make a new issue out of this?

rv0’s picture

rv0 CreditAttribution: rv0 commented
Status: Needs review » Closed (fixed)

Committed an edited version, thanks for the help.

Yes, there are many things in the code that could be done in a better way. Patches are always welcome in a new issue.
The sad part about major code changes (to eliminate code smell) is that they need a lot of testing because of all the possible usecases.