Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cookie decryption when using psr7 #9263

Merged
merged 1 commit into from
Aug 14, 2016

Conversation

jadb
Copy link
Contributor

@jadb jadb commented Aug 14, 2016

When upgrading an application that uses encrypted cookies, tests failed. After looking for quite some time, I was able to spot cookies being transformed from the PSR7 response to Cake's own but with the == encoded; which was unsurprisingly breaking decryption.

I should also add that I am not sure this is the best way to resolve this as I haven't played much with the new PSR7 support yet. Maybe the fix should go elsewhere.

Update Without this change, it also fails when using unencrypted cookies with serialized array values.

@lorenzo
Copy link
Member

lorenzo commented Aug 14, 2016

Cookies with serialized values sounds scary

@jadb
Copy link
Contributor Author

jadb commented Aug 14, 2016

@lorenzo, I agree. But issue is with any value with URL encodable characters (encrypted values are one example).

@markstory markstory added this to the 3.3.1 milestone Aug 14, 2016
*/
public function testCanAssertCookieEncryptedWithAesWhenUsingPsr7()
{
$this->_useHttpServer = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a method for this, but I can change that post merge.

@markstory markstory merged commit f6f293e into master Aug 14, 2016
@markstory markstory deleted the fix-encrypted-cookies-with-psr7 branch August 14, 2016 15:42
@markstory
Copy link
Member

Looks good to me

@argentum-websolutions
Copy link

I faced the same issues with CakePHP 3.3.5 while trying to implement a 'Remember Me' feature.

Post Login using Form authentication, I was setting the cookie using default encryption and security salt.

$this->Cookie->write('CookieAuth', [
                            'username' => $this->request->data['mail'],
                            'password' => $this->request->data['password']
                            ]);

In the AppController beforeFilter method, I have set up the script to read the cookie and relogin using the values.

if (!$this->Auth->user() && $this->Cookie->read('CookieAuth')) {
// do something
}

However $this->Cookie->read always returns an empty string.

Studying the Cookie Component Scripts, I have found the solution that solved the problem for me.

The values passed to the _decode and _decrypt functions of Cake\Utility\CookieCryptTrait are urlencoded which is encoding the '=' in the prefix and causing the issue.

I modified each of the two functions by adding a urldecode line to it (the first line), and that solved the problem.

protected function _decode($value, $encrypt, $key)
    {
        $value = urldecode($value);
        if (!$encrypt) {
            return $this->_explode($value);
        }
         ...
         ...
}

protected function _decrypt($values, $mode, $key = null)
    {
        $values = urldecode($values);
        if (is_string($values)) {
            return $this->_decode($values, $mode, $key);
        }

        $decrypted = [];
        foreach ($values as $name => $value) {
            $decrypted[$name] = $this->_decode($value, $mode, $key);
        }

        return $decrypted;
    }

@markstory
Copy link
Member

Can you try deleting the old cookies and creating new ones. This sounds like the issue that was fixed in #9557.

@argentum-websolutions
Copy link

argentum-websolutions commented Oct 16, 2016

I tries deleting the old cookies. It did not help. I even tried building a different new cooking and had the same issue. Incidentally, this is a fresh install of CakePHP 3.3.5 and not an upgrade from an older setup.

@markstory
Copy link
Member

Can you try upgrading to 3.3.6? That version contains the fix from #9557

@argentum-websolutions
Copy link

The upgrade took care of the issue. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants