Pass object type and name to external ACL verifiers

This requires changing the ACL verifier plumbing to pass object
type and name all the way through when verifying ACLs.  Hopefully
I caught everything.

9 files changed, 57 insertions, 44 deletions

diff --git a/NEWS b/NEWS
index eee61bd..aa9cf47 100644
--- a/NEWS
+++ b/NEWS
@@ -11,9 +11,9 @@ wallet 1.3 (unreleased)
 
     A new ACL type, external (Wallet::ACL::External), is now supported.
     This ACL runs an external command to check if access is allowed, and
-    passes the principal and the ACL identifier to that command.  To
-    enable this ACL type for an existing wallet database, use wallet-admin
-    to register the new verifier.
+    passes the principal, type and name of the object, and the ACL
+    identifier to that command.  To enable this ACL type for an existing
+    wallet database, use wallet-admin to register the new verifier.
 
     A new variation on the ldap-attr ACL type, ldap-attr-root
     (Wallet::ACL::LDAP::Attribute::Root), is now supported.  This is
diff --git a/docs/design-acl b/docs/design-acl
index b8bb8b3..836c411 100644
--- a/docs/design-acl
+++ b/docs/design-acl
@@ -31,9 +31,10 @@ Semantics
     used:  Iterate through each ACL entry in the ACL in question.  If the
     ACL entry is malformatted or the scheme is not recognized, skip it.
     Otherwise, dispatch the question to the check function of the ACL
-    implementation, passing it the principal identifying the client and
-    the <identifier> portion of the ACL entry.  This function returns
-    either authorized or unauthorized.  If authorized, end the search; if
+    implementation, passing it the principal identifying the client, the
+    <identifier> portion of the ACL entry, and the type and name of the
+    object the user is attempting to access.  This function returns either
+    authorized or unauthorized.  If authorized, end the search; if
     unauthorized, continue to the next ACL entry.
 
     There is no support in this scheme for negative ACLs.
diff --git a/perl/lib/Wallet/ACL.pm b/perl/lib/Wallet/ACL.pm
index 862b88f..69e6890 100644
--- a/perl/lib/Wallet/ACL.pm
+++ b/perl/lib/Wallet/ACL.pm
@@ -480,7 +480,7 @@ sub history {
 {
     my %verifier;
     sub check_line {
-        my ($self, $principal, $scheme, $identifier) = @_;
+        my ($self, $principal, $scheme, $identifier, $type, $name) = @_;
         unless ($verifier{$scheme}) {
             my $class = $self->scheme_mapping ($scheme);
             unless ($class) {
@@ -493,7 +493,8 @@ sub history {
                 return;
             }
         }
-        my $result = ($verifier{$scheme})->check ($principal, $identifier);
+        my $result = ($verifier{$scheme})->check ($principal, $identifier,
+                                                  $type, $name);
         if (not defined $result) {
             push (@{ $self->{check_errors} }, ($verifier{$scheme})->error);
             return;
@@ -503,13 +504,13 @@ sub history {
     }
 }
 
-# Given a principal, check whether it should be granted access according to
-# this ACL.  Returns 1 if access was granted, 0 if access was denied, and
-# undef on some error.  Errors from ACL verifiers do not cause an error
-# return, but are instead accumulated in the check_errors variable returned by
-# the check_errors() method.
+# Given a principal, object type, and object name, check whether that
+# principal should be granted access according to this ACL.  Returns 1 if
+# access was granted, 0 if access was denied, and undef on some error.  Errors
+# from ACL verifiers do not cause an error return, but are instead accumulated
+# in the check_errors variable returned by the check_errors() method.
 sub check {
-    my ($self, $principal) = @_;
+    my ($self, $principal, $type, $name) = @_;
     unless ($principal) {
         $self->error ('no principal specified');
         return;
@@ -520,7 +521,8 @@ sub check {
     $self->{check_errors} = [];
     for my $entry (@entries) {
         my ($scheme, $identifier) = @$entry;
-        my $result = $self->check_line ($principal, $scheme, $identifier);
+        my $result = $self->check_line ($principal, $scheme, $identifier,
+                                        $type, $name);
         return 1 if $result;
     }
     return 0;
diff --git a/perl/lib/Wallet/ACL/Base.pm b/perl/lib/Wallet/ACL/Base.pm
index 19ca612..3778c07 100644
--- a/perl/lib/Wallet/ACL/Base.pm
+++ b/perl/lib/Wallet/ACL/Base.pm
@@ -103,10 +103,12 @@ This method should be overridden by any child classes that want to
 implement validating the name of an ACL before creation.  The default
 implementation allows any name for an ACL.
 
-=item check(PRINCIPAL, ACL)
+=item check(PRINCIPAL, ACL, TYPE, NAME)
 
 This method should always be overridden by child classes.  The default
-implementation just declines all access.
+implementation just declines all access.  TYPE and NAME are the type and
+name of the object being accessed, which may be used by some ACL schemes
+or may be ignored.
 
 =item error([ERROR ...])
 
diff --git a/perl/lib/Wallet/ACL/External.pm b/perl/lib/Wallet/ACL/External.pm
index 77c2499..f1bd577 100644
--- a/perl/lib/Wallet/ACL/External.pm
+++ b/perl/lib/Wallet/ACL/External.pm
@@ -46,13 +46,12 @@ sub new {
 # The most trivial ACL verifier.  Returns true if the provided principal
 # matches the ACL.
 sub check {
-    my ($self, $principal, $acl) = @_;
+    my ($self, $principal, $acl, $type, $name) = @_;
     unless ($principal) {
         $self->error ('no principal specified');
         return;
     }
-    my @args = split (' ', $acl);
-    unshift @args, $principal;
+    my @args = ($principal, $type, $name, $acl);
     my $pid = open (EXTERNAL, '-|');
     if (not defined $pid) {
         $self->error ("cannot fork: $!");
@@ -134,14 +133,15 @@ an error.
 Creates a new ACL verifier.  For this verifier, this just confirms that
 the wallet configuration sets an external command.
 
-=item check(PRINCIPAL, ACL)
+=item check(PRINCIPAL, ACL, TYPE, NAME)
 
 Returns true if the external command returns success when run with that
-PRINCIPAL and ACL.  ACL will be split on whitespace and passed as multiple
-arguments.  So, for example, the ACL C<external mdbset shell> will, when
-triggered by a request from rra@EXAMPLE.COM, result in the command:
+PRINCIPAL, object TYPE and NAME, and ACL.  So, for example, the ACL C<external
+mdbset shell> will, when triggered by a request from rra@EXAMPLE.COM for the
+object C<file password>, result in the command:
 
-    $Wallet::Config::EXTERNAL_COMMAND rra@EXAMPLE.COM mdbset shell
+    $Wallet::Config::EXTERNAL_COMMAND rra@EXAMPLE.COM file password \
+        'mdbset shell'
 
 =item error()
 
diff --git a/perl/lib/Wallet/ACL/Nested.pm b/perl/lib/Wallet/ACL/Nested.pm
index 07833f8..3b6c827 100644
--- a/perl/lib/Wallet/ACL/Nested.pm
+++ b/perl/lib/Wallet/ACL/Nested.pm
@@ -59,7 +59,7 @@ sub syntax_check {
 # that entry.  We also want to keep track of things already checked in order
 # to avoid any loops.
 sub check {
-    my ($self, $principal, $group) = @_;
+    my ($self, $principal, $group, $type, $name) = @_;
     unless ($principal) {
         $self->error ('no principal specified');
         return;
@@ -78,8 +78,9 @@ sub check {
     # to go through each entry and decide if the given acl has access.
     my @members = $self->get_membership ($group);
     for my $entry (@members) {
-        my ($type, $name) = @{ $entry };
-        my $result = $acl->check_line ($principal, $type, $name);
+        my ($scheme, $identifier) = @{ $entry };
+        my $result = $acl->check_line ($principal, $scheme, $identifier,
+                                       $type, $name);
         return 1 if $result;
     }
     return 0;
diff --git a/perl/lib/Wallet/Config.pm b/perl/lib/Wallet/Config.pm
index 98b5dc9..e8bc00c 100644
--- a/perl/lib/Wallet/Config.pm
+++ b/perl/lib/Wallet/Config.pm
@@ -551,10 +551,10 @@ runs an external command to determine if access is granted.
 
 =item EXTERNAL_COMMAND
 
-Path to the command to run to determine whether access is granted.  The
-first argument to the command will be the principal requesting access.
-The identifier of the ACL will be split on whitespace and passed in as the
-remaining arguments to this command.
+Path to the command to run to determine whether access is granted.  The first
+argument to the command will be the principal requesting access.  The second
+and third arguments will be the type and name of the object that principal is
+requesting access to.  The final argument will be the identifier of the ACL.
 
 No other arguments are passed to the command, but the command will have
 access to all of the remctl environment variables seen by the wallet
diff --git a/perl/t/data/acl-command b/perl/t/data/acl-command
index e368118..b7c3066 100755
--- a/perl/t/data/acl-command
+++ b/perl/t/data/acl-command
@@ -18,26 +18,30 @@ if [ "$1" != 'eagle@eyrie.org' ]; then
     exit 1
 fi
 
-# Check that the second argument is test.
-if [ "$2" != 'test' ]; then
+# Check that the second and third arguments are file test (the test object).
+if [ "$2" != 'file' ]; then
     echo 'incorrect second argument' >&2
     exit 1
 fi
+if [ "$3" != 'test' ]; then
+    echo 'incorrect third argument' >&2
+    exit 1
+fi
 
-# Process the third argument.
-case $3 in
-    success)
+# Process the fourth argument.
+case $4 in
+    'test success')
         exit 0
         ;;
-    failure)
+    'test failure')
         exit 1
         ;;
-    error)
+    'test error')
         echo 'some error' >&2
         exit 1
         ;;
     *)
-        echo 'unknown third argument' >&2
+        echo 'unknown fourth argument' >&2
         exit 1
         ;;
 esac
diff --git a/perl/t/verifier/external.t b/perl/t/verifier/external.t
index 3e7e776..d1438de 100755
--- a/perl/t/verifier/external.t
+++ b/perl/t/verifier/external.t
@@ -22,11 +22,14 @@ $Wallet::Config::EXTERNAL_COMMAND = 't/data/acl-command';
 my $verifier = Wallet::ACL::External->new;
 ok (defined $verifier, 'Wallet::ACL::External creation');
 ok ($verifier->isa ('Wallet::ACL::External'), ' and class verification');
-is ($verifier->check ('eagle@eyrie.org', 'test success'), 1, 'Success');
-is ($verifier->check ('eagle@eyrie.org', 'test failure'), 0, 'Failure');
+is ($verifier->check ('eagle@eyrie.org', 'test success', 'file', 'test'),
+    1, 'Success');
+is ($verifier->check ('eagle@eyrie.org', 'test failure', 'file', 'test'),
+    0, 'Failure');
 is ($verifier->error, undef, 'No error set');
-is ($verifier->check ('eagle@eyrie.org', 'test error'), undef, 'Error');
+is ($verifier->check ('eagle@eyrie.org', 'test error', 'file', 'test'),
+    undef, 'Error');
 is ($verifier->error, 'some error', ' and right error');
-is ($verifier->check (undef, 'eagle@eyrie.org'), undef,
+is ($verifier->check (undef, 'eagle@eyrie.org', 'file', 'test'), undef,
     'Undefined principal');
 is ($verifier->error, 'no principal specified', ' and right error');