[Rt-commit] rt branch, 5.0-trunk, updated. rt-5.0.0-186-gb0e59a2e48

? sunnavy sunnavy at bestpractical.com
Wed Dec 30 16:55:25 EST 2020


The branch, 5.0-trunk has been updated
       via  b0e59a2e489975b8d1dca5d6236b26d05b791d20 (commit)
       via  923446a45187325078f4d42e65c0f229cefb1219 (commit)
       via  58646f19ec36bfb6d746fc5ee8281cb29800cad4 (commit)
       via  65afd9d3c60a5623766d4c603d84e697833a715e (commit)
       via  9ca22addf79b29bef3b4c0ba08a71c193123bb5b (commit)
      from  f27628549c754235e4615a1c3772496790d8d7c1 (commit)

Summary of changes:
 docs/UPGRADING-5.0                            |  7 +++++
 lib/RT/Config.pm                              | 25 ++++++++++++---
 share/html/Admin/Tools/Config/Elements/Option | 45 +++++++++++++++++----------
 share/html/Admin/Tools/EditConfig.html        | 38 +++++-----------------
 share/html/Widgets/Form/{Code => JSON}        |  2 +-
 t/web/admin_tools_editconfig.t                | 24 +++++++++-----
 6 files changed, 81 insertions(+), 60 deletions(-)
 copy share/html/Widgets/Form/{Code => JSON} (97%)

- Log -----------------------------------------------------------------
commit 9ca22addf79b29bef3b4c0ba08a71c193123bb5b
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Nov 10 04:41:23 2020 +0800

    Mark config items containing regexp immutable
    
    We are going to switch to JSON format, which doesn't support regexp :/
    
    Note that "path" in @StaticRoots is regexp, so we need to mark it too.

diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 64dd51a9a1..8f44f5f3ec 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -646,6 +646,7 @@ our %META;
 
     RTAddressRegexp => {
         Type    => 'SCALAR',
+        Immutable => 1,
         PostLoadCheck => sub {
             my $self = shift;
             my $value = $self->Get('RTAddressRegexp');
@@ -1793,9 +1794,6 @@ our %META;
     DefaultSearchResultOrderBy => {
         Widget => '/Widgets/Form/String',
     },
-    EmailSubjectTagRegex => {
-        Widget => '/Widgets/Form/String',
-    },
     EmailOutputEncoding => {
         Widget => '/Widgets/Form/String',
     },
@@ -1937,7 +1935,26 @@ our %META;
     },
     ShowMobileSite => {
         Widget => '/Widgets/Form/Boolean',
-    }
+    },
+    StaticRoots => {
+        Type      => 'ARRAY',
+        Immutable => 1,
+    },
+    EmailSubjectTagRegex => {
+        Immutable => 1,
+    },
+    ExtractSubjectTagMatch => {
+        Immutable => 1,
+    },
+    ExtractSubjectTagNoMatch => {
+        Immutable => 1,
+    },
+    WebNoAuthRegex => {
+        Immutable => 1,
+    },
+    SelfServiceRegex => {
+        Immutable => 1,
+    },
 );
 my %OPTIONS = ();
 my @LOADED_CONFIGS = ();

commit 65afd9d3c60a5623766d4c603d84e697833a715e
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Wed Aug 19 14:44:00 2020 -0400

    Use "JSON" format for array/hash items in system config edit web UI
    
    This is for security reasons, to prevent users from executing arbitrary
    code there.
    
    For complicated configuration items that couldn't be serialized as
    JSON(like regex or coderef), we keep showing them as readonly Perl code.
    
    Internally in the database, the items are still saved as Perl data
    structures.

diff --git a/share/html/Admin/Tools/Config/Elements/Option b/share/html/Admin/Tools/Config/Elements/Option
index 82acca3920..2769467d06 100644
--- a/share/html/Admin/Tools/Config/Elements/Option
+++ b/share/html/Admin/Tools/Config/Elements/Option
@@ -46,18 +46,31 @@
 %#
 %# END BPS TAGGED BLOCK }}}
 <%PERL>
-
+use JSON;
 use Data::Dumper;
 my $stringify = sub {
     my $value = shift;
-    return "" if !defined($value);
+    return ('', '') if !defined($value);
 
+    my $output;
     local $Data::Dumper::Terse = 1;
     local $Data::Dumper::Indent = 2;
     local $Data::Dumper::Sortkeys = 1;
-    my $output = Dumper $value;
-    chomp $output;
-    return $output;
+    my $format = '';
+    if (ref $value) {
+        eval { $output = to_json($value, {utf8 => 1, pretty => 1, canonical => 1}) };
+        if ( $@ ) {
+            $output = Dumper($value);
+            $format = 'code';
+        }
+        else {
+            $format = 'json';
+        }
+    } else {
+        $output = $value;
+    }
+    chomp($output);
+    return($output, $format);
 };
 
 my $doc_version = $RT::VERSION;
@@ -67,20 +80,18 @@ my $name = $option->{Name};
 my $meta = RT->Config->Meta( $name );
 return if $meta->{Invisible} || $meta->{Deprecated} || $meta->{Obfuscate};
 
-my $has_execute_code = $session{CurrentUser}->HasRight(Right => 'ExecuteCode', Object => RT->System);
-
 my $raw_value = RT->Config->Get( $name );
-my $val = $stringify->($raw_value);
+my ($val, $format) = $stringify->($raw_value);
 my $doc_url = "https://docs.bestpractical.com/rt/$doc_version/RT_Config.html#$option->{Help}";
-my $widget = $meta->{'Widget'} || '/Widgets/Form/Code';
-my $is_code = $widget eq '/Widgets/Form/Code';
+my $widget = $meta->{'Widget'} || '/Widgets/Form/JSON';
 
+# CustomDateRanges could contain subrefs, but we supply limited/safe
+# widgets that contain plain inputs only.
 my $is_immutable = $meta->{Immutable}
                 || $meta->{Obfuscate}
-                || ($is_code && $val =~ s/sub \{ "DUMMY" \}/sub { ... }/g)
-                || ($is_code && !$has_execute_code);
+                || ( $format eq 'code' && $widget ne '/Widgets/Form/CustomDateRanges' );
 
-my $current_value = $is_code ? $val : $raw_value;
+my $current_value = $format ? $val : $raw_value;
 my $args   = $meta->{'WidgetArguments'} || {};
 if ($widget eq '/Widgets/Form/Boolean') {
     %$args = (
@@ -111,8 +122,8 @@ my $row_end = qq{</div></div>};
 
 <!-- start option <% $name %> -->
 % if ( $meta->{EditLink} ) {
-% if ($widget eq '/Widgets/Form/MultilineString' || $widget eq '/Widgets/Form/Code') {
-<% $row_start |n %><textarea disabled class="<% $is_code ? 'code' : '' %> form-control" rows="6" cols="80"><% $current_value %></textarea><br />
+% if ($widget eq '/Widgets/Form/MultilineString' || $widget eq '/Widgets/Form/Code' || $widget eq '/Widgets/Form/JSON' ) {
+<% $row_start |n %><textarea disabled class="<% $format %> form-control" rows="6" cols="80"><% $current_value %></textarea><br />
 % } else {
 <% $row_start |n %><input type="text" disabled value="<% $current_value %>" class="form-control" /><br/>
 % }
@@ -126,8 +137,8 @@ my $row_end = qq{</div></div>};
 <div class="text-right"><em><% loc('Must modify in config file' ) %></em></div>
 <% $row_end |n%>
 % } elsif ( $is_immutable ) {
-% if ($widget eq '/Widgets/Form/MultilineString' || $widget eq '/Widgets/Form/Code') {
-<% $row_start |n %><textarea disabled class="<% $is_code ? 'code' : '' %> form-control" rows="6" cols="80"><% $current_value %></textarea>
+% if ($widget eq '/Widgets/Form/MultilineString' || $widget eq '/Widgets/Form/Code' || $widget eq '/Widgets/Form/JSON') {
+<% $row_start |n %><textarea disabled class="<% $format %> form-control" rows="6" cols="80"><% $current_value %></textarea>
 % } else {
 <% $row_start |n %><input type="text" disabled value="<% $current_value %>" class="form-control" />
 % }
diff --git a/share/html/Admin/Tools/EditConfig.html b/share/html/Admin/Tools/EditConfig.html
index 5de306fa45..2d60cd7898 100644
--- a/share/html/Admin/Tools/EditConfig.html
+++ b/share/html/Admin/Tools/EditConfig.html
@@ -53,8 +53,6 @@ unless ($session{'CurrentUser'}->HasRight( Object=> $RT::System, Right => 'Super
  Abort(loc('This feature is only available to system administrators'));
 }
 
-my $has_execute_code = $session{CurrentUser}->HasRight(Right => 'ExecuteCode', Object => RT->System);
-
 my $options = RT->Config->LoadSectionMap();
 my $active_context = {
     tab        => CSSClass( $ARGS{tab}        || $options->[0]->{Name}) ,
@@ -81,8 +79,8 @@ if (delete $ARGS{Update}) {
             next if !exists $ARGS{$key . '-Current'};
 
             my $meta = RT->Config->Meta( $key );
-            my $widget = $meta->{Widget} || '/Widgets/Form/Code';
-            my $is_code = $widget eq '/Widgets/Form/Code';
+            my $widget = $meta->{Widget} || '/Widgets/Form/JSON';
+            my $is_json = $widget eq '/Widgets/Form/JSON';
 
             my $val = $ARGS{$key};
             $val = '' if $val eq '__empty_value__';
@@ -100,38 +98,16 @@ if (delete $ARGS{Update}) {
                 next;
             }
 
-            if ($is_code) {
-                if (!$has_execute_code) {
-                    push @results, loc("Cannot change [_1]: Permission Denied", $key);
-                    $has_error++;
-                    next;
-                }
-
-                my $code = $val;
-                my $coderef;
-                # similar to RT::Scrip::CompileCheck
-                do {
-                    no strict 'vars';
-                    $coderef = eval "sub { $code \n }";
-                };
-                if ($@) {
+            if ($is_json) {
+                my $json = $val;
+                ($val) = RT::Configuration->new( $session{CurrentUser} )->_DeJSONContent($json);
+                if (!defined $val) {
                     my $error = $@;
-                    push @results, loc("Couldn't compile [_1] codeblock '[_2]': [_3]", $key, $code, $error);
+                    push @results, loc("Couldn't decode [_1] JSON '[_2]': [_3]", $key, $json, $error);
                     $has_error++;
                     next;
                 }
-
-                if ($coderef) {
-                    $val = eval { $coderef->() };
-                    if ($@) {
-                        my $error = $@;
-                        push @results, loc("Couldn't execute [_1] codeblock '[_2]': [_3]", $key, $code, $error);
-                        $has_error++;
-                        next;
-                    }
-                }
             }
-
             my $setting = RT::Configuration->new($session{CurrentUser});
             $setting->LoadByCols(Name => $key, Disabled => 0);
             if ($setting->Id) {
diff --git a/share/html/Widgets/Form/JSON b/share/html/Widgets/Form/JSON
new file mode 100644
index 0000000000..8f9d407c93
--- /dev/null
+++ b/share/html/Widgets/Form/JSON
@@ -0,0 +1,57 @@
+%# BEGIN BPS TAGGED BLOCK {{{
+%#
+%# COPYRIGHT:
+%#
+%# This software is Copyright (c) 1996-2020 Best Practical Solutions, LLC
+%#                                          <sales at bestpractical.com>
+%#
+%# (Except where explicitly superseded by other copyright notices)
+%#
+%#
+%# LICENSE:
+%#
+%# This work is made available to you under the terms of Version 2 of
+%# the GNU General Public License. A copy of that license should have
+%# been provided with this software, but in any event can be snarfed
+%# from www.gnu.org.
+%#
+%# This work is distributed in the hope that it will be useful, but
+%# WITHOUT ANY WARRANTY; without even the implied warranty of
+%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+%# General Public License for more details.
+%#
+%# You should have received a copy of the GNU General Public License
+%# along with this program; if not, write to the Free Software
+%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+%# 02110-1301 or visit their web page on the internet at
+%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+%#
+%#
+%# CONTRIBUTION SUBMISSION POLICY:
+%#
+%# (The following paragraph is not intended to limit the rights granted
+%# to you to modify and distribute this software under the terms of
+%# the GNU General Public License and is only of importance to you if
+%# you choose to contribute your changes and enhancements to the
+%# community by submitting them to Best Practical Solutions, LLC.)
+%#
+%# By intentionally submitting any modifications, corrections or
+%# derivatives to this work, or any other work intended for use with
+%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+%# you are the copyright holder for those contributions and you grant
+%# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+%# royalty-free, perpetual, license to use, copy, create derivative
+%# works based on those contributions, and sublicense and distribute
+%# those contributions and any derivatives thereof.
+%#
+%# END BPS TAGGED BLOCK }}}
+<%DOC>
+see docs/extending/using_forms_widgets.pod
+</%DOC>
+<& /Widgets/Form/MultilineString, Class => 'json', %ARGS &>
+<%METHOD InputOnly>
+<& /Widgets/Form/MultilineString:InputOnly, %ARGS &>
+</%METHOD>
+<%METHOD Process>
+<& /Widgets/Form/MultilineString:Process, %ARGS &>
+</%METHOD>

commit 58646f19ec36bfb6d746fc5ee8281cb29800cad4
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Wed Aug 19 15:19:43 2020 -0400

    Update unit test for JSON entry of hashes/arrays rather than Perl.

diff --git a/t/web/admin_tools_editconfig.t b/t/web/admin_tools_editconfig.t
index f23d2fdb04..2182411a4b 100644
--- a/t/web/admin_tools_editconfig.t
+++ b/t/web/admin_tools_editconfig.t
@@ -24,6 +24,10 @@ $m->follow_link_ok( { text => 'System Configuration' }, 'followed link to "Syste
 $m->follow_link_ok( { text => 'History' }, 'followed link to History page' );
 $m->follow_link_ok( { text => 'Edit' }, 'followed link to Edit page' );
 
+# In the tests below, new_value is *always* the string we feed
+# into the Web form.  For compound objects such as hashes and arrays,
+# we have a separate expected member that is the Perl data structure
+# resulting from feeding new_value into the Web interface.
 my $tests = [
     {
         name      => 'change a string value',
@@ -41,13 +45,15 @@ my $tests = [
         name      => 'change an arrayref value',
         form_id   => 'form-System-Extra_security',
         setting   => 'ReferrerWhitelist',
-        new_value => ['www.example.com:443', 'www3.example.com:80'],
+        new_value => '["www.example.com:443", "www3.example.com:80"]',
+        expected  => ['www.example.com:443', 'www3.example.com:80'],
     },
     {
         name      => 'change a hashref value',
         form_id   => 'form-System-Outgoing_mail',
         setting   => 'OverrideOutgoingMailFrom',
-        new_value => { 1 => 'new-outgoing-from at example.com' },
+        new_value => '{"1":"new-outgoing-from at example.com"}',
+        expected  => {1 => 'new-outgoing-from at example.com'},
     },
 ];
 
@@ -80,7 +86,7 @@ sub run_test {
         {
             form_id => $args{form_id},
             fields  => {
-                $args{setting} => stringify( $args{new_value} ),
+                $args{setting} => $args{new_value},
             },
         },
         'form was submitted successfully'
@@ -100,22 +106,26 @@ sub run_test {
 
     my $rt_config_value = RT->Config->Get( $args{setting} );
 
-    is( $rt_configuration_value, stringify($args{new_value}), 'value from RT::Configuration->Load matches new value' );
-    cmp_deeply( $rt_config_value, $args{new_value}, 'value from RT->Config->Get matches new value' );
+    is( $rt_configuration_value, stringify($args{expected}) || $args{new_value}, 'value from RT::Configuration->Load matches new value' );
+    cmp_deeply( $rt_config_value, $args{expected} || $args{new_value}, 'value from RT->Config->Get matches new value' );
 }
 
 sub check_transaction {
     my ($tx, $change) = @_;
     is($tx->ObjectType, 'RT::Configuration', 'tx is config change');
     is($tx->Field, $change->{setting}, 'tx matches field changed');
-    is($tx->NewValue, stringify($change->{new_value}), 'tx value matches');
+    is($tx->NewValue, stringify($change->{expected}) || $change->{new_value}, 'tx value matches');
 }
 
 sub check_history_page_item {
     my ($tx, $change) = @_;
     my $link = sprintf('ConfigHistory.html?id=%d#txn-%d', $tx->ObjectId, $tx->id);
     ok($m->find_link(url => $link), 'found tx link in history');
-    $m->text_contains(compactify($change->{new_value}), 'fetched tx has new value');
+    if ($change->{expected}) {
+        $m->text_contains(compactify($change->{expected}), 'fetched tx has new value');
+    } else {
+        $m->text_contains($change->{new_value});
+    }
     $m->text_contains("$change->{setting} changed", 'fetched tx has changed field');
 }
 

commit 923446a45187325078f4d42e65c0f229cefb1219
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Fri Dec 18 15:02:23 2020 -0500

    Document switch to JSON instead of Perl for System Configuration editor.

diff --git a/docs/UPGRADING-5.0 b/docs/UPGRADING-5.0
index 22b60cfc53..a194093668 100644
--- a/docs/UPGRADING-5.0
+++ b/docs/UPGRADING-5.0
@@ -242,6 +242,13 @@ If you are still seeing that error after updating to RT 5.0.1, edit the page, re
 the saved search, save, then add it back again. After saving again, it should appear
 as expected.
 
+=item *
+
+The System Configuration editor (Admin > Tools > System Configuration > Edit)
+now uses JSON rather than Perl syntax to represent arrays and hashes.  Be
+sure to enter valid JSON if you wish to modify an array- or hash-valued
+configuration setting.
+
 =back
 
 =cut

commit b0e59a2e489975b8d1dca5d6236b26d05b791d20
Merge: f27628549c 923446a451
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Dec 31 05:21:04 2020 +0800

    Merge branch '5.0/use-json-in-system-config-edit-ui' into 5.0-trunk


-----------------------------------------------------------------------


More information about the rt-commit mailing list