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
    • Rank:
      43314

      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.

        Activity

        Hide
        Petr Škoda added a comment -

        Thanks for the report.

        Show
        Petr Škoda 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: