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] Make stateful firewalls turn responses private only when needed#33663

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
fabpot merged 1 commit intosymfony:4.4fromnicolas-grekas:sec-dis-track2
Sep 24, 2019

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedSep 22, 2019
edited
Loading

QA
Branch?4.4
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#26769et al.
LicenseMIT
Doc PR-

Replaces#28089

By taking over session usage tracking and replacing it with token usage tracking, we can prevent responses that don't actually use the token from turning responses private without changing anything to the lifecycle of security listeners. This makes the behavior much more seamless, allowing to still log the user with the monolog processor, and display it in the profiler toolbar.

This works by using two separate token storage services:

  • security.token_storage now tracks access to the token and increments the session usage tracker when needed. This is the service that is injected in userland.
  • security.untracked_token_storage is a raw token storage that just stores the token and is disconnected from the session. This service is injected in places where reading the session doesn't impact the generated output in any way (as e.g. in Monolog processors, etc.)

@nicolas-grekas
Copy link
MemberAuthor

nicolas-grekas commentedSep 23, 2019
edited
Loading

PR ready. I verified locally thatdeps=high failure is going to be fixed once this is merged into master.

@fabpot
Copy link
Member

Thank you@nicolas-grekas.

fabpot added a commit that referenced this pull requestSep 24, 2019
…ate only when needed (nicolas-grekas)This PR was merged into the 4.4 branch.Discussion----------[Security] Make stateful firewalls turn responses private only when needed| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#26769 *et al.*| License       | MIT| Doc PR        | -Replaces#28089By taking over session usage tracking and replacing it with token usage tracking, we can prevent responses that don't actually use the token from turning responses private without changing anything to the lifecycle of security listeners. This makes the behavior much more seamless, allowing to still log the user with the monolog processor, and display it in the profiler toolbar.This works by using two separate token storage services:- `security.token_storage` now tracks access to the token and increments the session usage tracker when needed. This is the service that is injected in userland.- `security.untracked_token_storage` is a raw token storage that just stores the token and is disconnected from the session. This service is injected in places where reading the session doesn't impact the generated output in any way (as e.g. in Monolog processors, etc.)Commits-------20df3a1 [Security] Make stateful firewalls turn responses private only when needed
@fabpotfabpot merged commit20df3a1 intosymfony:4.4Sep 24, 2019
@nicolas-grekasnicolas-grekas deleted the sec-dis-track2 branchSeptember 24, 2019 16:08
chalasr added a commit that referenced this pull requestSep 27, 2019
…colas-grekas)This PR was merged into the 4.4 branch.Discussion----------[Security] add "anonymous: lazy" mode to firewalls| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       |Fixes#26769 et al.| License       | MIT| Doc PR        | -Contains#33663 until it is merged.This PR allows defining a firewall as such:```yamlsecurity:    firewalls:        main:            anonymous: lazy```This means that the corresponding area should not start the session / load the user unless the application actively gets access to it. On pages that don't fetch the user at all, this means the session is not started, which means the corresponding token neither is. Lazily, when the user is accessed, e.g. via a call to `is_granted()`, the user is loaded, starting the session if needed.See#27817 for previous explanations on the topic also.Note that thanks to the logic in#33633, this PR doesn't have the drawback spotted in#27817: here, the profiler works as expected.Recipe update pending atsymfony/recipes#649Commits-------5cd1d7b [Security] add "anonymous: lazy" mode to firewalls
@nicolas-grekasnicolas-grekas modified the milestones:next,4.4Oct 27, 2019
@Toflar
Copy link
Contributor

I don't quite get all the details that were discussed here and in the APIP issue (#34220) but this is really breaking the current caching implementation for Contao. We're usingscheb/two-factor-bundle for 2FA and a quick xdebug session pointed me tohttps://github.com/scheb/two-factor-bundle/blob/master/Security/Http/Firewall/TwoFactorListener.php#L152. So there's always someone callinggetToken() and thus, all responses will always be private.
Can you maybe elaborate on how that should be fixed? I mean, why shouldgetToken() cause tracking if there is none?

@nicolas-grekas
Copy link
MemberAuthor

Even when there is no token, the page cannot be cached. Why? Because if you cache it, it means you're going to serve a response from the cache that has been generated while no token was set - while the content of the resource could be different when there is a token.

Seehttps://symfony.com/blog/new-in-symfony-4-4-lazy-firewalls for a new feature in 4.4 that improves the situation.

@Toflar
Copy link
Contributor

Even when there is no token, the page cannot be cached. Why? Because if you cache it, it means you're going to serve a response from the cache that has been generated while no token was set - while the content of the resource could be different when there is a token.

I agree. However, that's kind of killing our concept. I'm fine with it in Symfony though, as it seems to make sense for the majority of apps. We have the option to disable the auto control by setting the header on the response. However, I would still like to be able to track whether a session has been used or not - independent fromgetToken() calls. And this is impossible now without decorating or overriding loads of services.
If, however, for all the ESI requests the whole kernel was shut down, it would work properly.
I did some draft PR and explained more over at#34688.

Seehttps://symfony.com/blog/new-in-symfony-4-4-lazy-firewalls for a new feature in 4.4 that improves the situation.

That's unrelated. AgetToken() call will still initialize the session and thus cause a response to turn private.

kayue reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

5 participants

@nicolas-grekas@fabpot@Toflar@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp