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

[Security] Making the component more flexible #30586

Closed
wants to merge 1 commit into from
Closed

[Security] Making the component more flexible #30586

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 17, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? yes/no (not sure yet)
Tests pass? yes
Fixed tickets none
License MIT
Doc PR none, yet.

This is the initial beginning to refactor a large part of the Security component. As purposed by @wouterj I start the initial refactoring. (cc: @nicolas-grekas)

@wouterj
Copy link
Member

wouterj commented Mar 17, 2019

Actually, I would do this just the other way: First remove/deprecate everything you'll find with User in this component. Then, we can reintroduce some user related features using this identity.
It'll be quite a challenge though, so I think it's better to split this into multiple PRs:

  1. Deprecate Token#getUser() and remove its usage in the component in favor of the other token methods;
  2. Deprecate UserProviderInterface and it's usage in the component and bundle;
  3. Introduce the IdentityInterface and build a BC layer between UserProviderInterface and the IdentityInterface.

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 17, 2019
@javiereguiluz
Copy link
Member

Actually, I wouldn't start with code for such a huge and difficult task!

This is what I'd do:

  • Make a list of the features that the Symfony component must and must not support by default. Lots of things have changed since the component was created. Some practices are now deprecated ("salting" passwords manually) and other edgy practices (2FA) are now common enough to be included in the core.
  • Show examples of how all those features would look like with the new code, so we can compare it with the current situation. For example, getting an object that represents the logged user or persisting users in the database are critical operations that must be solved perfectly. If the new systems is "better" but makes these common operations much more complicated (for the sake of "code purity") then it's a big "no go".
  • Finally, when you know what do you want to do and how it's going to be ... then you can code it. This is just a detail. Anyone with experience in PHP can do it. The important thing is the design of the new system.

@ghost ghost closed this Mar 26, 2019
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
This pull request was closed.
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

5 participants