Skip to content

[HttpFoundation] Added MarshallingSessionHandler #35804

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 25, 2020
Merged

[HttpFoundation] Added MarshallingSessionHandler #35804

merged 1 commit into from
Feb 25, 2020

Conversation

atailouloute
Copy link
Contributor

@atailouloute atailouloute commented Feb 20, 2020 •
edited
Loading

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR TODO

Added MarshallingSessionHandler, a decorator for session handlers which uses the cache marshaller in order to encrypt session data.

(This is an alternative solution to #35643)

To use it, we can simply decorate the session marshaller, after that all session data will be encrypted

Symfony\Component\Cache\Marshaller\SodiumMarshaller:
    decorates: 'session.marshaller'
    arguments:
        - ['%env(file:resolve:SODIUM_DECRYPTION_FILE)%']
        - '@Symfony\Component\Cache\Marshaller\SodiumMarshaller.inner'

TODO:

  • unit tests

@atailouloute atailouloute force-pushed the http-foundation-marshalling-session-handler branch from 45f08b1 to 4253566 Compare February 20, 2020 17:43
@nicolas-grekas nicolas-grekas added this to the next milestone Feb 20, 2020
@atailouloute atailouloute force-pushed the http-foundation-marshalling-session-handler branch 2 times, most recently from 46b8c7b to bed3c72 Compare February 20, 2020 17:58
@nicolas-grekas nicolas-grekas changed the title [HttpFoundation][Cache] Added MarshallingSessionHandler [HttpFoundation] Added MarshallingSessionHandler Feb 20, 2020
@atailouloute atailouloute force-pushed the http-foundation-marshalling-session-handler branch 2 times, most recently from 466738c to 3c75755 Compare February 20, 2020 18:02
@atailouloute atailouloute force-pushed the http-foundation-marshalling-session-handler branch from 3c75755 to 54e790f Compare February 20, 2020 18:25
@atailouloute atailouloute mentioned this pull request Feb 20, 2020
Closed
2 tasks
@stof
Copy link
Member

stof commented Feb 21, 2020

Why adding an IdentityMarshaller in HttpFoundation ?

@atailouloute
Copy link
Contributor Author

Why adding an IdentityMarshaller in HttpFoundation ?

We had it before in Cache, but it seemed that it will be used only by the MashallingSessionHandler

wdyt @nicolas-grekas ?

@nicolas-grekas
Copy link
Member

Because the only use case is in HttpFoundation, in Cache it is not used and won't ever be.

@atailouloute atailouloute force-pushed the http-foundation-marshalling-session-handler branch from 54e790f to 2c2dce6 Compare February 21, 2020 23:07
nicolas-grekas
nicolas-grekas reviewed Feb 23, 2020
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

What about adding some base services in framework-bundle to make using this as easy as possible?

We could have a session.marshaller service that defaults to the IdentityMarshaller, wired to wrap the session handler. Then, when this marshaller service is not decorated, we would remove the wrapper to optimize the handler in a compiler pass. WDYT? Doable?

(please update the description of the PR once we have a working way of doing so).

@atailouloute atailouloute force-pushed the http-foundation-marshalling-session-handler branch from 2c2dce6 to dc88fa7 Compare February 23, 2020 21:59
@atailouloute
Copy link
Contributor Author

What about adding some base services in framework-bundle to make using this as easy as possible?

We could have a session.marshaller service that defaults to the IdentityMarshaller, wired to wrap the session handler. Then, when this marshaller service is not decorated, we would remove the wrapper to optimize the handler in a compiler pass. WDYT? Doable?

(please update the description of the PR once we have a working way of doing so).

Yes, I already did that, I pushed the changes and updated the description

@atailouloute atailouloute force-pushed the http-foundation-marshalling-session-handler branch from dc88fa7 to 155d980 Compare February 23, 2020 22:34
nicolas-grekas
nicolas-grekas approved these changes Feb 25, 2020
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

That's great, the service decoration makes this trivial to use.
We can even remove the @Symfony\Component\Cache\Marshaller\SodiumMarshaller.inner argument when autowiring is enabled.

@fabpot
Copy link
Member

fabpot commented Feb 25, 2020

Thank you @atailouloute.

@fabpot fabpot mentioned this pull request Feb 25, 2020
Closed
@fabpot fabpot merged commit 28a249f into symfony:master Feb 25, 2020
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
Merged
@wouterj wouterj mentioned this pull request Dec 6, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Cache Feature HttpFoundation Status: Reviewed
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.
5 participants
@atailouloute @stof @nicolas-grekas @fabpot @carsonbot