Merge #3695 fix/sms-pincode-reuse

lib/pf/activation.pm

@@ -191,7 +191,6 @@ sub view_by_code {
             type => $type,
             activation_code => $activation_code,
         },
-        -no_auto_tenant_id => 1,
     );
     if (is_error($status)) {
         return undef;
@@ -203,6 +202,32 @@ sub view_by_code {
     return ($item->to_hash);
 }
 
+=head2 is_code_in_use
+
+is_code_in_use
+
+=cut
+
+sub is_code_in_use {
+    my ($type, $code, $mac) = @_;
+    my ($status, $iter) = pf::dal::activation->search(
+        -columns => [ \"1" ],
+        -where   => {
+            type            => $type,
+            activation_code => $code,
+            status          => $UNVERIFIED,
+            expiration => { ">=" => \['NOW()']},
+            ($type eq 'sms' && defined($mac)) ? ( mac => $mac ) : (),
+        },
+    );
+
+    if (is_error($status)) {
+        return undef;
+    }
+
+    return $iter->rows;
+}
+
 =head2 view_by_code_mac
 
 view_by_code_mac
@@ -403,27 +428,46 @@ sub _generate_activation_code {
     my $no_unique = $data{'no_unique'};
     my $type = $data{'type'};
     my $style = $data{'style'} // 'md5';
+    my $generator = $style eq 'digits' ? \&digits_generator : \&md5_generator;
+    my $mac = $data{mac};
     do {
         # generating something not so easy to guess (and hopefully not in rainbowtables)
-        if ($style eq 'digits') {
-            $code = int(rand(9999999999)) + 1;
-        } else {
-            $code = md5_hex(
-                join("|",
-                    time + int(rand(10)),
-                    grep {defined $_} @data{qw(expiration mac pid contact_info)})
-            );
-        }
+        $code = $generator->(\%data);
         if ($code_length > 0) {
             $code = substr($code, 0, $code_length);
         }
         # make sure the generated code is unique
-        $code = undef if (!$no_unique && view_by_code($type, $code));
+        $code = undef if (!$no_unique && is_code_in_use($type, $code, $mac));
     } while (!defined($code));
 
     return $code;
 }
 
+=head2 digits_generator
+
+digits_generator
+
+=cut
+
+sub digits_generator {
+    return int(rand(9999999999)) + 1;;
+}
+
+=head2 md5_generator
+
+md5_generator
+
+=cut
+
+sub md5_generator {
+    my ($data) = @_;
+    return md5_hex(
+        join( "|",
+            time + int( rand(10) ),
+            grep { defined $_ } @{$data}{qw(expiration mac pid contact_info)} )
+    );
+}
+
 =head2 send_email
 
 Send an email with the activation code

lib/pf/dal.pm

@@ -332,14 +332,15 @@ sub update_items {
         @args,
     );
     return $status, undef if is_error($status);
-
+    my $rows = $sth->rows;
+    $sth->finish;
     my $info = $sth->{Database}{mysql_info};
-    if ($info =~ /^.*: (\d+).*: (\d+).*: (\d+)/) {
+    if ($info && $info =~ /^.*: (\d+).*: (\d+).*: (\d+)/) {
         my ($matched, $row, $warning) = ($1, $2, $3);
         return $STATUS::OK, $matched;
     }
 
-    return $STATUS::OK, $sth->rows;
+    return $STATUS::OK, $rows;
 }
 
 =head2 insert

t/unittest/activation.t

@@ -0,0 +1,88 @@
+#!/usr/bin/perl
+
+=head1 NAME
+
+activation
+
+=head1 DESCRIPTION
+
+unit test for activation
+
+=cut
+
+use strict;
+use warnings;
+#
+use lib '/usr/local/pf/lib';
+
+BEGIN {
+    #include test libs
+    use lib qw(/usr/local/pf/t);
+    #Module for overriding configuration paths
+    use setup_test_config;
+}
+
+use Test::More tests => 6;
+use Utils;
+
+#This test will running last
+use Test::NoWarnings;
+use pf::activation qw($UNVERIFIED $SMS_ACTIVATION);
+my $mac =  Utils::test_mac();
+my $mac2 = Utils::test_mac();
+my $pid = "default";
+
+my %data = (
+    'pid' => $pid,
+    'mac' => $mac,
+    'contact_info' => 'test@inverse.ca',
+    'status' => $UNVERIFIED,
+    'type' => 'sms',
+    'portal' => 'portal',
+    'carrier_id' => 100056,
+    'code_length' => 8,
+    'style'    => 'md5',
+    timeout    => 2,
+    'source_id' => 'local',
+);
+
+my $code = pf::activation::create(\%data);
+
+ok (defined $code ,"Code generated for $mac for $pid");
+
+ok (pf::activation::is_code_in_use('sms', $code, $mac), "code in use");
+ok (!pf::activation::is_code_in_use('sms', $code, $mac2), "code not in use for mac2");
+ok (!pf::activation::is_code_in_use('sms', "${code}_$$", $mac), "Is not code in use");
+
+sleep(3);
+
+ok (!pf::activation::is_code_in_use('sms', $code, $mac), "Is not in use");
+
+=head1 AUTHOR
+
+Inverse inc. <info@inverse.ca>
+
+=head1 COPYRIGHT
+
+Copyright (C) 2005-2018 Inverse inc.
+
+=head1 LICENSE
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License
+as published by the Free Software Foundation; either version 2
+of the License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
+USA.
+
+=cut
+
+1;