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

Laravel 5.6.30 breaks passport #795

Closed
krve opened this issue Aug 9, 2018 · 14 comments
Closed

Laravel 5.6.30 breaks passport #795

krve opened this issue Aug 9, 2018 · 14 comments

Comments

@krve
Copy link

krve commented Aug 9, 2018

If you don't serialize the cookies this breaks passport authentication. On line 190 in src/Guards/TokenGuard.php.

/**
 * Decode and decrypt the JWT token cookie.
 *
 * @param  \Illuminate\Http\Request  $request
 * @return array
 */
protected function decodeJwtTokenCookie($request)
{
    return (array) JWT::decode(
        // This line does not disable serialization per EncryptCookies.php
        $this->encrypter->decrypt($request->cookie(Passport::cookie())),
        $this->encrypter->getKey(), ['HS256']
    );
}

This is only the case if you make use of your passport api on your own site. (https://laravel.com/docs/5.6/passport#consuming-your-api-with-javascript)

@krve krve changed the title Laravel 5.6.30 breaks passport Laravel 5.6.30 breaks laravel/passport Aug 9, 2018
@krve krve changed the title Laravel 5.6.30 breaks laravel/passport Laravel 5.6.30 breaks passport Aug 9, 2018
@philtweir
Copy link

philtweir commented Aug 9, 2018

Just to link in the context:

A temporary workaround within an individual app, until Passport no longer expects to deserialize, is to add

protected static $serialize = true;

to app/Http/Middleware/EncryptCookies.php (and clear cookies before trying to log in the first time). While this does let your app work, it also prevents you getting the benefit of the security fix (where someone who has access to your APP_KEY can create their own malicious cookies that get sent to the deserialize function).

@poxin13
Copy link

poxin13 commented Aug 9, 2018

Had to add that to EncryptCookies.php as well. Same issue.

@themsaid
Copy link
Member

themsaid commented Aug 9, 2018

Opened a PR to introduce a way to stop unserializing the cookie value:

#796

@plakhin
Copy link

plakhin commented Aug 10, 2018

@themsaid please don't forger about laravel/passport 4.0, the latest one supported by Laravel 5.5 LTS

@themsaid
Copy link
Member

@plakhin can you backport this PR to it?

@plakhin
Copy link

plakhin commented Aug 10, 2018

@themsaid I'm to busy until next week. If no one will do, then I do, but a bit later.

@clnt
Copy link

clnt commented Aug 10, 2018

@themsaid I have opened a PR to the 4.0 branch: #797

@simondavies
Copy link

I had this for version 4.0 and tried to composer update but it did not download the update, I had to target it directly

composer require laravel/passport:4.0.x-dev

@prolonginc
Copy link

this was driving me nutts for a couple of hours. @simondavies the version 5 will work just clear your cookies after adding in protected static $serialize = true; to the EncryptCookies.

@clnt
Copy link

clnt commented Aug 15, 2018

@prolonginc Why would you do that? That is just disabling the security fix. Version 5.0 is for Laravel 5.6 and not 5.5

@taylorotwell Can we not just get a version 4.0.4 tagged on the 4.0 branch??

@amacsmith
Copy link

amacsmith commented Aug 15, 2018

Could an artisan command be created to do this update for users? instead of breaking upon composer update? Just wondering didn't know if that would be a viable option.

@NastuzziSamy
Copy link

This issue made me tilt the whole day

NastuzziSamy added a commit to simde-utc/portail that referenced this issue Aug 16, 2018
@NastuzziSamy
Copy link

If you have updated Laravel\Passport you should follow this:

Passport 6.0.7

Passport 6.0.7 has been released with a new Laravel\Passport\Passport::withoutCookieSerialization() method. Once you have disabled cookie serialization, you should call this method within your application's AppServiceProvider.

Using protected static $serialize = true; create a security issue...

@krve krve closed this as completed Aug 17, 2018
huetremy pushed a commit to simde-utc/portail that referenced this issue Aug 19, 2018
@yaquawa
Copy link
Contributor

yaquawa commented Aug 28, 2018

I don't understand why Laravel serialize the cookie however...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests