Moodle
  1. Moodle
  2. MDL-34965

When the session expires redirect the user to the login page instead of login it as guest

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Authentication, Usability
    • Labels:
    • Testing Instructions:
      Hide

      This issue must be tested with MDL-35029 integrated.

      1. Login as an admin
      2. Enable Users -> Permissions -> User policies -> autologinguests
      3. Enable Server -> Session handling -> dbsessions if it's not already enabled
      4. Edit the database and set mdl_config->sessiontimeout to 3 seconds (or you can use the UI to set the session timeout to the lower value and wait until the session expires)
      5. Logged as an admin, teacher or student go to a course without guest access
      6. After 4 seconds refresh the screen
      7. You SHOULD be redirected to the login page
      8. Enter your username and pwd and click 'submit'
      9. You SHOULD be logged in and you SHOULD be redirected to the course page
      10. Probably you will want to restore your $CFG->sessiontimeout value
      Show
      This issue must be tested with MDL-35029 integrated. Login as an admin Enable Users -> Permissions -> User policies -> autologinguests Enable Server -> Session handling -> dbsessions if it's not already enabled Edit the database and set mdl_config->sessiontimeout to 3 seconds (or you can use the UI to set the session timeout to the lower value and wait until the session expires) Logged as an admin, teacher or student go to a course without guest access After 4 seconds refresh the screen You SHOULD be redirected to the login page Enter your username and pwd and click 'submit' You SHOULD be logged in and you SHOULD be redirected to the course page Probably you will want to restore your $CFG->sessiontimeout value
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34965_master
    • Rank:
      43541

      Description

      With guest autologin (Users -> Permissions -> User policies -> autologinguests) when a user session expires the user is autologged as a guest; probably the user is more interested in login again and view the page he/she was viewing with his/her own user instead of as a guest user.

      This behaviour has been detected peer reviewing MDL-32688, probably this is not the only place with the "autologin as guest" feature causes this sort of error when the session expires and it can be managed in a centralized way by require_login() allowing the user to relogin instead of showing an error or a guest-adapted interface

        Issue Links

          Activity

          Hide
          David Monllaó added a comment -

          Proposing an alternative behaviour

          Show
          David Monllaó added a comment - Proposing an alternative behaviour
          Hide
          Petr Škoda added a comment -

          Hehe, I started implementing it too. +1

          Show
          Petr Škoda added a comment - Hehe, I started implementing it too. +1
          Hide
          Frédéric Massart added a comment -

          Hi David. The patch looks good, but I was just wondering if I am auto logged in as a guest, can my session expire? And if it does, will I be redirected to the login page?

          If a guest has automatically been logged in, IMO they shouldn't see the login page when the session expires. But that implies that the guest session can expire, which might not be the case.

          Feel free to push for integration if my comment is irrelevant. Thanks!

          Show
          Frédéric Massart added a comment - Hi David. The patch looks good, but I was just wondering if I am auto logged in as a guest, can my session expire? And if it does, will I be redirected to the login page? If a guest has automatically been logged in, IMO they shouldn't see the login page when the session expires. But that implies that the guest session can expire, which might not be the case. Feel free to push for integration if my comment is irrelevant. Thanks!
          Hide
          Petr Škoda added a comment -

          Hmm, right, I take my +1 back because this would be probably quite confusing for guests. The problem is we do not know anything about previous session...

          Show
          Petr Škoda added a comment - Hmm, right, I take my +1 back because this would be probably quite confusing for guests. The problem is we do not know anything about previous session...
          Hide
          Petr Škoda added a comment -

          In any case changes like this should imho go into master only.

          Show
          Petr Škoda added a comment - In any case changes like this should imho go into master only.
          Hide
          David Monllaó added a comment -

          Thanks for the comment, I imagine myself reading moodle.org without being logged and being redirected every X mins to the login page and I don't like it. We can set a new $SESSION attribute to detect if the expired session comes from a guest and redirect or not depending on it (unsetting the var after the checking) or there is a restrictive policy for creating new $SESSION attrs?

          Show
          David Monllaó added a comment - Thanks for the comment, I imagine myself reading moodle.org without being logged and being redirected every X mins to the login page and I don't like it. We can set a new $SESSION attribute to detect if the expired session comes from a guest and redirect or not depending on it (unsetting the var after the checking) or there is a restrictive policy for creating new $SESSION attrs?
          Hide
          Frédéric Massart added a comment -

          You should check the behaviour first because I am not sure that the guest access can expire. I could not reproduce it but I didn't try hard.

          Show
          Frédéric Massart added a comment - You should check the behaviour first because I am not sure that the guest access can expire. I could not reproduce it but I didn't try hard.
          Hide
          David Monllaó added a comment -

          Thanks for the comments. Another proposal, with "autologin as a guest" enabled maybe there is no need to expire the guest sessions.

          Please Petr, can you peer review it? I'm not 100% confident with this, it adds s a different behaviour with $CFG->dbsession enabled, I see that legacy_file_session is not recommended but it's not deprecated. Thanks in advance

          I removed the 22 and 23 pull branches

          Show
          David Monllaó added a comment - Thanks for the comments. Another proposal, with "autologin as a guest" enabled maybe there is no need to expire the guest sessions. Please Petr, can you peer review it? I'm not 100% confident with this, it adds s a different behaviour with $CFG->dbsession enabled, I see that legacy_file_session is not recommended but it's not deprecated. Thanks in advance I removed the 22 and 23 pull branches
          Hide
          David Monllaó added a comment -

          Thanks for your comments Frédéric; after talking with Petr I've split the issue in two (MDL-35029 is the other one) so with "autologin guests" enabled the guest users will not be logged out, their session will be renewed; the cron garbage collector deletes the non-used sessions as it's currently doing.

          To maintain the current behaviour with dbsessions disabled I added a if (!empty($CFG->dbsessions))

          Show
          David Monllaó added a comment - Thanks for your comments Frédéric; after talking with Petr I've split the issue in two ( MDL-35029 is the other one) so with "autologin guests" enabled the guest users will not be logged out, their session will be renewed; the cron garbage collector deletes the non-used sessions as it's currently doing. To maintain the current behaviour with dbsessions disabled I added a if (!empty($CFG->dbsessions))
          Hide
          Dan Poltawski added a comment -

          Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..

          Show
          Dan Poltawski added a comment - Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..
          Hide
          Dan Poltawski added a comment -

          Thanks David, i've integrated this now

          Show
          Dan Poltawski added a comment - Thanks David, i've integrated this now
          Hide
          Michael de Raadt added a comment -

          Test result: Works a treat.

          Tested in master only.

          3 seconds is a really short time.

          Show
          Michael de Raadt added a comment - Test result: Works a treat. Tested in master only. 3 seconds is a really short time.
          Hide
          Dan Poltawski added a comment -

          Hurray!

          You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

          Show
          Dan Poltawski added a comment - Hurray! You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

            People

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

              Dates

              • Created:
                Updated:
                Resolved: