Moodle
  1. Moodle
  2. MDL-29665

auth/db plugin misses user_confirm() method

    Details

    • Testing Instructions:
      Hide
      1. Log in as an administrator and visit <yoursite>/admin/settings.php?section=manageauths.
      2. Ensure the 'Email-based self-registration' plugin is activated and 'Self registration' is set to 'Email-based self-registration'.
      3. Enable the 'External database' plugin.
      4. Click on the settings for the 'External database' and set it up as per http://docs.moodle.org/23/en/External_database_enrolment.
      5. Register an account on your Moodle site, do not click on the link sent by the activation email, we want the user to remain unconfirmed.
      6. As the administrator browse the list of users and choose to edit the newly registered user.
      7. Set the newly registered users authentication method to 'External database'.
      8. Set up an account on the external database for this user.
      9. Ensure you can log in as this user despite your account not being confirmed.
      10. Once logged in check the database and ensure the 'confirm' column in the 'user' table is set to 1 for that user.
      Show
      Log in as an administrator and visit <yoursite>/admin/settings.php?section=manageauths. Ensure the 'Email-based self-registration' plugin is activated and 'Self registration' is set to 'Email-based self-registration'. Enable the 'External database' plugin. Click on the settings for the 'External database' and set it up as per http://docs.moodle.org/23/en/External_database_enrolment . Register an account on your Moodle site, do not click on the link sent by the activation email, we want the user to remain unconfirmed. As the administrator browse the list of users and choose to edit the newly registered user. Set the newly registered users authentication method to 'External database'. Set up an account on the external database for this user. Ensure you can log in as this user despite your account not being confirmed. Once logged in check the database and ensure the 'confirm' column in the 'user' table is set to 1 for that user.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull 2.4 Branch:
      MDL-29665_m24
    • Pull Master Branch:
      MDL-29665_master
    • Rank:
      19172

      Description

      The auth db plugin does not has the user_confirm() method. This may lead into some problem with special auth flows depending on external db where confirmation is needed, like the auth/manual pluign does. Other standard plugins, depending on external backend such as auth/LDAP, has the methods user_confirm() and can_confirm() as well.

      Solution:
      Add the missing method to auth/db taking it from auth/manual

        Activity

        Hide
        Eloy Lafuente (stronk7) added a comment -

        +1

        Show
        Eloy Lafuente (stronk7) added a comment - +1
        Hide
        Saswat Padhi added a comment -

        Yes, this issue might arise if the user is created and then auth method is changed to auth/db before the user confirms.

        The patch is attached and is adapted from auth/manual.

        Show
        Saswat Padhi added a comment - Yes, this issue might arise if the user is created and then auth method is changed to auth/db before the user confirms. The patch is attached and is adapted from auth/manual.
        Hide
        Mark Nelson added a comment -

        Hi Saswat, thanks for the patch. I have applied the patch and pushed to my github.

        Show
        Mark Nelson added a comment - Hi Saswat, thanks for the patch. I have applied the patch and pushed to my github.
        Hide
        Mark Nelson added a comment -

        I have reviewed the patch and added testing instructions. Pushing to integration.

        Show
        Mark Nelson added a comment - I have reviewed the patch and added testing instructions. Pushing 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
        Eloy Lafuente (stronk7) added a comment -

        Uhm, sorry, I'm reopening this:

        1) if the auth method is changed (hacking the DB, I suppose), then you should confirm those users too.
        2) The user_confirm() method should only exist, IMO, if the auth supports confirmation.
        3) In any case, the proposed implementation seems incorrect, as far as it does not verify the secret at all and, instead, performs "automatic" confirmation always.

        I've added Petr here know a bit more about when and how that method should be available.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Uhm, sorry, I'm reopening this: 1) if the auth method is changed (hacking the DB, I suppose), then you should confirm those users too. 2) The user_confirm() method should only exist, IMO, if the auth supports confirmation. 3) In any case, the proposed implementation seems incorrect, as far as it does not verify the secret at all and, instead, performs "automatic" confirmation always. I've added Petr here know a bit more about when and how that method should be available. Ciao
        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
        Mark Nelson added a comment -

        Hi Eloy, I am fine with closing this issue and setting it to not fix. During our sprint our aim was to get patched issues closed, so I chose this one as there was a patch provided and it had your +1 (albeit from a while ago).

        Show
        Mark Nelson added a comment - Hi Eloy, I am fine with closing this issue and setting it to not fix. During our sprint our aim was to get patched issues closed, so I chose this one as there was a patch provided and it had your +1 (albeit from a while ago).
        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.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: