8000 Login throttling: error message is not translated · Issue #40863 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

Login throttling: error message is not translated #40863

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
Closed
jmsche opened this issue Apr 19, 2021 · 9 comments
Closed

Login throttling: error message is not translated #40863

jmsche opened this issue Apr 19, 2021 · 9 comments
Labels
Bug Missing translations Security Status: Needs Review

Comments

@jmsche
Copy link
Contributor
jmsche commented Apr 19, 2021

Symfony version(s) affected: 5.3.0@beta1

Description
When logging in with fr locale, I'm getting the following error message:

Too many failed login attempts, please try again in 1 minute.

While I'm expecting a translated error message.

How to reproduce
Add rate limiter to security.yaml & login using a different locale than en.

Possible Solution
Add translations in security translation files.

Additional context
I think the error message comes from here: https://github.com/symfony/symfony/blob/v5.3.0-BETA1/src/Symfony/Component/Security/Core/Exception/TooManyLoginAttemptsAuthenticationException.php#L44

The "later" part seems already translated, so I think messages handling "minute" & "minutes" should be added.
@jmsche jmsche added the Bug label Apr 19, 2021
@ro0NL
Copy link
Contributor
ro0NL commented Apr 20, 2021 •
edited
Loading

Variables should be extracted to getMessageData IIUC

edit: no my bad, actual variable values arent included, the translations are missing:

symfony/src/Symfony/Component/Security/Core/Resources/tran 8000 slations/security.en.xlf

Lines 65 to 67 in bb1e1e5

<trans-unit id="17">
<source>Too many failed login attempts, please try again later.</source>
<target>Too many failed login attempts, please try again later.</target>

@chalasr
Copy link
Member
chalasr commented Apr 20, 2021

Thanks for the report. PR welcome.

@jmsche jmsche mentioned this issue Apr 21, 2021
Closed
@derrabus derrabus added the Bug label Apr 21, 2021
@derrabus
Copy link
Member

I think this is not just about a missing translation. The problem is that a message key is generated that makes assumptions on pluralizations. Instead, we should use a message key like this one:

symfony/src/Symfony/Component/Validator/Constraints/Choice.php

Lines 43 to 44 in be384cf

public $minMessage = 'You must select at least {{ limit }} choice.|You must select at least {{ limit }} choices.';
public $maxMessage = 'You must select at most {{ limit }} choice.|You must select at most {{ limit }} choices.';

@wouterj
Copy link
Member
wouterj commented Apr 21, 2021

Contrary to the Validator component, we do not require the Translation component to be registered and enabled in Security. So the translation keys must be human readable without message formatter.

@derrabus
Copy link
Member

Thanks for the explanation, @wouterj. In that case it's really just about adding the translations.

@javiereguiluz
Copy link
Member

Maybe we can't translate these messages then. They would only be correct for languages with same pluralization rules as English, right?

@stof
Copy link
Member
stof commented Apr 21, 2021

The translator does not care about the content of the key to decide whether it should apply pluralization or no.

@jmsche jmsche mentioned this issue Apr 30, 2021
Merged
@fabpot fabpot closed this as completed May 1, 2021
@fabpot
Copy link
Member
fabpot commented May 1, 2021

Note that we will have issues with languages where plural is more complex.

fabpot added a commit that referenced this issue May 1, 2021
@fabpot
minor #41003 [Security] Add missing French translations for logging t…
c92acf4 
…hrottling (jmsche)

This PR was merged into the 4.4 branch.

Discussion
----------

[Security] Add missing French translations for logging throttling

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #40863 (at least for French)
| License       | MIT
| Doc PR        | N/A

Note: this PR replaces #40889 as somehow force-push closed the other PR.

Also this one targets 4.4 branch directly :)

Commits
-------

b64efd2 [Security] Add missing French translations for logging throttling
@mhujer
Copy link
Contributor
mhujer commented May 3, 2021

It is also problem in Czech (discovered it when working on #41040)

mhujer added a commit to mhujer/symfony that referenced this issue May 3, 2021
@mhujer
[Security] Update Czech translations
40a711d 
- closes symfony#41040
- The translations are not perfect for some %minutes% values
   as the pluralization is more complicated in Czech than in English.
- See: symfony#40863 (comment)
@mhujer mhujer mentioned this issue May 3, 2021
Merged
chalasr pushed a commit to mhujer/symfony that referenced this issue May 4, 2021
@mhujer @chalasr
[Security] Update Czech translations
076310c 
- closes symfony#41040
- The translations are not perfect for some %minutes% values
   as the pluralization is more complicated in Czech than in English.
- See: symfony#40863 (comment)
chalasr added a commit that referenced this issue May 4, 2021
@chalasr
minor #41093 [Security] Update Czech translations (mhujer)
98d3201 
This PR was submitted for the 5.x branch but it was merged into the 4.4 branch instead.

Discussion
----------

[Security] Update Czech translations

| Q             | A
| ------------- | ---
| Branch?       | 5.x for features
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? |no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #41040
| License       | MIT

Closes #41040

The translations are not perfect for some %minutes% values as the pluralization is more complicated in Czech than in English. (See: #40863 (comment))

Commits
-------

076310c [Security] Update Czech translations
symfony-splitter pushed a commit to symfony/security that referenced this issue May 4, 2021
@mhujer @chalasr
[Security] Update Czech translations
a6b20d5 
- closes #41040
- The translations are not perfect for some %minutes% values
   as the pluralization is more complicated in Czech than in English.
- See: symfony/symfony#40863 (comment)
symfony-splitter pushed a commit to symfony/security-core that referenced this issue May 4, 2021
@mhujer @chalasr
[Security] Update Czech translations
74391f9 
- closes #41040
- The translations are not perfect for some %minutes% values
   as the pluralization is more complicated in Czech than in English.
- See: symfony/symfony#40863 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Missing translations Security Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
0