[Bps-public-commit] rt-extension-resetpassword branch, master, updated. 1.04-32-gb2c154b
? sunnavy
sunnavy at bestpractical.com
Fri Oct 2 17:22:26 EDT 2020
The branch, master has been updated
via b2c154ba7351413982e3383f6ccd531b7bbc407c (commit)
via 3a394c7bde2258fa160b1db8e316ac78286c7390 (commit)
via faeafcb95597fb6352c9668281591ce2f446fbc2 (commit)
via 0bf7e73e8e99595eee5f31c86191b3575c82e92a (commit)
via 0da20d1004527deec7392dec198209b38f053b47 (commit)
from 467963088b9d2521683631c247c4b4530030204b (commit)
Summary of changes:
html/NoAuth/ResetPassword/Reset/dhandler | 138 +++++++++++++++++++++++--------
lib/RT/Extension/ResetPassword.pm | 25 ++++--
2 files changed, 125 insertions(+), 38 deletions(-)
- Log -----------------------------------------------------------------
commit 0da20d1004527deec7392dec198209b38f053b47
Author: Dianne Skoll <dianne at bestpractical.com>
Date: Fri Sep 4 14:15:02 2020 -0400
Refactor code to avoid duplicating token-generating code.
diff --git a/html/NoAuth/ResetPassword/Reset/dhandler b/html/NoAuth/ResetPassword/Reset/dhandler
index b42f34b..cb7f6c6 100644
--- a/html/NoAuth/ResetPassword/Reset/dhandler
+++ b/html/NoAuth/ResetPassword/Reset/dhandler
@@ -1,4 +1,6 @@
<%init>
+use RT::Extension::ResetPassword;
+
# The URL They're visitng
# @{[$RT::WebURL]}/NoAuth/Reset/@{[$token]}/@{[$u->id]}
my @results;
@@ -11,9 +13,7 @@ my $token;
my $u = RT::User->new($RT::SystemUser);
$u->LoadByCols( id => $id );
if ( $u->id ) {
- $token = Digest::MD5->new()->add( $u->id, $u->__Value('Password'),
- $RT::DatabasePassword, $u->LastUpdated,
- @{[$RT::WebPath]} . '/NoAuth/ResetPassword/Reset' )->hexdigest();
+ $token = RT::Extension::ResetPassword::CreateToken($u) || '';
}
else {
push @results,
diff --git a/lib/RT/Extension/ResetPassword.pm b/lib/RT/Extension/ResetPassword.pm
index 33315b5..d4e8e50 100644
--- a/lib/RT/Extension/ResetPassword.pm
+++ b/lib/RT/Extension/ResetPassword.pm
@@ -7,21 +7,27 @@ our $VERSION = '1.06';
RT->AddStyleSheets("resetpassword.css");
-sub CreateTokenAndResetPassword {
+sub CreateToken {
my $user = shift;
unless ( $user && $user->Id ) {
- RT::Logger->error( "Need to provide a loaded RT::User object for CreateTokenAndResetPassword." );
- return;
+ RT::Logger->error( "Need to provide a loaded RT::User object for CreateToken" );
+ return undef;
}
-
- my $token = Digest::MD5->new()->add(
+ return Digest::MD5->new()->add(
$user->id,
$user->__Value('Password'),
$RT::DatabasePassword,
$user->LastUpdated,
@{[$RT::WebPath]} . '/NoAuth/ResetPassword/Reset'
- )->hexdigest();
+ )->hexdigest();
+}
+
+sub CreateTokenAndResetPassword {
+ my $user = shift;
+
+ my $token = CreateToken($user);
+ return unless $token; # CreateToken will log error
my ($status, $msg) = RT::Interface::Email::SendEmailUsingTemplate(
To => $user->EmailAddress,
commit 0bf7e73e8e99595eee5f31c86191b3575c82e92a
Author: Dianne Skoll <dianne at bestpractical.com>
Date: Fri Sep 4 15:31:39 2020 -0400
Use SHA256 instead of MD5 to generate the token.
diff --git a/lib/RT/Extension/ResetPassword.pm b/lib/RT/Extension/ResetPassword.pm
index d4e8e50..55929e9 100644
--- a/lib/RT/Extension/ResetPassword.pm
+++ b/lib/RT/Extension/ResetPassword.pm
@@ -3,6 +3,8 @@ package RT::Extension::ResetPassword;
use strict;
use warnings;
+use Digest::SHA qw(sha256_hex);
+
our $VERSION = '1.06';
RT->AddStyleSheets("resetpassword.css");
@@ -14,13 +16,14 @@ sub CreateToken {
RT::Logger->error( "Need to provide a loaded RT::User object for CreateToken" );
return undef;
}
- return Digest::MD5->new()->add(
+
+ return sha256_hex(
$user->id,
$user->__Value('Password'),
$RT::DatabasePassword,
$user->LastUpdated,
@{[$RT::WebPath]} . '/NoAuth/ResetPassword/Reset'
- )->hexdigest();
+ );
}
sub CreateTokenAndResetPassword {
commit faeafcb95597fb6352c9668281591ce2f446fbc2
Author: Dianne Skoll <dianne at bestpractical.com>
Date: Fri Sep 11 11:05:44 2020 -0400
Make the password-change link expire after a configurable interval (default 4 hours)
diff --git a/html/NoAuth/ResetPassword/Reset/dhandler b/html/NoAuth/ResetPassword/Reset/dhandler
index cb7f6c6..29a3deb 100644
--- a/html/NoAuth/ResetPassword/Reset/dhandler
+++ b/html/NoAuth/ResetPassword/Reset/dhandler
@@ -21,6 +21,22 @@ else {
$show_form = 0;
}
+# Calculate time difference between now and when user object was updated
+my $age = $u->LastUpdatedObj->Diff;
+if (!defined($age)) {
+ # Could not get the time difference; make age negative which should
+ # be impossible; we'll catch it below
+ $age = -1000000;
+} else {
+ # The time difference returned by Diff should be negative, so correct
+ if ($age > 0) {
+ # Impossible... someone turned back the machine's clock
+ $age = -1000000;
+ } else {
+ $age = -1 * $age;
+ }
+}
+
# If the token validation fails, throw them an error
if ( $submitted_token ne $token ) {
push @results,
@@ -30,7 +46,17 @@ if ( $submitted_token ne $token ) {
$show_form = 0;
}
-# if the validation succeeds, continue on
+# If the link has expired, throw the same error. Default expiry time is 4 hours
+elsif ( ($age < 0) ||
+ ($age > (RT->Config->Get('PasswordChangeLinkExpirySeconds') || (4*60*60)))) {
+ push @results,
+ loc(
+ "It looks like the URL you clicked on has expired or wasn't quite right. Maybe you didn't paste the whole thing?"
+ );
+ $show_form = 0;
+}
+
+# Link is valid and has not expired
else {
# If they've supplied a new password twice, change it and redirect to home
diff --git a/lib/RT/Extension/ResetPassword.pm b/lib/RT/Extension/ResetPassword.pm
index 55929e9..f740163 100644
--- a/lib/RT/Extension/ResetPassword.pm
+++ b/lib/RT/Extension/ResetPassword.pm
@@ -29,6 +29,12 @@ sub CreateToken {
sub CreateTokenAndResetPassword {
my $user = shift;
+ # Update the LastUpdated time in the $user so that we can
+ # expire the password-change link that gets sent out. We
+ # need to do this before we create the token because $user->LastUpdated
+ # is part of the token hash
+ $user->_SetLastUpdated();
+
my $token = CreateToken($user);
return unless $token; # CreateToken will log error
commit 3a394c7bde2258fa160b1db8e316ac78286c7390
Author: Dianne Skoll <dianne at bestpractical.com>
Date: Fri Sep 25 14:34:49 2020 -0400
Save token into cookie and redirect to not expose sensitive information
The reset-password link contains a potentially-sensitive token. Although
the possibility for abuse is small since the token is invalidated after
it has been used, we don't want to leak the token in any Referrer: headers.
By storing the sensitive information in a cookie and immediately redirecting,
we should never see the token in a Referrer header.
diff --git a/html/NoAuth/ResetPassword/Reset/dhandler b/html/NoAuth/ResetPassword/Reset/dhandler
index 29a3deb..68e48d8 100644
--- a/html/NoAuth/ResetPassword/Reset/dhandler
+++ b/html/NoAuth/ResetPassword/Reset/dhandler
@@ -8,18 +8,60 @@ my $show_form = 1;
my $title = loc('Reset your password');
my $virtual_path = $m->dhandler_arg();
my ( $submitted_token, $id ) = split( '/', $virtual_path );
-my $token;
-# Validate the token
-my $u = RT::User->new($RT::SystemUser);
-$u->LoadByCols( id => $id );
-if ( $u->id ) {
- $token = RT::Extension::ResetPassword::CreateToken($u) || '';
+
+my $bland_error_message = loc(
+ "It looks like the URL you clicked on has expired or wasn't quite right. Maybe you didn't paste the whole thing?"
+);
+
+my $cookie_name = 'RTPasswdResetCookie';
+
+# If the token and ID are part of the URL path, set a cookie and redirect.
+# We do not want the original URL to accidentally leak info in the
+# Referrer field
+if ($submitted_token && $id) {
+ my $cookie = CGI::Cookie->new(
+ -name => $cookie_name,
+ -value => "$submitted_token/$id",
+ -path => RT->Config->Get('WebPath'),
+ -secure => (RT->Config->Get('WebSecureCookies') ? 1 : 0),
+ -httponly => (RT->Config->Get('WebHttpOnlyCookies') ? 1 : 0)
+ );
+ $r->err_headers_out->{'Set-Cookie'} = $cookie->as_string;
+ $r->status(302);
+ $m->redirect(RT->Config->Get('WebPath') . '/NoAuth/ResetPassword/Reset/',
+ '302 found');
+ $m->abort;
}
-else {
- push @results,
- loc("Something went wrong. Please contact your RT administrator.");
+
+# If we don't have a cookie, something's wrong
+my %cookies = CGI::Cookie->parse(RT::Interface::Web::RequestENV('HTTP_COOKIE'));
+
+if (!$cookies{$cookie_name}) {
+ push @results, $bland_error_message;
$show_form = 0;
}
+if ($show_form) {
+ ( $submitted_token, $id ) = split( '/', $cookies{$cookie_name}->value());
+ if (!$submitted_token || !$id) {
+ push @results, $bland_error_message;
+ $show_form = 0;
+ }
+}
+
+my $token;
+my $u = RT::User->new($RT::SystemUser);
+if ($show_form) {
+ # Validate the token
+ $u->LoadByCols( id => $id );
+ if ( $u->id ) {
+ $token = RT::Extension::ResetPassword::CreateToken($u) || '';
+ }
+ else {
+ push @results,
+ loc("Something went wrong. Please contact your RT administrator.");
+ $show_form = 0;
+ }
+}
# Calculate time difference between now and when user object was updated
my $age = $u->LastUpdatedObj->Diff;
@@ -37,44 +79,48 @@ if (!defined($age)) {
}
}
-# If the token validation fails, throw them an error
-if ( $submitted_token ne $token ) {
- push @results,
- loc(
- "It looks like the URL you clicked on has expired or wasn't quite right. Maybe you didn't paste the whole thing?"
- );
- $show_form = 0;
-}
-
-# If the link has expired, throw the same error. Default expiry time is 4 hours
-elsif ( ($age < 0) ||
- ($age > (RT->Config->Get('PasswordChangeLinkExpirySeconds') || (4*60*60)))) {
- push @results,
- loc(
- "It looks like the URL you clicked on has expired or wasn't quite right. Maybe you didn't paste the whole thing?"
- );
- $show_form = 0;
-}
-
-# Link is valid and has not expired
-else {
-
- # If they've supplied a new password twice, change it and redirect to home
- if ( ( $submitted_token eq $token )
- and $ARGS{'password'}
- and $ARGS{'password2'}
- and ( $ARGS{'password'} eq $ARGS{'password2'} ) )
- {
- my ( $val, $msg ) = $u->SetPassword( $ARGS{'password'} );
- push @results, $msg;
- if ($val) { $show_form = 0;}
+if ($show_form) {
+ # If the token validation fails, throw them an error
+ if ( $submitted_token ne $token ) {
+ push @results, $bland_error_message;
+ $show_form = 0;
}
- elsif ( $ARGS{'password'} ) {
- push @results, loc("The two passwords you typed didn't match.");
+ # If the link has expired, throw the same error. Default expiry time is 4 hours
+ elsif ( ($age < 0) ||
+ ($age > (RT->Config->Get('PasswordChangeLinkExpirySeconds') || (4*60*60)))) {
+ push @results, $bland_error_message;
+ $show_form = 0;
+ }
+ # Link is valid and has not expired
+ else {
+ # If they've supplied a new password twice, change it and redirect to home
+ if ( ( $submitted_token eq $token )
+ and $ARGS{'password'}
+ and $ARGS{'password2'}
+ and ( $ARGS{'password'} eq $ARGS{'password2'} ) )
+ {
+ my ( $val, $msg ) = $u->SetPassword( $ARGS{'password'} );
+ push @results, $msg;
+ if ($val) { $show_form = 0;}
+ # Ask the browser to delete the cookie
+ my $cookie = CGI::Cookie->new(
+ -name => $cookie_name,
+ -value => '',
+ -expires => '-10y',
+ -path => RT->Config->Get('WebPath'),
+ -secure => (RT->Config->Get('WebSecureCookies') ? 1 : 0),
+ -httponly => (RT->Config->Get('WebHttpOnlyCookies') ? 1 : 0)
+ );
+ $r->err_headers_out->{'Set-Cookie'} = $cookie->as_string;
+
+ }
+ elsif ( $ARGS{'password'} ) {
+ push @results, loc("The two passwords you typed didn't match.");
+ }
}
-
}
-# otherwise, show the form
+
+# otherwise, (potentially) show the form
</%init>
commit b2c154ba7351413982e3383f6ccd531b7bbc407c
Merge: 4679630 3a394c7
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Oct 3 05:19:43 2020 +0800
Merge branch 'sha256-instead-of-md5-for-token-generation'
-----------------------------------------------------------------------
More information about the Bps-public-commit
mailing list