8000 [Security] removed usage of the deprecated SecurityContextInterface by fabpot · Pull Request #13323 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Security] removed usage of the deprecated SecurityContextInterface #13323

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 2 commits into from
Jan 8, 2015
Merged

[Security] removed usage of the deprecated SecurityContextInterface #13323

merged 2 commits into from
Jan 8, 2015

Conversation

fabpot
Copy link
Member
@fabpot fabpot commented Jan 8, 2015
Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

This PR removed internal usage of SecurityContextInterface in favor of the new alternatives, it also fixes removes as many deprecation notices as possible for the Security component.

@fabpot fabpot mentioned this pull request Jan 8, 2015 8000
Closed
1 task
xabbuh
xabbuh reviewed Jan 8, 2015
View reviewed changes
src/Symfony/Component/Security/Core/SecurityContext.php
@@ -11,6 +11,8 @@

namespace Symfony\Component\Security\Core;

trigger_error('The '.__NAMESPACE__.'\SecurityContext method is deprecated since version 2.6 and will be removed in 3.0.', E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

method should be class, right?

@fabpot fabpot force-pushed the security-context-deprecation-fixes branch 2 times, most recently from 6246890 to d80c75a Compare January 8, 2015 12:02
@fabpot fabpot force-pushed the security-context-deprecation-fixes branch from d80c75a to 2c888e0 Compare January 8, 2015 13:09
@fabpot
Copy link
Member Author
fabpot commented Jan 8, 2015

Tests do not pass for lower/higher because SecurityBundle tests need a 2.7 version of symfony/security... but the subtree split won't be available before the merge of this PR :)

@fabpot
Copy link
Member Author
fabpot commented Jan 8, 2015

ping @symfony/deciders

Tobion
Tobion reviewed Jan 8, 2015
View reviewed changes
src/Symfony/Component/Security/Core/Validator/Constraints/UserPasswordValidator.php
/**
* @param SecurityContextInterface|TokenStorageInterface
*
* Passing a SecurityContextInterface as a first argument was deprecated in 2.7 and will be removed in 3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

should we not trigger a deprecation warning when the argument is a SecurityContextInterface?

Copy link
Contributor

Choose a reason for hiding this comment

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

actually you can remove all these warning statements in the phpdoc because as I said below SecurityContextInterface extends TokenStorageInterface. so this warning is irrelevant and we also dont need to trigger a deprecation warning in code (because that would already be done when people use the a class based on securitycontext).

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@fabpot fabpot force-pushed the security-context-deprecation-fixes branch from 2c888e0 to df25713 Compare January 8, 2015 15:39
stof
stof reviewed Jan 8, 2015
View reviewed changes
src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php
*/
public function __construct(SecurityContextInterface $securityContext, AuthenticationManagerInterface $authenticationManager, SessionAuthenticationStrategyInterface $sessionStrategy, HttpUtils $httpUtils, $providerKey, AuthenticationSuccessHandlerInterface $successHandler, AuthenticationFailureHandlerInterface $failureHandler, array $options = array(), LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null)
public function __construct($tokenStorage, AuthenticationManagerInterface $authenticationManager, SessionAuthenticationStrategyInterface $sessionStrategy, HttpUtils $httpUtils, $providerKey, AuthenticationSuccessHandlerInterface $successHandler, AuthenticationFailureHandlerInterface $failureHandler, array $options = array(), LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null)
Copy link
Member

Choose a reason for hiding this comment

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

I suggest forcing symfony/security-http to use symfony/security-core 2.6+ and typehinting the TokenStorageInterface

Copy link
Contributor

Choose a reason for hiding this comment

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

agree

Copy link
Member Author

Choose a reason for hiding this comment

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

The deps is already on 2.6.

Copy link
Member

Choose a reason for hiding this comment

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

then it is OK to typehint the new interface

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@fabpot fabpot force-pushed the security-context-deprecation-fixes branch from df25713 to 5f20448 Compare January 8, 2015 15:42
Tobion
Tobion reviewed Jan 8, 2015
View reviewed changes
src/Symfony/Component/Security/Core/SecurityContext.php
@@ -88,8 +90,6 @@ public function getToken()
*/
public function setToken(TokenInterface $token = null)
{
trigger_error('The '.__METHOD__.' method is deprecated since version 2.6 and will be removed in 3.0. Use the Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage::setToken() method instead.', E_USER_DEPRECATED);
Copy link
Contributor

Choose a reason for hiding this comment

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

you forgot getToken

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, fixed

stof
stof reviewed Jan 8, 2015
View reviewed changes
src/Symfony/Bundle/SecurityBundle/Templating/Helper/SecurityHelper.php
*/
public function __construct(SecurityContextInterface $context = null)
public function __construct($securityChecker = null)
Copy link
Member

Choose a reason for hiding this comment

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

should use the new interface as typehint too

@fabpot fabpot force-pushed the security-context-deprecation-fixes branch 2 times, most recently from 57c0388 to 966cac7 Compare January 8, 2015 15:57
stof
stof reviewed Jan 8, 2015
View reviewed changes
src/Symfony/Bridge/Twig/composer.json
@@ -17,7 +17,7 @@
],
"require": {
"php": ">=5.3.3",
"symfony/security-csrf": "~2.4|~3.0.0",
"symfony/security-csrf": "~2.6|~3.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

the dev requirement for other parts of the security component should also be bumped

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@fabpot
Copy link
Member Author
fabpot commented Jan 8, 2015

The whole PR should be reviewed again as I've changed much more than before now that all type hints have been changed. ping @symfony/deciders

@fabpot fabpot force-pushed the security-context-deprecation-fixes branch from 966cac7 to ba71b68 Compare January 8, 2015 16:02
@stof
Copy link
Member
stof commented Jan 8, 2015

👍

@fabpot fabpot merged commit ba71b68 into symfony:2.7 Jan 8, 2015
fabpot added a commit that referenced this pull request Jan 8, 2015
@fabpot
feature #13323 [Security] removed usage of the deprecated SecurityCon…
e104595 
…textInterface (fabpot)

This PR was merged into the 2.7 branch.

Discussion
----------

[Security] removed usage of the deprecated SecurityContextInterface

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

This PR removed internal usage of SecurityContextInterface in favor of the new alternatives, it also fixes removes as many deprecation notices as possible for the Security component.

Commits
-------

ba71b68 added type-hint
91d01d8 [Security] removed usage of the deprecated SecurityContextInterface
@fabpot fabpot deleted the security-context-deprecation-fixes branch January 9, 2015 18:03
@dosten dosten mentioned this pull request Jun 6, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fabpot @stof @Tobion @xabbuh
0