Avoid Username/Email is passed as clear GET Parameter to CheckEmailAction on password reset process

[BA-JBI]

Subject

Avoid Username/Email is passed as clear GET Parameter to CheckEmailAction on password reset process as discussed in #1692

I am targeting this branch, because this fixes an privacy issue and doesn't break anything.

Closes #1692.

Changelog

Removed

• After requesting new passwort the username isn't passed to CheckEmailAction anymode

[Hanmac]

need to update about changed parameters

SonataUserBundle/src/Resources/config/actions_resetting.php

Lines 38 to 46 in 8713dce

	->set('sonata.user.action.check_email', CheckEmailAction::class)
	->public()
	->args([
	service('twig'),
	service('router'),
	service('sonata.admin.pool'),
	service('sonata.admin.global_template_registry'),
	abstract_arg('token ttl'),
	])

[BA-JBI]

Sorry, I forgot the service definitions because I'm already way too spoiled by Symfony's autowiring technology

[Hanmac]

Ugh, I forgot another location again:

SonataUserBundle/src/DependencyInjection/SonataUserExtension.php

Lines 123 to 124 in 8713dce

	$container->getDefinition('sonata.user.action.check_email')
	->replaceArgument(4, $config['token_ttl']);

[Hanmac]

@VincentLanglet I think this MR is ready to run the Tests?

[VincentLanglet]

This is a BC break.

If someone does new CheckEmailAction($twig, $urlGenerator, $adminPool, ...) it will now crash.

We have to keep a BC signature. You can get inspiration from https://github.com/sonata-project/SonataUserBundle/blob/4.x/src/GoogleAuthenticator/RequestListener.php#L58-L88

public function __construct(
    private Environment $twig,
    Pool|UrlGeneratorInterface $adminPool,
    TemplateRegistryInterface|Pool $templateRegistry,
    int|TemplateRegistryInterface $tokenTtl,
    ?int $tokenTtl = null,
) {
    if ($adminPool instanceof UrlGeneratorInterface) {
            if (!$templateRegistry instanceof Pool) {
                throw new TypeError(...);
            }
            $this->adminPool = $templateRegistry;

            @trigger_error(sprintf(
                'Passing an instance of %s as argument 2 to "%s()" is deprecated since sonata-project/user-bundle 5.x and will only accept an instance of %s in version 6.0.',
                UrlGeneratorInterface::class,
                __METHOD__,
                Pool::class
            ), \E_USER_DEPRECATED);
        } else {
            $this->adminPool = $adminPool;
        }
        
        // And so on
}

[Hanmac]

@VincentLanglet about the native_function_invocation Lint, should I make an MR for this, or should a bot handle that?

[VincentLanglet]

A rebase should solve the issues

[BA-JBI]

Tried to apply the backward compatibility as good as possible similar to the suggested code.
Hope this matches the expected solution

[Hanmac]

For BC, the Request $request param probably needs to be optional.
(with Deprecate Warning)

[VincentLanglet]

No I don't think so.

There won't be any error if you pass an extra param

[Hanmac]

Ah, i misunderstand the errors there,

it was only Psalm/PHPStan complaining, not a real PHP error.

[VincentLanglet]

I rebased fixed and merged in #1695.

Thanks !

[BA-JBI]

Yey cool
Thank You !!!