Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Security] Allow configuring a target url when switching user#46338

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
chalasr merged 1 commit intosymfony:6.2from94noni:ft-switch-redirect
Jul 20, 2022

Conversation

@94noni
Copy link
Contributor

@94noni94noni commentedMay 12, 2022
edited
Loading

QA
Branch?6.2
Bug fix?no
New feature?yes
Deprecations?no
Ticketsnone
LicenseMIT
Doc PRif accepted

When using theuser switch feature, I sometime found myself needing to redirect to a specific url (I took as example the logout target config)

Tests will be checked as well as doc if PR is acceptable

Thus I am proposing this feature, thank you,

PR reworked after merge at#47343

HeahDude and maxhelias reacted with thumbs up emoji
@carsonbot
Copy link

Hey!

I think@edefimov has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@94noni
Copy link
ContributorAuthor

@wouterj@chalasr friendly ping as code owner
does this small feature would be accepted so I can continue working on the PR (tests etc ?)

many thanks !

@chalasr
Copy link
Member

Thanks for the ping@94noni, this looks sensible to me.

Should we throw an error if bothtarget_url andstateless are set?

@94noni
Copy link
ContributorAuthor

@chalasr I appreciate the fast feedback!

this looks sensible to me. Should we throw an error if both target_url and stateless are set?

I will check this part as I know a little bit on this side
Are you thinking on any pitfall regarding this?

@chalasr
Copy link
Member

chalasr commentedJun 29, 2022
edited
Loading

@94noni It's just about DX: setting atarget_url on a stateless firewall does not make sense as there is no redirect in that case, hence forbidding that combination would make sense I think. Better be explicit than silent :) especially in the security area. That check could be done in SecurityExtension::createSwitchUserListener

@94noni
Copy link
ContributorAuthor

94noni commentedJul 6, 2022
edited
Loading

@chalasr Indeed it makes sens, I will have a deeper look at it soon, thx

status: needs work

@94noni
Copy link
ContributorAuthor

@chalasr I've tried to implement yourrequest comment

feel free to give me feedbacks :)

@chalasr
Copy link
Member

Thank you@94noni.

94noni reacted with thumbs up emoji

@chalasrchalasr merged commite89f0ff intosymfony:6.2Jul 20, 2022
@94noni94noni deleted the ft-switch-redirect branchJuly 20, 2022 20:39
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestSep 20, 2022
… user (94noni)This PR was squashed before being merged into the 6.2 branch.Discussion----------[Security] Allow configuring a target url when switching userDocumentssymfony/symfony#47343Supersedsymfony/symfony#46338 (feature changed)Close#17021Commits-------90dc3c8 [Security] Allow configuring a target url when switching user
@fabpotfabpot mentioned this pull requestOct 24, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@chalasrchalasrchalasr approved these changes

@wouterjwouterjAwaiting requested review from wouterjwouterj is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

6.2

Development

Successfully merging this pull request may close these issues.

4 participants

@94noni@carsonbot@chalasr@fabpot

[8]ページ先頭

©2009-2025 Movatter.jp