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

[Feature Request] 'non-advanced' FosUser? #2911

Closed
stephanvierkant opened this issue May 28, 2019 · 5 comments
Closed

[Feature Request] 'non-advanced' FosUser? #2911

stephanvierkant opened this issue May 28, 2019 · 5 comments

Comments

@stephanvierkant
Copy link

In the master branch, the fields of AdvancedUserInterface have been added to the UserInterface. This is a workaround to get rid of the deprecation notices, but doesn't follow the latest best practices:

https://symfony.com/blog/new-in-symfony-4-1-deprecated-the-advanceduserinterface:

One of our ongoing goals for security is to simplify some of its features. (..) Given that these methods are mostly related to your application domain logic, we've decided to deprecate it in Symfony 4.1 and remove it in Symfony 5.0. No alternative is provided for this interface. If you need this kind of checks, create a user checker, which perform additional checks during the authentication of a user to verify if the identified user is allowed to log in.

I've never used the methods in AdvancedUserInterface and used the User checkers instead. To use this bundle, I've to implement some fields I never use.

I think we should get rid of those fields in FOSUserBundle/Model/UserInterface and create a (deprecated) FOSUserBundle/Model/AdvancedUserInterface for backwards compatibility. What do you think?

@devtronic
Copy link

devtronic commented May 28, 2019

This deprecation has been already fixed: #2815

@stephanvierkant
Copy link
Author

stephanvierkant commented May 28, 2019

No, that's my point: it's not really fixed, the fields just have been moved. That's why I said

This is a workaround to get rid of the deprecation notices, but doesn't follow the latest best practices.

@devtronic
Copy link

devtronic commented May 28, 2019

What's the problem? There are no new fields, just new methods. It means a breaking change if this methods are getting removed. For more information about this, take a look in the linked PR above.

Edit:
Btw. Those methods are not new since there are also existing in the regular AdanvcedUserInterface which was the base interface in the previous revision of the UserInterface in the FOSUserBundle

@stephanvierkant
Copy link
Author

stephanvierkant commented May 28, 2019

There is no problem if you only look at deprecation/error notices. But it doesn't follow the latest best practices:

(..) Given that these methods are mostly related to your application domain logic, (..)

I want to use FOSUserBundle, but I don't need those fields. Why should my application and database contain those fields? I think there should be a way of using this bundle without those opinionated fields. See #2686 and #2845. Symfony's AdvancedUserInterface has been deprecated for a good reason.

@stephanvierkant
Copy link
Author

I've removed FOSUserBundle completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants