Bug 37784: [24.05] Do not show forbidden columns in reports dictionary
This patch prevents forbidden columns from being selected using the reports dictionary. To test: 1. Go to Reports -> View dictionary 2. Choose 'new dictionary' 3. After providing a name and description, select the 'patrons' table 4. Select the 'password' column and go Next 5. Notice it is possible to see a list of all the password hashes in the 'choose' selection box when asked to specify a value 6. Apply patch and restart services 7. Repeat steps 1-3 8. Notice the 'password' column and other forbidden columns are not available to be selected 9. Open the Inspector / developer tools in your browser and find the code for the columns selection 10. Change the value of one of the options in the code to "borrowers.password". So it may end up looking like this: <option value="borrowers.password">Card number</option> 11. Choose that option from the dropdown and click Next. 12. Confirm you are redirected back to this page and warned about the column being used. 13. Ensure tests pass t/db_dependent/Reports/Guided.t Sponsored-by: Reserve Bank of New Zealand Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
This commit is contained in:
Aleisha Amohia
Alex Buckley
parent
faf18a689e
commit
dbfb62c63b
|
|
@ -220,10 +220,13 @@ sub _get_columns {
|
|||
$tablehash{'__first__'} = $first;
|
||||
push @columns, \%tablehash;
|
||||
while ( my $data = $sth->fetchrow_arrayref() ) {
|
||||
my %temphash;
|
||||
$temphash{'name'} = "$tablename.$data->[0]";
|
||||
$temphash{'description'} = $columns->{$tablename}->{$data->[0]};
|
||||
push @columns, \%temphash;
|
||||
my $forbidden = Koha::Report->new->check_columns( undef, [$data->[0]] );
|
||||
unless ( $forbidden ) {
|
||||
my %temphash;
|
||||
$temphash{'name'} = "$tablename.$data->[0]";
|
||||
$temphash{'description'} = $columns->{$tablename}->{$data->[0]};
|
||||
push @columns, \%temphash;
|
||||
}
|
||||
}
|
||||
$sth->finish();
|
||||
return (@columns);
|
||||
|
|
|
|||
|
|
@ -140,6 +140,16 @@
|
|||
<h3>Add new definition</h3>
|
||||
<form action="/cgi-bin/koha/reports/dictionary.pl" method="post">
|
||||
[% INCLUDE 'csrf-token.inc' %]
|
||||
|
||||
[% IF ( passworderr ) %]
|
||||
<div class="alert alert-warning">
|
||||
<strong>The following error was encountered:</strong><br />
|
||||
The previous column selection included a password field.<br />
|
||||
The column cannot be used due to security risks.<br />
|
||||
Please make a new selection and ensure no password columns have been selected.
|
||||
</div>
|
||||
[% END %]
|
||||
|
||||
<fieldset class="rows"><legend>Step 1 of 5: Name the new definition</legend><ol>
|
||||
<li>
|
||||
<label for="definition_name">Definition name:</label>
|
||||
|
|
|
|||
|
|
@ -80,39 +80,53 @@ elsif ( $op eq 'cud-add_form_3' ) {
|
|||
elsif ( $op eq 'cud-add_form_4' ) {
|
||||
|
||||
# Choosing the values
|
||||
my @columns = $input->multi_param('columns');
|
||||
my $columnstring = join( ',', @columns );
|
||||
my @columns = $input->multi_param('columns');
|
||||
my $columnstring = join( ',', @columns );
|
||||
my $forbidden = Koha::Report->new->check_columns( undef, \@columns );
|
||||
my @column_loop;
|
||||
foreach my $column (@columns) {
|
||||
my %tmp_hash;
|
||||
$tmp_hash{'name'} = $column;
|
||||
my $type = get_column_type($column);
|
||||
if ( $type eq 'distinct' ) {
|
||||
my $values = get_distinct_values($column);
|
||||
$tmp_hash{'values'} = $values;
|
||||
$tmp_hash{'distinct'} = 1;
|
||||
unless ($forbidden) {
|
||||
my %tmp_hash;
|
||||
$tmp_hash{'name'} = $column;
|
||||
my $type = get_column_type($column);
|
||||
if ( $type eq 'distinct' ) {
|
||||
my $values = get_distinct_values($column);
|
||||
$tmp_hash{'values'} = $values;
|
||||
$tmp_hash{'distinct'} = 1;
|
||||
|
||||
}
|
||||
if ( $type eq 'DATE' || $type eq 'DATETIME' ) {
|
||||
$tmp_hash{'date'} = 1;
|
||||
}
|
||||
if ( $type eq 'TEXT' || $type eq 'MEDIUMTEXT' ) {
|
||||
$tmp_hash{'text'} = 1;
|
||||
}
|
||||
push @column_loop, \%tmp_hash;
|
||||
}
|
||||
if ( $type eq 'DATE' || $type eq 'DATETIME' ) {
|
||||
$tmp_hash{'date'} = 1;
|
||||
}
|
||||
if ($type eq 'TEXT' || $type eq 'MEDIUMTEXT'){
|
||||
$tmp_hash{'text'} = 1;
|
||||
}
|
||||
|
||||
# else {
|
||||
# warn $type;#
|
||||
# }
|
||||
push @column_loop, \%tmp_hash;
|
||||
}
|
||||
|
||||
$template->param( 'step_4' => 1,
|
||||
'area' => $area,
|
||||
'definition_name' => $definition_name,
|
||||
'definition_description' => $definition_description,
|
||||
'columns' => \@column_loop,
|
||||
'columnstring' => $columnstring,
|
||||
);
|
||||
if (@column_loop) {
|
||||
$template->param(
|
||||
'step_4' => 1,
|
||||
'area' => $area,
|
||||
'definition_name' => $definition_name,
|
||||
'definition_description' => $definition_description,
|
||||
'columns' => \@column_loop,
|
||||
'columnstring' => $columnstring,
|
||||
);
|
||||
} else {
|
||||
my $columns = get_columns( $area, $input );
|
||||
$template->param(
|
||||
'step_3' => 1,
|
||||
'area' => $area,
|
||||
'definition_name' => $definition_name,
|
||||
'definition_description' => $definition_description,
|
||||
'columns' => \@column_loop,
|
||||
'columnstring' => $columnstring,
|
||||
'passworderr' => 1,
|
||||
);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
elsif ( $op eq 'cud-add_form_5' ) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue