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

[6.x] Split hasValidSignature method #30208

Merged
merged 1 commit into from
Oct 8, 2019

Conversation

dwightwatson
Copy link
Contributor

This splits up the functionality of the hasValidSignature method into two separate checks - whether the signature matches the given path/URL, and whether the expires timestamp has since passed or not.

The benefit of providing access to these separately is it allows apps to handle these cases differently. It would allow you to catch InvalidSignatureException and then provide a better user experience when a signature was actually correct, but has just since expired.

My use case is that some emails we send include signed URLs that will automatically log our users in and then redirect them to the page the user intended to go to. In the case that the URL has expired I would still like redirect them to the intended page, but just skip the automatic login.

@netpok
Copy link
Contributor

netpok commented Oct 8, 2019

Maybe we should introduce an ExpiredSinatureException which extends the InvalidSignatureException?

@taylorotwell taylorotwell merged commit 20ae4a5 into laravel:6.x Oct 8, 2019
@dwightwatson
Copy link
Contributor Author

@netpok I was actually ready to put a PR together that was exactly like that, but felt that implementation may have been a little too heavy. This PR was a simpler compromise.

https://github.com/laravel/framework/compare/6.x...dwightwatson:signature-expiry?expand=1

@ghost
Copy link

ghost commented Aug 28, 2023

Could you do this PR?

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