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

[HttpFoundation] Temporary URI signed #51501

Closed
BaptisteContreras opened this issue Aug 27, 2023 · 0 comments · Fixed by #51502
Closed

[HttpFoundation] Temporary URI signed #51501

BaptisteContreras opened this issue Aug 27, 2023 · 0 comments · Fixed by #51502

Comments

@BaptisteContreras
Copy link
Contributor

Description

I would like to add the possibility to create a temporary URI signed.

To achieve this, we can add an expiration parameter to the UriSigner's sign method.
By default at null, if a value is passed, a query parameter with the timestamp of the expiration date + time will be added before the computation of the URI hash.
Thus when checking the URI, if this query parameter is present, the timestamp will be used by the check method to determine if the URI has expired.

I propose a signature like this :

public function sign(string $uri, \DateTimeInterface|\DateInterval|int $expiration = null): string
  • If it's a \DatetimeInterface, it's converted to a timestamp.
  • If it's a \DateInterval, it's added to "now" then converted to a timestamp.
  • If it's an int, it's expected to be a timestamp in seconds.

If null, nothing is added.

Example

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpKernel\UriSigner;
use Symfony\Component\Routing\Annotation\Route;

class DemoController extends AbstractController
{

    #[Route('/', name: 'index')]
    public function index(UriSigner $signer): Response
    {
        $signed = $signer->sign('demo?foo=bar', new \DateTime('2050-01-01')); // Valid for years..
        dump($signer->check($signed)); // true

        $signed = $signer->sign('demo?foo=bar', new \DateInterval('PT10S')); // Valid for 10 seconds from now
        dump($signer->check($signed)); // true
        sleep(30);
        dump($signer->check($signed)); // false
        
        $signed = $signer->sign('demo?foo=bar', 4070908800); // 2099-01-01
        dump($signer->check($signed)); // true
    }
    
}
@xabbuh xabbuh added the Feature label Sep 4, 2023
@BaptisteContreras BaptisteContreras changed the title [HttpKernel] Temporary URI signed [HttpFoundation] Temporary URI signed Oct 30, 2023
@fabpot fabpot closed this as completed Mar 17, 2024
fabpot added a commit that referenced this issue Mar 17, 2024
This PR was merged into the 7.1 branch.

Discussion
----------

[HttpKernel] Add temporary URI signed

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #51501
| License       | MIT
| Doc PR        | TODO

This feature add the possibilty to create a temporary URI signed as mentionned in #51501.

Few things about this implementation :

- I added an optional parameter to the constructor of the `UriSigner`. I'm not sure if it can be considered as a **BC** ?
- The query parameter for expiration (by default "_expiration") is considered as a **reserved parameter** for the signer and if you try to sign an URI with this parameter, the method will throw
a LogicalException.  In fact, the `check `method will test the presence of the expiration query parameter to determine if it must (or not) verify the expiration.
A problem can arise in this case : Lets say you want to sign this URI : "/demo?_expiration=foo" and you don't want a temporary URL. The URI is signed without problem but when you try to call `check`, the method will notice the
presence of "_expiration" and try to determine if it has expired, considering the value "foo" as a timestamp.

Commits
-------

f0c2cfb [HttpFoundation] Add temporary URI signed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants