Moodle
  1. Moodle
  2. MDL-27953

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

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor 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 2.4 Branch:
      MDL-27953_m24
    • Pull 2.5 Branch:
      MDL-27953_m25
    • Pull Master Branch:
      MDL-27953_master
    • Story Points:
      8
    • Rank:
      17594
    • 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

      1. users-upload-test-auth.csv
        0.2 kB
        Mark Nelson
      2. USERUPLOAD-AUTH.csv
        0.3 kB
        John Paul Posada
      3. USERUPLOAD-NO-AUTH.csv
        0.3 kB
        John Paul Posada

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          Anthony Borrow added a comment -

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

          Show
          Anthony Borrow added a comment - Thanks for the updated instructions - those are quite helpful. Peace - Anthony
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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 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 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
          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
          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 added a comment -

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

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

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

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

          Carrying into the next sprint so it can be completed.

          Show
          Michael de Raadt added a comment - Carrying into the next sprint so it can be completed.
          Hide
          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
          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
          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
          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
          Petr Škoda 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
          Petr Škoda 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
          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
          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 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 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
          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
          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
          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
          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
          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
          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
          Mark Nelson added a comment -

          Hey Petr Škoda, 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
          Mark Nelson added a comment - Hey Petr Škoda , 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
          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
          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
          Mark Nelson added a comment -

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

          Show
          Mark Nelson added a comment - I changed the function name to can_be_manually_set. Submitting to integration.
          Hide
          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
          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
          Jérôme Mouneyrac added a comment - - edited

          tested on master, 2.5 and 2.4, all works.

          Show
          Jérôme Mouneyrac added a comment - - edited tested on master, 2.5 and 2.4, all works.
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Mark Nelson added a comment -

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

          Show
          Mark Nelson added a comment - I have fixed this in MDL-42421 - thanks for spotting this Tim.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Agile