[Jifty-commit] r4617 - in jifty/trunk: lib/Jifty/Action/Record

[jifty-commit at lists.jifty.org]

Author: sartak
Date: Wed Dec  5 15:03:51 2007
New Revision: 4617

Modified:
   jifty/trunk/   (props changed)
   jifty/trunk/lib/Jifty/Action/Record/Update.pm

Log:
 r48628 at onn:  sartak | 2007-12-05 15:03:20 -0500
 Make sure actions still fail when you set columns you can't update (or read) to undef


Modified: jifty/trunk/lib/Jifty/Action/Record/Update.pm
==============================================================================
--- jifty/trunk/lib/Jifty/Action/Record/Update.pm	(original)
+++ jifty/trunk/lib/Jifty/Action/Record/Update.pm	Wed Dec  5 15:03:51 2007
@@ -124,6 +124,26 @@
             $value = scalar <$value>;
         }
 
+        # Error on columns we can't update
+        # <Sartak> ah ha. I think I know why passing due => undef reports
+        #          action success
+        # <Sartak> Jifty::Action::Record::Update compares the value of the
+        #          field with what you passed in
+        # <Sartak> but since user can't read the field, it returns undef
+        # <Sartak> and so: they're both undef, no change, skip this column
+        # <Sartak> and since that's the only column that changed, it'll notice
+        #          that every column it did try to update (which is.. none of
+        #          them) succeeded
+        # <Sartak> I don't think we can just skip ACLs for reading the column
+        #          -- that's a potential security issue. an attacker could try
+        #          every value until the action succeeds because nothing changed
+        # <Sartak> it doesn't matter for HM but for other apps it may
+
+        unless ($self->record->current_user_can('update', $field => $value)) {
+            $self->result->field_error($field, _('Permission denied'));
+            next;
+        }
+
         # Skip fields that have not changed
         my $old = $self->record->$field;
         # XXX TODO: This ignore "by" on columns