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.Access site reports gives access to watchdog, which in turn gives access to error messages and things like content titles without any access restrictions at all, so it should probably have the 'give to trusted roles only' warning on it. Coming here from #965078: HTTP request checking is unreliable and should be removed in favor of watchdog() calls.
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | drupal-access_report_perm-2012468-21.patch | 432 bytes | joshi.rohit100 |
| #19 | drupal-access_report_perm-2012468-19.patch | 503 bytes | joshi.rohit100 |
| #11 | drupal-access_report_perm-2012468-11.patch | 432 bytes | David_Rothstein |
| #6 | drupal-access_report_perm-2012468-6.patch | 382 bytes | plach |
Comments
Comment #1
David_Rothstein CreditAttribution: David_Rothstein commentedIt would be a good idea to resolve this before Drupal 8 is released, especially given #965078: HTTP request checking is unreliable and should be removed in favor of watchdog() calls - the fact that D8 is logging URLs potentially including passwords or tokens in the logs is really not so different from something like https://drupal.org/node/912412, and bigger than basic access bypass issues (e.g. with node titles).
I agree this should probably just be marked a trusted permission... It's not ever going to be possible to keep all sensitive information out of the logs entirely (or desirable).
Comment #2
catchComment #3
catchComment #4
blueminds CreditAttribution: blueminds commentedPlease see attached patches.
Comment #5
David_Rothstein CreditAttribution: David_Rothstein commentedShouldn't it be TRUE, rather than FALSE?
Comment #6
plachAddressed #5
Comment #7
marco CreditAttribution: marco commentedI've verified #6, 'restrict access' needs to be TRUE.
Comment #8
B31 CreditAttribution: B31 commentedI've verified #6, too. I confirm that 'restrict access' needs to be TRUE.
Comment #9
Dries CreditAttribution: Dries commentedI committed this to HEAD. It may not be a bad idea to add a code comment explain "why"?
Comment #11
David_Rothstein CreditAttribution: David_Rothstein commentedThis is https://drupal.org/PSA-2014-002 and was already marked for D7 backport, so here's the patch.
Comment #12
dcam CreditAttribution: dcam commentedI've reviewed #11. It correctly adds the "Give to trusted roles only" warning to the View site reports permission in D7. It's also the exact same change that went into D8. #11 is RTBC.
Comment #14
dcam CreditAttribution: dcam commented11: drupal-access_report_perm-2012468-11.patch queued for re-testing.
Comment #15
dcam CreditAttribution: dcam commentedComment #17
dcam CreditAttribution: dcam commented11: drupal-access_report_perm-2012468-11.patch queued for re-testing.
Comment #19
joshi.rohit100Updated the patch for D7. Added 'warning' attribute.
Comment #21
joshi.rohit100rerolled #11.
I think, this test case is not relevent here.
Comment #22
dcam CreditAttribution: dcam commentedI'm not sure what was going on with #19 and #21. #11 didn't need a reroll and is still RTBC for D7.
Comment #24
David_Rothstein CreditAttribution: David_Rothstein commentedComment #25
joshi.rohit10021: drupal-access_report_perm-2012468-21.patch queued for re-testing.
Comment #27
plach21: drupal-access_report_perm-2012468-21.patch queued for re-testing.
Comment #28
dcam CreditAttribution: dcam commentedComment #31
dcam CreditAttribution: dcam commentedComment #34
dcam CreditAttribution: dcam commentedComment #35
David_Rothstein CreditAttribution: David_Rothstein commented7.29 was a security release, but I went ahead and committed this now to get it into 7.30 (since it's harmless and related to the above-mentioned PSA).
Someone could create a separate followup issue for both D7 and D8 for the suggestion by Dries above (to add a code comment).