4 files changed, 68 insertions, 60 deletions

diff --git a/perl/Wallet/Object/Base.pm b/perl/Wallet/Object/Base.pm
index 76bb799..59aee9f 100644
--- a/perl/Wallet/Object/Base.pm
+++ b/perl/Wallet/Object/Base.pm
@@ -34,12 +34,12 @@ $VERSION = '0.01';
 # type in the object.  If the object doesn't exist, returns undef.  This will
 # probably be usable as-is by most object types.
 sub new {
-    my ($class, $name, $type, $dbh) = shift;
+    my ($class, $type, $name, $dbh) = shift;
     $dbh->{AutoCommit} = 0;
     $dbh->{RaiseError} = 1;
     $dbh->{PrintError} = 0;
-    my $sql = 'select ob_name from objects where ob_name = ? and ob_type = ?';
-    my $data = $dbh->selectrow_array ($sql, undef, $name, $type);
+    my $sql = 'select ob_name from objects where ob_type = ? and ob_name = ?';
+    my $data = $dbh->selectrow_array ($sql, undef, $type, $name);
     die "cannot find ${type}:${name}\n" unless ($data and $data eq $name);
     my $self = {
         dbh  => $dbh,
@@ -55,18 +55,18 @@ sub new {
 # specified class.  Stores the database handle to use, the name, and the type
 # in the object.  Subclasses may need to override this to do additional setup.
 sub create {
-    my ($class, $name, $type, $dbh, $user, $host, $time) = @_;
+    my ($class, $type, $name, $dbh, $user, $host, $time) = @_;
     $dbh->{AutoCommit} = 0;
     $dbh->{RaiseError} = 1;
     $dbh->{PrintError} = 0;
     $time ||= time;
     eval {
-        my $sql = 'insert into objects (ob_name, ob_type, ob_created_by,
+        my $sql = 'insert into objects (ob_type, ob_name, ob_created_by,
             ob_created_from, ob_created_on) values (?, ?, ?, ?, ?)';
-        $dbh->do ($sql, undef, $name, $type, $user, $host, $time);
-        $sql = "insert into object_history (oh_object, oh_type, oh_action,
+        $dbh->do ($sql, undef, $type, $name, $user, $host, $time);
+        $sql = "insert into object_history (oh_type, oh_name, oh_action,
             oh_by, oh_from, oh_on) values (?, ?, 'create', ?, ?, ?)";
-        $dbh->do ($sql, undef, $name, $type, $user, $host, $time);
+        $dbh->do ($sql, undef, $type, $name, $user, $host, $time);
         $dbh->commit;
     };
     if ($@) {
@@ -183,16 +183,20 @@ sub log_set {
 # Returns undef on failure and the new value on success.
 sub _set_internal {
     my ($self, $attr, $value, $user, $host, $time) = @_;
+    if ($attr !~ /^[a-z_]+\z/) {
+        $self->{error} = "invalid attribute $attr";
+        return;
+    }
     $time ||= time;
     my $name = $self->{name};
     my $type = $self->{type};
     eval {
-        my $sql = "select ob_$attr from objects where ob_name = ? and
-            ob_type = ?";
-        my $old = $self->{dbh}->selectrow_array ($sql, undef, $name, $type);
-        $sql = "update objects set ob_$attr = ? where ob_name = ? and
-            ob_type = ?";
-        $self->{dbh}->do ($sql, undef, $value, $name, $type);
+        my $sql = "select ob_$attr from objects where ob_type = ? and
+            ob_name = ?";
+        my $old = $self->{dbh}->selectrow_array ($sql, undef, $type, $name);
+        $sql = "update objects set ob_$attr = ? where ob_type = ? and
+            ob_name = ?";
+        $self->{dbh}->do ($sql, undef, $value, $type, $name);
         $self->log_set ($attr, $old, $value, $user, $host, $time);
         $self->{dbh}->commit;
     };
@@ -208,10 +212,14 @@ sub _set_internal {
 # Get a particular attribute.  Returns the attribute value.
 sub _get_internal {
     my ($self, $attr) = @_;
+    if ($attr !~ /^[a-z_]+\z/) {
+        $self->{error} = "invalid attribute $attr";
+        return;
+    }
     my $name = $self->{name};
     my $type = $self->{type};
-    my $sql = "select $attr from objects where ob_name = ? and ob_type = ?";
-    my $value = $self->{dbh}->selectrow_array ($sql, undef, $name, $type);
+    my $sql = "select $attr from objects where ob_type = ? and ob_name = ?";
+    my $value = $self->{dbh}->selectrow_array ($sql, undef, $type, $name);
     return $value;
 }
 
@@ -312,9 +320,9 @@ sub show {
     my $fields = join (', ', map { $_->[0] } @attrs);
     my @data;
     eval {
-        my $sql = "select $fields from objects where ob_name = ? and
-            ob_type = ?";
-        @data = $self->{dbh}->selectrow_array ($sql, undef, $name, $type);
+        my $sql = "select $fields from objects where ob_type = ? and
+            ob_name = ?";
+        @data = $self->{dbh}->selectrow_array ($sql, undef, $type, $name);
     };
     if ($@) {
         $self->{error} = "cannot retrieve data for ${type}:${name}: $@";
@@ -343,13 +351,13 @@ sub destroy {
     my $name = $self->{name};
     my $type = $self->{type};
     eval {
-        my $sql = 'delete from flags where fl_object = ? and fl_type = ?';
-        $self->{dbh}->do ($sql, undef, $name, $type);
-        $sql = 'delete from objects where ob_name = ? and ob_type = ?';
-        $self->{dbh}->do ($sql, undef, $name, $type);
-        $sql = "insert into object_history (oh_object, oh_type, oh_action,
+        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, $name, $type, $user, $host, $time);
+        $self->{dbh}->do ($sql, undef, $type, $name, $user, $host, $time);
         $self->{dbh}->commit;
     };
     if ($@) {
@@ -398,11 +406,11 @@ the Wallet::Object::Type->new syntax).
 
 =over 4
 
-=item new(NAME, TYPE, DBH)
+=item new(TYPE, NAME, DBH)
 
-Creates a new object with the given object name and type, based on data
+Creates a new object with the given object type and name, based on data
 already in the database.  This method will only succeed if an object of the
-given NAME and TYPE is already present in the wallet database.  If no such
+given TYPE and NAME is already present in the wallet database.  If no such
 object exits, throws an exception.  Otherwise, returns an object blessed
 into the class used for the new() call (so subclasses can leave this method
 alone and not override it).
@@ -412,12 +420,12 @@ further operations.  This database handle is taken over by the wallet system
 and its settings (such as RaiseError and AutoCommit) will be modified by the
 object for its own needs.
 
-=item create(NAME, TYPE, DBH, PRINCIPAL, HOSTNAME [, DATETIME])
+=item create(TYPE, NAME, DBH, PRINCIPAL, HOSTNAME [, DATETIME])
 
 Similar to new() but instead creates a new entry in the database.  This
-method will throw an exception if an entry for that name and type already
+method will throw an exception if an entry for that type and name already
 exists in the database or if creating the database record fails.  Otherwise,
-a new database entry will be created with that name and type, no owner, no
+a new database entry will be created with that type and name, no owner, no
 ACLs, no expiration, no flags, and with created by, from, and on set to the
 PRINCIPAL, HOSTNAME, and DATETIME parameters.  If DATETIME isn't given, the
 current time is used.  The database handle is treated as with new().
diff --git a/perl/Wallet/Object/Keytab.pm b/perl/Wallet/Object/Keytab.pm
index e4cb00c..f83949c 100644
--- a/perl/Wallet/Object/Keytab.pm
+++ b/perl/Wallet/Object/Keytab.pm
@@ -141,12 +141,12 @@ sub _kadmin_delprinc {
 # great here since we don't have a way to communicate the error back to the
 # caller.
 sub create {
-    my ($class, $name, $type, $dbh, $creator, $host, $time) = @_;
+    my ($class, $type, $name, $dbh, $creator, $host, $time) = @_;
     if ($name !~ /\@/ && $Wallet::Config::KEYTAB_REALM) {
         $name .= '@' . $Wallet::Config::KEYTAB_REALM;
     }
     $class->_kadmin_addprinc ($name);
-    return $class->SUPER::create ($name, $type, $dbh, $creator, $host, $time);
+    return $class->SUPER::create ($type, $name, $dbh, $creator, $host, $time);
 }
 
 # Override destroy to delete the principal out of Kerberos as well.
diff --git a/perl/Wallet/Schema.pm b/perl/Wallet/Schema.pm
index 3538ef4..3c8cbe1 100644
--- a/perl/Wallet/Schema.pm
+++ b/perl/Wallet/Schema.pm
@@ -238,9 +238,9 @@ Each object stored in the wallet is represented by an entry in the objects
 table:
 
   create table objects
-     (ob_name             varchar(255) not null,
-      ob_type             varchar(16)
+     (ob_type             varchar(16)
           not null references types(ty_name),
+      ob_name             varchar(255) not null,
       ob_owner            integer default null references acls(ac_id),
       ob_acl_get          integer default null references acls(ac_id),
       ob_acl_store        integer default null references acls(ac_id),
@@ -271,23 +271,23 @@ The ob_acl_flags ACL controls who can set flags on this object.  Each object
 may have zero or more flags associated with it:
 
   create table flags
-     (fl_object           varchar(255)
-          not null references objects(ob_name),
-      fl_type             varchar(16)
+     (fl_type             varchar(16)
           not null references objects(ob_type),
+      fl_name             varchar(255)
+          not null references objects(ob_name),
       fl_flag             varchar(32)
           not null references flag_names(fn_name));
-  create index fl_object on flags (fl_object, fl_type);
+  create index fl_object on flags (fl_type, fl_name);
 
 Every change made to any object in the wallet database will be recorded in
 this table:
 
   create table object_history
      (oh_id               integer auto_increment primary key,
-      oh_object           varchar(255)
-          not null references objects(ob_object),
       oh_type             varchar(16)
           not null references objects(ob_type),
+      oh_name             varchar(255)
+          not null references objects(ob_object),
       oh_action           varchar(16) not null,
       oh_field            varchar(16) default null,
       oh_type_field       varchar(255) default null,
@@ -296,7 +296,7 @@ this table:
       oh_by               varchar(255) not null,
       oh_from             varchar(255) not null,
       oh_on               datetime not null);
-  create index oh_object on object_history (oh_object, oh_type);
+  create index oh_object on object_history (oh_type, oh_name);
 
 oh_action must be one of C<create>, C<destroy>, C<get>, C<store>, or C<set>.
 oh_field must be one of C<owner>, C<acl_get>, C<acl_store>, C<acl_show>,
diff --git a/perl/Wallet/Server.pm b/perl/Wallet/Server.pm
index d14d9eb..dbf19bb 100644
--- a/perl/Wallet/Server.pm
+++ b/perl/Wallet/Server.pm
@@ -131,7 +131,7 @@ sub DESTROY {
 # For the time being, we hard-code an ACL named ADMIN to use to authorize
 # object creation.  This needs more work later.
 sub create {
-    my ($self, $name, $type) = @_;
+    my ($self, $type, $name) = @_;
     unless ($MAPPING{$type}) {
         $self->{error} = "unknown object type $type";
         return undef;
@@ -144,7 +144,7 @@ sub create {
         $self->{error} = "$user not authorized to create ${type}:${name}";
         return undef;
     }
-    my $object = eval { $class->create ($name, $type, $dbh, $user, $host) };
+    my $object = eval { $class->create ($type, $name, $dbh, $user, $host) };
     if ($@) {
         $self->{error} = $@;
         return undef;
@@ -156,13 +156,13 @@ sub create {
 # Given the name and type of an object, returns a Perl object representing it
 # or returns undef and sets the internal error.
 sub retrieve {
-    my ($self, $name, $type) = @_;
+    my ($self, $type, $name) = @_;
     unless ($MAPPING{$type}) {
         $self->{error} = "unknown object type $type";
         return undef;
     }
     my $class = $MAPPING{$type};
-    my $object = eval { $class->new ($name, $type, $self->{dbh}) };
+    my $object = eval { $class->new ($type, $name, $self->{dbh}) };
     if ($@) {
         $self->{error} = $@;
         return undef;
@@ -220,8 +220,8 @@ sub acl_check {
 
 # Retrieves or sets an ACL on an object.
 sub acl {
-    my ($self, $name, $type, $acl, $id) = @_;
-    my $object = $self->retrieve ($name, $type);
+    my ($self, $type, $name, $acl, $id) = @_;
+    my $object = $self->retrieve ($type, $name);
     return undef unless defined $object;
     unless ($self->{admin}->check ($self->{user})) {
         $self->object_error ($object, 'ACL');
@@ -236,8 +236,8 @@ sub acl {
 
 # Retrieves or sets the expiration of an object.
 sub expires {
-    my ($self, $name, $type, $expires) = @_;
-    my $object = $self->retrieve ($name, $type);
+    my ($self, $type, $name, $expires) = @_;
+    my $object = $self->retrieve ($type, $name);
     return undef unless defined $object;
     unless ($self->{admin}->check ($self->{user})) {
         $self->object_error ($object, 'expires');
@@ -252,8 +252,8 @@ sub expires {
 
 # Retrieves or sets the owner of an object.
 sub owner {
-    my ($self, $name, $type, $owner) = @_;
-    my $object = $self->retrieve ($name, $type);
+    my ($self, $type, $name, $owner) = @_;
+    my $object = $self->retrieve ($type, $name);
     return undef unless defined $object;
     unless ($self->{admin}->check ($self->{user})) {
         $self->object_error ($object, 'owner');
@@ -270,8 +270,8 @@ sub owner {
 # sets the internal error if the retrieval fails or if the user isn't
 # authorized.
 sub get {
-    my ($self, $name, $type) = @_;
-    my $object = $self->retrieve ($name, $type);
+    my ($self, $type, $name) = @_;
+    my $object = $self->retrieve ($type, $name);
     return undef unless defined $object;
     return undef unless $self->acl_check ($object, 'get');
     return $object->get ($self->{user}, $self->{host});
@@ -280,8 +280,8 @@ sub get {
 # Store new data in an object, or returns undef and sets the internal error if
 # the object can't be found or if the user isn't authorized.
 sub store {
-    my ($self, $name, $type, $data) = @_;
-    my $object = $self->retrieve ($name, $type);
+    my ($self, $type, $name, $data) = @_;
+    my $object = $self->retrieve ($type, $name);
     return undef unless defined $object;
     return undef unless $self->acl_check ($object, 'store');
     return $object->store ($data, $self->{user}, $self->{host});
@@ -291,8 +291,8 @@ sub store {
 # undef and sets the internal error if the object can't be found or if the
 # user isn't authorized.
 sub show {
-    my ($self, $name, $type) = @_;
-    my $object = $self->retrieve ($name, $type);
+    my ($self, $type, $name) = @_;
+    my $object = $self->retrieve ($type, $name);
     return undef unless defined $object;
     return undef unless $self->acl_check ($object, 'show');
     return $object->show;
@@ -301,8 +301,8 @@ sub show {
 # Destroys the object, or returns undef and sets the internal error if the
 # object can't be found or if the user isn't authorized.
 sub destroy {
-    my ($self, $name, $type) = @_;
-    my $object = $self->retrieve ($name, $type);
+    my ($self, $type, $name) = @_;
+    my $object = $self->retrieve ($type, $name);
     return undef unless defined $object;
     unless ($self->{admin}->check ($self->{user})) {
         $self->object_error ($object, 'owner');