[freeside-commits] branch master updated. 10f2ba3c0fa4c1147be6535ef10890d2f2defe9f

Jonathan Prykop jonathan at 420.am
Wed Jul 22 23:44:46 PDT 2015


The branch, master has been updated
       via  10f2ba3c0fa4c1147be6535ef10890d2f2defe9f (commit)
      from  68d0a78fcbdaac34656af91640886cce5dd6e501 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 10f2ba3c0fa4c1147be6535ef10890d2f2defe9f
Author: Jonathan Prykop <jonathan at freeside.biz>
Date:   Thu Jul 23 01:43:40 2015 -0500

    RT#37163 Disconnect Users via Radclient [die on error]

diff --git a/FS/FS/part_export/sqlradius.pm b/FS/FS/part_export/sqlradius.pm
index 1f5b018..048a244 100644
--- a/FS/FS/part_export/sqlradius.pm
+++ b/FS/FS/part_export/sqlradius.pm
@@ -10,6 +10,7 @@ use FS::svc_acct;
 use FS::export_svc;
 use Carp qw( cluck );
 use NEXT;
+use Net::OpenSSH;
 
 @ISA = qw(FS::part_export);
 @EXPORT_OK = qw( sqlradius_connect );
@@ -79,8 +80,8 @@ tie %options, 'Tie::IxHash',
   'disconnect_port' => {
     label => 'Port to send disconnection requests to, default 1700',
   },
-  'disconnect_log' => {
-    label => 'Print disconnect output and errors to the queue log (will otherwise fail silently)',
+  'disconnect_ignore_error' => {
+    label => 'Ignore disconnection request errors',
     type => 'checkbox',
   },
 ;
@@ -194,22 +195,6 @@ sub _export_replace {
   my $dbh = dbh;
 
   my $jobnum = '';
-
-  # disconnect users before changing username
-  if ($self->option('disconnect_ssh')) {
-    my $err_or_queue = $self->sqlradius_queue( $new->svcnum, 'user_disconnect',
-      'disconnect_ssh'    => $self->option('disconnect_ssh'),
-      'svc_acct_username' => $old->username,
-      'disconnect_port'   => $self->option('disconnect_port'),
-      'disconnect_log'    => $self->option('disconnect_log'),
-    );
-    unless ( ref($err_or_queue) ) {
-      $dbh->rollback if $oldAutoCommit;
-      return $err_or_queue;
-    }
-    $jobnum = $err_or_queue->jobnum; # chain all of these dependencies
-  }
-
   if ( $self->export_username($old) ne $self->export_username($new) ) {
     my $usergroup = $self->option('usergroup') || 'usergroup';
     my $err_or_queue = $self->sqlradius_queue( $new->svcnum, 'rename',
@@ -218,13 +203,6 @@ sub _export_replace {
       $dbh->rollback if $oldAutoCommit;
       return $err_or_queue;
     }
-    if ( $jobnum ) {
-      my $error = $err_or_queue->depend_insert( $jobnum );
-      if ( $error ) {
-        $dbh->rollback if $oldAutoCommit;
-        return $error;
-      }
-    }
     $jobnum = $err_or_queue->jobnum;
   }
 
@@ -274,7 +252,7 @@ sub _export_replace {
   my $error;
   my (@oldgroups) = $old->radius_groups('hashref');
   my (@newgroups) = $new->radius_groups('hashref');
-  $error = $self->sqlreplace_usergroups( $new->svcnum,
+  ($error,$jobnum) = $self->sqlreplace_usergroups( $new->svcnum,
                                          $self->export_username($new),
                                          $jobnum ? $jobnum : '',
                                          \@oldgroups,
@@ -285,6 +263,28 @@ sub _export_replace {
     return $error;
   }
 
+  # radius database is used for authorization, so to avoid users reauthorizing
+  # before the database changes, disconnect users after changing database
+  if ($self->option('disconnect_ssh')) {
+    my $err_or_queue = $self->sqlradius_queue( $new->svcnum, 'user_disconnect',
+      'disconnect_ssh'    => $self->option('disconnect_ssh'),
+      'svc_acct_username' => $old->username,
+      'disconnect_port'   => $self->option('disconnect_port'),
+      'ignore_error'      => $self->option('disconnect_ignore_error'),
+    );
+    unless ( ref($err_or_queue) ) {
+      $dbh->rollback if $oldAutoCommit;
+      return $err_or_queue;
+    }
+    if ( $jobnum ) {
+      my $error = $err_or_queue->depend_insert( $jobnum );
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return $error;
+      }
+    }
+  }
+
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
 
   '';
@@ -309,21 +309,6 @@ sub _export_suspend {
 
   my $jobnum = '';
 
-  # disconnect users before changing anything
-  if ($self->option('disconnect_ssh')) {
-    my $err_or_queue = $self->sqlradius_queue( $new->svcnum, 'user_disconnect',
-      'disconnect_ssh'    => $self->option('disconnect_ssh'),
-      'svc_acct_username' => $svc_acct->username,
-      'disconnect_port'   => $self->option('disconnect_port'),
-      'disconnect_log'    => $self->option('disconnect_log'),
-    );
-    unless ( ref($err_or_queue) ) {
-      $dbh->rollback if $oldAutoCommit;
-      return $err_or_queue;
-    }
-    $jobnum = $err_or_queue->jobnum;
-  }
-
   my @newgroups = $self->suspended_usergroups($svc_acct);
 
   unless (@newgroups) { #don't change password if assigning to a suspended group
@@ -334,16 +319,11 @@ sub _export_suspend {
       $dbh->rollback if $oldAutoCommit;
       return $err_or_queue;
     }
-    if ( $jobnum ) {
-      my $error = $err_or_queue->depend_insert( $jobnum );
-      if ( $error ) {
-        $dbh->rollback if $oldAutoCommit;
-        return $error;
-      }
-    }
+    $jobnum = $err_or_queue->jobnum;
   }
 
-  my $error =
+  my $error;
+  ($error,$jobnum) =
     $self->sqlreplace_usergroups(
       $new->svcnum,
       $self->export_username($new),
@@ -355,6 +335,28 @@ sub _export_suspend {
     $dbh->rollback if $oldAutoCommit;
     return $error;
   }
+
+  # radius database is used for authorization, so to avoid users reauthorizing
+  # before the database changes, disconnect users after changing database
+  if ($self->option('disconnect_ssh')) {
+    my $err_or_queue = $self->sqlradius_queue( $new->svcnum, 'user_disconnect',
+      'disconnect_ssh'    => $self->option('disconnect_ssh'),
+      'svc_acct_username' => $svc_acct->username,
+      'disconnect_port'   => $self->option('disconnect_port'),
+    );
+    unless ( ref($err_or_queue) ) {
+      $dbh->rollback if $oldAutoCommit;
+      return $err_or_queue;
+    }
+    if ( $jobnum ) {
+      my $error = $err_or_queue->depend_insert( $jobnum );
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return $error;
+      }
+    }
+  }
+
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
 
   '';
@@ -404,24 +406,25 @@ sub _export_delete {
 
   my $jobnum = '';
 
-  # disconnect users before changing anything
+  my $usergroup = $self->option('usergroup') || 'usergroup';
+  my $err_or_queue = $self->sqlradius_queue( $svc_x->svcnum, 'delete',
+    $self->export_username($svc_x), $usergroup );
+  $jobnum = $err_or_queue->jobnum;
+
+  # radius database is used for authorization, so to avoid users reauthorizing
+  # before the database changes, disconnect users after changing database
   if ($self->option('disconnect_ssh')) {
     my $err_or_queue = $self->sqlradius_queue( $svc_x->svcnum, 'user_disconnect',
       'disconnect_ssh'    => $self->option('disconnect_ssh'),
       'svc_acct_username' => $svc_x->username,
       'disconnect_port'   => $self->option('disconnect_port'),
-      'disconnect_log'    => $self->option('disconnect_log'),
+      'ignore_error'      => $self->option('disconnect_ignore_error'),
     );
     return $err_or_queue unless ref($err_or_queue);
-    $jobnum = $err_or_queue->jobnum;
-  }
-
-  my $usergroup = $self->option('usergroup') || 'usergroup';
-  my $err_or_queue = $self->sqlradius_queue( $svc_x->svcnum, 'delete',
-    $self->export_username($svc_x), $usergroup );
-  if ( $jobnum ) {
-    my $error = $err_or_queue->depend_insert( $jobnum );
-    return $error if $error;
+    if ( $jobnum ) {
+      my $error = $err_or_queue->depend_insert( $jobnum );
+      return $error if $error;
+    }
   }
 
   ref($err_or_queue) ? '' : $err_or_queue;
@@ -616,6 +619,8 @@ sub sqlradius_connect {
   DBI->connect(@_) or die $DBI::errstr;
 }
 
+# on success, returns '' in scalar context, ('',$jobnum) in list context
+# on error, always just returns error
 sub sqlreplace_usergroups {
   my ($self, $svcnum, $username, $jobnum, $old, $new) = @_;
 
@@ -657,8 +662,9 @@ sub sqlreplace_usergroups {
       my $error = $err_or_queue->depend_insert( $jobnum );
       return $error if $error;
     }
+    $jobnum = $err_or_queue->jobnum; # chain all of these dependencies
   }
-  '';
+  wantarray ? ('',$jobnum) : '';
 }
 
 
@@ -1252,7 +1258,7 @@ I<svc_acct_username> - the user to be disconnected (required)
 
 I<disconnect_port> - the port (on the nas) to send disconnect requests to (defaults to 1700)
 
-I<disconnect_log> - if true, print disconnect command & output to the error log
+I<ignore_error> - do not die on error with the disconnect request
 
 Note this is NOT the opposite of sqlradius_connect.
 
@@ -1269,21 +1275,26 @@ sub sqlradius_user_disconnect {
   $dbh->disconnect();
   die "No nas found in radius db" unless @$nas;
   # set up ssh connection
-  eval "use Net::SSH";
   my $ssh = Net::OpenSSH->new($opt{'disconnect_ssh'});
   die "Couldn't establish SSH connection: " . $ssh->error
     if $ssh->error;
   # send individual disconnect requests
   my $user = $opt{'svc_acct_username'}; #svc_acct username
   my $port = $opt{'disconnect_port'} || 1700; #or should we pull this from the db?
+  my $error = '';
   foreach my $nas (@$nas) {
     my $nasname = $nas->{'nasname'};
     my $secret  = $nas->{'secret'};
     my $command = qq(echo "User-Name=$user" | radclient -r 1 $nasname:$port disconnect '$secret');
     my ($output, $errput) = $ssh->capture2($command);
-    warn $command . "\n" . $output . $errput . $ssh->error . "\n"
-      if $opt{'disconnect_log'};
+    $error .= "Error running $command: $errput " . $ssh->error . " "
+      if $errput || $ssh->error;
   }
+  $error .= "Some clients may have successfully disconnected"
+    if $error && (@$nas > 1);
+  $error = "No clients found"
+    unless @$nas;
+  die $error if $error && !$opt{'ignore_error'};
   return '';
 }
 

-----------------------------------------------------------------------

Summary of changes:
 FS/FS/part_export/sqlradius.pm |  141 ++++++++++++++++++++++------------------
 1 file changed, 76 insertions(+), 65 deletions(-)




More information about the freeside-commits mailing list