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 Master Branch:
      w26_MDL-40289_m26_badgescaps

      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...

        Gliffy Diagrams

          Activity

          Hide
          Petr Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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 Skoda 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: