From 45d8382fb2cc3f32b176675c0edb7eef5ca6aa21 Mon Sep 17 00:00:00 2001
From: Russ Allbery <rra@stanford.edu>
Date: Sat, 1 Dec 2007 01:55:18 +0000
Subject: Expiration dates are now expressed in YYYY-MM-DD HH:MM:SS instead of
 seconds since epoch and returned the same way.  Timestamps are now stored in
 the database as correct date and time types rather than seconds since epoch
 to work properly with MySQL.

---
 NEWS                       |  5 +++++
 client/wallet.pod          |  8 ++++----
 perl/Wallet/ACL.pm         |  9 +++++----
 perl/Wallet/Object/Base.pm | 35 ++++++++++++++++++++---------------
 perl/Wallet/Server.pm      |  9 +++++----
 perl/t/keytab.t            | 12 ++++++------
 perl/t/object.t            |  8 ++++----
 perl/t/server.t            | 19 ++++++++++---------
 server/wallet-backend      | 14 +++++++-------
 tests/server/backend-t.in  |  4 ++--
 10 files changed, 68 insertions(+), 55 deletions(-)

diff --git a/NEWS b/NEWS
index 6be0444..ee51205 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,11 @@ wallet 0.3 (unreleased)
     The keytab backend now supports limiting generated keytabs to
     particular enctypes by setting an attribute on the object.
 
+    Expiration dates are now expressed in YYYY-MM-DD HH:MM:SS instead of
+    seconds since epoch and returned the same way.  Timestamps are now
+    stored in the database as correct date and time types rather than
+    seconds since epoch to work properly with MySQL.
+
     The wallet backend test suite now supports using a database other than
     SQLite for testing.
 
diff --git a/client/wallet.pod b/client/wallet.pod
index 779e691..af7042d 100644
--- a/client/wallet.pod
+++ b/client/wallet.pod
@@ -195,10 +195,10 @@ If <expires> is not given, displays the current expiration of the object
 identified by <type> and <name>, or C<No expiration set> if none is set.
 The expiration will be displayed in seconds since epoch.
 
-If <expires> is given, sets the expiration on the object identified by
-<type> and <name> to <expires>.  <expires> should be given in seconds
-since epoch.  If <expires> is the empty string, clears the expiration of
-the object.
+If <date> is given, sets the expiration on the object identified by <type>
+and <name> to <date> and (if given) <time>.  <date> must be in the format
+C<YYYY-MM-DD> and <time> in the format C<HH:MM:SS>.  If <date> is the empty
+string, clears the expiration of the object.
 
 Currently, the expiration of an object is not used.
 
diff --git a/perl/Wallet/ACL.pm b/perl/Wallet/ACL.pm
index f04217e..bc318a1 100644
--- a/perl/Wallet/ACL.pm
+++ b/perl/Wallet/ACL.pm
@@ -87,9 +87,10 @@ sub create {
         $dbh->do ($sql, undef, $name);
         $id = $dbh->last_insert_id (undef, undef, 'acls', 'ac_id');
         die "unable to retrieve new ACL ID" unless defined $id;
+        my $date = strftime ('%Y-%m-%d %T', localtime $time);
         $sql = "insert into acl_history (ah_acl, ah_action, ah_by, ah_from,
             ah_on) values (?, 'create', ?, ?, ?)";
-        $dbh->do ($sql, undef, $id, $user, $host, $time);
+        $dbh->do ($sql, undef, $id, $user, $host, $date);
         $dbh->commit;
     };
     if ($@) {
@@ -143,10 +144,11 @@ sub log_acl {
     unless ($action =~ /^(add|remove)\z/) {
         die "invalid history action $action";
     }
+    my $date = strftime ('%Y-%m-%d %T', localtime $time);
     my $sql = 'insert into acl_history (ah_acl, ah_action, ah_scheme,
         ah_identifier, ah_by, ah_from, ah_on) values (?, ?, ?, ?, ?, ?, ?)';
     $self->{dbh}->do ($sql, undef, $self->{id}, $action, $scheme, $identifier,
-                      $user, $host, $time);
+                      $user, $host, $date);
 }
 
 ##############################################################################
@@ -317,8 +319,7 @@ sub history {
         $sth->execute ($self->{id});
         my @data;
         while (@data = $sth->fetchrow_array) {
-            my $time = strftime ('%Y-%m-%d %H:%M:%S', localtime $data[5]);
-            $output .= "$time  ";
+            $output .= "$data[5]  ";
             if ($data[0] eq 'add' or $data[0] eq 'remove') {
                 $output .= "$data[0] $data[1] $data[2]";
             } else {
diff --git a/perl/Wallet/Object/Base.pm b/perl/Wallet/Object/Base.pm
index 1371f7f..2fe6ed9 100644
--- a/perl/Wallet/Object/Base.pm
+++ b/perl/Wallet/Object/Base.pm
@@ -65,12 +65,13 @@ sub create {
     die "invalid object type\n" unless $type;
     die "invalid object name\n" unless $name;
     eval {
+        my $date = strftime ('%Y-%m-%d %T', localtime $time);
         my $sql = 'insert into objects (ob_type, ob_name, ob_created_by,
             ob_created_from, ob_created_on) values (?, ?, ?, ?, ?)';
-        $dbh->do ($sql, undef, $type, $name, $user, $host, $time);
+        $dbh->do ($sql, undef, $type, $name, $user, $host, $date);
         $sql = "insert into object_history (oh_type, oh_name, oh_action,
             oh_by, oh_from, oh_on) values (?, ?, 'create', ?, ?, ?)";
-        $dbh->do ($sql, undef, $type, $name, $user, $host, $time);
+        $dbh->do ($sql, undef, $type, $name, $user, $host, $date);
         $dbh->commit;
     };
     if ($@) {
@@ -131,20 +132,21 @@ sub log_action {
     # the object record itself.  Commit both changes as a transaction.  We
     # assume that AutoCommit is turned off.
     eval {
+        my $date = strftime ('%Y-%m-%d %T', localtime $time);
         my $sql = 'insert into object_history (oh_type, oh_name, oh_action,
             oh_by, oh_from, oh_on) values (?, ?, ?, ?, ?, ?)';
         $self->{dbh}->do ($sql, undef, $self->{type}, $self->{name}, $action,
-                          $user, $host, $time);
+                          $user, $host, $date);
         if ($action eq 'get') {
             $sql = 'update objects set ob_downloaded_by = ?,
                 ob_downloaded_from = ?, ob_downloaded_on = ? where
                 ob_type = ? and ob_name = ?';
-            $self->{dbh}->do ($sql, undef, $user, $host, $time, $self->{type},
+            $self->{dbh}->do ($sql, undef, $user, $host, $date, $self->{type},
                               $self->{name});
         } elsif ($action eq 'store') {
             $sql = 'update objects set ob_stored_by = ?, ob_stored_from = ?,
                 ob_stored_on = ? where ob_type = ? and ob_name = ?';
-            $self->{dbh}->do ($sql, undef, $user, $host, $time, $self->{type},
+            $self->{dbh}->do ($sql, undef, $user, $host, $date, $self->{type},
                               $self->{name});
         }
         $self->{dbh}->commit;
@@ -178,11 +180,12 @@ sub log_set {
     unless ($fields{$field}) {
         die "invalid history field $field";
     }
+    my $date = strftime ('%Y-%m-%d %T', localtime $time);
     my $sql = "insert into object_history (oh_type, oh_name, oh_action,
         oh_field, oh_type_field, oh_old, oh_new, oh_by, oh_from, oh_on)
         values (?, ?, 'set', ?, ?, ?, ?, ?, ?, ?)";
     $self->{dbh}->do ($sql, undef, $self->{type}, $self->{name}, $field,
-                      $type_field, $old, $new, $user, $host, $time);
+                      $type_field, $old, $new, $user, $host, $date);
 }
 
 ##############################################################################
@@ -301,7 +304,7 @@ sub attr_show {
 sub expires {
     my ($self, $expires, $user, $host, $time) = @_;
     if ($expires) {
-        if ($expires !~ /^\d+\z/ || $expires == 0) {
+        if ($expires !~ /^\d{4}-\d\d-\d\d( \d\d:\d\d:\d\d)?\z/) {
             $self->error ("malformed expiration time $expires");
             return;
         }
@@ -465,8 +468,7 @@ sub history {
         $sth->execute ($self->{type}, $self->{name});
         my @data;
         while (@data = $sth->fetchrow_array) {
-            my $time = strftime ('%Y-%m-%d %H:%M:%S', localtime $data[7]);
-            $output .= "$time  ";
+            $output .= "$data[7]  ";
             my ($old, $new) = @data[3..4];
             if ($data[0] eq 'set' and $data[1] eq 'flags') {
                 if (defined ($data[4])) {
@@ -619,13 +621,14 @@ sub destroy {
         return;
     }
     eval {
+        my $date = strftime ('%Y-%m-%d %T', localtime $time);
         my $sql = 'delete from flags where fl_type = ? and fl_name = ?';
         $self->{dbh}->do ($sql, undef, $type, $name);
         $sql = 'delete from objects where ob_type = ? and ob_name = ?';
         $self->{dbh}->do ($sql, undef, $type, $name);
         $sql = "insert into object_history (oh_type, oh_name, oh_action,
             oh_by, oh_from, oh_on) values (?, ?, 'destroy', ?, ?, ?)";
-        $self->{dbh}->do ($sql, undef, $type, $name, $user, $host, $time);
+        $self->{dbh}->do ($sql, undef, $type, $name, $user, $host, $date);
         $self->{dbh}->commit;
     };
     if ($@) {
@@ -779,11 +782,13 @@ string.
 
 Sets or retrieves the expiration date of an object.  If no arguments are
 given, returns the current expiration or undef if no expiration is set.  If
-arguments are given, change the expiration to EXPIRES, which should be in
-seconds since epoch, and return true on success and false on failure.  Pass
-in the empty string for EXPIRES to clear the expiration date.  The other
-arguments are used for logging and history and should indicate the user and
-host from which the change is made and the time of the change.
+arguments are given, change the expiration to EXPIRES and return true on
+success and false on failure.  EXPIRES must be in the format C<YYYY-MM-DD
+HH:MM:SS>, although the time portion may be omitted.  Pass in the empty
+string for EXPIRES to clear the expiration date.
+
+The other arguments are used for logging and history and should indicate the
+user and host from which the change is made and the time of the change.
 
 =item flag_check(FLAG)
 
diff --git a/perl/Wallet/Server.pm b/perl/Wallet/Server.pm
index 04e8fd9..c119ad4 100644
--- a/perl/Wallet/Server.pm
+++ b/perl/Wallet/Server.pm
@@ -920,10 +920,11 @@ isn't set and a failure to retrieve the expiration, the caller should call
 error() after an undef return.  If error() also returns undef, that ACL
 wasn't set; otherwise, error() will return the error message.
 
-If EXPIRES is given, sets the expiration to EXPIRES, which should be in
-seconds since epoch.  To set an expiration, the current user must be
-authorized by the ADMIN ACL.  Returns true for success and false for
-failure.
+If EXPIRES is given, sets the expiration to EXPIRES.  EXPIRES must be in
+the format C<YYYY-MM-DD +HH:MM:SS>, although the time portion may be
+omitted.  Pass in the empty +string for EXPIRES to clear the expiration
+date.  To set an expiration, the current user must be authorized by the
+ADMIN ACL.  Returns true for success and false for failure.
 
 =item flag_clear(TYPE, NAME, FLAG)
 
diff --git a/perl/t/keytab.t b/perl/t/keytab.t
index fbd66f6..48abd57 100755
--- a/perl/t/keytab.t
+++ b/perl/t/keytab.t
@@ -278,10 +278,10 @@ SKIP: {
            Name: wallet/one
      Created by: $user
    Created from: $host
-     Created on: $trace[2]
+     Created on: $date
   Downloaded by: $user
 Downloaded from: $host
-  Downloaded on: $trace[2]
+  Downloaded on: $date
 EOO
     is ($object->show, $expected, 'Show output is correct');
 
@@ -518,7 +518,7 @@ SKIP: {
            Name: wallet/one
      Created by: $user
    Created from: $host
-     Created on: $trace[2]
+     Created on: $date
 EOO
     is ($one->show, $expected, 'Show output displays no attributes');
     is ($one->attr ('foo', [ 'bar' ], @trace), undef,
@@ -547,7 +547,7 @@ EOO
     Synced with: kaserver
      Created by: $user
    Created from: $host
-     Created on: $trace[2]
+     Created on: $date
 EOO
     is ($one->show, $expected, ' and show now displays the attribute');
     $history .= <<"EOO";
@@ -731,10 +731,10 @@ EOO
        Enctypes: $eshow
      Created by: $user
    Created from: $host
-     Created on: $trace[2]
+     Created on: $date
   Downloaded by: $user
 Downloaded from: $host
-  Downloaded on: $trace[2]
+  Downloaded on: $date
 EOO
     is ($one->show, $expected, ' and show now displays the enctype list');
     $keytab = $one->get (@trace);
diff --git a/perl/t/object.t b/perl/t/object.t
index 3591885..101110a 100755
--- a/perl/t/object.t
+++ b/perl/t/object.t
@@ -81,7 +81,7 @@ is ($object->owner ('ADMIN', @trace), 1, ' and setting it again works');
 
 # Expires.
 is ($object->expires, undef, 'Expires is not set to start');
-my $now = time;
+my $now = strftime ('%Y-%m-%d %T', localtime time);
 if ($object->expires ($now, @trace)) {
     ok (1, ' and setting it works');
 } else {
@@ -193,6 +193,7 @@ is ($object->error, "cannot store keytab:$princ: object type is immutable",
     ' with the right error');
 
 # Test show.
+my $date = strftime ('%Y-%m-%d %H:%M:%S', localtime $trace[2]);
 my $output = <<"EOO";
            Type: keytab
            Name: $princ
@@ -206,7 +207,7 @@ my $output = <<"EOO";
           Flags: unchanging
      Created by: $user
    Created from: $host
-     Created on: $trace[2]
+     Created on: $date
 
 Members of ACL ADMIN (id: 1) are:
   krb5 $user
@@ -226,7 +227,7 @@ $output = <<"EOO";
           Flags: locked unchanging
      Created by: $user
    Created from: $host
-     Created on: $trace[2]
+     Created on: $date
 
 Members of ACL ADMIN (id: 1) are:
   krb5 $user
@@ -252,7 +253,6 @@ $object = eval {
     Wallet::Object::Base->create ('keytab', $princ, $dbh, @trace)
   };
 ok (defined ($object), 'Recreating the object succeeds');
-my $date = strftime ('%Y-%m-%d %H:%M:%S', localtime $trace[2]);
 $output = <<"EOO";
 $date  create
     by $user from $host
diff --git a/perl/t/server.t b/perl/t/server.t
index f7826b6..229d58d 100755
--- a/perl/t/server.t
+++ b/perl/t/server.t
@@ -10,6 +10,7 @@
 
 use Test::More tests => 321;
 
+use POSIX qw(strftime);
 use Wallet::Config;
 use Wallet::Server;
 
@@ -193,7 +194,7 @@ is ($server->destroy ('base', 'service/test'), undef, ' but not twice');
 is ($server->error, 'cannot find base:service/test', ' with the right error');
 
 # Test manipulating expires.
-my $now = time;
+my $now = strftime ('%Y-%m-%d %T', localtime time);
 is ($server->expires ('base', 'service/test'), undef,
     'Retrieving expires on an unknown object fails');
 is ($server->error, 'cannot find base:service/test', ' with the right error');
@@ -225,7 +226,7 @@ is ($server->show ('base', 'service/test'), undef,
     'Cannot show nonexistent object');
 is ($server->error, 'cannot find base:service/test', ' with the right error');
 my $show = $server->show ('base', 'service/admin');
-$show =~ s/(Created on:) \d+$/$1 0/;
+$show =~ s/(Created on:) [\d-]+ [\d:]+$/$1 0/;
 my $expected = <<"EOO";
            Type: base
            Name: service/admin
@@ -504,7 +505,7 @@ is ($server->error,
     "cannot store base:service/user1: object type is immutable",
     ' and the method is called');
 $show = $server->show ('base', 'service/user1');
-$show =~ s/(Created on:) \d+$/$1 0/m;
+$show =~ s/(Created on:) [\d-]+ [\d:]+$/$1 0/m;
 $expected = <<"EOO";
            Type: base
            Name: service/user1
@@ -574,7 +575,7 @@ is ($server->flag_set ('base', 'service/both', 'unchanging'), 1,
     ' and set flags on an object we have an ACL');
 is ($server->flag_set ('base', 'service/both', 'locked'), 1, ' both flags');
 $show = $server->show ('base', 'service/both');
-$show =~ s/(Created on:) \d+$/$1 0/m;
+$show =~ s/(Created on:) [\d-]+ [\d:]+$/$1 0/m;
 $expected = <<"EOO";
            Type: base
            Name: service/both
@@ -656,7 +657,7 @@ is ($server->error,
     "cannot store base:service/user2: object type is immutable",
     ' and the method is called');
 $show = $server->show ('base', 'service/user2');
-$show =~ s/(Created on:) \d+$/$1 0/m;
+$show =~ s/(Created on:) [\d-]+ [\d:]+$/$1 0/m;
 $expected = <<"EOO";
            Type: base
            Name: service/user2
@@ -779,7 +780,7 @@ is ($server->error, "$user2 not authorized to create base:service/foo",
     ' with the right error');
 $show = $server->show ('base', 'service/default');
 if (defined $show) {
-    $show =~ s/(Created on:) \d+$/$1 0/m;
+    $show =~ s/(Created on:) [\d-]+ [\d:]+$/$1 0/m;
     $expected = <<"EOO";
            Type: base
            Name: service/default
@@ -805,7 +806,7 @@ is ($server->error, "ACL both exists and doesn't match default",
 is ($server->create ('base', 'service/default-2'), 1,
     'Creating an object with an existing ACL works');
 $show = $server->show ('base', 'service/default-2');
-$show =~ s/(Created on:) \d+$/$1 0/m;
+$show =~ s/(Created on:) [\d-]+ [\d:]+$/$1 0/m;
 $expected = <<"EOO";
            Type: base
            Name: service/default-2
@@ -824,7 +825,7 @@ $result = eval { $server->get ('base', 'service/default-get') };
 is ($result, undef, 'Auto-creation on get...');
 is ($@, "Do not instantiate Wallet::Object::Base directly\n", ' ...works');
 $show = $server->show ('base', 'service/default-get');
-$show =~ s/(Created on:) \d+$/$1 0/m;
+$show =~ s/(Created on:) [\d-]+ [\d:]+$/$1 0/m;
 $expected = <<"EOO";
            Type: base
            Name: service/default-get
@@ -847,7 +848,7 @@ is ($server->error,
     "cannot store base:service/default-store: object type is immutable",
     ' ...works');
 $show = $server->show ('base', 'service/default-store');
-$show =~ s/(Created on:) \d+$/$1 0/m;
+$show =~ s/(Created on:) [\d-]+ [\d:]+$/$1 0/m;
 $expected = <<"EOO";
            Type: base
            Name: service/default-store
diff --git a/server/wallet-backend b/server/wallet-backend
index 1e067d1..c2be5e7 100755
--- a/server/wallet-backend
+++ b/server/wallet-backend
@@ -186,7 +186,7 @@ sub command {
         check_args (2, 2, [], @args);
         $server->destroy (@args) or failure ($server->error, @_);
     } elsif ($command eq 'expires') {
-        check_args (2, 3, [], @args);
+        check_args (2, 4, [], @args);
         if (@args > 2) {
             $server->expires (@args) or failure ($server->error, @_);
         } else {
@@ -397,16 +397,16 @@ object will be usable.
 Destroy the object identified by <type> and <name>.  With some backends,
 this will trigger destruction of an object in an external system as well.
 
-=item expires <type> <name> [<expires>]
+=item expires <type> <name> [<date> [<time>]]
 
-If <expires> is not given, displays the current expiration of the object
+If <date> is not given, displays the current expiration of the object
 identified by <type> and <name>, or C<No expiration set> if none is set.
 The expiration will be displayed in seconds since epoch.
 
-If <expires> is given, sets the expiration on the object identified by
-<type> and <name> to <expires>.  <expires> should be given in seconds
-since epoch.  If <expires> is the empty string, clears the expiration of
-the object.
+If <date> is given, sets the expiration on the object identified by <type>
+and <name> to <date> and (if given) <time>.  <date> must be in the format
+C<YYYY-MM-DD> and <time> in the format C<HH:MM:SS>.  If <date> is the empty
+string, clears the expiration of the object.
 
 Currently, the expiration of an object is not used.
 
diff --git a/tests/server/backend-t.in b/tests/server/backend-t.in
index 44a4a48..8509177 100644
--- a/tests/server/backend-t.in
+++ b/tests/server/backend-t.in
@@ -9,7 +9,7 @@
 
 use strict;
 use IO::String;
-use Test::More tests => 1219;
+use Test::More tests => 1222;
 
 # Create a dummy class for Wallet::Server that prints what method was called
 # with its arguments and returns data for testing.
@@ -195,7 +195,7 @@ is ($out, "$new\n", ' and nothing ran');
 # Check too few, too many, and bad arguments for every command.
 my %commands = (create  => [2, 2],
                 destroy => [2, 2],
-                expires => [2, 3],
+                expires => [2, 4],
                 get     => [2, 2],
                 getacl  => [3, 3],
                 getattr => [3, 3],
-- 
cgit v1.2.3