Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
? wrong permission for admin/structure/menu/parents
Proposed resolution
? Use administer menu permission
Remaining tasks
- (done) commit to 8.x Patch in #26 committed in #29
- (novice) Backport patch in #26 to 7.x | Contributor task document for backporting a patch
- git instructions for creating patch | Contributor task documentation for creating a patch
- Review patch to check it fixes the issue, the change is properly documented and for coding standards. Make sure patch stays within scope of just this issue. | Contributor task documentation for reviewing patch
User interface changes
No.
API changes
No.
Original report by @tsvenson
Taking alpha 6 for a test spin and got a weird access denied when logged in as user with administrator role (all permissions ticked).
Type access denied
Location http://d7/admin/structure/menu/parents
Referrer http://d7/admin/structure/types/manage/page?render=overlay
Message admin/structure/menu/parents
Severity warning
When I tried the location URL I got access denied as the same user. Then I logged in as user 1 and this time I was prompted with a download (Firefox 3.6.6) for parents of a "application/json" type.
When looking at that file it contained only "[]" nothing else.
Sorry but I have no idea where this is coming from, hope its enough though.
Comment | File | Size | Author |
---|---|---|---|
#35 | menu-parents-access-849624-35.patch | 1.4 KB | dcam |
Comments
Comment #1
int CreditAttribution: int commentedComment #2
webchickEr, no. Unless we can track this down to something pervasive in the system, this is just normal.
Can anyone reproduce this?
Comment #3
Stevel CreditAttribution: Stevel commentedaccess arguments for admin/structure/menu/parents is set to array(TRUE), which is no permission to be granted by user_access, so that returns access denied by anyone but user 1.
Getting a JSON-file looks correct to me.
This patch sets the permission to administer menu, the same as the other menu items at admin/structure/menu/*
edit:
sorry, crosspost.
I'm not sure why it had TRUE as a permission though, was it just a mistake, or is it there intentionally?
Comment #4
Stevel CreditAttribution: Stevel commentedComment #5
Stevel CreditAttribution: Stevel commentedAnd this is normal as it doesn't prevent usage of anything really, just allows an administrator to select a default parent item that's not in one of the selected menus on admin/structure/types/manage/page (under Menu Settings).
To reproduce, just visit admin/structure/types/manage/page and view the menu settings. The error then shows up in the log.
Comment #6
Heine CreditAttribution: Heine commentedComment #7
rfayD8 first. Let's get these fixed on D8 and into D7.
Comment #8
oriol_e9gD8 roll.
Comment #9
oriol_e9gWould be good to write a test.
Comment #10
Chris Pergantis CreditAttribution: Chris Pergantis commentedI had a couple of log entries that revealed this same behavior (Top 'access denied' errors ), but the site is on D7 not D8. As stated above I go to the URL and all I get is the double brackets. Just an FYI - perhaps this issue is a legacy from D7?
Comment #11
Chris Pergantis CreditAttribution: Chris Pergantis commentedreturning your posts to D8
Comment #12
oriol_e9g@cperg We fix first all bugs in D8 and then we backport the solution to D7, this is our normal process.
Comment #13
Chris Pergantis CreditAttribution: Chris Pergantis commentedit was just a heads up - didn't expect your process to be different from what you stated.
Comment #14
amanaplan CreditAttribution: amanaplan commentedsub
Comment #15
Alan Evans CreditAttribution: Alan Evans commentedNote - steps to reproduce the bug in the UI:
Comment #16
Alan Evans CreditAttribution: Alan Evans commentedAdded a failing test for this: https://skitch.com/e-alanevans/8q2du/test-result-testd85.localhost
Test result after patching: https://skitch.com/e-alanevans/8q2ry/test-result-testd85.localhost ... https://skitch.com/e-alanevans/8q2rb/test-result-testd85.localhost
Patch including test attached. Leaving in review for now - the test itself needs review, and I'm not 100% sure that "administer menu" is the correct permission for this: The feature only shows up on a node type creation form, so in the sense of the items returned by the callback it's related to menu admin, and in the sense of where the data is used, it's only related to node type admin. To make this callback useful, you first need access to create new content types, and to see the menu items you also need permission to view all those menu items (which is only guaranteed for menu admin).
On the other hand, maybe the only reasonable permission for the data returned by the callback itself is menu admin, as menu admins are the only users who should be able to see all menu items.
Comment #17
arelem CreditAttribution: arelem commentedApril 1 2012
Yes I am running into this all the time in Drupal 7.12, it prevents the use of some Node References and prevents the use of .
I have Page, Article, Event, Contact, Date and Test2 Content Types.
I have 2 views for Contacts which display first name only and full name. These show as drop down fields in the Event form but no selection is entered into the database, content type or view.
With "Content types that can be referenced", Page and Article work ok, but any other created Content Types do not work.
A further problem is that some Content types do not display in the selector, this too produces error message. The Content type is selectable from a user menu however.
Comment #18
catchChanging the version back to 7.x makes this less rather than more likely to get fixed, see http://drupal.org/node/767608
Not sure about the permissions either, should it be administer node types + menu admin?
Comment #19
dawehnerThis kind of javascript could be be used by other modules so i would think that just "administer menu" is enough.
Comment #20
Alan Evans CreditAttribution: Alan Evans commentedMakes sense - using just "administer menu" keeps this callback decoupled from where it's used, so that the permission just expresses the level of access that you should have in order to view all of the data that it returns. Although it's only used in one place that we know of, there is potential for it to be used elsewhere.
+1 then for keeping "administer menu"
Comment #21
kscheirer#16: 849624-menu-parents-access.patch queued for re-testing.
Comment #22
kscheirerRetesting against latest HEAD since it has been over a year.
Comment #24
brad.bulger CreditAttribution: brad.bulger commentedreroll of patch from 16 against 8.x - moved test into MenuTest.php. that test passed when i tried the patch on a local install.
Comment #25
dawehnerThis should be marked as public. Additional we should check that a user without the permission does not have access
Comment #26
brad.bulger CreditAttribution: brad.bulger commentedredid patch against latest 8.x, and added 'public' to this patch's test function. i didn't change the other plain 'function' declarations in MenuTest.php, that seemed out of scope. i also added some lines to check for a 403 return to an unprivileged standard user. Alan or someone with more experience with writing tests should probably review that code. it worked when i tried it on a local install.
Comment #27
dawehnerThank you!
Comment #28
YesCT CreditAttribution: YesCT commentedThis issue was RTBC and passing tests on July 1, the beginning of API freeze.
Comment #29
catchCommitted/pushed to 8.x.
Moving to 7.x for backport.
Comment #30
David_Rothstein CreditAttribution: David_Rothstein commented@mikeytown2 has the start of a patch in #1633130: Path admin/structure/menu/parents returns 403 unless UID=1 (which I closed as a duplicate), but the test isn't backported yet.
Comment #31
memcinto CreditAttribution: memcinto commentedHow is it that this still hasn't been applied to 7.x? Every time I update core I have to go and manually patch the menu module on each of my 50+ drupal sites. Annoying.
Comment #32
webchickBecause instead of uploading a D7 patch for review, which could then be RTBCed and committed, people keep manually patching menu module on their D7 sites. :)
Comment #33
milos.kroulik CreditAttribution: milos.kroulik commented#32 Well, patch mentioned in #30 seems to be working fine. Maybe it would be way to go?
Comment #34
YesCT CreditAttribution: YesCT commentedadded instructions for doing a backport.
Comment #35
dcam CreditAttribution: dcam commentedBackported #26 to D7.
Comment #36
mikeytown2 CreditAttribution: mikeytown2 commentedPatch looks good +1 for RTBC.
Comment #37
dcam CreditAttribution: dcam commentedFor any potential reviewers: steps to reproduce the issue are in #15.
Comment #38
dcam CreditAttribution: dcam commentedI know I normally shouldn't do this to my own patch, but we had one person RTBC it. I think that's all the RTBCing it's going to get.
Comment #41
dcam CreditAttribution: dcam commentedComment #44
dcam CreditAttribution: dcam commentedComment #47
dcam CreditAttribution: dcam commentedComment #52
mikeytown2 CreditAttribution: mikeytown2 commentedComment #55
dcam CreditAttribution: dcam commentedComment #58
dcam CreditAttribution: dcam commentedComment #59
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks!