[#11624] - return 403 instead of a 404 for a not accessible category
- Closed
- 4 Sep 2016
- Medium
- Build: staging
- # 11624
- Diff
- rdeutz:pr_for_issue_5466
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
PR for issue #5466
Executive summary
This PR modifies the process when checking a category in JViewCategory. It allows to skip the access rights checks in the first place to differentiate between a not existing category and a category a user hasn’t the access rights to access. The part of checking the access right was something that couldn’t be reached because for a not accessible category
$this->get('Category')
was always false.
Background information
As discussed in the issue, if you have a menu item set to publish and the category assigned to that category is set to registered you will get a 404. I agree that this is not the right return code. But I don’t agree that the right behavior is to show a login. In this case I think 403 is that right return code. Anything above this is a new feature. Just because we have a redirect to login for articles doesn’t mean we need that for categories also.
Backwards compatibility
Shouldn’t be a problem, but not 100% sure
Translation impact
nothing
Testing instruction
- Install a clean Joomla with test data
- Set Category "Park Blog" access level to registered
- Don’t change articles access level
- On front-end: > All Front End Views > clicking on "Article Category Blog"
You will get a 404
- Apply patch and try again
You will get a 403
Need also testing with some 3rd part code
joomla-cms-bot
- | Category | ⇒ | Front End Components Libraries |
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
|
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11624.
| Status | Pending | ⇒ | Ready to Commit |
| Labels |
Added:
?
|
||
Don't merge CMS-Maintainers, this needs more testing, we have to make sure that this hasn't side effects.
I think this should be in 3.7 so that it can be highlighted for testing on
real sites during the beta and rc phase
On 16 August 2016 at 09:18, Robert Deutz notifications@github.com wrote:
Don't merge CMS-Maintainers, this needs more testing, we have to make sure
that this hasn't side effects.—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#11624 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8b8S6q4Do7Go3TbKDA7Zq-I6u_s9ks5qgXJogaJpZM4JkySL
.
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Agreed 3.7
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-09-04 13:39:23 |
| Closed_By | ⇒ | wilsonge |
| Labels |
Removed:
?
|
||
Large regression: f50eb41#commitcomment-18893449
I have tested this item✅ successfully on a8afaac
Tested with category blog and category list menu items
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11624.