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

          Attachments

            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: