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

Upload users option only allow the following authentication options: Manual account, or No login

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.3
    • Fix Version/s: 2.4.6, 2.5.2
    • Component/s: Administration
    • Labels:
    • Database:
      MySQL
    • Testing Instructions:
      Hide
      1. Enable LDAP Authentication and Shibboleth by visiting Site administration > Plugins > Authentication > Manage authentication.
      2. Upload test CSV user file (users-upload-test-auth.csv) via the page Site administration > Users > Accounts > Upload users > Choose file and upload.
      3. In the Upload users preview page scroll down to the Default values section and select the drop-down menu in 'Choose an authentication method' and ensure you see 'LDAP server' in the following options but not 'Shibboleth'.
      4. Upload the file and ensure it works as expected.

      For master only

      1. Check that there is a nice explanation in auth/upgrade.txt documenting the new function.
      Show
      Enable LDAP Authentication and Shibboleth by visiting Site administration > Plugins > Authentication > Manage authentication. Upload test CSV user file (users-upload-test-auth.csv) via the page Site administration > Users > Accounts > Upload users > Choose file and upload. In the Upload users preview page scroll down to the Default values section and select the drop-down menu in 'Choose an authentication method' and ensure you see 'LDAP server' in the following options but not 'Shibboleth'. Upload the file and ensure it works as expected. For master only Check that there is a nice explanation in auth/upgrade.txt documenting the new function.
    • Workaround:
      Hide

      Specify the authentication plugin in the CSV file by adding an 'auth' column.

      Show
      Specify the authentication plugin in the CSV file by adding an 'auth' column.
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-27953_master
    • Sprint:
      BACKEND Sprint 3
    • Story Points (Obsolete):
      8
    • Sprint:
      BACKEND Sprint 3

      Description

      When uploading a CSV file of users in 2.0.3 the "Choose an authentication" drop-down only gives the following options:

      Manual account
      No login

      In 1.9.x I get the following authentication options:

      Manual account
      No login
      LDAP server
      Email-based self-registration

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              aborrow Anthony Borrow added a comment -

              John Paul - Thanks for filing this issue. I would expect LDAP to be listed if it is enabled. Would you mind updating the testing instructions so that it is more step-by-step for the developer. Perhaps you could provide a sample csv file that could be used. In creating a new user, currently all of the authentication plugins are listed. I'm not sure if that is really helpful if they are not enabled. Might it be better if it only showed the available plugins. I am wanting to ensure that the behavior is consistent whether we are creating a new user from the UI, uploading new users via CSV, etc. Thanks for helping to think through exactly what we desire to happen in these scenarios. Peace - Anthony

              Show
              aborrow Anthony Borrow added a comment - John Paul - Thanks for filing this issue. I would expect LDAP to be listed if it is enabled. Would you mind updating the testing instructions so that it is more step-by-step for the developer. Perhaps you could provide a sample csv file that could be used. In creating a new user, currently all of the authentication plugins are listed. I'm not sure if that is really helpful if they are not enabled. Might it be better if it only showed the available plugins. I am wanting to ensure that the behavior is consistent whether we are creating a new user from the UI, uploading new users via CSV, etc. Thanks for helping to think through exactly what we desire to happen in these scenarios. Peace - Anthony
              Hide
              johnpaultrack John Paul Posada added a comment -

              Hi Anthony,

              I agree that the behaviour should be consistant. I've updated the instructions as requested also.

              Cheers,
              John Paul

              Show
              johnpaultrack John Paul Posada added a comment - Hi Anthony, I agree that the behaviour should be consistant. I've updated the instructions as requested also. Cheers, John Paul
              Hide
              aborrow Anthony Borrow added a comment -

              Thanks for the updated instructions - those are quite helpful. Peace - Anthony

              Show
              aborrow Anthony Borrow added a comment - Thanks for the updated instructions - those are quite helpful. Peace - Anthony
              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for reporting this and providing additional details.

              I've put it on our backlog and we'll try to get to it as soon as we can.

              In the meantime adding if you have a chance to work on a code solution, sharing that will help us and other users.

              Show
              salvetore Michael de Raadt added a comment - Thanks for reporting this and providing additional details. I've put it on our backlog and we'll try to get to it as soon as we can. In the meantime adding if you have a chance to work on a code solution, sharing that will help us and other users.
              Hide
              snydert Tamara Snyder added a comment -

              Hi - can you please confirm if this workaround works in 2.1? I don't have any dropdown to select authentication mode, I am using auth column in my upload users csv file with value set to cas which is what we are using. This worked fine in 2.0 but is not working with 2.1. Did test using same exact file to confirm and what succeeds in 2.0 gives error message in 2.1: "auth plugin not supported here." Need to upload several hundred users before classes start in few days. Any help greatly appreciated.

              Thanks,
              Tammy

              Show
              snydert Tamara Snyder added a comment - Hi - can you please confirm if this workaround works in 2.1? I don't have any dropdown to select authentication mode, I am using auth column in my upload users csv file with value set to cas which is what we are using. This worked fine in 2.0 but is not working with 2.1. Did test using same exact file to confirm and what succeeds in 2.0 gives error message in 2.1: "auth plugin not supported here." Need to upload several hundred users before classes start in few days. Any help greatly appreciated. Thanks, Tammy
              Hide
              snydert Tamara Snyder added a comment -

              Apologies - the workaround is working in 2.1. I did not check after receiving the error that user was properly created. They are in fact created despite the error. We just did not get the error in 2.0.

              Show
              snydert Tamara Snyder added a comment - Apologies - the workaround is working in 2.1. I did not check after receiving the error that user was properly created. They are in fact created despite the error. We just did not get the error in 2.0.
              Hide
              antonvaltaz Daniel Walters added a comment -

              The workaround does not work for me in 2.0.3+ (I am updating existing users, not creating new ones).

              I get the error:

              manual
              manual-->ldap
              Auth plugin not supported here

              But when I go to check on individual existing users, they are still listed as using manual authentication.

              Show
              antonvaltaz Daniel Walters added a comment - The workaround does not work for me in 2.0.3+ (I am updating existing users, not creating new ones). I get the error: manual manual-->ldap Auth plugin not supported here But when I go to check on individual existing users, they are still listed as using manual authentication.
              Hide
              hagelk Kris Hagel added a comment -

              This is still broken in Moodle 2.2.3+ (Build: 20120615). Also, the work around suggested does not work.

              Show
              hagelk Kris Hagel added a comment - This is still broken in Moodle 2.2.3+ (Build: 20120615). Also, the work around suggested does not work.
              Hide
              markn Mark Nelson added a comment -

              There is a function 'uu_supported_auths' that returns the authentication options you can choose when uploading users, however this has been hard coded to use 'manual', 'nologin', 'none' and 'email'. The PHPDocs for the function specify "If ppl want to use some other auth type they have to include it in the CSV file next on each line.". I need to discuss whether this should be changed, and if so, how are we going to implement it. Maybe by default the authentication method can not be selected unless it specifies that it can ..

              Show
              markn Mark Nelson added a comment - There is a function 'uu_supported_auths' that returns the authentication options you can choose when uploading users, however this has been hard coded to use 'manual', 'nologin', 'none' and 'email'. The PHPDocs for the function specify "If ppl want to use some other auth type they have to include it in the CSV file next on each line.". I need to discuss whether this should be changed, and if so, how are we going to implement it. Maybe by default the authentication method can not be selected unless it specifies that it can ..
              Hide
              markn Mark Nelson added a comment -

              I am changing this from a bug to an improvement and lowering the priority. The code is working as expected, although it can be improved on.

              Show
              markn Mark Nelson added a comment - I am changing this from a bug to an improvement and lowering the priority. The code is working as expected, although it can be improved on.
              Hide
              markn Mark Nelson added a comment -

              Ok, so 1.9 any plugin that was enabled was added to the drop down list, there was no restriction. In 2.0 there was an array created that had a hard coded list of authentication plugins that can be used, as stated above. The master diff I created introduces a new function that authentication plugins can override if they wish to be added to the list, the 2.3 and 2.4 fix is simply to add ldap to the array of allowed plugins.

              Show
              markn Mark Nelson added a comment - Ok, so 1.9 any plugin that was enabled was added to the drop down list, there was no restriction. In 2.0 there was an array created that had a hard coded list of authentication plugins that can be used, as stated above. The master diff I created introduces a new function that authentication plugins can override if they wish to be added to the list, the 2.3 and 2.4 fix is simply to add ldap to the array of allowed plugins.
              Hide
              abgreeve Adrian Greeve added a comment -

              [Y] Syntax
              [Y] Output
              [Y] Whitespace
              [-] Language
              [-] Databases
              [*] Testing
              [-] Security
              [-] Documentation
              [Y] Git
              [Y] Sanity check

              Hello Mark,

              I like the solution that you have given here. I can't spot anything wrong with it.

              Just one small thing. Perhaps you could change the name of the file that you are supposed to download and use to USERUPLOAD-NO-AUTH.csv. The other file has the 'auth' field filled out and so the option that you mention in the testing instructions does not get displayed.

              Thanks.

              Show
              abgreeve Adrian Greeve added a comment - [Y] Syntax [Y] Output [Y] Whitespace [-] Language [-] Databases [*] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check Hello Mark, I like the solution that you have given here. I can't spot anything wrong with it. Just one small thing. Perhaps you could change the name of the file that you are supposed to download and use to USERUPLOAD-NO-AUTH.csv. The other file has the 'auth' field filled out and so the option that you mention in the testing instructions does not get displayed. Thanks.
              Hide
              markn Mark Nelson added a comment -

              As discussed, Adrian was looking at the workaround instructions, not my testing instructions. The file to use is users-upload-test-auth.csv as specified. Submitting to integration.

              Show
              markn Mark Nelson added a comment - As discussed, Adrian was looking at the workaround instructions, not my testing instructions. The file to use is users-upload-test-auth.csv as specified. Submitting to integration.
              Hide
              damyon Damyon Wiese added a comment -

              The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

              Thanks!

              Show
              damyon Damyon Wiese added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Thanks!
              Hide
              poltawski Dan Poltawski added a comment -

              Hi Mark,

              Looks fine, but we've not had Petr involved in this issue? Please involve him as component maintainer. Add him as a watcher at the least.

              Show
              poltawski Dan Poltawski added a comment - Hi Mark, Looks fine, but we've not had Petr involved in this issue? Please involve him as component maintainer. Add him as a watcher at the least.
              Hide
              cibot CiBoT added a comment -

              Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

              Show
              cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
              Hide
              cibot CiBoT added a comment -

              Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

              Show
              cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
              Hide
              salvetore Michael de Raadt added a comment -

              Carrying into the next sprint so it can be completed.

              Show
              salvetore Michael de Raadt added a comment - Carrying into the next sprint so it can be completed.
              Hide
              markn Mark Nelson added a comment -

              Hi Petr, as the maintainer of the enrolment plugins, are you able to view these changes and let me know if they are acceptable or not? Thanks!

              Show
              markn Mark Nelson added a comment - Hi Petr, as the maintainer of the enrolment plugins, are you able to view these changes and let me know if they are acceptable or not? Thanks!
              Hide
              markn Mark Nelson added a comment -

              Hi Petr, I will be on holiday after today until April, so if you do view this and it seems fine can you push to integrations? Thanks.

              Show
              markn Mark Nelson added a comment - Hi Petr, I will be on holiday after today until April, so if you do view this and it seems fine can you push to integrations? Thanks.
              Hide
              skodak Petr Skoda added a comment -

              Hello,

              1/ please use get_auth_plugin() instead of manually creating instance
              2/ I like the new methods.
              3/ I do not understand why should admins upload ldap users, instead just execute ldap sync and done. It would be different if there was option to prevent adding of new accounts in ldap sync, but it is not there. my -2

              Show
              skodak Petr Skoda added a comment - Hello, 1/ please use get_auth_plugin() instead of manually creating instance 2/ I like the new methods. 3/ I do not understand why should admins upload ldap users, instead just execute ldap sync and done. It would be different if there was option to prevent adding of new accounts in ldap sync, but it is not there. my -2
              Hide
              markn Mark Nelson added a comment -

              Thanks Petr,

              1) Thanks, wasn't aware of that function. I agree, using this is better. Changed.
              3) I am not too sure myself as I have not had much experiencing using LDAP and Moodle. Maybe someone in the community can explain why ldap sync can not be used?

              Show
              markn Mark Nelson added a comment - Thanks Petr, 1) Thanks, wasn't aware of that function. I agree, using this is better. Changed. 3) I am not too sure myself as I have not had much experiencing using LDAP and Moodle. Maybe someone in the community can explain why ldap sync can not be used?
              Hide
              moodle.com moodle.com added a comment -

              Mark is away on holidays and will be back in a few weeks.

              I've carried this over to the next sprint so it can be completed when he returns.

              Show
              moodle.com moodle.com added a comment - Mark is away on holidays and will be back in a few weeks. I've carried this over to the next sprint so it can be completed when he returns.
              Hide
              markn Mark Nelson added a comment -

              The code is fine as I have addressed Petr's small point about using get_auth_plugin. Now it is simply a question as to why we should do this. As Petr stated in his third point, "why should admins upload ldap users, instead just execute ldap sync and done." I do not have enough knowledge on using LDAP and Moodle so cannot answer this. We are waiting on someone in the community to answer Petr's third point to provide a reason why this should be integrated.

              Show
              markn Mark Nelson added a comment - The code is fine as I have addressed Petr's small point about using get_auth_plugin. Now it is simply a question as to why we should do this. As Petr stated in his third point, "why should admins upload ldap users, instead just execute ldap sync and done." I do not have enough knowledge on using LDAP and Moodle so cannot answer this. We are waiting on someone in the community to answer Petr's third point to provide a reason why this should be integrated.
              Hide
              hagenjam James Hagen added a comment -

              I've been tracking this issue since discovering that when I uploaded details of users (i.e. existing LDAP-authenticated users) via CSV file in order to populate an existing cohort (see "Uploading users to a cohort" at http://docs.moodle.org/22/en/Cohorts), the users' records changed by default from LDAP authenticated to manual authenticated. This is unavoidable (we're using 2.4) because LDAP is not in the array of allowed plugins, as noted above. To answer the question at least in part, the ability to upload (ldap) users via csv file to populate cohorts definitely has benefits and "just executing an ldap sync" is not an alternative - for us.

              Show
              hagenjam James Hagen added a comment - I've been tracking this issue since discovering that when I uploaded details of users (i.e. existing LDAP-authenticated users) via CSV file in order to populate an existing cohort (see "Uploading users to a cohort" at http://docs.moodle.org/22/en/Cohorts ), the users' records changed by default from LDAP authenticated to manual authenticated. This is unavoidable (we're using 2.4) because LDAP is not in the array of allowed plugins, as noted above. To answer the question at least in part, the ability to upload (ldap) users via csv file to populate cohorts definitely has benefits and "just executing an ldap sync" is not an alternative - for us.
              Hide
              markn Mark Nelson added a comment -

              Thanks for your input James. Petr, what is your opinion on James's situation? I am going to take this off the current sprint and set it to backlog until a solution is finalised.

              Show
              markn Mark Nelson added a comment - Thanks for your input James. Petr, what is your opinion on James's situation? I am going to take this off the current sprint and set it to backlog until a solution is finalised.
              Hide
              markn Mark Nelson added a comment -

              Hey Petr Skoda, I have been going through my old issues and found this. Can you please review James's last comment abt why this would be useful and let me know if you think this should be integrated (obviously after a rebase since it was done so long ago).

              Show
              markn Mark Nelson added a comment - Hey Petr Skoda , I have been going through my old issues and found this. Can you please review James's last comment abt why this would be useful and let me know if you think this should be integrated (obviously after a rebase since it was done so long ago).
              Hide
              markn Mark Nelson added a comment -

              I spoke to Petr today and he thinks that the concept is fine. I will be pushing to integration shortly once I have rebased and considered a new function name other than "can_be_manually_assigned", as we use the term "assign" when talking about roles so it may be confusing. This will go into 2.4, 2.5 and master.

              Show
              markn Mark Nelson added a comment - I spoke to Petr today and he thinks that the concept is fine. I will be pushing to integration shortly once I have rebased and considered a new function name other than "can_be_manually_assigned", as we use the term "assign" when talking about roles so it may be confusing. This will go into 2.4, 2.5 and master.
              Hide
              markn Mark Nelson added a comment -

              I changed the function name to can_be_manually_set. Submitting to integration.

              Show
              markn Mark Nelson added a comment - I changed the function name to can_be_manually_set. Submitting to integration.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks Mark this has been integrated now.

              Great to see the discussion on the requirement for these changes, that was the first thing I thought myself reading thought the description.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Mark this has been integrated now. Great to see the discussion on the requirement for these changes, that was the first thing I thought myself reading thought the description.
              Hide
              jerome Jérôme Mouneyrac added a comment - - edited

              tested on master, 2.5 and 2.4, all works.

              Show
              jerome Jérôme Mouneyrac added a comment - - edited tested on master, 2.5 and 2.4, all works.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Huzzah, your code made it into Moodle. Perhaps now things are ever so slightly better!

              "The ship can't take this much pressure. Sometimes it falls apart just sitting in the hangar."
              ~ Professor Farnsworth

              Show
              samhemelryk Sam Hemelryk added a comment - Huzzah, your code made it into Moodle. Perhaps now things are ever so slightly better! "The ship can't take this much pressure. Sometimes it falls apart just sitting in the hangar." ~ Professor Farnsworth
              Hide
              cttxg Teresa Gibbison added a comment - - edited

              I found this issue while reviewing our core code hacks. We use an institution specific authentication method so have to manually add it to the uu_supported_auths $whitelist array. I understand the thought of hardcoding these as a "guarantee" they will work but it occurred to me that as an Administrator using an authentication method of our choice which obviously works properly (or we wouldn't be able to use it!) I should be able to configure whitelist (rather than just hacking the code).
              What are other's thoughts on allowing admins to configure their uu_supported_auths?

              --edit: sorry, looking at the master diff (rather than the 2.5 diff) this looks like it's going to be configurable - thanks

              Show
              cttxg Teresa Gibbison added a comment - - edited I found this issue while reviewing our core code hacks. We use an institution specific authentication method so have to manually add it to the uu_supported_auths $whitelist array. I understand the thought of hardcoding these as a "guarantee" they will work but it occurred to me that as an Administrator using an authentication method of our choice which obviously works properly (or we wouldn't be able to use it!) I should be able to configure whitelist (rather than just hacking the code). What are other's thoughts on allowing admins to configure their uu_supported_auths? --edit: sorry, looking at the master diff (rather than the 2.5 diff) this looks like it's going to be configurable - thanks
              Hide
              markn Mark Nelson added a comment -

              Hi Teresa,

              We don't tend to introduce new functions into existing APIs in stable branches, which is why the diffs differ dramatically (alliteration ftw).

              Regards,

              Mark

              Show
              markn Mark Nelson added a comment - Hi Teresa, We don't tend to introduce new functions into existing APIs in stable branches, which is why the diffs differ dramatically (alliteration ftw). Regards, Mark
              Hide
              timhunt Tim Hunt added a comment -

              Good news! the feature I need has already been implemented.

              Bad news it is 2.6 only.

              So, the fact the upgrade.txt file was committed to 2.5.2 is just confusing, since can_be_manually_set does not actually exist there.

              It is also a bit confusing that the can_be_manually_set API does not seem to affect the standard create users form, just the bulk upload one.

              Show
              timhunt Tim Hunt added a comment - Good news! the feature I need has already been implemented. Bad news it is 2.6 only. So, the fact the upgrade.txt file was committed to 2.5.2 is just confusing, since can_be_manually_set does not actually exist there. It is also a bit confusing that the can_be_manually_set API does not seem to affect the standard create users form, just the bulk upload one.
              Hide
              markn Mark Nelson added a comment -

              Wow Tim, you are right. The documentation should not have been pushed into 2.4 or 2.5! I will create a tracker issue to remove these. In the testing instructions it does say 'for master only' so I assume it was mistakenly integrated into those branches.

              Show
              markn Mark Nelson added a comment - Wow Tim, you are right. The documentation should not have been pushed into 2.4 or 2.5! I will create a tracker issue to remove these. In the testing instructions it does say 'for master only' so I assume it was mistakenly integrated into those branches.
              Hide
              markn Mark Nelson added a comment -

              The commit I created describing the change in the documentation has been altered for 2.5 and 2.4 to display those version numbers. I only ever created one for master. Seems like there was some confusion when it was integrated where it as assumed the new API was applied to all branches.

              Show
              markn Mark Nelson added a comment - The commit I created describing the change in the documentation has been altered for 2.5 and 2.4 to display those version numbers. I only ever created one for master. Seems like there was some confusion when it was integrated where it as assumed the new API was applied to all branches.
              Hide
              markn Mark Nelson added a comment -

              I have fixed this in MDL-42421 - thanks for spotting this Tim.

              Show
              markn Mark Nelson added a comment - I have fixed this in MDL-42421 - thanks for spotting this Tim.
              Hide
              poltawski Dan Poltawski added a comment -

              Does anyone know why this new auth api feature shouldn't apply to manual user creation? See MDL-50462

              Show
              poltawski Dan Poltawski added a comment - Does anyone know why this new auth api feature shouldn't apply to manual user creation? See MDL-50462
              Hide
              jennerm Matt Jenner added a comment -

              Sorry to dig this up - but when I use Upload Users and upload a bunch with 'shibboleth' as the auth the tool states unknown authentication plugin and seems to fail. But it does not fail; users can be changed from Manual to Shibboleth (or created as new) and you just ignore the 'failure' message.

              Secondly, if I update users with no auth field (to bulk enrol) it'll revert the auth back to 'manual'. This was how I discover the above, as I had to bulk change them all back to shibboleth and while ignoring errors, it does all work.

              i expect there's a gremlin in the code somewhere that's not showing much love for other auth types

              Show
              jennerm Matt Jenner added a comment - Sorry to dig this up - but when I use Upload Users and upload a bunch with 'shibboleth' as the auth the tool states unknown authentication plugin and seems to fail. But it does not fail; users can be changed from Manual to Shibboleth (or created as new) and you just ignore the 'failure' message. Secondly, if I update users with no auth field (to bulk enrol) it'll revert the auth back to 'manual'. This was how I discover the above, as I had to bulk change them all back to shibboleth and while ignoring errors, it does all work. i expect there's a gremlin in the code somewhere that's not showing much love for other auth types
              Hide
              markn Mark Nelson added a comment -

              Hi Matt, please create another issue and if possible include replication steps and the version of Moodle you are using. Thanks.

              Show
              markn Mark Nelson added a comment - Hi Matt, please create another issue and if possible include replication steps and the version of Moodle you are using. Thanks.

                People

                • Votes:
                  5 Vote for this issue
                  Watchers:
                  17 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    9/Sep/13