NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[SecurityBundle] Add missing argument to security.authentication.provider.simple#26762

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

Jump to bottom

Closed

nicolas-grekas wants to merge1 commit intosymfony:3.4fromnicolas-grekas:fix-missing-arg

Closed

[SecurityBundle] Add missing argument to security.authentication.provider.simple#26762

nicolas-grekas wants to merge1 commit intosymfony:3.4fromnicolas-grekas:fix-missing-arg

Conversation

Copy link

Member

nicolas-grekas commentedApr 3, 2018

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#26753
License	MIT
Doc PR	-

nicolas-grekas added this to the3.4 milestone

Apr 3, 2018

carsonbot added Status: Needs Review SecurityBundle Bug labels

Apr 3, 2018

nicolas-grekas mentioned this pull request

Apr 3, 2018

[Security] Service "security.authentication.provider.simple_form.secured_area": The index "3" is not in the range [0, 2].#26753

Closed

[SecurityBundle] Add missing argument to security.authentication.prov…

e8433cb

…ider.simple

nicolas-grekas force-pushed thefix-missing-arg branch from58c08c6 toe8433cbCompare

April 3, 2018 10:55

nicolas-grekas commented

Apr 3, 2018

View reviewed changes

src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml

		<argument /><!-- Simple Authenticator-->
		<argument /><!-- User Provider-->
		<argument /><!-- Provider-shared Key-->
		<argument>null</argument><!-- User Checker-->

Copy link

MemberAuthor

nicolas-grekasApr 3, 2018•
edited
Loading

Choose a reason for hiding this comment

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

Explicitnull because whileSimpleFormFactory::createAuthProvider() does replace the argument,SimplePreAuthenticationFactory::create() doesn't, thus keeps it as is.

Copy link

Contributor

andreyboloninApr 3, 2018•
edited
Loading

Choose a reason for hiding this comment

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

@nicolas-grekas success tested on4.0.7

Copy link

divyeshasplApr 5, 2018•
edited
Loading

Choose a reason for hiding this comment

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

yes its works for me Thnaks@nicolas-grekas

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Apr 3, 2018

xabbuh approved these changes

Apr 3, 2018

View reviewed changes

Copy link

MemberAuthor

nicolas-grekas commentedApr 3, 2018

Introduced in#26370.@i3or1s can you please have a look?

nicolas-grekas mentioned this pull request

Apr 3, 2018

[Security] added userChecker to SimpleAuthenticationProvider#26370

Closed

Copy link

Contributor

i3or1s commentedApr 3, 2018

@nicolas-grekas Yes i will take a look

Copy link

Contributor

@nicolas-grekas I would also replace the argument in theSimplePreAuthenticationFactory::create() since there is defaultUserChacker that can be usedSecurity/Core/User/UserChecker.php rather than usingnull. I guess in the long run it is better to deal with an object rather thannull.
I must have missed that part :/

Copy link

MemberAuthor

nicolas-grekas commentedApr 3, 2018•
edited
Loading

@i3or1s would you mind opening a PR doing so? (you can borrow the patch here also so I'll close this one.)

Copy link

Contributor

i3or1s commentedApr 3, 2018

@nicolas-grekas No problems I will do so now.

i3or1s mentioned this pull request

Apr 3, 2018

[SecurityBundle] Add missing argument to security.authentication.provider.simple#26774

Merged

Copy link

Member

chalasr commentedApr 3, 2018

Closing in favor of#26774, thanks@nicolas-grekas.

chalasr closed this

Apr 3, 2018

nicolas-grekas added a commit that referenced this pull request

Apr 4, 2018

bug#26774[SecurityBundle] Add missing argument to security.authenti…

3c54c4a

…cation.provider.simple (i3or1s, chalasr)This PR was merged into the 2.8 branch.Discussion----------[SecurityBundle] Add missing argument to security.authentication.provider.simple| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#26753| License       | MIT| Doc PR        | -Created PR in relation to a conversation in [PR](#26762)#26762Commits-------c82c2f1 [SecurityBundle] Add test for simple authentication config1b26aac [SecurityBundle] Add missing argument to security.authentication.provider.simple