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] Change FormAuthenticator if condition #30347

Merged
merged 1 commit into from Feb 23, 2019

Conversation

Projects
None yet
6 participants
@PReimers
Copy link
Contributor

PReimers commented Feb 22, 2019

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

I changed the if condition in SimpleFormAuthenticationListener and UsernamePasswordFormAuthenticationListener based on the solution provided by @nikic in issue #30341

#OpenSourceFriday

@PReimers

This comment has been minimized.

Copy link
Contributor Author

PReimers commented Feb 22, 2019

Just discovered I need to write some additional tests.
They'll follow later today.

@xabbuh xabbuh added the Security label Feb 22, 2019

@PReimers

This comment has been minimized.

Copy link
Contributor Author

PReimers commented Feb 22, 2019

I changed the if condition and added some test cases to test additional scenarios.

I hope it's no problem I added a class in the test file. This allowed me to test if the $username variable contains a class containing a __toString() method

@chalasr chalasr added this to the 3.4 milestone Feb 22, 2019

@nicolas-grekas nicolas-grekas changed the title Change FormAuthenticator if condition [Security] Change FormAuthenticator if condition Feb 23, 2019

@nicolas-grekas nicolas-grekas force-pushed the PReimers:issue-30341 branch from 5ed95c4 to 67ae121 Feb 23, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 23, 2019

Thank you @PReimers.

@nicolas-grekas nicolas-grekas merged commit 67ae121 into symfony:3.4 Feb 23, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Feb 23, 2019

bug #30347 [Security] Change FormAuthenticator if condition (PReimers)
This PR was squashed before being merged into the 3.4 branch (closes #30347).

Discussion
----------

[Security] Change FormAuthenticator if condition

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

I changed the if condition in `SimpleFormAuthenticationListener` and `UsernamePasswordFormAuthenticationListener` based on the solution provided by @nikic in issue #30341

#OpenSourceFriday

Commits
-------

67ae121 [Security] Change FormAuthenticator if condition

This was referenced Mar 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.