feat: allow signing verification URLs without the host by MichaelBrauner · Pull Request #142 · SymfonyCasts/verify-email-bundle

MichaelBrauner · 2023-05-05T11:56:05Z

Closes #136.

Verification links are signed as absolute URLs, so a link is bound to the host it was generated for. This PR adds a sign_without_host option: with it enabled the path is signed instead, and the link can be redeemed under any host the application answers on.

eidenschink · 2023-05-22T11:30:25Z

This configuration option would be great. I have a use case where I want to target to a completely different host running on another framework; the hosts endpoint then proxies the request (or rather re-creates it) to the Symfony Controller.
If only the relative path is signed I can simply make the necessary hostname change in the TWIG template.

shakaran · 2025-05-27T10:45:01Z

@MichaelBrauner could you rebase for resolve the conflicts? Thanks

MichaelBrauner · 2025-06-03T20:13:35Z

Hi, this PR is now ready for review.

However, I wasn't able to run the test suite locally – vendor/bin/simple-phpunit fails with:

Trait "Symfony\Bridge\PhpUnit\Legacy\PolyfillAssertTrait" not found in Assert.php

I tried with PHP 8.1 and 8.2, removed and reinstalled the vendor folder and the PHPUnit Bridge multiple times, but no success.
Maybe you could verify if the test suite runs on your end?

Thanks!

MichaelBrauner · 2025-06-07T16:36:52Z

Can you rerun the checks?
It should pass: https://github.com/MichaelBrauner/verify-email-bundle/actions/runs/15507669530

bocharsky-bw · 2025-08-15T14:35:58Z

Sorry for the delay.

I reran the CI, but we still have errors here

melbings · 2025-12-22T09:46:20Z

@bocharsky-bw would you be so kind to re-run CI so that we can see the errors? The old logs have been archived.

bocharsky-bw · 2025-12-22T14:08:27Z

@melbings I reran the CI for you, I hope this helps! Though I see this PR now should be rebased due to some changes in master, and there are some conflicts that should be resolved too.

UPD: Yeah, probably the conflicts should be resolved first. If you want to follow up on this PR and push it forward - please, feel free to create a new PR based on these changes

melbings · 2026-01-22T09:56:45Z

@bocharsky-bw I'm afraid we're abandoning this pull request on our side. We will use Symfony's UriSigner component to deal with signed URLs instead. Nevertheless, thank you for your work and your support.

ToshY · 2026-07-01T13:48:07Z

@bocharsky-bw @jrushlow Any chance this can still be merged?

melbings · 2026-07-02T06:39:14Z

@bocharsky-bw @jrushlow Any chance this can still be merged?

I'm afraid that someone would need to resolve merge issues that have come up since we originally worked on this. Unfortunately, my company cannot work on this anymore. I assume that Claude & co. should have quite an easy time finishing this up, though.

MichaelBrauner · 2026-08-20T17:05:39Z

Rebased onto main, conflicts resolved. The option is called sign_without_host now.

Could you kick off the checks? 40 tests green locally, PHPStan and CS at parity with main.

Renames the option to sign_without_host. When it is enabled, validateEmailConfirmation() and validateEmailConfirmationFromRequest() now check the path instead of the full URL.

MichaelBrauner · 2026-08-20T19:29:23Z

The --prefer-lowest job was the only red one: three tests called validateEmailConfirmationFromRequest() without the class_exists(UriSigner::class) guard.

Could you re-run when you get a chance?

bocharsky-bw

Thank you for returning to work on it! I see many people are interested in it, so I would like to see this feature merged. I left some comments, could you take a look at them too?

Some extra tests like next ones would be very welcome:

In testValidateEmailSignatureFromRequestWithoutHost we do Request::create($signedUrl) that uses localhost. It worth checking on a different host as well, e.g. Request::create('https://other.example'.$signedUrl).
A regression test for sign_without_host: false to make sure the link with different host is declined. That's very important security concern.
A test to make sure a changed path or query is declined for sign_without_host: false
VerifyEmailHelperFunctionalTest::getHelper() now has a new arg but no tests are tested it with true

MichaelBrauner · 2026-10-20T18:17:39Z

@bocharsky-bw Great news, thanks for the review! I’ll look into your comments and the suggested tests in the next few days.

bocharsky-bw · 2026-10-21T10:48:19Z

Awesome, thanks for working on it! I would like to see it finally merged :)

The property can hold the legacy HttpKernel\UriSigner, which PHPStan cannot resolve on current Symfony. Both classes share the same API, so annotating it as the new one removes four suppressed errors without changing behaviour.

The returned URL still carries scheme and host from the router context, so it stays a working link that can go straight into an email. Routes that cannot be generated as a path - those declaring schemes or their own host - now throw a LogicException instead of producing a signature that silently covers more than the path.

With the option off, a changed host, path or query must be rejected. Switching the option in either direction invalidates links that have already been sent.

bocharsky-bw

Thanks for making more changes on it! I think we're very close to finish now, I left some minor comments, could you take a look please?

I see all tests are green too! 🎉

bocharsky-bw · 2026-10-26T18:11:19Z

+    {
+        // creating a new RequestStack is fine here, because it keeps UrlHelper on the router context,
+        // the same source generate() uses
+        return (new UrlHelper(new RequestStack(), $this->router))->getAbsoluteUrl($pathAndQuery);


Could you also explain in the comment above why we didn't inject this service instead? I think it's easy to miss, would be important to clarify

bocharsky-bw · 2026-10-26T19:32:47Z

+    private $signWithoutHost;

-    public function __construct(UrlGeneratorInterface $router, /* no typehint for BC with legacy PHP */ $uriSigner, VerifyEmailQueryUtility $queryUtility, VerifyEmailTokenGenerator $generator, int $lifetime)
+    public function __construct(UrlGeneratorInterface $router, /* no typehint for BC with legacy PHP */ $uriSigner, VerifyEmailQueryUtility $queryUtility, VerifyEmailTokenGenerator $generator, int $lifetime, bool $signWithoutHost = false)


Do you think we should duplicate setting false here explicitly? The option should be defaulted to false anyway, so we can expect it here, no need to make it optional, right?

Yes, in the DI container the service gets its parameters with the defaults from the configuration. But the class isn't marked as internal, so it may be instantiated directly in projects — dropping the default would be a BC break in a minor version.

I've added a @legacy note on the constructor so it doesn't get lost, and we can make it required in 2.0. What do you think?

bocharsky-bw · 2026-10-26T19:56:22Z

+
+        if (\is_string($host)) {
+            throw new \LogicException(\sprintf('The route "%s" generates a URL with the host "%s" instead of a path, so it cannot be signed while the "sign_without_host" option is enabled. Remove the host or scheme from the route, or set "framework.router.default_uri" to match it.', $routeName, $host));
+        }


Fairly speaking, I'm not sure we should throw in this case. We're signing without host on purpose (expclicitly), so I think it's safe to take the path if the uri was generated with host and sign only the path. I.e just force ignoring host and schema if it exists. Wdyt?

But your solution is defensive and I'm fine with it too, just trying to find the best DX path here. So it's up to you

Let's take the case where a developer declares a route with its own host. If we strip it and then prepend the host from the router context, the link in the email points at the wrong domain and 404s. You would only notice that in production. That's why I think staying defensive is the right call here. What do you think?

I see, OK, fine to me 👍🏼

bocharsky-bw

This looks solid to me! The only question left is in test stability.

Before this PR every test kernel had the same config, so that was harmless. Now kernels differ by customConfig and routes, and some tests boot 2–3 of them. I think it's possible that a test could end up running with anothertest's sign_without_host value and fail randomly.

The chance is small, but flaky CI is no fun to debug. A per-instance unique id would make it impossible:

private $uniqueId;

public function __construct()
{
    $this->uniqueId = uniqid('', true);
    parent::__construct();
}

public function getCacheDir()
{
    return sys_get_temp_dir().'/cache/'.$this->uniqueId;
}

public function getLogDir()
{
    return sys_get_temp_dir().'/logs/'.$this->uniqueId;
}

Each kernel gets a fresh compile, which is exactly what happens today anyway, just guaranteed now. Wdyt?

MichaelBrauner · 2026-10-27T11:47:11Z

This looks solid to me! The only question left is in test stability.

Before this PR every test kernel had the same config, so that was harmless. Now kernels differ by customConfig and routes, and some tests boot 2–3 of them. I think it's possible that a test could end up running with anothertest's sign_without_host value and fail randomly.

The chance is small, but flaky CI is no fun to debug. A per-instance unique id would make it impossible:
private $uniqueId;

public function __construct()
{
    $this->uniqueId = uniqid('', true);
    parent::__construct();
}

public function getCacheDir()
{
    return sys_get_temp_dir().'/cache/'.$this->uniqueId;
}

public function getLogDir()
{
    return sys_get_temp_dir().'/logs/'.$this->uniqueId;
}
Each kernel gets a fresh compile, which is exactly what happens today anyway, just guaranteed now. Wdyt?

You're right — there are nicer things in life than debugging flaky CI. I'll do exactly that.

bocharsky-bw

Thank you!

MichaelBrauner · 2026-10-27T16:36:42Z

Thank you, Victor! It was a pleasure working with you on this.

melbings · 2026-10-27T20:53:17Z

Good job, guys 👍 @MichaelBrauner @bocharsky-bw

MichaelBrauner changed the title ~~Email signature verification without domain~~ Make signature generation more flexible by allowing relative URLs May 5, 2023

MichaelBrauner force-pushed the email-signature-verification-without-domain branch 2 times, most recently from b434387 to 93d5392 Compare June 2, 2025 06:23

MichaelBrauner changed the title ~~Make signature generation more flexible by allowing relative URLs~~ feat: make signature generation more flexible by allowing relative URLs Jun 3, 2025

MichaelBrauner force-pushed the email-signature-verification-without-domain branch from 93d5392 to 90ec899 Compare June 3, 2025 20:09

MichaelBrauner marked this pull request as ready for review June 3, 2025 20:14

michaelbrauner_mysteryminds and others added 3 commits August 20, 2026 14:39

Signiture verification with relative path

a54acfd

refactor(signature): simplify signature verification for relative URLs

fcac3d4

fix(tests): adjust for relative URL signatures

8fe5bb2

MichaelBrauner force-pushed the email-signature-verification-without-domain branch from 146a026 to 9c06fb4 Compare August 20, 2026 16:35

MichaelBrauner changed the title ~~feat: make signature generation more flexible by allowing relative URLs~~ feat: allow signing verification URLs without the host Aug 20, 2026

fix: sign an absolute path, not a request-relative one

0e29805

Renames the option to sign_without_host. When it is enabled, validateEmailConfirmation() and validateEmailConfirmationFromRequest() now check the path instead of the full URL.

MichaelBrauner force-pushed the email-signature-verification-without-domain branch from 9c06fb4 to 0e29805 Compare August 20, 2026 19:25

bocharsky-bw closed this Oct 18, 2026

bocharsky-bw reopened this Oct 18, 2026

bocharsky-bw reviewed Oct 18, 2026

View reviewed changes

MichaelBrauner added 7 commits October 26, 2026 08:09

refactor(config): use defaultFalse() for sign_without_host

ed36ae7

docs(services): fix the comment on the sign_without_host argument

257d78c

test(kernel): allow routes to be configured with Route objects

95429aa

test: cover tampered URLs and switching the sign_without_host option

5b7ba04

With the option off, a changed host, path or query must be rejected. Switching the option in either direction invalidates links that have already been sent.

docs: rewrite the sign_without_host section

8ebeafb

bocharsky-bw reviewed Oct 26, 2026

View reviewed changes

MichaelBrauner added 6 commits October 27, 2026 08:51

docs: explain why UrlHelper is created instead of injected

7ed028a

test: reject tampered URLs in both signing modes

a944be1

refactor(tests): drop the redundant $helper variable

18b1547

docs: note that $signWithoutHost should become required in 2.0

2c3e628

docs: apply the suggested wording for switching the option

dea6147

test: cover switching the sign_without_host option back

679728a

bocharsky-bw reviewed Oct 27, 2026

View reviewed changes

test(kernel): give each kernel instance its own cache directory

9ec633d

bocharsky-bw approved these changes Oct 27, 2026

View reviewed changes

bocharsky-bw merged commit 98b02dc into SymfonyCasts:main Oct 27, 2026
10 checks passed

MichaelBrauner deleted the email-signature-verification-without-domain branch October 27, 2026 17:12

Conversation

MichaelBrauner commented May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eidenschink commented May 22, 2023

Uh oh!

shakaran commented May 27, 2025

Uh oh!

MichaelBrauner commented Jun 3, 2025

Uh oh!

MichaelBrauner commented Jun 7, 2025

Uh oh!

bocharsky-bw commented Aug 15, 2025

Uh oh!

melbings commented Dec 22, 2025

Uh oh!

bocharsky-bw commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melbings commented Jan 22, 2026

Uh oh!

ToshY commented Jul 1, 2026

Uh oh!

melbings commented Jul 2, 2026

Uh oh!

MichaelBrauner commented Aug 20, 2026

Uh oh!

MichaelBrauner commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bocharsky-bw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaelBrauner commented Oct 20, 2026

Uh oh!

bocharsky-bw commented Oct 21, 2026

Uh oh!

bocharsky-bw left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bocharsky-bw Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bocharsky-bw Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

MichaelBrauner Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

bocharsky-bw Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

MichaelBrauner Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

bocharsky-bw Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bocharsky-bw left a comment

Choose a reason for hiding this comment

Uh oh!

MichaelBrauner commented Oct 27, 2026

Uh oh!

bocharsky-bw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MichaelBrauner commented Oct 27, 2026

Uh oh!

melbings commented Oct 27, 2026

MichaelBrauner commented May 5, 2023 •

edited

Loading

bocharsky-bw commented Dec 22, 2025 •

edited

Loading

MichaelBrauner commented Aug 20, 2026 •

edited

Loading

bocharsky-bw left a comment •

edited

Loading