Bläddra i källkod

Give each password reset its own user's name and address

process_change_request kept its dispatch table in a state hash, so the
reset_pass closure was built once, on the first call, and kept that
call's $display and $contactemail.  Each later reset in the same
process wrote the first user's display name and contact address onto
the user being reset.  Because useradd is an INSERT OR REPLACE and the
display name is unique, it also deleted the first user's row.

The table is my now, so each call builds a closure over its own row.
The closure's $user was a second variable of that name, which
Variables::ProhibitReusedNames reported.  It is $name now.

t/Trog-Auth-change-request.t resets two users in one process, and fails
against the old code: the second gets the first one's name, and the
first is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
troglodyne-bot 1 vecka sedan
förälder
incheckning
3ca16414cb
2 ändrade filer med 46 tillägg och 7 borttagningar
  1. 7 7
      lib/Trog/Auth.pm
  2. 39 0
      t/Trog-Auth-change-request.t

+ 7 - 7
lib/Trog/Auth.pm

@@ -531,16 +531,16 @@ sub process_change_request ($token) {
     my $secret       = $rows->[0]{secret};
     my $contactemail = $rows->[0]{contact_email};
 
-    state %dispatch = (
-        reset_pass => sub {
-            my ( $user, $pass ) = @_;
+    # my, not state: each closure reads this request's $display and $contactemail.
+    my %dispatch = (
+        reset_pass => sub ( $name, $pass ) {
 
             #XXX The fact that this is an INSERT OR REPLACE means all the entries in change_request for this user will get cascade wiped.  Which is good, as the secrets aren't salted.
             # This is also why we have to snag the user's ACLs or they will be wiped.
-            my @acls = acls4user($user);
-            useradd( $user, $display, $pass, \@acls, $contactemail );
-            killsession($user);
-            return "Password set to $pass for $user";
+            my @acls = acls4user($name);
+            useradd( $name, $display, $pass, \@acls, $contactemail );
+            killsession($name);
+            return "Password set to $pass for $name";
         },
     );
     my $res = $dispatch{$type}->( $user, $secret );

+ 39 - 0
t/Trog-Auth-change-request.t

@@ -0,0 +1,39 @@
+#!/usr/bin/env perl
+use 5.040;
+use re '/aasx';
+use warnings FATAL => 'all';
+
+use FindBin::libs;
+
+use Test::More;
+use Cwd();
+use File::Copy();
+use File::Path();
+use File::Temp();
+
+# Auth keeps its database at config/auth.db under the cwd, so it gets a sandbox.
+my $repo = Cwd::getcwd();
+my $root = File::Temp::tempdir( CLEANUP => 1 );
+File::Path::make_path( "$root/config", "$root/schema" );
+File::Copy::copy( "$repo/schema/auth.schema", "$root/schema/auth.schema" ) or die $!;
+chdir($root)                                                               or die "chdir $root: $!";
+END { chdir($repo) if $repo }
+
+require_ok('Trog::Auth') or BAIL_OUT("Can't find SUT");
+
+my $dbh = Trog::SQLite::dbh( 'schema/auth.schema', 'config/auth.db' );
+sub contact_of ($user) { return $dbh->selectrow_hashref( 'SELECT display_name, contact_email FROM user WHERE name = ?', undef, $user ) }
+
+subtest 'process_change_request: each reset keeps its own user' => sub {
+    Trog::Auth::useradd( qw{alice Alice old}, [], 'alice@test.test' );
+    Trog::Auth::useradd( qw{bob Bob old},     [], 'bob@test.test' );
+    Trog::Auth::add_change_request( user => $_, token => "t-$_", type => 'reset_pass', secret => 'new' ) for qw{alice bob};
+
+    like( Trog::Auth::process_change_request('t-alice'), qr/for[ ]alice/, 'the first reset is done' );
+    like( Trog::Auth::process_change_request('t-bob'),   qr/for[ ]bob/,   'and the second' );
+
+    is_deeply( contact_of('alice'), { display_name => 'Alice', contact_email => 'alice@test.test' }, 'the first user is as they were' );
+    is_deeply( contact_of('bob'),   { display_name => 'Bob',   contact_email => 'bob@test.test' },   'and the second keeps their own name and address, not the first one\'s' );
+};
+
+done_testing();