[#11162] - Create new Category on the fly is heavily broken
- Closed
- 7 Sep 2016
- Medium
- Build: master
- # 11162
The new feature implemented here is heavily broken: #8623
There are several issues:
- There is no ACL check (see from @brianteeman: #8623 (comment) and from @ggppdk: #8623 (comment))
- Create a new user with "manager" rights
- Remove the "Create" permission for managers in com_content
- => User can still create "categories on the fly"
- If you cannot create new categories in the way described above, you run into the next bug: @ggppdk mentioned, that the article was assigned to the "ROOT" category. The reason is very easy:
- you edit the article with ID 1.
- https://github.com/joomla/joomla-cms/pull/8623/files#diff-9b292088b08fa3a892af0f75631302aeR179 => Here the model call does not ignore the request (parameter), so the category model takes the ID from the $_REQUEST (https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_categories/models/category.php#L145).
- the returned ID is the ROOT ID.
- Ofc this works with every category which has the same ID as the article...There is also a special case: the category title with the same ID as the articleID will be updated to the name of the "new category" we wanted to create
- The category selection breaks on several combinations
- if there is no category yet, you cannot create one
- ...
Solutions:
1. Add proper ACL checks(!!!)
2. Add proper cleaning of request variables (ignore_request e.g.)
3. Add proper validation of request variables
4. Fix bug in select dropdown
I see what happend now, i had replied to a PR that closed / merged already !, no wonder that most people just passed it without reading my comment
| Category | ⇒ | ACL Administration |
IMO, we should consider for once reusing the categories view itself in a iframe modal to choose or create categories. The button can be added to the input field to trigger the modal.
Like this:
or this:
This way the ACL, parent category etc will be handled by com_categories implicitly. However, I understand that what I am saying is not same as on-the-fly.
If this is acceptable, I can submit a PR myself.
On a very plain install of J3.6 this does not work
When saving the new category is not created and the item is assigned to category "ROOT".
| Labels |
Added:
?
|
||
@izharaazmi I love the idea, however it won't feel as on the fly. So instead I suggest having the current method but having a ACL button appear below when a new category is added so that those who want to configure the ACL can do so while not slowing down those who are content with the current setup.
So instead I suggest having the current method but having a ACL button appear below when a new category is added
Again: the problem is NOT, that there is no ACL for the new category, the problem is, that the new function is broken and ignores the global category ACL...a Privilege Escalation Vulnerability
If some of us thinks it would be acceptable to have little less than "on
the fly" then I'd do the PR with my 2nd approach above. Opinions please!
@izharaazmi I would support the idea. However I do wonder about the "Refresh" button, is there a way to avoid adding that?
@JoshuaLewis @izharaazmi yup listen for change event on the select and do a re-initialization, something like: #11040
@dgt41 As in #11040 we have a "position" dropdown to listen to, and act on the "other modules" dropdown refresh thing.
Here I don't see such a thing that we can listen to. [New] opens a modal to "category.add" page, that after save doesn't communicate so to the actual page (better if it did), hence the user must do some trigger to fetch the updated list of categories.
How do you suggest this to go around?
I prefer the PR of @izharaazmi instead of creating a new category "on the fly" because it would also solve annoying issues like this one #11224
@izharaazmi it's not a one to one exact solution but the modal close could trigger the event (or a hidden button) and then the same ajax call as the other PR will refresh the select element. I just shared that as the second part could be used (with the appropriate changes) here as well
In fact I took the idea from
And yes it has to be an ajax call anyway to fetch the updated list of categories. We just need to find the appropriate trigger for the call. IMO, modal close is okay but not the best one. And I think user should also have the choice to manually trigger the refresh.
@izharaazmi you can also check the way modules update the infos in the menu edit page (but please lets use ajax here, as that one is way too messy javascript code)
Okay. Now I'll work on this this weekend. Hope a PR on or around Monday.
Closing as we have #11244 and #11238 that address all three issues
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11162.
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-07-22 18:00:27 |
| Closed_By | ⇒ | brianteeman |
Hello @brianteeman the 4th point is still unresolved.
Missed that - reopening
How can you have a scenario without a single category?
| Status | Closed | ⇒ | New |
| Closed_Date | 2016-07-22 18:00:27 | ⇒ | |
| Closed_By | brianteeman | ⇒ |
Thanks,
if you have no article, you can delete all categories
Hi @bembelimen I am closing this.
Point 4 isnt really related to the create categories on the fly feature at all. Its more fundamental than that - the category field is a required field and that is the issue not the categories on the fly feature. If you want to create a new issue to address that one specific scenario by all means do it but its certainly not a high priority to fix.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11162.
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-09-07 08:03:53 |
| Closed_By | ⇒ | brianteeman |
I replied twice about proper ACL check missing, and then i got no response
But i understand now that sometimes, people are busy, to read all the comments , especially when something is marked already RTC