Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-20772

move forward with stackable auth modules

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Minor
    • Resolution: Inactive
    • Affects Version/s: 2.0
    • Fix Version/s: DEV backlog
    • Component/s: Authentication
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE

      Description

      Background discussion here: http://moodle.org/mod/forum/discuss.php?d=136675

      I've attached a first attempt at a patch for actually using the AUTH_* definitions already in lib/authlib.php. This code respects AUTH_DENIED so that it will be possible to create stackable blacklist authentication modules in addition to the stackable whitelist modules we already have.

      I wrote this with the idea of keeping the patch as small as possible, but I think it would be better if we made it possible to provide more information back to the user. For example, it would be useful if, while configuring your authentication modules stack, you could turn on some more verbosity and be told "login denied for XYZ reason by ABC module" instead of simply "Login denied".

      Is there any interest in considering this for inclusion in 2.0?

      Cheers,
      Matt

        Gliffy Diagrams

        1. moquist_auth_fail3.patch
          4 kB
          Matt Oquist
        2. more-stackable.diff
          3 kB
          Matt Oquist
        3. skodak_auth_fail2.patch
          5 kB
          Petr Skoda

          Activity

          Hide
          aborrow Anthony Borrow added a comment -

          Matt - Looks good. My only suggestion for improvement has to do with communication with the user. Might it be helpful to indicate a reason(s) why the access is being denied. For example, Login access denied: Blacklisted IP. I realize that the more information you give the easier it allows some folks to get back in; however, looking at it from the perspective of most good users it may be helpful to specify a more exact cause for the denial. In any case, just something to consider. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - Matt - Looks good. My only suggestion for improvement has to do with communication with the user. Might it be helpful to indicate a reason(s) why the access is being denied. For example, Login access denied: Blacklisted IP. I realize that the more information you give the easier it allows some folks to get back in; however, looking at it from the perspective of most good users it may be helpful to specify a more exact cause for the denial. In any case, just something to consider. Peace - Anthony
          Hide
          skodak Petr Skoda added a comment -

          why not use loginpage_hook() instead, you may disable login page completely from some IP addresses?

          Show
          skodak Petr Skoda added a comment - why not use loginpage_hook() instead, you may disable login page completely from some IP addresses?
          Hide
          skodak Petr Skoda added a comment -

          attaching patch with some more minor modifications, please review

          Show
          skodak Petr Skoda added a comment - attaching patch with some more minor modifications, please review
          Hide
          moquist Matt Oquist added a comment -

          skodak_auth_fail2.patch looks good to me.

          Should we make AUTH_OK 1 and AUTH_FAIL 0? That way they'll still correspond to the 'true' and 'false' that are being used now. I've attached another variant that does this.

          Show
          moquist Matt Oquist added a comment - skodak_auth_fail2.patch looks good to me. Should we make AUTH_OK 1 and AUTH_FAIL 0? That way they'll still correspond to the 'true' and 'false' that are being used now. I've attached another variant that does this.
          Hide
          moquist Matt Oquist added a comment -

          This swaps the values of AUTH_FAIL and AUTH_OK so they correspond to 'false' and 'true'.

          Show
          moquist Matt Oquist added a comment - This swaps the values of AUTH_FAIL and AUTH_OK so they correspond to 'false' and 'true'.
          Hide
          iarenaza Iñaki Arenaza added a comment -

          Matt,

          while your last change makes certain sense, I'd explicitly code a check for AUTH_FAIL or 'false' in the last hunk of the patch. This would make the code more obvious to anyone having a look at it, which is a good thing when dealing with security related code Other than that, it looks good to me.

          Saludos
          Iñaki.

          Show
          iarenaza Iñaki Arenaza added a comment - Matt, while your last change makes certain sense, I'd explicitly code a check for AUTH_FAIL or 'false' in the last hunk of the patch. This would make the code more obvious to anyone having a look at it, which is a good thing when dealing with security related code Other than that, it looks good to me. Saludos Iñaki.
          Hide
          marina Marina Glancy added a comment -

          We have detected that this issue has been inactive for over two years and also did not collect many votes. It is possible that it has been already implemented in a more recent version of Moodle, or it is not highly demanded. There are unlimited number of ways Moodle functinality can be expanded and improved but we would like to concentrate on the features that will benefit majority of users, and which can not be implemented as plugins. If you have a suggestion for improving Moodle core, and there is no open issue for it in the tracker, please start a new forum discussion to see how many other users agree with you, and then create a new issue providing as many details as possible.

          ==BLK2YIMP20141121==

          Show
          marina Marina Glancy added a comment - We have detected that this issue has been inactive for over two years and also did not collect many votes. It is possible that it has been already implemented in a more recent version of Moodle, or it is not highly demanded. There are unlimited number of ways Moodle functinality can be expanded and improved but we would like to concentrate on the features that will benefit majority of users, and which can not be implemented as plugins. If you have a suggestion for improving Moodle core, and there is no open issue for it in the tracker, please start a new forum discussion to see how many other users agree with you, and then create a new issue providing as many details as possible. ==BLK2YIMP20141121==

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: