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

[FrameworkBundle] makeKernelBrowser::loginUser() session available for updating after login#47001

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

Open
arderyp wants to merge1 commit intosymfony:7.4
base:7.4
Choose a base branch
Loading
fromarderyp:6.2

Conversation

arderyp
Copy link
Contributor

@arderyparderyp commentedJul 21, 2022
edited by OskarStark
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
Tickets#46961
LicenseMIT
Doc PRsymfony/symfony-docs#...

After upgrading from 4.4 to 5.4, I started running into deprecation warnings about fetchingsecurity.csrf.token_manager andsecurity.csrf.token_storage from the test container. The issue was that, in my functional tests, I was logging in, then I needed to generate and apply CSRF tokens to the logged in session so that I could directly submit data arrays to POST controller action methods without having to crawl to the form page first.

Since I was not crawling to the form page first, no CSRF for the form was generated and applied to the session. Oddly enough, generating tokens from the CSRF token storage services did work, despite the deprecation warnings, and I'm not entirely sure how that was working without an activated session pointer. I prefer to not crawl to the form page first as it would double the amount of crawler requests in my test suite, so this approach is mostly for convenience (easier to abstract than crawler DOM interactions) and speed.

So, this is a simply PR that probably needs tweaks, and test coverage and docs, but I didn't want to invest the time into the later two if the Symfony team thinks this is not an idea they'd consider for implementation.

Anyways, all this PR really does is track an internal pointer to the generated test session onKernelBrowser, which can be manipulated after callingloginUser().

I've explained the workaround to my problem, which implements this kind of logic here:#46961

I suppose another simpler option would be to continue allowing the use of the csrf token storage services without an active sessionwithin the test container (to basically function as they do now but without the deprecation warnings). I suspect this might not be possible given the system migration towardsRequestStack.

Given this PR, I could now do:

useSymfony\Component\Security\Csrf\TokenStorage\SessionTokenStorage;$user =...;$client =static::createClient();$client->loginUser($user);// Technically, you don't need to generate a real token here, and instead could use any test string$tokenId =...;$csrfToken =static::getContainer()->get('security.csrf.token_generator')->generateToken();$client->setLoginSessionValue(SessionTokenStorage::SESSION_NAMESPACE ."/$tokenId",$csrfToken);

I imagine there are wide applications for this feature beyond CSRF tokens.

marien-probesys, solverat, jdreesen, chm0815, syther101, and endelwar reacted with thumbs up emoji
@carsonbotcarsonbot added this to the6.2 milestoneJul 21, 2022
@carsonbotcarsonbot changed the title[feature] make KernelBrowser::loginUser() session available for updating after loginmake KernelBrowser::loginUser() session available for updating after loginJul 21, 2022
@arderyp
Copy link
ContributorAuthor

On a related note, it would be nice to be able to tellloginUser() which token type to user, for exampleUsernamePasswordToken instead ofTestBrowserToken.

@nicolas-grekasnicolas-grekas modified the milestones:6.2,6.3Nov 5, 2022
@nicolas-grekasnicolas-grekas modified the milestones:6.3,6.4May 23, 2023
@nicolas-grekasnicolas-grekas modified the milestones:6.4,7.1Nov 15, 2023
@arderyp
Copy link
ContributorAuthor

Any thoughts on this?

@xabbuhxabbuh modified the milestones:7.1,7.2May 15, 2024
@carsonbotcarsonbot changed the titlemake KernelBrowser::loginUser() session available for updating after login[FrameworkBundle] make KernelBrowser::loginUser() session available for updating after loginMay 15, 2024
@fabpotfabpot modified the milestones:7.2,7.3Nov 20, 2024
@arderyp
Copy link
ContributorAuthor

@fabpot@nicolas-grekas thoughts?

@OskarStarkOskarStark changed the title[FrameworkBundle] make KernelBrowser::loginUser() session available for updating after login[FrameworkBundle] makeKernelBrowser::loginUser() session available for updating after loginFeb 11, 2025
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Just wondering: did you try using$container->get('session.factory')->createSession(); directly in your test cases?

@@ -33,6 +34,7 @@ class KernelBrowser extends HttpKernelBrowser
private bool $hasPerformedRequest = false;
private bool $profiler = false;
private bool $reboot = true;
private SessionInterface $session;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Keeping the session in a property might be a bad idea, it's a stateful object.

@arderyp
Copy link
ContributorAuthor

Just wondering: did you try using$container->get('session.factory')->createSession(); directly in your test cases?

yes@nicolas-grekas, in fact I am using that in my workaround. callingstatic::getContainer()->get('security.csrf.token_generator')->generateToken(); doesn't seem to apply said token to the authenticated test session, so CSRF tokens are always invalid (unless I apply them myself via my workaround, which applies the same principle as this PR)

@fabpotfabpot modified the milestones:7.3,7.4May 26, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

Assignees
No one assigned
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.

5 participants
@arderyp@nicolas-grekas@fabpot@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp