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

Allow auth plugins to provide custom user signup forms

    Details

      Description

      It would be very useful if authentication plugins that allow self-registration were able to override the built-in signup form. Plugins appear to be able to implement sign-up however they like using the user_signup method, but they're stuck with the existing form which won't always be appropriate.

      The main use case I'm thinking of here is for (a custom version of) the Shibboleth plugin to be able to capture Shibboleth attributes as part of the user sign-up. At our institution we're using Shibboleth for all logins, and are hoping eventually to register our server with the UK Federation to enable us to give access to colleagues from other universities. (We were unable to do this before we upgraded to Moodle 2 and were able to switch on "authpreventaccountcreation" since anyone in a trusted university could have potentially created an account, but now we're in a position to do so.) What we need now is a way for people to request Moodle accounts, and I don't see any technical reason why this shouldn't be the self registration page if it was Shibboleth protected.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            maherne Michael Aherne added a comment -

            Attached a small patch that implements this functionality.

            Show
            maherne Michael Aherne added a comment - Attached a small patch that implements this functionality.
            Hide
            skodak Petr Skoda added a comment -

            I like this idea, +1. Maybe it would be better to move the form include together with the constructor.

            Show
            skodak Petr Skoda added a comment - I like this idea, +1. Maybe it would be better to move the form include together with the constructor.
            Hide
            maherne Michael Aherne added a comment -

            Thanks, Petr. I've incorporated your suggestion into the patch.

            Show
            maherne Michael Aherne added a comment - Thanks, Petr. I've incorporated your suggestion into the patch.
            Hide
            maherne Michael Aherne added a comment -

            Just realised that in the issue description I've outlined a pretty complex use case and missed the most obvious one: adding extra validation to the existing form (e.g. allowing only ".ac.uk" email addresses or whatever)

            Show
            maherne Michael Aherne added a comment - Just realised that in the issue description I've outlined a pretty complex use case and missed the most obvious one: adding extra validation to the existing form (e.g. allowing only ".ac.uk" email addresses or whatever)
            Hide
            skodak Petr Skoda added a comment -

            For me this change means "let the auth plugin decide how to sign up", please submit it for integration once you see it ready.

            Show
            skodak Petr Skoda added a comment - For me this change means "let the auth plugin decide how to sign up", please submit it for integration once you see it ready.
            Hide
            maherne Michael Aherne added a comment -

            Thanks, Petr. I'm happy for this to be submitted for integration as it is, but I don't have the tracker permissions to do it myself.

            Show
            maherne Michael Aherne added a comment - Thanks, Petr. I'm happy for this to be submitted for integration as it is, but I don't have the tracker permissions to do it myself.
            Hide
            skodak Petr Skoda added a comment -

            oh, done

            Show
            skodak Petr Skoda added a comment - oh, done
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Guys,

            This change looks good and useful to me. Just two things missing:

            • We should mention this new capability in upgrade.txt. Michael are you able to add a brief mention of it in that file in your patch?
            • We need some testing instructions for this, regression testing current pugins with signup forms should suffice. Though if there is a template plugin, it'd be great if we could test with that.

            Once this is integrated, we should also update the docs to reflect this new capability, i've added the api_change tag for this.

            Show
            poltawski Dan Poltawski added a comment - Hi Guys, This change looks good and useful to me. Just two things missing: We should mention this new capability in upgrade.txt. Michael are you able to add a brief mention of it in that file in your patch? We need some testing instructions for this, regression testing current pugins with signup forms should suffice. Though if there is a template plugin, it'd be great if we could test with that. Once this is integrated, we should also update the docs to reflect this new capability, i've added the api_change tag for this.
            Hide
            maherne Michael Aherne added a comment -

            Thanks for the feedback, Dan. I've added a bit to upgrade.txt and rebased it to the latest master. I've also added some regression testing instructions. Cheers!

            Show
            maherne Michael Aherne added a comment - Thanks for the feedback, Dan. I've added a bit to upgrade.txt and rebased it to the latest master. I've also added some regression testing instructions. Cheers!
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Michael, i've integrated this now.

            Show
            poltawski Dan Poltawski added a comment - Thanks Michael, i've integrated this now.
            Hide
            abgreeve Adrian Greeve added a comment -

            I tested the self registration with email. That worked fine. I had a talk with Dan and he said that skipping testing with LDAP, while not ideal, would be okay in this situation.
            Test passed.

            Show
            abgreeve Adrian Greeve added a comment - I tested the self registration with email. That worked fine. I had a talk with Dan and he said that skipping testing with LDAP, while not ideal, would be okay in this situation. Test passed.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Fixed STOP Closed STOP Thanks STOP

            Yay, imagination! Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Fixed STOP Closed STOP Thanks STOP Yay, imagination! Ciao
            Hide
            skodak Petr Skoda added a comment -

            Removing the dev docs flag, the info in upgrade.txt should be enough, thanks again

            Show
            skodak Petr Skoda added a comment - Removing the dev docs flag, the info in upgrade.txt should be enough, thanks again

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Dec/12