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

[11.x] Support regular expressions for cookies to exclude from encryption #53724

Closed
wants to merge 7 commits into from

Conversation

selcukcukur
Copy link
Contributor

Cookies that should be excluded from encryption currently support saving individually and directly with the cookie name. However, sometimes we may need to create multiple cookies that start with a specific name and exclude all of them from encryption. When this happens, we can make it easier to exclude all cookies that start with the '*' character, rather than excluding cookies one by one.

For example, when you add collapse and expand feature to the components in the dashboard, you want to store the data in cookies or you want to store the user's preferences such as language and currency in cookies. Although it is possible to do this on the Laravel side, when you try to change this in normal javascript frameworks, you may have problems getting the cookie value due to encryption.

However, once this pull request is implemented, you can now exclude such cookies as follows instead of excluding them one by one.

laravel_user_*
laravel_online_widget_*

When you define the $except property in this way, all cookies starting with laravel_user_xxx or laravel_online_widget_xxxx are considered excluded from encryption.

Comment on lines 219 to 227
$exclude = Collection::make($this->except)
->merge(static::$neverEncrypt)
->filter(fn ($cookie) => Str::contains($cookie,'*'));

return $exclude->isEmpty()
? in_array($name, array_merge($this->except, static::$neverEncrypt))
: $exclude->filter(fn ($cookie) => Str::startsWith(
$name, Str::replace('*', '', $cookie)
))->isNotEmpty();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be simplified by using some() over chaining filter() with isEmpty()

Suggested change
$exclude = Collection::make($this->except)
->merge(static::$neverEncrypt)
->filter(fn ($cookie) => Str::contains($cookie,'*'));
return $exclude->isEmpty()
? in_array($name, array_merge($this->except, static::$neverEncrypt))
: $exclude->filter(fn ($cookie) => Str::startsWith(
$name, Str::replace('*', '', $cookie)
))->isNotEmpty();
$exclude = Collection::make($this->except)
->merge(static::$neverEncrypt)
->filter(fn ($cookie) => Str::contains($cookie,'*'));
return $exclude->isEmpty()
? in_array($name, array_merge($this->except, static::$neverEncrypt))
: $exclude->some(fn ($cookie) => Str::startsWith(
$name, Str::replace('*', '', $cookie)
));

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, seems like it would be easier if the class property $except would be a collection (especially, with the comment below) 🤔

Copy link
Contributor

@shaedrich shaedrich Dec 2, 2024

Choose a reason for hiding this comment

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

Not sure if you still want to simplify the filter(...)->isNotEmpty() to some(...) to be consistent with doesntContain(...)

Comment on lines 219 to 227
$exclude = Collection::make($this->except)
->merge(static::$neverEncrypt)
->filter(fn ($cookie) => Str::contains($cookie,'*'));

return $exclude->isEmpty()
? in_array($name, array_merge($this->except, static::$neverEncrypt))
: $exclude->filter(fn ($cookie) => Str::startsWith(
$name, Str::replace('*', '', $cookie)
))->isNotEmpty();
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, seems like it would be easier if the class property $except would be a collection (especially, with the comment below) 🤔

@taylorotwell
Copy link
Member

Would rather just see something like Str::is() usage, but this could be a breaking change in general.

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.

3 participants