Moodle
  1. Moodle
  2. MDL-33562

mdl_user_private_key's are not cleaned up when a user is deleted

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3, 2.3.7, 2.4.4, 2.5
    • Fix Version/s: 2.3.8, 2.4.5, 2.5.1
    • Component/s: Libraries
    • Labels:
    • Testing Instructions:
      Hide

      BEFORE UPGRADE:

      1. Enable RSS in advanced settings and create a forum with rss enabled
      2. Create a user
      3. Login as that user and make a note of their userid
      4. visit the forum as that user
      5. Click the rss link in the settings block of any forum (you should get a link with a token in the middle)
      6. Logout as that user
      7. Use database to verify that a token exists for the user you created and logged in as
        SELECT * FROM mdl_user_private_key WHERE userid = USERIDYOUFOUND;
      8. Login as an admin and delete the user you created
      9. Use the database to verify that the token still exists

      UPGRADE..

      1. VERIFY: that the users token that you just checked for has now been removed
      2. VERIFY: Repeat the pre-upgrade steps and VERIFY that the users token is deleted now when the user is deleted.
      Show
      BEFORE UPGRADE: Enable RSS in advanced settings and create a forum with rss enabled Create a user Login as that user and make a note of their userid visit the forum as that user Click the rss link in the settings block of any forum (you should get a link with a token in the middle) Logout as that user Use database to verify that a token exists for the user you created and logged in as SELECT * FROM mdl_user_private_key WHERE userid = USERIDYOUFOUND; Login as an admin and delete the user you created Use the database to verify that the token still exists UPGRADE.. VERIFY: that the users token that you just checked for has now been removed VERIFY: Repeat the pre-upgrade steps and VERIFY that the users token is deleted now when the user is deleted.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull 2.4 Branch:
    • Pull 2.5 Branch:
    • Pull Master Branch:
      MDL-33562-master
    • Rank:
      41491

      Description

      I discovered while testing MDL-33514 that user_private_key information is not removed when a user is deleted.

      This issue is to remove it when user is deleted and add an upgrade step to remove old keys which aren't deleted.

        Issue Links

          Activity

          Hide
          Dan Poltawski added a comment -

          Sending for peer review the master only branch.

          Show
          Dan Poltawski added a comment - Sending for peer review the master only branch.
          Hide
          Dan Poltawski added a comment -

          No peer reviewer, sending for integration.

          Show
          Dan Poltawski added a comment - No peer reviewer, sending for integration.
          Hide
          Dan Poltawski added a comment -

          I've run the upgrade step on all dbs to verify its ok, I think the subselect is the only cross-db way to do it..

          It is a bit of a weird step to do NOT EXISTS rather than exists with deleted = 1, but this means we can get rid of all the bad records.

          Show
          Dan Poltawski added a comment - I've run the upgrade step on all dbs to verify its ok, I think the subselect is the only cross-db way to do it.. It is a bit of a weird step to do NOT EXISTS rather than exists with deleted = 1, but this means we can get rid of all the bad records.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yeah, "delete joins" and really horrible, specially under MySQL (see MDL-29589).

          So right now, yes, surely it's the only way to make it cross-db, although it would be worth trying it in the moodle.org testing site (massive number of users). Or at least change the "DELETE" by a "SELECT" and try it.

          Anyway, I'm holding this for next week... once the on-sync period expires 100% (we are right now over the line).

          Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Yeah, "delete joins" and really horrible, specially under MySQL (see MDL-29589 ). So right now, yes, surely it's the only way to make it cross-db, although it would be worth trying it in the moodle.org testing site (massive number of users). Or at least change the "DELETE" by a "SELECT" and try it. Anyway, I'm holding this for next week... once the on-sync period expires 100% (we are right now over the line). Thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23, 24, 25 & master), fixing via merge commit all the versions, thanks!

          My only concern is if this can be extremely slow with zillions of users (under MySQL) and perhaps it would need some notice saying it "may be long". NVN.

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23, 24, 25 & master), fixing via merge commit all the versions, thanks! My only concern is if this can be extremely slow with zillions of users (under MySQL) and perhaps it would need some notice saying it "may be long". NVN.
          Hide
          Ankit Agarwal added a comment -

          Works as described.
          Thanks

          Show
          Ankit Agarwal added a comment - Works as described. Thanks
          Hide
          Dan Poltawski added a comment -

          Thanks for your contributions!

          _main:
          @ BB#0:
                  push    {r7, lr}
                  mov     r7, sp
                  sub     sp, #4
                  movw    r0, :lower16:(L_.str-(LPC0_0+4))
                  movt    r0, :upper16:(L_.str-(LPC0_0+4))
          LPC0_0:
                  add     r0, pc
                  bl      _printf
                  movs    r1, #0
                  movt    r1, #0
                  str     r0, [sp]                @ 4-byte Spill
                  mov     r0, r1
                  add     sp, #4
                  pop     {r7, pc}
          
                  .section        __TEXT,__cstring,cstring_literals
          L_.str:                                 @ @.str
                  .asciz   "This code is now upstream!"
          
          Show
          Dan Poltawski added a comment - Thanks for your contributions! _main: @ BB#0: push {r7, lr} mov r7, sp sub sp, #4 movw r0, :lower16:(L_.str-(LPC0_0+4)) movt r0, :upper16:(L_.str-(LPC0_0+4)) LPC0_0: add r0, pc bl _printf movs r1, #0 movt r1, #0 str r0, [sp] @ 4- byte Spill mov r0, r1 add sp, #4 pop {r7, pc} .section __TEXT,__cstring,cstring_literals L_.str: @ @.str .asciz "This code is now upstream!"

            People

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

              Dates

              • Created:
                Updated:
                Resolved: