Return the name of the ACL instead of the numeric ID

The owner and getacl commands now return the current name of the ACL
instead of its numeric ID, matching the documentation of owner.

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

3 files changed, 27 insertions, 13 deletions

diff --git a/perl/lib/Wallet/Object/Base.pm b/perl/lib/Wallet/Object/Base.pm
index 4939bf5..a6a78bf 100644
--- a/perl/lib/Wallet/Object/Base.pm
+++ b/perl/lib/Wallet/Object/Base.pm
@@ -26,7 +26,7 @@ use Wallet::ACL;
 # This version should be increased on any code change to this module.  Always
 # use two digits for the minor version with a leading zero if necessary so
 # that it will sort properly.
-$VERSION = '0.07';
+$VERSION = '0.08';
 
 ##############################################################################
 # Constructors
@@ -302,7 +302,14 @@ sub acl {
     } elsif (defined $id) {
         return $self->_set_internal ($attr, undef, $user, $host, $time);
     } else {
-        return $self->_get_internal ($attr);
+        my $id = $self->_get_internal ($attr);
+        return unless defined $id;
+        my $acl = eval { Wallet::ACL->new ($id, $self->{schema}) };
+        if ($@) {
+            $self->error ($@);
+            return;
+        }
+        return $acl->name;
     }
 }
 
@@ -380,7 +387,14 @@ sub owner {
     } elsif (defined $owner) {
         return $self->_set_internal ('owner', undef, $user, $host, $time);
     } else {
-        return $self->_get_internal ('owner');
+        my $id = $self->_get_internal ('owner');
+        return unless defined $id;
+        my $acl = eval { Wallet::ACL->new ($id, $self->{schema}) };
+        if ($@) {
+            $self->error ($@);
+            return;
+        }
+        return $acl->name;
     }
 }
 
diff --git a/perl/lib/Wallet/Server.pm b/perl/lib/Wallet/Server.pm
index e278489..95fd4e6 100644
--- a/perl/lib/Wallet/Server.pm
+++ b/perl/lib/Wallet/Server.pm
@@ -831,7 +831,7 @@ failure to get the error message.
 Gets or sets the ACL type ACL to ID for the object identified by TYPE and
 NAME.  ACL should be one of C<get>, C<store>, C<show>, C<destroy>, or
 C<flags>.  If ID is not given, returns the current setting of that ACL as
-a numeric ACL ID or undef if that ACL isn't set or on failure.  To
+the name of the ACL or undef if that ACL isn't set or on failure.  To
 distinguish between an ACL that isn't set and a failure to retrieve the
 ACL, the caller should call error() after an undef return.  If error()
 also returns undef, that ACL wasn't set; otherwise, error() will return
@@ -1041,9 +1041,9 @@ owner ACL will not be checked.
 =item owner(TYPE, NAME [, OWNER])
 
 Gets or sets the owner for the object identified by TYPE and NAME.  If
-OWNER is not given, returns the current owner as a numeric ACL ID or undef
-if no owner is set or on an error.  To distinguish between an owner that
-isn't set and a failure to retrieve the owner, the caller should call
+OWNER is not given, returns the current owner as the name of the ACL or
+undef if no owner is set or on an error.  To distinguish between an owner
+that isn't set and a failure to retrieve the owner, 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.
 
diff --git a/perl/t/object/base.t b/perl/t/object/base.t
index 11f18b7..ee9ff4b 100755
--- a/perl/t/object/base.t
+++ b/perl/t/object/base.t
@@ -70,7 +70,7 @@ if ($object->owner ('ADMIN', @trace)) {
 } else {
     is ($object->error, '', ' and setting it to ADMIN works');
 }
-is ($object->owner, $acl->id, ' at which point it is ADMIN');
+is ($object->owner, $acl->name, ' at which point it is ADMIN');
 ok (! $object->owner ('unknown', @trace),
     ' but setting it to something bogus fails');
 is ($object->error, 'ACL unknown not found', ' with the right error');
@@ -128,7 +128,7 @@ for my $type (qw/get store show destroy flags/) {
     } else {
         is ($object->error, '', ' and setting it to ADMIN (numeric) works');
     }
-    is ($object->acl ($type), $acl->id, ' at which point it is ADMIN');
+    is ($object->acl ($type), $acl->name, ' at which point it is ADMIN');
     ok (! $object->acl ($type, 22, @trace),
         ' but setting it to something bogus fails');
     is ($object->error, 'ACL 22 not found', ' with the right error');
@@ -138,8 +138,8 @@ for my $type (qw/get store show destroy flags/) {
         is ($object->error, '', ' and clearing it works');
     }
     is ($object->acl ($type), undef, ' at which point it is cleared');
-    is ($object->acl ($type, $acl->id, @trace), 1,
-        ' and setting it again works');
+    is ($object->acl ($type, $acl->name, @trace), 1,
+        ' and setting it again by name works');
 }
 
 # Flags.
@@ -189,7 +189,7 @@ is ($object->error, "cannot store keytab:${princ}: object is locked",
 is ($object->owner ('', @trace), undef, ' and setting owner fails');
 is ($object->error, "cannot modify keytab:${princ}: object is locked",
     ' for the same reason');
-is ($object->owner, 1, ' but retrieving the owner works');
+is ($object->owner, 'ADMIN', ' but retrieving the owner works');
 is ($object->expires ('', @trace), undef, ' and setting expires fails');
 is ($object->error, "cannot modify keytab:${princ}: object is locked",
     ' for the same reason');
@@ -198,7 +198,7 @@ for my $acl (qw/get store show destroy flags/) {
     is ($object->acl ($acl, '', @trace), undef, " and setting $acl ACL fails");
     is ($object->error, "cannot modify keytab:${princ}: object is locked",
         ' for the same reason');
-    is ($object->acl ($acl), 1, " but retrieving $acl ACL works");
+    is ($object->acl ($acl), 'ADMIN', " but retrieving $acl ACL works");
 }
 is ($object->flag_check ('locked'), 1, ' and checking flags works');
 @flags = $object->flag_list;