Skip to content

Commit

Permalink
bug #52724 [Security] make secret required for DefaultLoginRateLimite…
Browse files Browse the repository at this point in the history
…r (RobertMe)

This PR was merged into the 6.4 branch.

Discussion
----------

[Security] make secret required for DefaultLoginRateLimiter

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes/no
| New feature?  | no
| Deprecations? | yes/no
| Issues        |
| License       | MIT

This tickets results from the discussion here: #52469 (review) and `@nicolas`-grekas requested a PR for it.

The `secret` parameter has been added in #51434 with a default value of `''` and a deprecation message that it is required / may not be empty. Which is fine and doesn't hurt backwards compatibility.

The later ticket #52469 changes the deprecation into an exception, as it is undesirable that no secret is used (in any scenario). This leads to the unintended side effect that there is a BC breakage when a developer manually creates a `DefaultLoginRateLimiter` as it is now actually required to provide a (non empty) value due to the check and exception.

Allowing the service / class to be used without providing the secret parameter, in a backwards compatible manner, but then still breaking the backwards compatibility by throwing due to the default value is confusing. So making the `secret` required makes more sense from a developer perspective as it is clear in that the parameter must be provided.

Commits
-------

ecbf0e9 [Security] make secret required for DefaultLoginRateLimiter
  • Loading branch information
nicolas-grekas committed Nov 26, 2023
2 parents 4f4ae19 + ecbf0e9 commit d3aa478
Showing 1 changed file with 1 addition and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ final class DefaultLoginRateLimiter extends AbstractRequestRateLimiter
/**
* @param non-empty-string $secret A secret to use for hashing the IP address and username
*/
public function __construct(RateLimiterFactory $globalFactory, RateLimiterFactory $localFactory, #[\SensitiveParameter] string $secret = '')
public function __construct(RateLimiterFactory $globalFactory, RateLimiterFactory $localFactory, #[\SensitiveParameter] string $secret)
{
if (!$secret) {
throw new InvalidArgumentException('A non-empty secret is required.');
Expand Down

0 comments on commit d3aa478

Please sign in to comment.