8000 [Security] Change to `BadCredentialsException` when empty username / password by llupa · Pull Request #57378 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content
8000

[Security] Change to BadCredentialsException when empty username / password #57378

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

Merged
merged 1 commit into from
Jun 12, 2024
Merged

[Security] Change to BadCredentialsException when empty username / password #57378

merged 1 commit into from
Jun 12, 2024

Conversation

llupa
Copy link
Contributor
@llupa llupa commented Jun 12, 2024
Q A
Branch? 7.1
Bug fix? no
New feature? no
Deprecations? no
Issues Fix #53851 (comment)
License MIT

Tests will likely fail since they are running flipped.

@llupa llupa requested a review from chalasr as a code owner June 12, 2024 10:34
@carsonbot carsonbot added this to the 7.1 milestone Jun 12, 2024
@llupa llupa changed the title [Security] Change to BadCredentialsException when empty username / … [Security] Change to BadCredentialsException when empty username / password Jun 12, 2024
Copy link
Member
@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Works for me given the the way form-login authentication is handled. Thanks!

@wouterj wouterj merged commit 302938c into symfony:7.1 Jun 12, 2024
9 of 10 checks passed
@wouterj
Copy link
Member
wouterj commented Jun 12, 2024

Thank you @llupa!

Fyi, we only flip the high deps test on the dev and x.4 branches.

8000

@J-Ben87
Copy link
J-Ben87 commented Jun 13, 2024

Thanks for the fix ❤️

@fabpot fabpot mentioned this pull request Jun 28, 2024
bobvandevijver added a commit to bobvandevijver/symfony that referenced this pull request Dec 3, 2024
fabpot added a commit that referenced this pull request Dec 6, 2024
… item (bobvandevijver)

This PR was merged into the 7.1 branch.

Discussion
----------

[Security] Update incorrect form authenticator changelog item

| Q             | A
| ------------- | ---
| Branch?       | 7.1 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | # <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->
| License       | MIT

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the latest branch.
 - For new features, provide some code snippets to help understand usage.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

Related to #53851, #57378 & #59079. ~~Whether or not this an actual CVE, I believe this should be removed from the changelog anyways as it does not throw a bad request anymore.~~

~~If we do keep considering it a new feature, it should probably be changed to reflect the correct exception.~~

As discussed, now only an update to note the actual exception being thrown.

Commits
-------

38f8ec2 Fix change log to mentioned thrown exception
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.

6 participants
0