Moodle
  1. Moodle
  2. MDL-27360

Web service tokens are displayed for deleted users

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2.1
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Web Services
    • Labels:
    • Testing Instructions:
      Hide
      • Create a service and authorise a user on it
      • Create a token for this user and this service
      • Delete the user but not the token
        => you should not see the token anymore displayed in the administration
        => the deleted user should not be authorised on the service anymore
      Show
      Create a service and authorise a user on it Create a token for this user and this service Delete the user but not the token => you should not see the token anymore displayed in the administration => the deleted user should not be authorised on the service anymore
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:

      Description

      1- Even though the core web service servers check if the user related to the token is deleted, it would be better to delete tokens when users are deleted. (specially for third party server not using the webservice_server class containing the authentication method then this would be a security issue)

      2- In the same time the administration should not display token for deleted users (patch in http://moodle.org/mod/forum/discuss.php?d=174506#p765320)

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            cheryl added a comment -

            In addition, you should not be able to see deleted users in the user list when adding new tokens. It is like the deleted user is not really deleted.

            Show
            cheryl added a comment - In addition, you should not be able to see deleted users in the user list when adding new tokens. It is like the deleted user is not really deleted.
            Hide
            Jérôme Mouneyrac added a comment -

            Thanks Cheryl for your initial report. Regarding your last comment, it's what I wanted to explain in the 2-

            Show
            Jérôme Mouneyrac added a comment - Thanks Cheryl for your initial report. Regarding your last comment, it's what I wanted to explain in the 2-
            Hide
            Jérôme Mouneyrac added a comment -

            And another issue to peer review Rosie

            Show
            Jérôme Mouneyrac added a comment - And another issue to peer review Rosie
            Hide
            Jérôme Mouneyrac added a comment -

            Note that in fact number 2) has been fixed in a previous issue a month ago.

            Show
            Jérôme Mouneyrac added a comment - Note that in fact number 2) has been fixed in a previous issue a month ago.
            Hide
            Rossiani Wijaya added a comment -

            Hi Jerome,

            If the external service is set for authorized user and the deleted user is in part of this authorize users, I think the deleted user should also be removed from the external_services_users table. Currently it doesn't show on the page, but it still exist in the database.

            Show
            Rossiani Wijaya added a comment - Hi Jerome, If the external service is set for authorized user and the deleted user is in part of this authorize users, I think the deleted user should also be removed from the external_services_users table. Currently it doesn't show on the page, but it still exist in the database.
            Hide
            Jérôme Mouneyrac added a comment -

            ah good catch Fixing it...

            Show
            Jérôme Mouneyrac added a comment - ah good catch Fixing it...
            Hide
            Eloy Lafuente (stronk7) 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.

            TIA and ciao

            Show
            Eloy Lafuente (stronk7) 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. TIA and ciao
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
            Hide
            Gerard Caulfield added a comment -

            Test passed in master, 22 and 21. Well done.

            Show
            Gerard Caulfield added a comment - Test passed in master, 22 and 21. Well done.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            A bit later this week, but finally your changes have been accepted and are now available in all the upstream git/cvs servers.

            Many thanks & ciao

            Show
            Eloy Lafuente (stronk7) added a comment - A bit later this week, but finally your changes have been accepted and are now available in all the upstream git/cvs servers. Many thanks & ciao

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: