Record ACL names in the ACL history

Change-Id: I0d7a088bb34dda2fc554b9f104c2a33e5faf879e
Reviewed-on: https://gerrit.stanford.edu/1554
Reviewed-by: Russ Allbery <rra@stanford.edu>
Tested-by: Russ Allbery <rra@stanford.edu>

5 files changed, 23 insertions, 14 deletions

diff --git a/NEWS b/NEWS
index f6e3fdd..08a7e14 100644
--- a/NEWS
+++ b/NEWS
@@ -46,6 +46,8 @@ wallet 1.1 (unreleased)
     the DateTime::Format::* module corresponding to the DBD::* module used
     for the server database.
 
+    ACL renames are now recorded in the ACL history.
+
     The initial creation and membership of the ADMIN ACL during database
     initialization or reinitialization is no longer recorded in the
     acl_history table.  (This is fallout from making a specific type of
diff --git a/perl/lib/Wallet/ACL.pm b/perl/lib/Wallet/ACL.pm
index 6f5172a..b488b43 100644
--- a/perl/lib/Wallet/ACL.pm
+++ b/perl/lib/Wallet/ACL.pm
@@ -161,7 +161,7 @@ sub scheme_mapping {
 # change and should be committed with that change.
 sub log_acl {
     my ($self, $action, $scheme, $identifier, $user, $host, $time) = @_;
-    unless ($action =~ /^(add|remove)\z/) {
+    unless ($action =~ /^(add|remove|rename)\z/) {
         die "invalid history action $action";
     }
     my $date = DateTime->from_epoch (epoch => $time);
@@ -184,7 +184,8 @@ sub log_acl {
 # logged since it isn't a change to any of the data stored in the wallet.
 # Returns true on success, false on failure.
 sub rename {
-    my ($self, $name) = @_;
+    my ($self, $name, $user, $host, $time) = @_;
+    $time ||= time;
     if ($name =~ /^\d+\z/) {
         $self->error ("ACL name may not be all numbers");
         return;
@@ -195,6 +196,7 @@ sub rename {
         my $acls = $self->{schema}->resultset('Acl')->find (\%search);
         $acls->ac_name ($name);
         $acls->update;
+        $self->log_acl ('rename', undef, undef, $user, $host, $time);
         $guard->commit;
     };
     if ($@) {
@@ -381,6 +383,8 @@ sub history {
             if ($data->ah_action eq 'add' || $data->ah_action eq 'remove') {
                 $output .= sprintf ("%s %s %s", $data->ah_action,
                                     $data->ah_scheme, $data->ah_identifier);
+            } elsif ($data->ah_action eq 'rename') {
+                $output .= 'rename from ' . $data->ah_name;
             } else {
                 $output .= $data->ah_action;
             }
diff --git a/perl/lib/Wallet/Schema.pm b/perl/lib/Wallet/Schema.pm
index 74b4c99..cb4c93e 100644
--- a/perl/lib/Wallet/Schema.pm
+++ b/perl/lib/Wallet/Schema.pm
@@ -1,7 +1,7 @@
 # Database schema and connector for the wallet system.
 #
 # Written by Jon Robertson <jonrober@stanford.edu>
-# Copyright 2012, 2013
+# Copyright 2012, 2013, 2014
 #     The Board of Trustees of the Leland Stanford Junior University
 #
 # See LICENSE for licensing terms.
@@ -160,6 +160,7 @@ table.
   create table acl_history
      (ah_id               integer auto_increment primary key,
       ah_acl              integer not null,
+      ah_name             varchar(255) default null,
       ah_action           varchar(16) not null,
       ah_scheme           varchar(32) default null,
       ah_identifier       varchar(255) default null,
@@ -168,14 +169,13 @@ table.
       ah_on               datetime not null);
   create index ah_acl on acl_history (ah_acl);
 
-ah_action must be one of C<create>, C<destroy>, C<add>, or C<remove>
-(enums aren't used for compatibility with databases other than MySQL).
-For a change of type create or destroy, only the action and the trace
-records (by, from, and on) are stored.  For a change to the lines of an
-ACL, the scheme and identifier of the line that was added or removed is
-included.  Note that changes to the ACL name are not recorded; ACLs are
-always tracked by system-generated ID, so name changes are purely
-cosmetic.
+ah_action must be one of C<create>, C<destroy>, C<add>, C<remove>, or
+C<rename> (enums aren't used for compatibility with databases other than
+MySQL).  For a change of type create, destroy, or rename, only the action,
+the ACL name (in the case of rename, the old ACL name prior to the
+rename), and the trace records (by, from, and on) are stored.  For a
+change to the lines of an ACL, the scheme and identifier of the line that
+was added or removed are included.
 
 ah_by stores the authenticated identity that made the change, ah_from
 stores the host from which they made the change, and ah_on stores the time
diff --git a/perl/lib/Wallet/Server.pm b/perl/lib/Wallet/Server.pm
index 2765d34..e278489 100644
--- a/perl/lib/Wallet/Server.pm
+++ b/perl/lib/Wallet/Server.pm
@@ -681,7 +681,7 @@ sub acl_rename {
             return;
         }
     }
-    unless ($acl->rename ($name)) {
+    unless ($acl->rename ($name, $self->{user}, $self->{host})) {
         $self->error ($acl->error);
         return;
     }
diff --git a/perl/t/general/acl.t b/perl/t/general/acl.t
index 01b4801..1dd5c53 100755
--- a/perl/t/general/acl.t
+++ b/perl/t/general/acl.t
@@ -63,7 +63,7 @@ ok ($acl->isa ('Wallet::ACL'), ' and the right class');
 is ($acl->name, 'test', ' and the right name');
 
 # Test rename.
-if ($acl->rename ('example')) {
+if ($acl->rename ('example', @trace)) {
     ok (1, 'Renaming the ACL');
 } else {
     is ($acl->error, '', 'Renaming the ACL');
@@ -83,7 +83,8 @@ ok (defined ($acl), ' and it can still found by ID');
 is ($@, '', ' with no exceptions');
 is ($acl->name, 'example', ' and the right name');
 is ($acl->id, 2, ' and the right ID');
-ok (! $acl->rename ('ADMIN'), ' but renaming to an existing name fails');
+ok (! $acl->rename ('ADMIN', @trace),
+    ' but renaming to an existing name fails');
 like ($acl->error, qr/^cannot rename ACL 2 to ADMIN: /,
       ' with the right error');
 
@@ -195,6 +196,8 @@ my $date = strftime ('%Y-%m-%d %H:%M:%S', localtime $trace[2]);
 my $history = <<"EOO";
 $date  create
     by $admin from $host
+$date  rename from test
+    by $admin from $host
 $date  add krb5 $user1
     by $admin from $host
 $date  add krb5 $user2