Merge lp:~nurlan0000/psiphon/sprint3-576643 into lp:psiphon
- sprint3-576643
- Merge into trunk
Proposed by
Nurlan Turdaliev
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Adam Kruger | ||||
Approved revision: | 123 | ||||
Merged at revision: | 130 | ||||
Proposed branch: | lp:~nurlan0000/psiphon/sprint3-576643 | ||||
Merge into: | lp:psiphon | ||||
Diff against target: |
429 lines (+283/-19) (has conflicts) 8 files modified
trunk/mod_psiphon/httpd_conf/httpd.conf (+1/-1) trunk/sql/upgrade-2.6.sql (+2/-0) trunk/tools/psiphon_install.sh (+1/-1) trunk/www/config.php (+4/-0) trunk/www/includes/lang.php (+65/-0) trunk/www/includes/url_helpers.php (+6/-0) trunk/www/p.php (+193/-0) trunk/www/reset_password.php (+11/-17) Text conflict in trunk/www/includes/lang.php |
||||
To merge this branch: | bzr merge lp:~nurlan0000/psiphon/sprint3-576643 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Adam Kruger | Approve | ||
Rod | Needs Fixing | ||
Nurlan Turdaliev (community) | Needs Resubmitting | ||
Review via email: mp+34057@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
Adam Kruger (adam-kruger) wrote : | # |
review:
Disapprove
(group review)
Revision history for this message
Adam Kruger (adam-kruger) : | # |
review:
Needs Fixing
(group review)
- 122. By Nurlan Turdaliev <ubuntu@ubuntu-desktop>
-
Implemented psiphon team suggestions
Revision history for this message
Nurlan Turdaliev (nurlan0000) : | # |
review:
Needs Resubmitting
Revision history for this message
Rod (rod-psiphon) wrote : | # |
Looks ok except this 403 should be 404:
reset_password.php
if (!$proxy_record || $proxy_
{
include(
...
The idea is to *always* return 404 any time invalid input/incorrect secret value is provided to a page outside of an authenticated session. This is to defend against an adversary scanning arbitrary web hosts for Psiphon proxies that respond to HTTP requests with some fingerprint-able response.
review:
Needs Fixing
- 123. By Nurlan Turdaliev <ubuntu@ubuntu-desktop>
-
return 404 error instead 403
Revision history for this message
Adam Kruger (adam-kruger) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'trunk/mod_psiphon/httpd_conf/httpd.conf' |
2 | --- trunk/mod_psiphon/httpd_conf/httpd.conf 2010-10-18 21:11:30 +0000 |
3 | +++ trunk/mod_psiphon/httpd_conf/httpd.conf 2010-11-01 07:42:40 +0000 |
4 | @@ -156,7 +156,7 @@ |
5 | |
6 | <Location /> |
7 | PsiphonAuthEnable On |
8 | -PsiphonNoAuth /a.php /e.php /w.php /chk.php /reset_password.php /logout.php |
9 | +PsiphonNoAuth /a.php /e.php /w.php /chk.php /reset_password.php /logout.php /p.php |
10 | PsiphonAuthLoginPath /auth.php |
11 | PsiphonLoginURLCheck On |
12 | </Location> |
13 | |
14 | === added file 'trunk/sql/upgrade-2.6.sql' |
15 | --- trunk/sql/upgrade-2.6.sql 1970-01-01 00:00:00 +0000 |
16 | +++ trunk/sql/upgrade-2.6.sql 2010-11-01 07:42:40 +0000 |
17 | @@ -0,0 +1,2 @@ |
18 | +ALTER TABLE `user` ADD COLUMN `pass_reset_token` CHAR(32) DEFAULT '' NOT NULL; |
19 | +ALTER TABLE `user` ADD COLUMN `pass_reset_date` INTEGER DEFAULT 0 NOT NULL; |
20 | |
21 | === modified file 'trunk/tools/psiphon_install.sh' |
22 | --- trunk/tools/psiphon_install.sh 2010-06-01 17:50:42 +0000 |
23 | +++ trunk/tools/psiphon_install.sh 2010-11-01 07:42:40 +0000 |
24 | @@ -271,7 +271,7 @@ |
25 | echo "Enter current password for mysql root (enter for none)" |
26 | |
27 | cat sql/create_db.sql sql/grant.new.sql sql/structure.2.3.sql sql/init.sql \ |
28 | -sql/upgrade-2.4.sql sql/upgrade-2.5.sql sql/jsf.sql sql/youtube.sql | mysql -uroot -p |
29 | +sql/upgrade-2.4.sql sql/upgrade-2.5.sql sql/upgrade-2.6.sql sql/jsf.sql sql/youtube.sql | mysql -uroot -p |
30 | |
31 | rm sql/grant.new.sql |
32 | |
33 | |
34 | === modified file 'trunk/www/config.php' |
35 | --- trunk/www/config.php 2010-06-28 20:44:20 +0000 |
36 | +++ trunk/www/config.php 2010-11-01 07:42:40 +0000 |
37 | @@ -247,4 +247,8 @@ |
38 | |
39 | $config["geoip_type"]= "country"; |
40 | $config["geoip_data_file"] = "/usr/share/GeoIP/GeoIP.dat"; |
41 | + |
42 | +// Password reset |
43 | +// 3 days |
44 | +$config['password_reset_ttl'] = 7 * 24 * 60 * 60; |
45 | ?> |
46 | |
47 | === modified file 'trunk/www/includes/lang.php' |
48 | --- trunk/www/includes/lang.php 2010-10-14 16:26:29 +0000 |
49 | +++ trunk/www/includes/lang.php 2010-11-01 07:42:40 +0000 |
50 | @@ -680,6 +680,7 @@ |
51 | ), |
52 | |
53 | 'reset_password_error' => array( |
54 | +<<<<<<< TREE |
55 | 'en' => "To reset your password, enter your username or e-mail address, and click the reset button. A temporary password will be e-mailed to you.", |
56 | 'fa' => "برای باز نشاندن رمزعبور خود، نام کاربری یا آدرس ایمیل خود را وارد کنید و روی دکمه \"باز نشاندن\" کلیک کنید. یک رمز عبور موقتی به شما ایمیل خواهد شد. ", |
57 | 'ru' => "Для сброса пароля введите имя пользователя или электронный адрес почты и нажмите кнопку сброса. Временный пароль будет отправлен на указанный Вами адрес электронной почты", |
58 | @@ -691,9 +692,35 @@ |
59 | 'fr' => "To reset your password, enter your username or e-mail address, and click the reset button. A temporary password will be e-mailed to you.", |
60 | 'es' => "To reset your password, enter your username or e-mail address, and click the reset button. A temporary password will be e-mailed to you.", |
61 | 'be' => "Для сброса пароля введите имя пользователя или электронный адрес почты и нажмите кнопку сброса. Временный пароль будет отправлен на указанный Вами адрес электронной почты", |
62 | +======= |
63 | + 'en' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
64 | + 'fa' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
65 | + 'ru' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
66 | + 'uz' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
67 | + 'tk' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
68 | + 'cn' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
69 | + 'ar' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
70 | + 'vt' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
71 | + 'fr' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
72 | + 'es' => "To reset your password, enter your username or e-mail address, and click the reset button. A password reset link will be e-mailed to you.", |
73 | + ), |
74 | + |
75 | + 'password_changed' => array( |
76 | + 'en' => "Password changed", |
77 | + 'fa' => "Password changed", |
78 | + 'ru' => "Password changed", |
79 | + 'uz' => "Password changed", |
80 | + 'tk' => "Password changed", |
81 | + 'cn' => "Password changed", |
82 | + 'ar' => "Password changed", |
83 | + 'vt' => "Password changed", |
84 | + 'fr' => "Password changed", |
85 | + 'es' => "Password changed", |
86 | +>>>>>>> MERGE-SOURCE |
87 | ), |
88 | |
89 | 'reset_password_subject' => array( |
90 | +<<<<<<< TREE |
91 | 'en' => "Your temporary Psiphon password", |
92 | 'fa' => "رمز عبور موقت سایفون", |
93 | 'ru' => "Ваш временный пароль в Psiphon ", |
94 | @@ -705,9 +732,22 @@ |
95 | 'fr' => "Votre mot de passe Psiphon temporaire", |
96 | 'es' => "Su contraseña temporal de Psiphon", |
97 | 'be' => "Ваш временный пароль в Psiphon ", |
98 | +======= |
99 | + 'en' => "Psiphon password reset", |
100 | + 'fa' => "Psiphon password reset", |
101 | + 'ru' => "Psiphon password reset", |
102 | + 'uz' => "Psiphon password reset", |
103 | + 'tk' => "Psiphon password reset", |
104 | + 'cn' => "Psiphon password reset", |
105 | + 'ar' => "Psiphon password reset", |
106 | + 'vt' => "Psiphon password reset", |
107 | + 'fr' => "Psiphon password reset", |
108 | + 'es' => "Psiphon password reset", |
109 | +>>>>>>> MERGE-SOURCE |
110 | ), |
111 | |
112 | 'reset_password_body' => array( |
113 | +<<<<<<< TREE |
114 | 'en' => "Your temporary Psiphon password is: %s. Please login and change your password immediately.", |
115 | 'fa' => "رمز عبور موقت شما: %s. لطفآ وارد حساب کاربری خود شده و این رمز را عوض کنید. ", |
116 | 'ru' => "Ваш временный пароль в Psiphon: %s. Войдите в систему и немедленно замените свой пароль.", |
117 | @@ -719,9 +759,22 @@ |
118 | 'fr' => "Votre mot de passe Psiphon temporaire est : %s. Veuillez vous connecter et changer immédiatement votre mot de passe.", |
119 | 'es' => "Su contraseña temporal de Psiphon es %s. Inicie la sesión y cambie la contraseña de inmediato.", |
120 | 'be' => "Ваш временный пароль в Psiphon: %s. Войдите в систему и немедленно замените свой пароль.", |
121 | +======= |
122 | + 'en' => "Follow this link to reset your password: %s.", |
123 | + 'fa' => "Follow this link to reset your password: %s.", |
124 | + 'ru' => "Follow this link to reset your password: %s.", |
125 | + 'uz' => "Follow this link to reset your password: %s.", |
126 | + 'tk' => "Follow this link to reset your password: %s.", |
127 | + 'cn' => "Follow this link to reset your password: %s.", |
128 | + 'ar' => "Follow this link to reset your password: %s.", |
129 | + 'vt' => "Follow this link to reset your password: %s.", |
130 | + 'fr' => "Follow this link to reset your password: %s.", |
131 | + 'es' => "Follow this link to reset your password: %s.", |
132 | +>>>>>>> MERGE-SOURCE |
133 | ), |
134 | |
135 | 'password_sent' => array( |
136 | +<<<<<<< TREE |
137 | 'en' => "A temporary password was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
138 | 'fa' => "یک رمز عبور موقت به آدرس ایمیل شما فرستاده شده است. اگر ایمیل دریافت نکردید نام کاربری یا آدرس ایمیل خود را چک کنید و دوباره امتحان کنید. ", |
139 | 'ru' => "Временный пароль отправлен на указанный Вами адрес электронной почты. Если Вы не получили сообщение, еще раз проверьте правильность имени пользователя и электронного адреса и повторите попытку.", |
140 | @@ -733,6 +786,18 @@ |
141 | 'fr' => "Un mot de passe temporaire a été envoyé à votre adresse électronique. Si vous ne recevez pas de courriel, vérifiez à nouveau votre nom utilisateur ou votre adresse électronique et essayez une nouvelle fois.", |
142 | 'es' => "Se le ha enviado una contraseña temporal a su dirección de correo electrónico. Si no recibe el mensaje con la contraseña, verifique nuevamente su nombre de usuario o dirección de correo electrónico y vuelva a intentar.", |
143 | 'be' => "Временный пароль отправлен на указанный Вами адрес электронной почты. Если Вы не получили сообщение, еще раз проверьте правильность имени пользователя и электронного адреса и повторите попытку.", |
144 | +======= |
145 | + 'en' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
146 | + 'fa' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
147 | + 'ru' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
148 | + 'uz' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
149 | + 'tk' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
150 | + 'cn' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
151 | + 'ar' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
152 | + 'vt' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
153 | + 'fr' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
154 | + 'es' => "A password reset link was sent to your e-mail address. If you do not receive an e-mail, double check your username or e-mail address and try again.", |
155 | +>>>>>>> MERGE-SOURCE |
156 | ), |
157 | |
158 | 'no_address' => array( |
159 | |
160 | === modified file 'trunk/www/includes/url_helpers.php' |
161 | --- trunk/www/includes/url_helpers.php 2010-06-08 17:58:34 +0000 |
162 | +++ trunk/www/includes/url_helpers.php 2010-11-01 07:42:40 +0000 |
163 | @@ -106,4 +106,10 @@ |
164 | |
165 | } |
166 | |
167 | +function make_password_reset_url($config, $proxy_id, $token) { |
168 | + // URL --> https://<proxy ip address|hostname>/p.php?token=01234567890123456789 |
169 | + |
170 | + return "https://".get_proxy_host($config, $proxy_id)."/p.php?"."token=".$token; |
171 | +} |
172 | + |
173 | ?> |
174 | |
175 | === added file 'trunk/www/p.php' |
176 | --- trunk/www/p.php 1970-01-01 00:00:00 +0000 |
177 | +++ trunk/www/p.php 2010-11-01 07:42:40 +0000 |
178 | @@ -0,0 +1,193 @@ |
179 | +<?php |
180 | +/* |
181 | + Psiphon Circumvention Platform |
182 | + Copyright (C) 2009-2010 Psiphon Inc.. |
183 | + |
184 | + This program is free software: you can redistribute it and/or modify |
185 | + it under the terms of the GNU General Public License as published by |
186 | + the Free Software Foundation, either version 3 of the License, or |
187 | + (at your option) any later version. |
188 | + |
189 | + This program is distributed in the hope that it will be useful, |
190 | + but WITHOUT ANY WARRANTY; without even the implied warranty of |
191 | + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
192 | + GNU General Public License for more details. |
193 | + |
194 | + You should have received a copy of the GNU General Public License |
195 | + along with this program. If not, see <http://www.gnu.org/licenses/>. |
196 | +*/ |
197 | + |
198 | + |
199 | +include_once($_SERVER[DOCUMENT_ROOT]."/includes/common_includes.php"); |
200 | +include_once($_SERVER[DOCUMENT_ROOT]."/includes/cr.php"); |
201 | + |
202 | + |
203 | + |
204 | +$token = $_GET['token']; |
205 | +$passwords_submitted = isset($_POST['submit_button']); |
206 | + |
207 | + |
208 | +// check token |
209 | +if (strlen($token) != 32) { |
210 | + include($_SERVER[DOCUMENT_ROOT]."/http-errors/404.php"); |
211 | +} |
212 | + |
213 | + |
214 | +/// Fetch the user to reset password |
215 | +$record_user = null; |
216 | + |
217 | +if ( $token ) |
218 | +{ |
219 | + $query = |
220 | + "SELECT * FROM user ". |
221 | + "WHERE pass_reset_token = :token AND UNIX_TIMESTAMP() < pass_reset_date + ".$config['password_reset_ttl']; |
222 | + |
223 | + $result = db_query_execute($config, $query, |
224 | + array(":token" => $token) |
225 | + ); |
226 | + |
227 | + $record_user = db_fetch_result($config, $result); |
228 | + |
229 | + |
230 | + if ( ! $record_user ) |
231 | + { |
232 | + include($_SERVER[DOCUMENT_ROOT]."/http-errors/404.php"); |
233 | + } |
234 | +} |
235 | + |
236 | +$query = "SELECT * FROM proxy WHERE hostname = :hostname"; |
237 | +$result = db_query_execute($config, $query, convert_null_array(array(":hostname" => $_SERVER[HTTP_HOST]))); |
238 | +$proxy_record = db_fetch_result($config, $result); |
239 | + |
240 | +if ( ! can_current_user_access_proxy($config, $record_user, $proxy_record['id'])) |
241 | +{ |
242 | + include($_SERVER[DOCUMENT_ROOT]."/http-errors/404.php"); |
243 | +} |
244 | + |
245 | + |
246 | + |
247 | +$password_error = null; |
248 | + |
249 | +if ($passwords_submitted) |
250 | +{ |
251 | + $password1 = $_POST['password1']; |
252 | + $password2 = $_POST['password2']; |
253 | + |
254 | + |
255 | + // password validation |
256 | + |
257 | + if (!is_valid_password($config, $password1, $record_user['uname'])) |
258 | + $password_error = sprintf(msg("password_invalid"), $config[min_pass_length]); |
259 | + |
260 | + if ($password1 != $password2) |
261 | + $password_error = msg("password_mismatch"); |
262 | + |
263 | + // Update user password |
264 | + if ( ! $password_error ) |
265 | + { |
266 | + $pass_salt = bin2hex(secure_rand()); |
267 | + $password = sha1($password1.$pass_salt); |
268 | + |
269 | + // unguessable session ID |
270 | + $session_id = bin2hex(secure_rand()); |
271 | + |
272 | + |
273 | + // reset password, password salt and invalidate password reset token. |
274 | + $query = |
275 | + 'UPDATE `user` '. |
276 | + 'SET '. |
277 | + 'pass = :password, '. |
278 | + 'pass_salt = :pass_salt, '. |
279 | + 'session = :session, '. |
280 | + 'pass_reset_token = "" '. |
281 | + 'WHERE id = :id'; |
282 | + |
283 | + db_query_execute( |
284 | + $config, |
285 | + $query, |
286 | + array( |
287 | + ':password' => $password, |
288 | + ':pass_salt' => $pass_salt, |
289 | + ':session' => $session_id, |
290 | + ':id' => $record_user['id'] |
291 | + ) |
292 | + ); |
293 | + } |
294 | +} |
295 | + |
296 | +$password_changed = $passwords_submitted && ! $password_error; |
297 | +if ($password_changed) |
298 | +{ |
299 | + $login_url = $proxy_record['login_url']; |
300 | + header('Location: '.$config['ret'].$login_url, true, 302); |
301 | + exit; |
302 | +} |
303 | + |
304 | + |
305 | +// set page title |
306 | +$page_title = msg("reset_password"); |
307 | + |
308 | +header("Content-type: text/html; charset=".charset()); |
309 | +?> |
310 | +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"> |
311 | +<html> |
312 | + <head> |
313 | + <title><?= $page_title; ?></title> |
314 | + <?=echo_css()?> |
315 | + </head> |
316 | + |
317 | + <body style="direction: <?=direction()?>"> |
318 | + |
319 | + <table> |
320 | + <tr> |
321 | + <td style="width:50%"> |
322 | +<?php |
323 | + include($_SERVER[DOCUMENT_ROOT]."/images/psiphonlogo.html"); |
324 | +?> |
325 | + </td> |
326 | + <td> |
327 | + <?php if ( ! $password_changed ) : ?> |
328 | + <font class="sixteenbluebold"><?=msg("reset_password")?></font> |
329 | + <?php else: ?> |
330 | + <font class="sixteenbluebold"><?=msg("password_changed")?></font> |
331 | + <?php endif; ?> |
332 | + </td> |
333 | + </tr> |
334 | + </table> |
335 | + <br> |
336 | + |
337 | + |
338 | + <?php if ($password_error) : ?> |
339 | + <font class="processing_error"><?= $password_error; ?></font> |
340 | + <br /><br /> |
341 | + <?php endif;?> |
342 | + |
343 | + <form name="reset_password" action="<?=escape_html(getenv("REQUEST_URI"))?>" method="post"> |
344 | + <table> |
345 | + <tr> |
346 | + <td> |
347 | + <label for="password1"><?= msg('password'); ?></label></td> |
348 | + <td> |
349 | + <input type="password" name="password1" id="password1" /> |
350 | + </td> |
351 | + </tr> |
352 | + <tr> |
353 | + <td> |
354 | + <label for="password2"><?= msg('verify_password'); ?></label> |
355 | + </td> |
356 | + <td> |
357 | + <input type="password" name="password2" id="password2" /><br /> |
358 | + </td> |
359 | + </tr> |
360 | + <tr> |
361 | + <td colspan="2"> |
362 | + <input type="submit" name="submit_button" value="<?= msg('save'); ?>" /> |
363 | + </td> |
364 | + </tr> |
365 | + </table> |
366 | + </form> |
367 | + |
368 | + <br> |
369 | + <br> |
370 | + </body> |
371 | +</html> |
372 | \ No newline at end of file |
373 | |
374 | === modified file 'trunk/www/reset_password.php' |
375 | --- trunk/www/reset_password.php 2010-07-07 15:55:51 +0000 |
376 | +++ trunk/www/reset_password.php 2010-11-01 07:42:40 +0000 |
377 | @@ -86,33 +86,27 @@ |
378 | break; |
379 | } |
380 | |
381 | - // The temporary password should be strong relative to the minimum allowed password. |
382 | - // We'll make it as strong as a password that's twice as long as the minimum. |
383 | - |
384 | - $temp_password_length = |
385 | - log(pow(75, $config[min_pass_length]), // there are about 75 possible characters in a strong password |
386 | - 16); // temp password is hex |
387 | - $temp_password_length = ceil($temp_password_length); |
388 | - $random_bytes_needed = ceil($temp_password_length/2.0); |
389 | - $login_pass = strtoupper(substr(bin2hex(secure_rand($random_bytes_needed)), 0, $temp_password_length)); |
390 | - |
391 | - $pass_salt = bin2hex(secure_rand()); |
392 | + $pass_reset_token = bin2hex(secure_rand()); |
393 | |
394 | // update user record with new, temporary password |
395 | // user.session is set to a large random number to invalidate the session. |
396 | // TODO: Set session to null instead. |
397 | $query = "UPDATE user ". |
398 | - "SET pass = :set_pass, pass_salt = :pass_salt, session = :session_id ". |
399 | + "SET pass_reset_token = :token, ". |
400 | + "pass_reset_date = UNIX_TIMESTAMP() ". |
401 | "WHERE id = :user_id"; |
402 | |
403 | db_query_execute( |
404 | $config, |
405 | $query, |
406 | convert_null_array(array( |
407 | - ":set_pass" => sha1($login_pass.$pass_salt), |
408 | - ":pass_salt" => $pass_salt, |
409 | - ":session_id" => bin2hex(secure_rand()), |
410 | - ":user_id" => $user_record['id']))); |
411 | + ":token" => $pass_reset_token, |
412 | + ":user_id" => $user_record['id'] |
413 | + )) |
414 | + ); |
415 | + |
416 | + $passwordResetLink = make_password_reset_url($config, $proxy_id = $proxy_record['id'], $pass_reset_token). |
417 | + ' '.msg("reset_password_subject", $user_record['last_lang']); |
418 | |
419 | // send the email |
420 | // TODO: should be some throttling here, as anyone can send an |
421 | @@ -121,7 +115,7 @@ |
422 | $config, |
423 | $user_record['email'], |
424 | msg("reset_password_subject", $user_record['last_lang']), |
425 | - sprintf(msg("reset_password_body", $user_record['last_lang']),$login_pass), |
426 | + sprintf(msg("reset_password_body", $user_record['last_lang']), $passwordResetLink), |
427 | $user_record['last_lang']); |
428 | |
429 | // The feedback here leaked information and allowed anyone to probe |
Code review comments:
- The alter table statements should have default values if NOT NULL is used
- We would like to follow the pattern of using ttl from the config file and storing only the reset date in the database
- In lang.php, reset_password_ error and password_sent: change "Password reset link" to "A password reset link"
- Always return 404 for invalid conditions in p.php. (In diff line 214 403 is used)
- Use of start_session() and _SESSION is unnecessary. The token GET param will automatically be passed in the POST request as it uses REQUEST_URI
- We noticed inconsistent code formatting: inconsistent braces, we always put braces around single line statements following if statements, multiple empty lines, multiple whitespace characters around
"=" signs, inconsistent php style using <?php echo ...> vs <?=...>
- Diff lines 395-398 is no-op code
- We don't understand the purpose of the "@" symbol at diff lines 261-262
- See Bug #634452 for the correct use of redirects
- The else block at diff line 378 is useless and should be removed. The if block starting at diff line 353 should be unconditional
- The following line is absent: "Content- type: text/html; charset= ".charset( ));?> www.w3. org/TR/ html4/loose. dtd">
<?=header(
It should appear above
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://
- The language module expects a global variable $record_user to be set in order to retrieve the current language.
So, $record_user should be set to $user_to_reset
- It is not necessary to escape_html the output of msg() because this is not user input
- Diff line 390: The language buttons should be removed, and instead the $record_ user[last_ lang] will be automatically used to set the email text and the reset link language to the user's correct language.
- Do not put html markup in the email text. The email is sent formatted as plain text. .make_password_ reset_url( $config, $proxy_id = $proxy_ record[ 'id'], $pass_reset_ token). '">'.
$passwordResetLink = '<a href="'
"<a href=" should not appear in the email body text.
- It is no longer necessary to invalidate the user session in reset_password.php when a password reset is requested (diff line 426: session = :session_id,)