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

          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