349337 - The time between two successive token requests should be a constant

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Frédéric Buclin]

Currently, the time between two successive token requests for a forgotten password or a new user account creation is hardcoded and set to 10 minutes. This should be a constant. One advantage is that the error message could say "you have to wait for 10 minutes before requesting a token again" instead of "wait a while and try again".

Comment 1

[Koosha KM]

Attachment: This patch implements the enhancement

This patch implements the enhancement by introducing a new config variable in localconfig: $token_retry_wait. This is set to 10 by default. This value is reflected in error messages. In addition, values greater than 59 make error messages display the waiting time in a combination of hour and minute. To try different possible error messages, set the variable to: 1, 59, 60, 61, and 62.

Comment 2

[Frédéric Buclin]

Comment on attachment 614160 [details] [diff] [review]
This patch implements the enhancement

Constants go into Bugzilla/Constants.pm. Localconfig only contains some very specific info. Also, the goal of this bug is to keep things simple. No need to worry about times larger than 60 minutes.

Comment 3

[Koosha KM]

Attachment: This one uses the Constants.pm

OK. This one uses the Constants.pm to define the constant. I've also changed the respective template to show only the time in minutes although I think that handling values greater than 59 would be a good idea.

Comment 4

[Frédéric Buclin]

Comment on attachment 614183 [details] [diff] [review]
This one uses the Constants.pm

This looks much better now. :) Still a few things, though.


>=== modified file 'Bugzilla/Constants.pm'

>+    TOKEN_RETRY_WAIT

Move this constant close to MAX_TOKEN_AGE as they are related. Also, rename this constant to ACCOUNT_CHANGE_INTERVAL or something like that. TOKEN_RETRY_WAIT is too cryptic.


>+# The time between two successive token requests in minute. This is used
>+# when a user wants to have an email received containing a link to reset his
>+# forgotten password or to create a new user account and retries after a short
>+# time. If the user wants to try his request again, he will have to wait
>+# the minutes set by this constant.

Make the comment much shorter. Something like:
# Interval in minutes before a user can request a new token to create/edit his account.


>=== modified file 'Bugzilla/Token.pm'

>     # But to prevent using this way to mailbomb an email address, make sure
>-    # the last request is at least 10 minutes old before sending a new email.
>+    # the last request is at least the minutes old set by TOKEN_RETRY_WAIT.

... the last request is old enough before sending a new email (default: 10 minutes).


>-    ThrowUserError('too_soon_for_new_token', {'type' => 'account'}) if $pending_requests;
>+    ThrowUserError('too_soon_for_new_token', {'type' => 'account', 'mins' => TOKEN_RETRY_WAIT}) if $pending_requests;

No need to pass constants to the hashref. Templates can already access them using constants.FOO_BAR.


>-    ThrowUserError('too_soon_for_new_token', {'type' => 'password'}) if $too_soon;
>+    ThrowUserError('too_soon_for_new_token', {'type' => 'password', 'mins' => TOKEN_RETRY_WAIT}) if $too_soon;

Same here.


>=== modified file 'template/en/default/global/user-error.html.tmpl'

>+    Please wait 
>+    [% IF mins > 1 %]
>+      [% mins %] minutes
>+    [% ELSIF mins > 0 %]
>+      [% mins %] minute
>+    [% END %]

Nobody will set it to 0 or 1 minute, because it would defeat the goal of the token check. So no need for this distinction. Remember that it's an internal constant, not a parameter admins will edit from the UI.

Comment 5

[Koosha KM]

Attachment: V3

Comment 6

[Frédéric Buclin]

Comment on attachment 614478 [details] [diff] [review]
V3

Perfect! Thanks for the patch! :) r=LpSolit

Comment 7

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Constants.pm
modified Bugzilla/Token.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 8187.