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

[5.1] Disable encryption for certain cookies #9150

Merged
merged 2 commits into from
Jun 9, 2015

Conversation

franzliedke
Copy link
Contributor

This has been a long-requested feature (#3440, #4134, #6421, #6679), so I wanted to try to get this in before the 5.1 release.

For example, in my FluxBB auth bridge package, I could put this code in the service provider:

Illuminate\Cookie\Middleware\EncryptCookies::disableFor('fluxbb_cookie_1234');

After considering the options, I think a blacklist in the middleware is the best middle ground between small code changes, ease of use and keeping the concept to this contained in this middleware.

/cc @davejamesmiller @RomainLanz @raulp @maclof

Closes #6679.

@taylorotwell
Copy link
Member

This is a pretty sensible approach. Thanks!

taylorotwell added a commit that referenced this pull request Jun 9, 2015
[5.1] Disable encryption for certain cookies
@taylorotwell taylorotwell merged commit ad8a241 into laravel:5.1 Jun 9, 2015
@maclof
Copy link

maclof commented Jun 9, 2015

@franzliedke great job dude, definitely going to be trying this out today!

*/
public static function disableFor($cookieName)
{
static::$disabled[] = array_merge(static::$disabled, (array) $cookieName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't that be static::$disabled = rather than disabled[]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the heads-up. One late-night refactoring too much. I sent a PR fixing this.

@d13r
Copy link
Contributor

d13r commented Jun 9, 2015

Thanks @franzliedke, that looks like a good solution.

@iJoyCode
Copy link

@franzliedke +1

@RomainLanz
Copy link

Thanks @franzliedke

@noil
Copy link

noil commented Jul 1, 2015

I try to use this functional (I put this code in the service provider: Illuminate\Cookie\Middleware\EncryptCookies::disableFor('fluxbb_cookie_1234');) but error happened. "Non-static method Illuminate\Cookie\Middleware\EncryptCookies::disableFor() should not be called statically, assuming $this from incompatible context"

@willrowe
Copy link
Contributor

willrowe commented Jul 3, 2015

The final implementation of this feature made the method non-static. The docs explain how to exclude cookies.

@d13r
Copy link
Contributor

d13r commented Jul 3, 2015

The docs explain how to exclude them from the app itself, but not from a package / service provider...

I haven't tested it yet, but my guess would be something like this:

// Check in case the user removed the middleware from their application
if (class_exists('App\Http\Middleware\EncryptCookies')) {
    // Get an instance of the middleware from the IoC container, and call the method
    app('App\Http\Middleware\EncryptCookies')->disableFor('fluxbb_cookie_1234');
}

I'm not sure if app('Illuminate\Cookie\Middleware\EncryptCookies') would work - it might return a different object than app('App\Http\Middleware\EncryptCookies').

Edit: Or, alternatively, do nothing special in the code and tell the user to add the exclude to App\Http\Middleware\EncryptCookies in the installation instructions.

@revengel
Copy link

Hi. What news about 'disableFor' method? How I can disable some cookie encryption in package? (Laravel 5.1)

How to use method 'disableFor' in my code? Please help.

app('App\Http\Middleware\EncryptCookies')->disableFor('foo');  // no works for me

@tillkruss
Copy link
Collaborator

app('App\Http\Middleware\EncryptCookies') returns a different object each call.

How exactly can disableFor() be called?

@mattcrowe
Copy link

I don't know if this is the best way to do this, but to get this to work I added the following to the "register" method in:

App\Providers\AppServiceProvider:

use Illuminate\Cookie\Middleware\EncryptCookies;

//...

public function register() {
$this->app->singleton('Illuminate\Cookie\Middleware\EncryptCookies', function($app) {
return new EncryptCookies($app['Illuminate\Contracts\Encryption\Encrypter']);
});
}

@d13r
Copy link
Contributor

d13r commented Oct 27, 2015

That can be reduced to:

public function register() {
    $this->app->singleton('Illuminate\Cookie\Middleware\EncryptCookies');
}

@tillkruss
Copy link
Collaborator

Ah, thanks, cool solution. I solved it like this in my tests:

$this->app->resolving(EncryptCookies::class, function ($object) {
    $object->disableFor('c');
});

@gibtang
Copy link

gibtang commented Aug 31, 2016

Is this going to the next release? Disabling encryption for certain cookies is useful for me too, especially when I need my cookie to interact with JS

@larabelt
Copy link

Disabling cookie encryption appears to be easier in 5.3:

https://laravel.com/docs/5.3/responses#cookies-and-encryption

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

Successfully merging this pull request may close these issues.