Logic error in phpCAS::checkAuthentication(), having to do with $_SESSION['phpCAS']['auth_checked']

[WiredAcademic]

Background:
phpCAS::checkAuthentication() has three parts in an IF-ELSEIF-ELSE statement.

1. The first part short-circuits out of this function, if phpCAS::isAuthenticated() returns true.
2. The second part removes the $_SESSION['phpCAS']['auth_checked'] variable when it is not needed anymore
3. The last part assigns the $_SESSION['phpCAS']['auth_checked'] variable, and an $_SESSION['phpCAS']['auth_count'] variable to decide when to check again if the user was not logged in the first time.

Problem:
When the short-circuit occurs in the first part of the IF-ELSEIF-ELSE statement, the variable $_SESSION['phpCAS']['auth_checked'] remains set when it would otherwise be unset in the second part of the IF-ELSEIF-ELSE statement.

Here is what the snapshot that I have looks like:

public function checkAuthentication()
{
    phpCAS::traceBegin();
    $res = false;
    if ( $this->isAuthenticated() ) {
        phpCAS::trace('user is authenticated');
        $res = true;
    } else if (isset($_SESSION['phpCAS']['auth_checked'])) {
        // the previous request has redirected the client to the CAS server
        // with gateway=true
        unset($_SESSION['phpCAS']['auth_checked']);
        $res = false;
    } else {
...

Solution:
Here is what the change should probably include:

public function checkAuthentication()
{
    ...
    if ( $this->isAuthenticated() ) {
        phpCAS::trace('user is authenticated');
        /* Here is where the 'auth_checked' variable is removed just in case it's set. */
        /* The variable is also removed in the second part of the IF-ELSEIF-ELSE statement. */
        phpCAS::trace('Removing "auth_checked" in case it is set);
        unset($_SESSION['phpCAS']['auth_checked']);
        $res = true;
    } else if (isset($_SESSION['phpCAS']['auth_checked'])) {
...

[jfritschi]

I think you are right. Have you had any issue with this part of the code? It's seems to me that it simply works since you are logged in in the "if" branch and doesn't really matter at that point anyway.

But it's an probably an error and could lead to other problems.

[WiredAcademic]

I was trying to force the gateway check to occur each page load. I probably should have just written new methods, but I was curious to learn more how phpCAS::checkAuthentication() worked. I'm glad I tried it that way, because I learned a lot.

[jfritschi]

@WiredAcademic: Thanks for the report.

Patch is commited.