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

Add new twig bridge function to generate impersonation path #50030

Merged
merged 1 commit into from Oct 1, 2023

Conversation

PhilETaylor
Copy link
Contributor

@PhilETaylor PhilETaylor commented Apr 16, 2023

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
License MIT
Doc PR symfony/symfony-docs#...

Before this PR

So we already have impersonation features in Symfony (https://symfony.com/doc/current/security/impersonating_user.html) and we have two twig helper functions impersonation_exit_url and impersonation_exit_path which both work with the configuration parameter for the switch user.

If the developer changes the switch parameter (_switch_user), then these helper functions will dynamically update the _switch_user=_exit type urls/paths.

However, to switch TO a user, hand crafted urls with ?_switch_user=MYIDENTIFIER like http://example.com/somewhere?_switch_user=thomas need to be hand crafted currently.

The problem

if we now go and change _switch_user to be something else, like _want_to_be_this_user in the Symfony configuration (Because the boss told us to do that), then all our exit path/urls will dynamically update, but our hard coded ?_switch_user=MYIDENTIFIER` will stop working.

The solution this PR provides

The solution this PR provides is to provide a new Twig Helper function for the impersonation path only, taking into account the configured value in Symfony config of the parameter (default is still _switch_user but can be anything like _want_to_be_this_user as per the docs)

This new twig function can be used as such:

<a href="{{ impersonation_path('mike') }}">Impersonate Mike</a>

This would output ?_want_to_be_this_user=mike or if the default parameter still used would be ?_switch_user=mike

The PR repurposes the existing code to generate the paths and is backward compatible.

@fabpot
Copy link
Member

fabpot commented Oct 1, 2023

Thank you @PhilETaylor.

@fabpot fabpot merged commit 6ef8975 into symfony:6.4 Oct 1, 2023
4 of 9 checks passed
fabpot added a commit that referenced this pull request Oct 2, 2023
…ory and add `impersonation_url()` (alexandre-daubois)

This PR was merged into the 6.4 branch.

Discussion
----------

[Security] Make `impersonation_path()` argument mandatory and add `impersonation_url()`

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT

Follow-up of #50030

When documenting this function, I found out that the`identifier` argument was optional, which seemed weird to me given the function purpose.

I then had a look at the implementation, and I saw that `ImpersonateUrlGenerator::generateImpersonationPath()` accepts a nullable string. However, the underlying call to `ImpersonateUrlGenerator::buildPath()` doesn't accept a nullable string. I propose to make the `identifier` argument mandatory, which makes more sense here.

I also added the missing Changelog line and `impersonation_url()`

Commits
-------

5d71d95 [Security] Make `impersonation_path()` argument mandatory and add `impersonation_url()`
This was referenced Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants