Moodle
  1. Moodle
  2. MDL-40289

badges capabilities include incorrect risks, levels and archetypes

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5.1
    • Component/s: Badges
    • Labels:
    • Testing Instructions:
      Hide

      note: this depends on major version bump to update core capabilities table, test in fresh new install

      1/ perform install
      2/ review all badges capabilities in following roles: coursecreator (no caps), user (defaults to view and earn), manager and editing teacher (configure everything on course level)
      3/ try badge capabilities overriding at course level

      Show
      note: this depends on major version bump to update core capabilities table, test in fresh new install 1/ perform install 2/ review all badges capabilities in following roles: coursecreator (no caps), user (defaults to view and earn), manager and editing teacher (configure everything on course level) 3/ try badge capabilities overriding at course level
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull 2.5 Branch:
      w26_MDL-40289_m25_badgescaps
    • Pull Master Branch:
      w26_MDL-40289_m26_badgescaps
    • Rank:
      51060

      Description

      Configuration risk is intended for site level settings that may pose security risks such as paths to executables.

      I believe the badges should not have this because they are allowed for teachers by defaults.

      The current rule is that only admins may have capabilities with configuration risks.

      The level of each capability is used for overriding, it must be the lowest level where you can override the capability...

        Activity

        Hide
        Petr Škoda added a comment -

        Grrr, there is absolutely no reasons to give any badge capabilities to course creators!!! Who was reviewing the badges patch before integration???

        Show
        Petr Škoda added a comment - Grrr, there is absolutely no reasons to give any badge capabilities to course creators!!! Who was reviewing the badges patch before integration???
        Hide
        Petr Škoda added a comment -

        Also if you give something to 'user' by default then do not include other roles such as manager or student!

        Show
        Petr Škoda added a comment - Also if you give something to 'user' by default then do not include other roles such as manager or student!
        Hide
        Petr Škoda added a comment -

        To integrators: please make sure that everybody who was involved in this gets educated how to define new capabilities, thanks.

        Show
        Petr Škoda added a comment - To integrators: please make sure that everybody who was involved in this gets educated how to define new capabilities, thanks.
        Hide
        Sam Hemelryk added a comment -

        Hi Petr,

        Before this gets integrated just a couple of questions:

        1. Should block/badges:myaddinstance be specifying any risks at all?
        2. Should block/badges:addinstance be defining risks as RISK_SPAM | RISK_XSS?
        3. Changing the context level to course for these contexts, much of these can be handled at the system level as well. I take it you're changing to course because thats where they are most relevant?

        I've also added Yuliya here as this is probably of interest to her.

        Show
        Sam Hemelryk added a comment - Hi Petr, Before this gets integrated just a couple of questions: Should block/badges:myaddinstance be specifying any risks at all? Should block/badges:addinstance be defining risks as RISK_SPAM | RISK_XSS? Changing the context level to course for these contexts, much of these can be handled at the system level as well. I take it you're changing to course because thats where they are most relevant? I've also added Yuliya here as this is probably of interest to her.
        Hide
        Yuliya Bozhko added a comment -

        Hi Sam,

        Just wanted to clarify what this block does, and you decide what is best Badges block shows to a user their own earned badges. Users who have this block on the page won't see other users badges, only their own.

        Yuliya

        Show
        Yuliya Bozhko added a comment - Hi Sam, Just wanted to clarify what this block does, and you decide what is best Badges block shows to a user their own earned badges. Users who have this block on the page won't see other users badges, only their own. Yuliya
        Hide
        Petr Škoda added a comment -

        1. no risks would be probably ok (risks can be changed at any time with just a version bump, there is no upgrade trouble here, I did not think much about them)
        2. other :addinstance caps do not seem to have correct risks, this one particular does not seem to have any risks to me
        3. if you want to override something at course level or category the capability needs to have CONTEXT_COURSE, when reviewing code you look for lowest possible level where the capability is used, in this case it is course

        Show
        Petr Škoda added a comment - 1. no risks would be probably ok (risks can be changed at any time with just a version bump, there is no upgrade trouble here, I did not think much about them) 2. other :addinstance caps do not seem to have correct risks, this one particular does not seem to have any risks to me 3. if you want to override something at course level or category the capability needs to have CONTEXT_COURSE, when reviewing code you look for lowest possible level where the capability is used, in this case it is course
        Hide
        Sam Hemelryk added a comment -

        Great, thanks for the clarification guys.

        Re: risks no doubt we need to go through all of our core blocks and assess risk usage.
        Certainly in this case it appears fine.

        This has been integrated now.

        Many thanks
        Sam

        Show
        Sam Hemelryk added a comment - Great, thanks for the clarification guys. Re: risks no doubt we need to go through all of our core blocks and assess risk usage. Certainly in this case it appears fine. This has been integrated now. Many thanks Sam
        Hide
        Sam Hemelryk added a comment -

        Tested and passed thanks Petr.

        Show
        Sam Hemelryk added a comment - Tested and passed thanks Petr.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Thanks for giving me joys and smiles
        Thanks for sharing my trouble's pile

        Thanks for wipeing the tears of my eye
        Thanks for showing me the glad view of sky

        Thanks for lending me your shoulders to lean
        Thanks for giving my words a proper mean

        Thanks for telling me the value of life
        Thanks for showing me the rules to survive

        Thanks for lending me the sympathetic ears
        Thanks for showing how much you care

        From all this what I mean in the end
        Is thanks for being my special friend.

        – Seema Chowdhury

        Sent upstream so... closing, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Thanks for giving me joys and smiles Thanks for sharing my trouble's pile Thanks for wipeing the tears of my eye Thanks for showing me the glad view of sky Thanks for lending me your shoulders to lean Thanks for giving my words a proper mean Thanks for telling me the value of life Thanks for showing me the rules to survive Thanks for lending me the sympathetic ears Thanks for showing how much you care From all this what I mean in the end Is thanks for being my special friend. – Seema Chowdhury Sent upstream so... closing, thanks!
        Hide
        Helen Foster added a comment -

        Just noting that I have added a note to http://docs.moodle.org/dev/Moodle_2.5.1_release_notes for sites which are upgrading from 2.5 with a link to info on how to correctly set badge permissions for each role archetype - http://docs.moodle.org/25/en/Upgrading#Upgrading_from_Moodle_2.5_to_2.5.1_or_later .

        Show
        Helen Foster added a comment - Just noting that I have added a note to http://docs.moodle.org/dev/Moodle_2.5.1_release_notes for sites which are upgrading from 2.5 with a link to info on how to correctly set badge permissions for each role archetype - http://docs.moodle.org/25/en/Upgrading#Upgrading_from_Moodle_2.5_to_2.5.1_or_later .

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: