Moodle

Category creation is only allowed by permisisons defined at system context

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9.1
  • Fix Version/s: 1.9.4
  • Component/s: Roles / Access
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

First noticed by the following steps:

1/ Assign a user a role which has moodle/category:create allowed to a category
2/ As that user, try to add a subcategory from the category
3/ The form will be displayed to add a new category
4/ Submiting the form returns you back to the form

Looking at the top of categoryedit.php seems to indicate that when categories are being added with also use system context.. Need to dig further..

Issue Links

This issue has a non-specific relationship to:
MDL-15265 Backport course sortorder rewrite to 1.9.x Major Open
MDL-14580 Sortorder incorrectly updated leading to interface failure Major Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Howard Miller added a comment -

When I looked at this I was actually assigning Admin rights at category level (which works). At the time there wasn't even a UI for adding categories at other than site level.

Show
Howard Miller added a comment - When I looked at this I was actually assigning Admin rights at category level (which works). At the time there wasn't even a UI for adding categories at other than site level.
Hide
Permalink
Dan Poltawski added a comment -

Hmm, i'm looking more at this, and the editcategory form is just broken in multiple places so i'm refactoring, patch on the way

Show
Dan Poltawski added a comment - Hmm, i'm looking more at this, and the editcategory form is just broken in multiple places so i'm refactoring, patch on the way
Hide
Permalink
Dan Poltawski added a comment - - edited

Big patch to fix some of the problems with the current edit category form which are:

  • Category context permissions are not respected (e.g. creating sub-categories can only be done by a user with permissions at system-level)
  • A category can be 'lost' by changing the parent to itself in the edit form
  • Lots of obsolete code which hasn't been implemented

I've refactored it a bit to address these issues (primarily to allow category ctx permission to be respected) and checks permissions where the parent is changed.

course/editcategory.php | 289 ++++++------------------------------------
course/editcategory_form.php | 47 +++++++-
lang/en_utf8/error.php | 3 +
3 files changed, 87 insertions, 252 deletions

Show
Dan Poltawski added a comment - - edited Big patch to fix some of the problems with the current edit category form which are:
  • Category context permissions are not respected (e.g. creating sub-categories can only be done by a user with permissions at system-level)
  • A category can be 'lost' by changing the parent to itself in the edit form
  • Lots of obsolete code which hasn't been implemented
I've refactored it a bit to address these issues (primarily to allow category ctx permission to be respected) and checks permissions where the parent is changed. course/editcategory.php | 289 ++++++------------------------------------ course/editcategory_form.php | 47 +++++++- lang/en_utf8/error.php | 3 + 3 files changed, 87 insertions, 252 deletions
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

just linking this bug with MDL-14580 because of potential relation, to have them under control.

Show
Eloy Lafuente (stronk7) added a comment - just linking this bug with MDL-14580 because of potential relation, to have them under control.
Hide
Permalink
Petr Škoda (skodak) added a comment -

I would prefer to have two optional parameters:

  • id for category - 0 means create new category
  • parent for category parent
    and do some verifications in definitional_after_data() and validation().
Show
Petr Škoda (skodak) added a comment - I would prefer to have two optional parameters:
  • id for category - 0 means create new category
  • parent for category parent and do some verifications in definitional_after_data() and validation().
Hide
Permalink
Dan Poltawski added a comment -

Makes sense, I was keeping with the meaning of the original parms, I will refactor this some more

Show
Dan Poltawski added a comment - Makes sense, I was keeping with the meaning of the original parms, I will refactor this some more
Hide
Permalink
Petr Škoda (skodak) added a comment -

thanks for working on this!

Show
Petr Škoda (skodak) added a comment - thanks for working on this!
Hide
Permalink
Dan Poltawski added a comment -

New patch with Petrs suggestions (which make it 100000x more logical!)

danp@cream:~/git/moodle-r2$ git diff --stat d5176
course/category.php | 3 +-
course/editcategory.php | 325 ++++++++----------------------------------
course/editcategory_form.php | 47 ++++++-
course/index.php | 7 +-
lang/en_utf8/error.php | 3 +
5 files changed, 112 insertions, 273 deletions

Show
Dan Poltawski added a comment - New patch with Petrs suggestions (which make it 100000x more logical!) danp@cream:~/git/moodle-r2$ git diff --stat d5176 course/category.php | 3 +- course/editcategory.php | 325 ++++++++---------------------------------- course/editcategory_form.php | 47 ++++++- course/index.php | 7 +- lang/en_utf8/error.php | 3 + 5 files changed, 112 insertions, 273 deletions
Hide
Permalink
Dan Poltawski added a comment -

Hi, has anyone had a chance to review this patch?

I think the problems mentioned are still broken in head, so I was thinking of updating this for HEAD so we can test for breakage

thanks

Show
Dan Poltawski added a comment - Hi, has anyone had a chance to review this patch? I think the problems mentioned are still broken in head, so I was thinking of updating this for HEAD so we can test for breakage thanks
Hide
Permalink
Howard Miller added a comment -

AFAIK, it's working fine in 1.9 Weekly. I did test it.

Unfortunately, you still can't delete them or move them about at Category level.

Show
Howard Miller added a comment - AFAIK, it's working fine in 1.9 Weekly. I did test it. Unfortunately, you still can't delete them or move them about at Category level.
Hide
Permalink
Tim Hunt added a comment -

Grabbing a bunch of related course category editing bugs that I plan to work on. Sorry for all the emails.

Show
Tim Hunt added a comment - Grabbing a bunch of related course category editing bugs that I plan to work on. Sorry for all the emails.
Hide
Permalink
Tim Hunt added a comment -

After some more testing tomorrow, and merging it to HEAD, I intend to commit this patch (categories.patch.txt) to the 1.9 stable branch.

It fixes MDL-17479, MDL-16426, MDL-16063, MDL-16013, MDL-15658, MDL-15556, MDL-15161, MDL-14925, MDL-13742, MDL-11557.

I chose this bug to attach the patch to because it had the most watchers!

Apologies for the fact it is one big patch, rather than a series of smaller patches. However, categoryedit.php, category.php and index.php where in pretty bad shape and needed significant cleaning up. categoryedit.php, in particular, was almost completely rewritten to bring it in line with modern Moodle coding practices. Doing that fixed a lot of bugs at once.

There are still about a dozen category related bugs on my list, but I am able to separate them out, and hope to be committing fixes for them in the next few days.

If anyone has some spare cycles to review or test this before tomorrow, that would be great, but I am not expecting anything. Thanks.

Show
Tim Hunt added a comment - After some more testing tomorrow, and merging it to HEAD, I intend to commit this patch (categories.patch.txt) to the 1.9 stable branch. It fixes MDL-17479, MDL-16426, MDL-16063, MDL-16013, MDL-15658, MDL-15556, MDL-15161, MDL-14925, MDL-13742, MDL-11557. I chose this bug to attach the patch to because it had the most watchers! Apologies for the fact it is one big patch, rather than a series of smaller patches. However, categoryedit.php, category.php and index.php where in pretty bad shape and needed significant cleaning up. categoryedit.php, in particular, was almost completely rewritten to bring it in line with modern Moodle coding practices. Doing that fixed a lot of bugs at once. There are still about a dozen category related bugs on my list, but I am able to separate them out, and hope to be committing fixes for them in the next few days. If anyone has some spare cycles to review or test this before tomorrow, that would be great, but I am not expecting anything. Thanks.
Hide
Permalink
Howard Miller added a comment -

Putting my sycophant hat on for a moment - that's excellent news, Tim The grimness of these pages has been a big hassle for a long time.

Show
Howard Miller added a comment - Putting my sycophant hat on for a moment - that's excellent news, Tim The grimness of these pages has been a big hassle for a long time.

People

Vote (0)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.