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] Add support for previous apps keys in signed URL verification #51222

Conversation

Krisell
Copy link
Contributor

@Krisell Krisell commented Apr 26, 2024

Description

Laravel 11 introduced the ability to add app.previous_keys to migrate the app-key for encryption transparently, added in #49962

This PR adds the same support for signed URLs, by allowing URLs generated under one of the previous keys (listed in app.previous_keys) to be considered as valid.

Implementation

The implementation iterates over the acceptable app keys, with the current app key first, and returns as soon as a valid hmac is found. Of course, signing a new URL always uses the current app key.

To support this, I updated the default $keyResolver in UrlGenerator (set in RoutingServiceProvider) to return an array of keys. I can't see this return value being typed anywhere, and I kept support for single keys being returned, in case anyone injects their own key resolver.

Inspiration

The idea for this PR comes from @valorin and the Securing Laravel newsletter, where the missing support for signed URLs was mentioned.

@Krisell Krisell changed the title Add support for previous apps keys in signed URL verification [11.x] Add support for previous apps keys in signed URL verification Apr 26, 2024
@valorin
Copy link
Contributor

valorin commented May 2, 2024

This is great, nice work! 😁

I can't see any issues with it, and I like how you're still supporting single keys for backwards compatibility.

@taylorotwell taylorotwell merged commit 6659c30 into laravel:11.x May 9, 2024
28 checks passed
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