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

Allow admin to remain logged in after login as procedure

    Details

    • Testing Instructions:
      Hide
      1. With new setting $CFG->simplereturnloginas disabled (default):
        • log in as somebody else and make sure you can only logout and are not able to return to your user
      1. Enable $CFG->simplereturnloginas (in experimental settings)
      2. Log in as another user
      3. Make sure you can return to original session through three different links (as in attached screenshot)
      4. Log in as another user again
      5. Visit /my/ page
      6. Make sure you can not return to original session and can only log out
      Show
      With new setting $CFG->simplereturnloginas disabled (default): log in as somebody else and make sure you can only logout and are not able to return to your user Enable $CFG->simplereturnloginas (in experimental settings) Log in as another user Make sure you can return to original session through three different links (as in attached screenshot) Log in as another user again Visit /my/ page Make sure you can not return to original session and can only log out
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_28_STABLE
    • Pull Master Branch:
      wip-MDL-24120-master
    • Story Points (Obsolete):
      20

      Description

      log in as admin to moodle 2
      go to a course (home page is enough)
      from participants block, select a student and login as him/her
      do something (at least nothing) and return to your personal account
      at the end of this process, you are always logged out

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            skodak Petr Skoda added a comment -

            yes, this is necessary for security reasons. We did not find any better way, sorry

            Show
            skodak Petr Skoda added a comment - yes, this is necessary for security reasons. We did not find any better way, sorry
            Hide
            andreabix Andrea Bicciolo added a comment -

            Petr,
            thank you for your clarification. That's werd , anyway if it is for security and there are not better way....

            However, we could help users who know the login as function (working since the very beginning up to moodle 1.9) to avoid to be stuck and think it is a bug (like me ).

            What about warn users when they click on "login as" ? Or let the know in some other way, such as a message? or any other better way I'm sure you can think of.

            Show
            andreabix Andrea Bicciolo added a comment - Petr, thank you for your clarification. That's werd , anyway if it is for security and there are not better way.... However, we could help users who know the login as function (working since the very beginning up to moodle 1.9) to avoid to be stuck and think it is a bug (like me ). What about warn users when they click on "login as" ? Or let the know in some other way, such as a message? or any other better way I'm sure you can think of.
            Hide
            skodak Petr Skoda added a comment -

            Yes, we could improve the loginas confirmation message. Unfortunately to make it really secure you should close and relaunch the browser.

            Show
            skodak Petr Skoda added a comment - Yes, we could improve the loginas confirmation message. Unfortunately to make it really secure you should close and relaunch the browser.
            Hide
            tsala Helen Foster added a comment -

            How about simply removing the Admin user link at the top right of the page, so that it only says "You are logged in as Sam Student (Logout)". Having an Admin link there too suggests that you can click on it and return to being logged in as an admin.

            Show
            tsala Helen Foster added a comment - How about simply removing the Admin user link at the top right of the page, so that it only says "You are logged in as Sam Student (Logout)". Having an Admin link there too suggests that you can click on it and return to being logged in as an admin.
            Hide
            skodak Petr Skoda added a comment -

            Helen: the link is useful because if you click it you get returned to the same course after login, if you click logout you go to the frontpage.

            Show
            skodak Petr Skoda added a comment - Helen: the link is useful because if you click it you get returned to the same course after login, if you click logout you go to the frontpage.
            Hide
            tsala Helen Foster added a comment -

            Wow, I never would have noticed that feature if you hadn't pointed it out Petr! Just wondering how useful it actually is though i.e. would an admin always want to be returned to the same course, or can we remove the feature for the sake of simplicity? Any comments from our watchers?

            Show
            tsala Helen Foster added a comment - Wow, I never would have noticed that feature if you hadn't pointed it out Petr! Just wondering how useful it actually is though i.e. would an admin always want to be returned to the same course, or can we remove the feature for the sake of simplicity? Any comments from our watchers?
            Hide
            andreabix Andrea Bicciolo added a comment -

            No great ideas here, sorry.... I think a messages could suffice.

            Show
            andreabix Andrea Bicciolo added a comment - No great ideas here, sorry.... I think a messages could suffice.
            Hide
            tsala Helen Foster added a comment -

            Suggestion from Bryan Dawson in http://moodle.org/mod/forum/discuss.php?d=161162:

            I think a tooltip with a message like 'Log out as other user and log back in as yourself' would explain what would happen without cluttering up the screen.

            Show
            tsala Helen Foster added a comment - Suggestion from Bryan Dawson in http://moodle.org/mod/forum/discuss.php?d=161162: I think a tooltip with a message like 'Log out as other user and log back in as yourself' would explain what would happen without cluttering up the screen.
            Hide
            digitalmess Aaron Cowell added a comment -

            I never noticed this before in my Beta and RC testing but I now see this happening on the migrated and upgraded server. This is certainly very strange behavior since 1.9+ did the user switching seemlessly. Now I use automatic login so logging back in as administrator is as simple as a finger swipe. I agree with Andrea that some sort of message could be displayed on the page after login as where you click continue alerting the administrator of being logged out when switching back to admin and/or a tooltip when hover over "admin" link at the end of the currently logged in user string. I'm going to imagin that other administrators will be reporting this as a bug when they upgrade to 2.0. Eventually it'll become the way of life once pervious versions of servers or phased out.

            What about an administrative option to relaunch browser upon user switching? As it was stated, true security would force logoff and reload browser. So if the Admin could enforce browser reload or not and obviously the browser reload would be off by default... Just a thought...

            Show
            digitalmess Aaron Cowell added a comment - I never noticed this before in my Beta and RC testing but I now see this happening on the migrated and upgraded server. This is certainly very strange behavior since 1.9+ did the user switching seemlessly. Now I use automatic login so logging back in as administrator is as simple as a finger swipe. I agree with Andrea that some sort of message could be displayed on the page after login as where you click continue alerting the administrator of being logged out when switching back to admin and/or a tooltip when hover over "admin" link at the end of the currently logged in user string. I'm going to imagin that other administrators will be reporting this as a bug when they upgrade to 2.0. Eventually it'll become the way of life once pervious versions of servers or phased out. What about an administrative option to relaunch browser upon user switching? As it was stated, true security would force logoff and reload browser. So if the Admin could enforce browser reload or not and obviously the browser reload would be off by default... Just a thought...
            Hide
            daveyboond Steve Bond added a comment -

            I'm probably being dumb but why is this necessary for security?

            This new feature is already really annoying me!

            Show
            daveyboond Steve Bond added a comment - I'm probably being dumb but why is this necessary for security? This new feature is already really annoying me!
            Hide
            djanjic Dragic Janjic added a comment -

            Am I understanding this issue correctly that the logout should happen and is expected due to security? If so, here is a question. When I login as a student for example, and click back on my name I am not logged out, just redirected back to the home page. A coworker of mine, gets logged out.

            Show
            djanjic Dragic Janjic added a comment - Am I understanding this issue correctly that the logout should happen and is expected due to security? If so, here is a question. When I login as a student for example, and click back on my name I am not logged out, just redirected back to the home page. A coworker of mine, gets logged out.
            Hide
            markpearson Mark Pearson added a comment -

            I second Steve Bond's comment. Why is this essential for security on 2.0 when it was not on 1.9? I hope that we're not going to use 'security' as a catch all reason for any arbitrary policy decision. As the Moodle admin I frequently need to 'login as' a user to check how things look to that user and then I do need to return to the same place.
            I guess I'm intrigued about what possible security breach could occur – cookie malfeasance?

            Show
            markpearson Mark Pearson added a comment - I second Steve Bond's comment. Why is this essential for security on 2.0 when it was not on 1.9? I hope that we're not going to use 'security' as a catch all reason for any arbitrary policy decision. As the Moodle admin I frequently need to 'login as' a user to check how things look to that user and then I do need to return to the same place. I guess I'm intrigued about what possible security breach could occur – cookie malfeasance?
            Hide
            leandrovilante Leandro Vilante added a comment -

            I agree with everyone that said it's an annoying functionality. When im configuring the moodle i need to test all the user roles and permissions, witch means a lot of 'login as' and 'login back'. And now it also means a lot of typing of password and loss of time. I'd like to point that i can't see how this will improve security. As administrator i would never leave my computer while logged in moodle or any other system, not even the OS, depending on the case. Also, no one will never blame the system if an administrator does it!

            Show
            leandrovilante Leandro Vilante added a comment - I agree with everyone that said it's an annoying functionality. When im configuring the moodle i need to test all the user roles and permissions, witch means a lot of 'login as' and 'login back'. And now it also means a lot of typing of password and loss of time. I'd like to point that i can't see how this will improve security. As administrator i would never leave my computer while logged in moodle or any other system, not even the OS, depending on the case. Also, no one will never blame the system if an administrator does it!
            Hide
            rrredmond Richard Redmond added a comment -

            This feature in 2.0 really slows down my technical support for my site. Many times I need to log in as a user in order to troubleshoot a problem. Other times, a student will ask me to upload work that they cannot upload and I need to login as that student. It seems to me that some of the beauty of Moodle is being lost in the upgrade to 2.0. The administrator is losing many of his/her management tools and it seems like the designers feel that the administrators cannot be trusted. ("Login As"/"Viewing Activity Reports from Profile"/"Allowing Course Files"

            Show
            rrredmond Richard Redmond added a comment - This feature in 2.0 really slows down my technical support for my site. Many times I need to log in as a user in order to troubleshoot a problem. Other times, a student will ask me to upload work that they cannot upload and I need to login as that student. It seems to me that some of the beauty of Moodle is being lost in the upgrade to 2.0. The administrator is losing many of his/her management tools and it seems like the designers feel that the administrators cannot be trusted. ("Login As"/"Viewing Activity Reports from Profile"/"Allowing Course Files"
            Hide
            jlynch John Lynch added a comment -

            At the risk of repeating what people have been asking on this thread for months, can someone please clarify what the "security issue" here is? For example, is there actual data that show that, when admin users "log in as" another user, they are more likely to forget that they are also logged in as an admin and therefore create a security risk? This is quite annoying to our entire team, and if Moodle doesn't want to remove this "feature" all together, we'd at least like the ability to easily turn it off.

            Show
            jlynch John Lynch added a comment - At the risk of repeating what people have been asking on this thread for months, can someone please clarify what the "security issue" here is? For example, is there actual data that show that, when admin users "log in as" another user, they are more likely to forget that they are also logged in as an admin and therefore create a security risk? This is quite annoying to our entire team, and if Moodle doesn't want to remove this "feature" all together, we'd at least like the ability to easily turn it off.
            Hide
            mikeyg007 Michael Green added a comment -

            I'd also like to know why it's necessary for security. Admins should be trusted to use this responsibility. It's a real drag to have to keep logging back in.

            Show
            mikeyg007 Michael Green added a comment - I'd also like to know why it's necessary for security. Admins should be trusted to use this responsibility. It's a real drag to have to keep logging back in.
            Hide
            skodak Petr Skoda added a comment -

            The problem is the user that you "login-as" - usually student, they could attack the admin account easily - the attack JS would simply switch back to real user and do whatever it wants. I would personally recommend to close the browser and open it again after switching to any other user.

            Show
            skodak Petr Skoda added a comment - The problem is the user that you "login-as" - usually student, they could attack the admin account easily - the attack JS would simply switch back to real user and do whatever it wants. I would personally recommend to close the browser and open it again after switching to any other user.
            Hide
            strom@augsburg.edu Eric Strom added a comment -

            Petr,

            Does this vulnerability exist across the entire moodle installation or is the risk limited to the machine/browser the admin user used to perform the 'login as' function?

            Show
            strom@augsburg.edu Eric Strom added a comment - Petr, Does this vulnerability exist across the entire moodle installation or is the risk limited to the machine/browser the admin user used to perform the 'login as' function?
            Hide
            skodak Petr Skoda added a comment -

            Limited? Any registered user could attack anybody else with capability to login as that user. Once you can trick somebody else's browser to execute your javascript code it is game over - attackers can do pretty much anything user can do with mouse or keyboard in that browser frame (for example create new users, change permissions, change passwords, delete courses, etc.)

            Show
            skodak Petr Skoda added a comment - Limited? Any registered user could attack anybody else with capability to login as that user. Once you can trick somebody else's browser to execute your javascript code it is game over - attackers can do pretty much anything user can do with mouse or keyboard in that browser frame (for example create new users, change permissions, change passwords, delete courses, etc.)
            Hide
            bobpuffer Bob Puffer added a comment -

            For those of us watching this issue (lots) I believe what Eric is asking (and I'd echo the question) is, "Is the security weakness limited to the scenario of the exploiting user needing to run the exploit from the same individual machine from which the "login as" took place?

            Show
            bobpuffer Bob Puffer added a comment - For those of us watching this issue (lots) I believe what Eric is asking (and I'd echo the question) is, "Is the security weakness limited to the scenario of the exploiting user needing to run the exploit from the same individual machine from which the "login as" took place?
            Hide
            skodak Petr Skoda added a comment -

            If anybody is interested in more details:
            1/ first study http://en.wikipedia.org/wiki/Cross-site_scripting
            2/ the XSS would happens through My page when user embeds persistent XSS payload that would switch back to original account
            3/ exploit would require a bit of social engineering that tricks the admin to visit user's own My page when logged-as
            4/ XSS is limited only by the permissions of user that is being successfully attacked

            Show
            skodak Petr Skoda added a comment - If anybody is interested in more details: 1/ first study http://en.wikipedia.org/wiki/Cross-site_scripting 2/ the XSS would happens through My page when user embeds persistent XSS payload that would switch back to original account 3/ exploit would require a bit of social engineering that tricks the admin to visit user's own My page when logged-as 4/ XSS is limited only by the permissions of user that is being successfully attacked
            Hide
            bobpuffer Bob Puffer added a comment -

            Is there something wrong with the way we're asking this question? Are you able to answer it?

            Show
            bobpuffer Bob Puffer added a comment - Is there something wrong with the way we're asking this question? Are you able to answer it?
            Hide
            rex Rex Lorenzo added a comment - - edited

            Wouldn't the cross-site scripting attacks succeed if a user is logged in as a site admin anyways? How does logging in as someone else and viewing any page be different than viewing that same page as a site admin?

            It seems that cross-site scripting is an issue anywhere and preventing someone from returning to their admin role after logging in as someone doesn't solve that problem.

            Show
            rex Rex Lorenzo added a comment - - edited Wouldn't the cross-site scripting attacks succeed if a user is logged in as a site admin anyways? How does logging in as someone else and viewing any page be different than viewing that same page as a site admin? It seems that cross-site scripting is an issue anywhere and preventing someone from returning to their admin role after logging in as someone doesn't solve that problem.
            Hide
            skodak Petr Skoda added a comment -

            Rex: originally in 1.9.x regular students could not add javascript to html block in their personal "MyPage". In early 2.0dev it was decided to allow JS there in MyPage html blocks for everybody (to allow embedding of external widgets, etc.). Normally you can not view somebody else's MyPage that is why it is usually not a security concern because you may XSS only yourself. Once you LoginAs you may visit MyPage of that other user and that is where it gets tricky, I am personally not sure if the logout mitigates all risks, you should probably close/reopen your browser. I was not involved in the My Page and user profile redesign later in 2.0dev, I did not review or study the code and I did not use this part of Moodle much - so please do not ask me how it works or should work or how to fix bugs there. I only maintain the LoginAs internals in lib/accesslib.php, the logout action is not part of that from my perspective. Since last summer I am not responsible for Moodle security and I am not doing code reviews of the whole code base any more, sorry.

            Show
            skodak Petr Skoda added a comment - Rex: originally in 1.9.x regular students could not add javascript to html block in their personal "MyPage". In early 2.0dev it was decided to allow JS there in MyPage html blocks for everybody (to allow embedding of external widgets, etc.). Normally you can not view somebody else's MyPage that is why it is usually not a security concern because you may XSS only yourself. Once you LoginAs you may visit MyPage of that other user and that is where it gets tricky, I am personally not sure if the logout mitigates all risks, you should probably close/reopen your browser. I was not involved in the My Page and user profile redesign later in 2.0dev, I did not review or study the code and I did not use this part of Moodle much - so please do not ask me how it works or should work or how to fix bugs there. I only maintain the LoginAs internals in lib/accesslib.php, the logout action is not part of that from my perspective. Since last summer I am not responsible for Moodle security and I am not doing code reviews of the whole code base any more, sorry.
            Hide
            bobpuffer Bob Puffer added a comment -

            Petr thanks for the reply. Is it possible to get someone involved in this issue from the core team that actually deals with this issue? How would we proceed?

            Show
            bobpuffer Bob Puffer added a comment - Petr thanks for the reply. Is it possible to get someone involved in this issue from the core team that actually deals with this issue? How would we proceed?
            Hide
            juonio Juho Viitasalo added a comment - - edited

            If the MyPage is really the only thing causing this XSS threat I'd suggest that the javascript on MyPage could be disabled. This would automatically prevent the log out after using the login as functionality. What do you say?

            Show
            juonio Juho Viitasalo added a comment - - edited If the MyPage is really the only thing causing this XSS threat I'd suggest that the javascript on MyPage could be disabled. This would automatically prevent the log out after using the login as functionality. What do you say?
            Hide
            paul.n Paul Nicholls added a comment -

            Petr, to expand on Juho's suggestion, could the following be an acceptable solution/workaround?
            If you are using "login as" to view somebody else's "MyPage", custom javascript added by that user is stripped out, and a notice is added to the top of the page along the lines of "For security reasons, any JavaScript added by [user's name] has been removed from this page. Any third-party widgets or other customisations relying on JavaScript will therefore not function as expected." If you think it necessary, the notice could contain a link (which gives a security warning and asks for confirmation before doing anything) which can be clicked to re-display the page with the javascript still in place.

            Alternatively, could we have some kind of mechanism which disables the return to being you which ONLY gets triggered when visiting the other user's "MyPage", perhaps with a warning/confirmation screen being slotted in before the "MyPage" which states that continuing to the user's MyPage will, for security reasons, require you to log in again to return to your account?

            It just seems like overkill to require you to log back in again no matter what you've done if there's only one page in the whole site that's deemed to be a security risk; I'd imagine that the admin would only ever be viewing the user's "MyPage" in a very very small proportion of the cases where they use "login as". It'd be nice to mitigate the actual risk rather than inconvenience people who may never go near the risky page (it may not seem like much of an inconvenience, but as others have suggested, sometimes you can be pinging back and forth between users and it quickly mounts up!).

            Show
            paul.n Paul Nicholls added a comment - Petr, to expand on Juho's suggestion, could the following be an acceptable solution/workaround? If you are using "login as" to view somebody else's "MyPage", custom javascript added by that user is stripped out, and a notice is added to the top of the page along the lines of "For security reasons, any JavaScript added by [user's name] has been removed from this page. Any third-party widgets or other customisations relying on JavaScript will therefore not function as expected." If you think it necessary, the notice could contain a link (which gives a security warning and asks for confirmation before doing anything) which can be clicked to re-display the page with the javascript still in place. Alternatively, could we have some kind of mechanism which disables the return to being you which ONLY gets triggered when visiting the other user's "MyPage", perhaps with a warning/confirmation screen being slotted in before the "MyPage" which states that continuing to the user's MyPage will, for security reasons, require you to log in again to return to your account? It just seems like overkill to require you to log back in again no matter what you've done if there's only one page in the whole site that's deemed to be a security risk; I'd imagine that the admin would only ever be viewing the user's "MyPage" in a very very small proportion of the cases where they use "login as". It'd be nice to mitigate the actual risk rather than inconvenience people who may never go near the risky page (it may not seem like much of an inconvenience, but as others have suggested, sometimes you can be pinging back and forth between users and it quickly mounts up!).
            Hide
            salvetore Michael de Raadt added a comment -

            I'm shifting this to the DEV backlog as it would require a significant change in the way we have implemented "login as".

            Show
            salvetore Michael de Raadt added a comment - I'm shifting this to the DEV backlog as it would require a significant change in the way we have implemented "login as".
            Hide
            hanna123 hanna edelman added a comment -

            Any news?

            Show
            hanna123 hanna edelman added a comment - Any news?
            Hide
            dougiamas Martin Dougiamas added a comment -

            I agree this is an overkill security solution for most sites.

            I think it's probably fair to probably add a setting (off by default) something like:

            "loginas_returntome": Admins logging in as a user will be returned to their original user account. Note that this may expose you to some possible attacks via JS hacks so there is a little risk here if you don't trust all your users.

            Show
            dougiamas Martin Dougiamas added a comment - I agree this is an overkill security solution for most sites. I think it's probably fair to probably add a setting (off by default) something like: "loginas_returntome": Admins logging in as a user will be returned to their original user account. Note that this may expose you to some possible attacks via JS hacks so there is a little risk here if you don't trust all your users.
            Hide
            peterbulmer Peter Bulmer added a comment -

            Hi Martin,
            Thanks for waking this issue up - looks promising

            If the problem is that we are letting users put unfiltered content on their "my" page, could we give site admins the option to choose between:
            a) Users can put what they like on their 'my' page, but you loginas logs out afterwards
            b) Loginas works like it used to, but user content gets escaped as it goes out.

            Another thought is to restrict loginas from quite doing the right thing for 'my' pages - show escaped content for loginas'd users. In the event that a user's 'my' page needed debugging, you would need a full login as option, but that could suffer the same restriction as we currently have.

            A lot of the time when admins are using loginas functionality, they don't need to view the 'my' page at all, or seeing escaped content would be ok, if we can make this scenario pain-free as possible, then I think that this would be a big win.

            I worry that giving admins an easy-to-use switch that says: "Make everything seem to work, but silently insecure", is very tempting to turn on. When nothing immediately explodes the admin will leave it on and forget about it. They'll either deliberately leave it on, or intend to turn it off after this little task
            Eventually many will suffer at the hands of their users, and not know why. Sure, it's their fault, but their perception of Moodle will suffer as a result.

            Thoughts?

            Show
            peterbulmer Peter Bulmer added a comment - Hi Martin, Thanks for waking this issue up - looks promising If the problem is that we are letting users put unfiltered content on their "my" page, could we give site admins the option to choose between: a) Users can put what they like on their 'my' page, but you loginas logs out afterwards b) Loginas works like it used to, but user content gets escaped as it goes out. Another thought is to restrict loginas from quite doing the right thing for 'my' pages - show escaped content for loginas'd users. In the event that a user's 'my' page needed debugging, you would need a full login as option, but that could suffer the same restriction as we currently have. A lot of the time when admins are using loginas functionality, they don't need to view the 'my' page at all, or seeing escaped content would be ok, if we can make this scenario pain-free as possible, then I think that this would be a big win. I worry that giving admins an easy-to-use switch that says: "Make everything seem to work, but silently insecure", is very tempting to turn on. When nothing immediately explodes the admin will leave it on and forget about it. They'll either deliberately leave it on, or intend to turn it off after this little task Eventually many will suffer at the hands of their users, and not know why. Sure, it's their fault, but their perception of Moodle will suffer as a result. Thoughts?
            Hide
            skodak Petr Skoda added a comment -

            here is the code to "unlogin-as" for course/loginas.php instead of the require_logout() :

             
                $_SESSION['SESSION'] = $_SESSION['REALSESSION'];
                $_SESSION['USER'] = $_SESSION['REALUSER'];
             
                unset($_SESSION['REALSESSION']);
                unset($_SESSION['REALUSER']);
             
                if ($id and $id != SITEID) {
                    redirect("$CFG->wwwroot/course/view.php?id=".$id);
                } else {
                    redirect("$CFG->wwwroot/");
                }

            I do not want to put my name next to this code that intentionally opens XSS in Moodle. Maybe there are other places with similar problems as the my moodle page or profiles, somebody needs to carefully review all blocks and user profiles code.

            Show
            skodak Petr Skoda added a comment - here is the code to "unlogin-as" for course/loginas.php instead of the require_logout() :   $_SESSION['SESSION'] = $_SESSION['REALSESSION']; $_SESSION['USER'] = $_SESSION['REALUSER'];   unset($_SESSION['REALSESSION']); unset($_SESSION['REALUSER']);   if ($id and $id != SITEID) { redirect("$CFG->wwwroot/course/view.php?id=".$id); } else { redirect("$CFG->wwwroot/"); } I do not want to put my name next to this code that intentionally opens XSS in Moodle. Maybe there are other places with similar problems as the my moodle page or profiles, somebody needs to carefully review all blocks and user profiles code.
            Hide
            fern Fernando Oliveira added a comment - - edited

            Thanks Petr. Just to be clear, the code you provide will prevent admin user logout when using the "login as" function? If so, where can I insert this code?

            ...nevermind. Found the "require_logout()" line. Seems to do the trick. Thanks for for this.

            Show
            fern Fernando Oliveira added a comment - - edited Thanks Petr. Just to be clear, the code you provide will prevent admin user logout when using the "login as" function? If so, where can I insert this code? ...nevermind. Found the "require_logout()" line. Seems to do the trick. Thanks for for this.
            Hide
            dougiamas Martin Dougiamas added a comment -

            Unfortunately we don't know for sure if the My page is the only one which causes risk here. Remember there are all kinds of other 3rd party plugins and capability combinations.

            As far as I can see, we need to either:

            a) let the admin assume the risk as I suggested above, with a site setting.
            b) force full logout whenever non-secure areas have been viewed. One way would be to invent a new session flag that if set will force logout, and review all code to check for security issues (eg in HTML block) and make sure the variable is set in that affected code.

            Show
            dougiamas Martin Dougiamas added a comment - Unfortunately we don't know for sure if the My page is the only one which causes risk here. Remember there are all kinds of other 3rd party plugins and capability combinations. As far as I can see, we need to either: a) let the admin assume the risk as I suggested above, with a site setting. b) force full logout whenever non-secure areas have been viewed. One way would be to invent a new session flag that if set will force logout, and review all code to check for security issues (eg in HTML block) and make sure the variable is set in that affected code.
            Hide
            marina Marina Glancy added a comment - - edited

            I have the first version of the code. It does not yet have checking of config variable and/or being on "trusted" page (as Martin said above)
            This is my commit: https://github.com/moodle/moodle/compare/master...marinaglancy:wip-MDL-24120-master

            But before proceeding I need to understand the following:
            This functionality was removed in MDL-23911 without any discussion or review. Also I found the code in lib/navigationlib.php that adds a link to return to original user but later this link is being removed. It looks to me like a regression from commit from MDL-22044, particularly this part:

            @@ -2539,11 +1925,12 @@ class settings_navigation extends navigation_node {
                     }
             
                     // Make sure the first child doesnt have proceed with hr set to true
            -        reset($this->children);
            -        current($this->children)->preceedwithhr = false;
             
            -        $this->remove_empty_root_branches();
            -        $this->respect_forced_open();
            +        foreach ($this->children as $key=>$node) {
            +            if ($node->nodetype != self::NODETYPE_BRANCH || $node->children->count()===0) {
            +                $node->remove();
            +            }
            +        }
                 }
            

            Sam Hemelryk, can you confirm please that this was unintentional and I don't break anything else by changing this line?

            And there is some weird CSS that breaks the output. If I remove it, it works fine and does not seem to affect anything else.

            Show
            marina Marina Glancy added a comment - - edited I have the first version of the code. It does not yet have checking of config variable and/or being on "trusted" page (as Martin said above) This is my commit: https://github.com/moodle/moodle/compare/master...marinaglancy:wip-MDL-24120-master But before proceeding I need to understand the following: This functionality was removed in MDL-23911 without any discussion or review. Also I found the code in lib/navigationlib.php that adds a link to return to original user but later this link is being removed. It looks to me like a regression from commit from MDL-22044 , particularly this part: @@ -2539,11 +1925,12 @@ class settings_navigation extends navigation_node { } // Make sure the first child doesnt have proceed with hr set to true - reset($this->children); - current($this->children)->preceedwithhr = false; - $this->remove_empty_root_branches(); - $this->respect_forced_open(); + foreach ($this->children as $key=>$node) { + if ($node->nodetype != self::NODETYPE_BRANCH || $node->children->count()===0) { + $node->remove(); + } + } } Sam Hemelryk , can you confirm please that this was unintentional and I don't break anything else by changing this line? And there is some weird CSS that breaks the output. If I remove it, it works fine and does not seem to affect anything else.
            Hide
            marina Marina Glancy added a comment -

            Show
            marina Marina Glancy added a comment -
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Marina,

            That change was from right after the creation of navigation structure and blocks. It was the first bit of new functionality I wrote for Moodle.
            In terms of that line I can't remember it explicitly of course, but I believe it lies somewhere between intentional and bug. At the time the navigation loaded by AJAX and although the settings navigation didn't it we had in mind that one day it may. In addition to this we had decided that settings should always be categorised, such that settings should always be within a branch and should never be loosely hanging.
            I believe that was the idea behind that line of code - of course I may be mistaken. It is a totalitarian piece of code - all that is not expected is removed but that is how it has been for a very long time now.

            In terms of the CSS - no idea - likely an artefact from the above decisions, no loose settings so this will have gone unseen since its conception.

            In regards to your solution, I have to admit having to log in again did always slow me down - but then the security concerns behind opening this up in such a way were so severe in my mind that it was never worth tackling it in code.
            In terms of the placement of the link to keep navigation consistent I think the link should be placed within "My profile settings", this will be more consistent with its placement in the top right user menu. It would also continue to keep true the idea of not having loose hanging settings.

            As for security here is how I understand it. This change allows just about any XSS attack to be broadened to a permission elevation (to the admin no less).
            The following scenario for instance:

            • Student discovers XSS attack vector.
            • Writes some crafty JS that does some nasty that would open a window with the "return to admin user" link.
            • Files a support ticket with helpdesk, stating a problem on the given page.
            • Unsuspecting admin uses loginas to see what the student sees.
            • XSS code opens new window to complete the "return to admin" action.
            • Original window with the XSS code now has admin access to the site.

            I have no idea if this is actually possible of course - it requires an XSS attack vector, some JS, and code that has not previously existed.
            Truthfully I never considered the action worth the link.
            The setting was a good idea though - people could turn it on if they want. Really I suppose there is a matter of trust, but for a security conscious site I would bet they would not want this setting on if it does indeed expose this kind of security risk.

            Hope all of this helps.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Marina, That change was from right after the creation of navigation structure and blocks. It was the first bit of new functionality I wrote for Moodle. In terms of that line I can't remember it explicitly of course, but I believe it lies somewhere between intentional and bug. At the time the navigation loaded by AJAX and although the settings navigation didn't it we had in mind that one day it may. In addition to this we had decided that settings should always be categorised, such that settings should always be within a branch and should never be loosely hanging. I believe that was the idea behind that line of code - of course I may be mistaken. It is a totalitarian piece of code - all that is not expected is removed but that is how it has been for a very long time now. In terms of the CSS - no idea - likely an artefact from the above decisions, no loose settings so this will have gone unseen since its conception. In regards to your solution, I have to admit having to log in again did always slow me down - but then the security concerns behind opening this up in such a way were so severe in my mind that it was never worth tackling it in code. In terms of the placement of the link to keep navigation consistent I think the link should be placed within "My profile settings", this will be more consistent with its placement in the top right user menu. It would also continue to keep true the idea of not having loose hanging settings. As for security here is how I understand it. This change allows just about any XSS attack to be broadened to a permission elevation (to the admin no less). The following scenario for instance: Student discovers XSS attack vector. Writes some crafty JS that does some nasty that would open a window with the "return to admin user" link. Files a support ticket with helpdesk, stating a problem on the given page. Unsuspecting admin uses loginas to see what the student sees. XSS code opens new window to complete the "return to admin" action. Original window with the XSS code now has admin access to the site. I have no idea if this is actually possible of course - it requires an XSS attack vector, some JS, and code that has not previously existed. Truthfully I never considered the action worth the link. The setting was a good idea though - people could turn it on if they want. Really I suppose there is a matter of trust, but for a security conscious site I would bet they would not want this setting on if it does indeed expose this kind of security risk. Hope all of this helps. Cheers Sam
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Just a parting thought before I left for the day - its worth noting that the admin user account is the account to protect at all costs of course.
            I had a chat to Petr about this over lunch today and he mentioned there was at least one place known to him in which a student can add executable JS to a page that would make this an easy exploit if you could get the admin to visit the page while loggedinas and with an option to return to the admin user without re-logging in.

            Show
            samhemelryk Sam Hemelryk added a comment - Just a parting thought before I left for the day - its worth noting that the admin user account is the account to protect at all costs of course. I had a chat to Petr about this over lunch today and he mentioned there was at least one place known to him in which a student can add executable JS to a page that would make this an easy exploit if you could get the admin to visit the page while loggedinas and with an option to return to the admin user without re-logging in.
            Hide
            marina Marina Glancy added a comment -

            Thanks Sam.
            I did not like myself how this "returnto" link is hanging, just found that it was already there in the code. Moving to my profile is probably a good idea.

            This is the potential solution that I thought of:
            1. Create a setting in "Experimental" section for now, saying "Require password when return from loginas", default yes, as it is now. With a disclaimer that it can be dangerous on the production sites.
            2. Mark some pages as "untrustworthy", they must call $PAGE->protect_loginas_session(); - I immediately can only think of /my/ page. Any other page where user without RISK_XSS can include js should be considered security issue.
            3. If admin/manager being logged in as somebody else requests at least one of the "dangerous" pages, we put a flag in his session that he must enter password on return. Does not matter if it is on this page or 5 requests later. (Mainly because we can't identify from which page the "return" link was requested.)
            4. We notify plugin devs and ask them to call this $PAGE method on their dangerous pages as well.
            5. After 1-2 releases we can consider moving setting out of experimental.

            Show
            marina Marina Glancy added a comment - Thanks Sam. I did not like myself how this "returnto" link is hanging, just found that it was already there in the code. Moving to my profile is probably a good idea. This is the potential solution that I thought of: 1. Create a setting in "Experimental" section for now, saying "Require password when return from loginas", default yes, as it is now. With a disclaimer that it can be dangerous on the production sites. 2. Mark some pages as "untrustworthy", they must call $PAGE->protect_loginas_session(); - I immediately can only think of /my/ page. Any other page where user without RISK_XSS can include js should be considered security issue. 3. If admin/manager being logged in as somebody else requests at least one of the "dangerous" pages, we put a flag in his session that he must enter password on return. Does not matter if it is on this page or 5 requests later. (Mainly because we can't identify from which page the "return" link was requested.) 4. We notify plugin devs and ask them to call this $PAGE method on their dangerous pages as well. 5. After 1-2 releases we can consider moving setting out of experimental.
            Hide
            marina Marina Glancy added a comment -

            Re Atto. Student is able to put JS, for example, in the forum post, but it will be stripped later. If teacher login as this student and edit his post, there will be no js there

            I thought about setting with three options:

            • always request password
            • request password for admin
            • never request password

            but what is the definition of "admin" - is_site_admin() or somebody with capability to 'moodle/site:config' ?

            Show
            marina Marina Glancy added a comment - Re Atto. Student is able to put JS, for example, in the forum post, but it will be stripped later. If teacher login as this student and edit his post, there will be no js there I thought about setting with three options: always request password request password for admin never request password but what is the definition of "admin" - is_site_admin() or somebody with capability to 'moodle/site:config' ?
            Hide
            marina Marina Glancy added a comment -

            requesting peer review

            Show
            marina Marina Glancy added a comment - requesting peer review
            Hide
            cibot CiBoT added a comment -

            Fails against automated checks.

            Checked MDL-24120 using repository: git://github.com/marinaglancy/moodle.git

            More information about this report

            Show
            cibot CiBoT added a comment - Fails against automated checks. Checked MDL-24120 using repository: git://github.com/marinaglancy/moodle.git master (4 errors / 4 warnings) [branch: wip-MDL-24120-master | CI Job ] phplint (0/0) , php (4/4) , js (0/0) , css (0/0) , phpdoc (0/0) , commit (0/0) , savepoint (0/0) , thirdparty (0/0) , More information about this report
            Hide
            timhunt Tim Hunt added a comment -

            Some comments, not a full peer review.

            1. The language string should not be config..., they should be

            configsimplereturnloginas_desc
            simplereturnloginasexperimental_desc

            2. Why is this change right? https://github.com/moodle/moodle/compare/master...marinaglancy:wip-MDL-24120-master#diff-ab388c1436e96891018161d201801943L3517

            3. Given the docs so far in the code, I have no idea why I might need to call loginas_dangerous. I think you should document that function as if you were explaining it to a GSoC student new to Moodle.

            4. Even if the correct use of that function is understandable, once you have found it, how does anyone ever discover that that function exists, and that they are supposed to call it sometimes.

            --------

            Instead of implementing this, and alternative would be to implement MDL-4578 / MDL-13703. That way, when the user gets logged out, their session is still destroyed as not, and they have to be redirected to login/index.php to get a new session, it is just that they won't have to type their password, which is the thing people are complaining about.

            Does that solve the security concerns?

            Show
            timhunt Tim Hunt added a comment - Some comments, not a full peer review. 1. The language string should not be config..., they should be configsimplereturnloginas_desc simplereturnloginasexperimental_desc 2. Why is this change right? https://github.com/moodle/moodle/compare/master...marinaglancy:wip-MDL-24120-master#diff-ab388c1436e96891018161d201801943L3517 3. Given the docs so far in the code, I have no idea why I might need to call loginas_dangerous. I think you should document that function as if you were explaining it to a GSoC student new to Moodle. 4. Even if the correct use of that function is understandable, once you have found it , how does anyone ever discover that that function exists, and that they are supposed to call it sometimes. -------- Instead of implementing this, and alternative would be to implement MDL-4578 / MDL-13703 . That way, when the user gets logged out, their session is still destroyed as not, and they have to be redirected to login/index.php to get a new session, it is just that they won't have to type their password, which is the thing people are complaining about. Does that solve the security concerns?
            Hide
            marina Marina Glancy added a comment -

            Tim, user can set browser to remember the password. Not sure we need to implement it on moodle side.

            as for your comments:
            1. there are two alternative naming conventions for strings in lang/en/admin.php, I chose the one that is used in this tree
            2. it's a bug I noticed meanwhile. Actually after I moved the item into profile, this change is not needed. I moved it into MDL-49667
            3&4. I added more phpdocs and upgrade.txt note

            Show
            marina Marina Glancy added a comment - Tim, user can set browser to remember the password. Not sure we need to implement it on moodle side. as for your comments: 1. there are two alternative naming conventions for strings in lang/en/admin.php, I chose the one that is used in this tree 2. it's a bug I noticed meanwhile. Actually after I moved the item into profile, this change is not needed. I moved it into MDL-49667 3&4. I added more phpdocs and upgrade.txt note
            Hide
            timhunt Tim Hunt added a comment -

            Marina, even with the browser saving your password, there is a big difference between having to see the login page and click the button, and just being automatically logged in when you return (like here in the tracker, and in almost any other web system).

            Also, it makes it a user's choice. Normal users who can't cause serious damage if they hit an XSS can turn the option on for convenience, while admins leave it turned off.

            Show
            timhunt Tim Hunt added a comment - Marina, even with the browser saving your password, there is a big difference between having to see the login page and click the button, and just being automatically logged in when you return (like here in the tracker, and in almost any other web system). Also, it makes it a user's choice. Normal users who can't cause serious damage if they hit an XSS can turn the option on for convenience, while admins leave it turned off.
            Hide
            lameze Simey Lameze added a comment -

            Hi Marina.

            Firstly thank you very much for work on this highly voted issue, it is definitely a GREAT addition to moodle, I personally hated when I was using Log in as and had to logout and login again every time. I've tested here and works really well.

            I just noticed the correct setting name is simplereturnloginas rather then returntooriginaluser. I was able to test the patch only after enable simplereturnloginas. So your testing instructions need to be updated.

            Probably we need a simple behat test for this, as you're introducing a new feature.

            Rest looks really good, can't wait to see this on moodle!

            [Y] Syntax
            [Y] Whitespace
            [Y] Output
            [Y] Language
            [-] Databases
            [N] Testing (instructions and automated tests)
            [-] Security
            [N] Documentation
            [Y] Git
            [-] Third party code
            [Y] Sanity check
            [-] Icons
            

            Feel free to push for integration review.

            Thanks.

            Show
            lameze Simey Lameze added a comment - Hi Marina. Firstly thank you very much for work on this highly voted issue, it is definitely a GREAT addition to moodle, I personally hated when I was using Log in as and had to logout and login again every time. I've tested here and works really well. I just noticed the correct setting name is simplereturnloginas rather then returntooriginaluser . I was able to test the patch only after enable simplereturnloginas . So your testing instructions need to be updated. Probably we need a simple behat test for this, as you're introducing a new feature. Rest looks really good, can't wait to see this on moodle! [Y] Syntax [Y] Whitespace [Y] Output [Y] Language [-] Databases [N] Testing (instructions and automated tests) [-] Security [N] Documentation [Y] Git [-] Third party code [Y] Sanity check [-] Icons Feel free to push for integration review. Thanks.
            Hide
            marina Marina Glancy added a comment -

            Thanks for review Simey, behat tests added, testing instructions corrected.
            Sending for integration

            Show
            marina Marina Glancy added a comment - Thanks for review Simey, behat tests added, testing instructions corrected. Sending for integration
            Hide
            timhunt Tim Hunt added a comment -

            TO INTEGRATORS: I would appreciate it if you would read my comments above when considering this. Thanks.

            Show
            timhunt Tim Hunt added a comment - TO INTEGRATORS: I would appreciate it if you would read my comments above when considering this. Thanks.
            Hide
            cibot CiBoT added a comment -

            Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

            Show
            cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
            Hide
            cibot CiBoT added a comment -

            Fails against automated checks.

            Checked MDL-24120 using repository: git://github.com/marinaglancy/moodle.git

            More information about this report

            Show
            cibot CiBoT added a comment - Fails against automated checks. Checked MDL-24120 using repository: git://github.com/marinaglancy/moodle.git master [branch: wip-MDL-24120-master | CI Job ] Error: Unable to fetch information from wip- MDL-24120 -master branch at git://github.com/marinaglancy/moodle.git. More information about this report
            Hide
            dmonllao David Monllaó added a comment -

            Thanks for working on this Marina, as developer/tester I would love this feature.

            I think that the gap between teachers and students is too big and it shouldn't; I would not allow any admin or manager to be XSSed by any other user (https://tracker.moodle.org/browse/MDL-47639?focusedCommentId=331908&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-331908) not even teachers, doing that (and keeping loginas capability only for admin/manager) we could allow this return to the real session after logging as without being worried about 3rd party code. A student can be teacher using course request, and the same depending on which capabilities they are granted; we opened MDL-47639 few months ago and not much people is commenting there, IMO they are quite related. I will leave this in review for some time so other people can comment about it.

            Regarding MDL-13703, for what I remember, systems where administrators can log in don't have a keep me logged in option, I agree with Petr in https://tracker.moodle.org/browse/MDL-13703?focusedCommentId=43912&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-43912.

            Show
            dmonllao David Monllaó added a comment - Thanks for working on this Marina, as developer/tester I would love this feature. I think that the gap between teachers and students is too big and it shouldn't; I would not allow any admin or manager to be XSSed by any other user ( https://tracker.moodle.org/browse/MDL-47639?focusedCommentId=331908&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-331908 ) not even teachers, doing that (and keeping loginas capability only for admin/manager) we could allow this return to the real session after logging as without being worried about 3rd party code. A student can be teacher using course request, and the same depending on which capabilities they are granted; we opened MDL-47639 few months ago and not much people is commenting there, IMO they are quite related. I will leave this in review for some time so other people can comment about it. Regarding MDL-13703 , for what I remember, systems where administrators can log in don't have a keep me logged in option, I agree with Petr in https://tracker.moodle.org/browse/MDL-13703?focusedCommentId=43912&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-43912 .
            Hide
            damyon Damyon Wiese added a comment -

            I think something that is worth considering is whether the votes for this feature are coming from devs who use testing sites, or teachers / admins of real sites. From the comments (I did not read through the code), but this solution seems like it
            a) creates a new responsibility for pages to mark if they are "dangerous" (IMO secure options should be the default with an option to disable).
            b) then leads to confusing "sometimes" I have to re-enter my password situations

            My vote would be a new setting to allow return from loginas without logging in again - that can only be enabled if debugging is set to developer (never ever for real sites)?

            Show
            damyon Damyon Wiese added a comment - I think something that is worth considering is whether the votes for this feature are coming from devs who use testing sites, or teachers / admins of real sites. From the comments (I did not read through the code), but this solution seems like it a) creates a new responsibility for pages to mark if they are "dangerous" (IMO secure options should be the default with an option to disable). b) then leads to confusing "sometimes" I have to re-enter my password situations My vote would be a new setting to allow return from loginas without logging in again - that can only be enabled if debugging is set to developer (never ever for real sites)?
            Hide
            marina Marina Glancy added a comment -

            I also have a suspicion that the issue's votes are from developers.
            I implemented the "dangerous" pages because of Martin's suggestion above but actually I like the idea of strictly bounding to the development mode.

            It will be very easy for me to change the logic of when the return is allowed. Can we have a voting?

            Show
            marina Marina Glancy added a comment - I also have a suspicion that the issue's votes are from developers. I implemented the "dangerous" pages because of Martin's suggestion above but actually I like the idea of strictly bounding to the development mode. It will be very easy for me to change the logic of when the return is allowed. Can we have a voting?
            Hide
            dmonllao David Monllaó added a comment -

            Support users with admin privileges would also use this feature as they usually login as users that reports doubts or unexpected behaviours and they need to check what the user sees. I don't +1 to only devs.

            Show
            dmonllao David Monllaó added a comment - Support users with admin privileges would also use this feature as they usually login as users that reports doubts or unexpected behaviours and they need to check what the user sees. I don't +1 to only devs.
            Hide
            marina Marina Glancy added a comment -

            but login as is always possible, it's just a quesiton of re-entering the password

            Show
            marina Marina Glancy added a comment - but login as is always possible, it's just a quesiton of re-entering the password
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Bloody lazy people! (said with love)

            We are just 1-password™ (automated or manual) away from returning to the admin account or, alternatively, can use two sessions/browsers.

            Not to talk about why an admin should be ever logging-in as another user and performing actions on behalf on him/her, this particular (ab)use leads to problems everywhere, logs not really performed by real user, restore admin conflicts...

            And, no only that, we still want to make it worse by re-implementing the automated-back-to-admin, when it's clear that it would lead to privileges escalation really easily.

            My -1 to accept this, neither as default neither as optional/experimental setting unless the system is 100% ready to prevent any dangerous content when using loginas.

            Ciao

            PS: Have you thought that, maybe, instead of marking pages... it would be simpler to hack outpur/format_xxx() in order to be more strict when a logginas session is running? Just an idea.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Bloody lazy people! (said with love) We are just 1-password™ (automated or manual) away from returning to the admin account or, alternatively, can use two sessions/browsers. Not to talk about why an admin should be ever logging-in as another user and performing actions on behalf on him/her, this particular (ab)use leads to problems everywhere, logs not really performed by real user, restore admin conflicts... And, no only that, we still want to make it worse by re-implementing the automated-back-to-admin, when it's clear that it would lead to privileges escalation really easily. My -1 to accept this, neither as default neither as optional/experimental setting unless the system is 100% ready to prevent any dangerous content when using loginas . Ciao PS: Have you thought that, maybe, instead of marking pages... it would be simpler to hack outpur/format_xxx() in order to be more strict when a logginas session is running? Just an idea.
            Hide
            dmonllao David Monllaó added a comment -

            Thanks for commenting Damyon and Eloy.

            I am sorry to reopen this because it has many votes but we should include this on a wider discussion about how we allow XSS. IMO admins/managers should never be XSSs by other users, ideally (although not sure it is possible to archive that allowing users to do all they can currently do) no users should XSS other people, and not because they have good hearts, but because they we prevent them to do it.

            Moving forward things like MDL-47639 would help here.

            Show
            dmonllao David Monllaó added a comment - Thanks for commenting Damyon and Eloy. I am sorry to reopen this because it has many votes but we should include this on a wider discussion about how we allow XSS. IMO admins/managers should never be XSSs by other users, ideally (although not sure it is possible to archive that allowing users to do all they can currently do) no users should XSS other people, and not because they have good hearts, but because they we prevent them to do it. Moving forward things like MDL-47639 would help here.
            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.

              Dates

              • Created:
                Updated: