Moodle
  1. Moodle
  2. MDL-34819

CAS authentication : Alternative logout return URL

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.4
    • Component/s: Authentication
    • Labels:
    • Testing Instructions:
      Hide
      • Activate CAS authentication with the right settings of your CAS server
      • Logout of Moodle
      • Login with a CAS user
      • Logout of Moodle
        -> You should be redirect to the Moodle root of your server
      • Go back to the management page of CAS authentication :
        • Set "Multi-authentication" to "Yes"
        • Set "Alternative logout return URL" to "http://moodle.org"
      • Logout of Moodle
      • Login with a CAS user
      • Logout of Moodle
        -> You should be redirect to "http://moodle.org"
      • Login with a manual user (NOCAS)
      • Logout of Moodle
        -> You should be redirect to the Moodle root of your server
      Show
      Activate CAS authentication with the right settings of your CAS server Logout of Moodle Login with a CAS user Logout of Moodle -> You should be redirect to the Moodle root of your server Go back to the management page of CAS authentication : Set "Multi-authentication" to "Yes" Set "Alternative logout return URL" to "http://moodle.org" Logout of Moodle Login with a CAS user Logout of Moodle -> You should be redirect to "http://moodle.org" Login with a manual user (NOCAS) Logout of Moodle -> You should be redirect to the Moodle root of your server
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34819-master

      Description

      Add a new setting in CAS authentication configuration to allow alternative logout return URL.

      In our case, we choose not to logout from CAS (SSO in multiple web app) when the user logout from Moodle and so we want to be able to redirect the user to a page where a message would tell him that he's not actually logout from CAS.

        Gliffy Diagrams

          Activity

          Hide
          Petr Skoda added a comment -

          Thanks for the report.

          Show
          Petr Skoda added a comment - Thanks for the report.
          Hide
          Dan Poltawski added a comment -

          This looks good to me, so submitting for integration.

          For the record, this same feature is present in the shibboleth auth, so it makes sense here too IMO.

          Show
          Dan Poltawski added a comment - This looks good to me, so submitting for integration. For the record, this same feature is present in the shibboleth auth, so it makes sense here too IMO.
          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
          Jean-Philippe Gaudreau added a comment -

          Branch rebased!

          Show
          Jean-Philippe Gaudreau added a comment - Branch rebased!
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Sorry Jean-Philippe, I'm reopening this because:

          • I've detected it includes a couple of changes (whitespace) in backup code?
          • It contains a dozen of merges (you should be rebasing, not merging your feature/bugfix branches.
          • There is some incorrect whitespace in auth/cas/auth.php (@ logoutpage_hook() method).

          Surely I could have cherry-picked the cas-related commit, but I wanted to share the situation with you this time to allow to know and fix the problem.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Sorry Jean-Philippe, I'm reopening this because: I've detected it includes a couple of changes (whitespace) in backup code? It contains a dozen of merges (you should be rebasing, not merging your feature/bugfix branches. There is some incorrect whitespace in auth/cas/auth.php (@ logoutpage_hook() method). Surely I could have cherry-picked the cas-related commit, but I wanted to share the situation with you this time to allow to know and fix the problem. 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
          Jean-Philippe Gaudreau added a comment -

          Hi Eloy,

          Branch is fixed! Sorry for all the problems with my git repositry and the whitespaces... Still learning and improving

          Putting the task back to peer review.

          Show
          Jean-Philippe Gaudreau added a comment - Hi Eloy, Branch is fixed! Sorry for all the problems with my git repositry and the whitespaces... Still learning and improving Putting the task back to peer review.
          Hide
          Dan Poltawski added a comment -

          Sending for integration again - checked the commits are clean this time, oops!

          Show
          Dan Poltawski added a comment - Sending for integration again - checked the commits are clean this time, oops!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

          Apologises for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologises for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Aparup Banerjee added a comment -

          Thanks, improvement integrated into master only.

          (any further help testing this would be really useful here)

          Show
          Aparup Banerjee added a comment - Thanks, improvement integrated into master only. (any further help testing this would be really useful here)
          Hide
          Aparup Banerjee added a comment -

          Jason has said he is setting up this test. Jason, could you update the status to testing in progress.

          Show
          Aparup Banerjee added a comment - Jason has said he is setting up this test. Jason, could you update the status to testing in progress.
          Hide
          Jason Fowler added a comment -

          okay, so I have the CAS server setup and working, just getting errors:

          CAS Authentication failed!
          You were not authenticated.

          Everytime I login now.

          I am now waiting on a CAS VM from Inaki Arenaza in the hopes it will solve my problems.

          Show
          Jason Fowler added a comment - okay, so I have the CAS server setup and working, just getting errors: CAS Authentication failed! You were not authenticated. Everytime I login now. I am now waiting on a CAS VM from Inaki Arenaza in the hopes it will solve my problems.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note that as far as this is master-only (can accept some minor breackage there), it's NOT being reverted, so it's being rolled upstream.

          BUT, it needs testing to get it properly passed and closed so I'm keeping it under "Waiting for testing" status.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Note that as far as this is master-only (can accept some minor breackage there), it's NOT being reverted, so it's being rolled upstream. BUT, it needs testing to get it properly passed and closed so I'm keeping it under "Waiting for testing" status. Ciao
          Hide
          Jason Fowler added a comment -

          Unable to complete testing instructions. Despite logging out as a the CAS user, everytime I click the login link to try to login as a NOCAS user, it logs me back in as the CAS user.

          Show
          Jason Fowler added a comment - Unable to complete testing instructions. Despite logging out as a the CAS user, everytime I click the login link to try to login as a NOCAS user, it logs me back in as the CAS user.
          Hide
          Aparup Banerjee added a comment -

          Hi Jason, it would be great if you could provide some specific errors or traces to use to follow on from this improvement.

          Also can you confirm that this is or isn't a problem about this patch by doing your test on last week's master release.

          Show
          Aparup Banerjee added a comment - Hi Jason, it would be great if you could provide some specific errors or traces to use to follow on from this improvement. Also can you confirm that this is or isn't a problem about this patch by doing your test on last week's master release.
          Hide
          Jason Fowler added a comment -

          There are no traces or errors to show.

          I login as a CAS user, fine
          Logout and get directed to moodle.org as expected.
          Go back to my test instance, and click login - I am automatically logged in as the CAS user. No options offered.

          Show
          Jason Fowler added a comment - There are no traces or errors to show. I login as a CAS user, fine Logout and get directed to moodle.org as expected. Go back to my test instance, and click login - I am automatically logged in as the CAS user. No options offered.
          Hide
          Dan Poltawski added a comment -

          Sounds to me like you need to logout of your CAS server and this isn't a bug, this is the point of single sign on.

          Show
          Dan Poltawski added a comment - Sounds to me like you need to logout of your CAS server and this isn't a bug, this is the point of single sign on.
          Hide
          Jason Fowler added a comment -

          Tested in last week's master too, and the issue is still present there. I could prevent it by enabling the CAS logout Option - but that then over-rides the Alternative Logout return URL in this patch.

          Show
          Jason Fowler added a comment - Tested in last week's master too, and the issue is still present there. I could prevent it by enabling the CAS logout Option - but that then over-rides the Alternative Logout return URL in this patch.
          Hide
          Jason Fowler added a comment -

          In reality, the patch here works as intended, it just doesn't allow the other error I found to be handled well.

          Show
          Jason Fowler added a comment - In reality, the patch here works as intended, it just doesn't allow the other error I found to be handled well.
          Hide
          Aparup Banerjee added a comment -

          resetting for Jason

          Show
          Aparup Banerjee added a comment - resetting for Jason
          Hide
          Jason Fowler added a comment -

          Passing this, because the error is not in the patch, but rather in the general implementation of the CAS system

          Show
          Jason Fowler added a comment - Passing this, because the error is not in the patch, but rather in the general implementation of the CAS system
          Hide
          Jean-Philippe Gaudreau added a comment -

          Hi Jason,

          Even if it's a wierd behaviour to not logout from CAS when logout from Moodle, I think it's the expected way because it's a single sign authentication method and so it could be used by other applications.

          In fact, I did this task so in our environnement, the user would be redirect to a page saying that he's not actually logged out and why it is so.

          Anyway many thx for testing

          Show
          Jean-Philippe Gaudreau added a comment - Hi Jason, Even if it's a wierd behaviour to not logout from CAS when logout from Moodle, I think it's the expected way because it's a single sign authentication method and so it could be used by other applications. In fact, I did this task so in our environnement, the user would be redirect to a page saying that he's not actually logged out and why it is so. Anyway many thx for testing
          Hide
          Aparup Banerjee added a comment -

          Your issue has dug up some gold.
          It works great i've been told.
          Go forth, be brave, be bold.

          yay! "All your thoughts are belong to everyone."

          Thanks and ciao!

          Show
          Aparup Banerjee added a comment - Your issue has dug up some gold. It works great i've been told. Go forth, be brave, be bold. yay! "All your thoughts are belong to everyone." Thanks and ciao!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: