From 0d2a5d985229f9f5cb62fe6844287b0e25c6835b Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Sun, 7 Jun 2026 22:07:11 +0100 Subject: [PATCH 1/2] Require access to current record for historical (#860) --- lib/GADS.pm | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/GADS.pm b/lib/GADS.pm index 64238c880..ed51fb8c3 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -1933,6 +1933,21 @@ any qr{/(record|history|purge|purgehistory)/([0-9]+)} => require_login sub { : $action eq 'purgehistory' ? $record->find_deleted_recordid($id) : $record->find_current_id($id); + # If it's a historical version, check that the user has access to the + # current version. This prevents the scenario whereby a user has a view + # limit on a particular condition, which potentially allows them access + # to the historical values of the record but not the current version. + # For the purposes of consistency, require that they always need access + # to the current version + if ($action eq 'history') + { + my $current = GADS::Record->new( + user => $user, + schema => schema, + ); + # Will bork if no access + $current->find_current_id($record->current_id); + } }; if ($@) From 10290ec454c96fb86db55a5326abec374ef02764 Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Sun, 13 Sep 2026 22:27:10 +0100 Subject: [PATCH 2/2] Do not allow access to chronology if no current access (#860) --- lib/GADS/Record.pm | 10 +++++++- t/003_chronology.t | 63 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/lib/GADS/Record.pm b/lib/GADS/Record.pm index d6fb7f5f2..b9cf38a02 100644 --- a/lib/GADS/Record.pm +++ b/lib/GADS/Record.pm @@ -724,7 +724,7 @@ sub find_chronology_id or error __x"Record ID {id} not found", id => $current_id; my $instance_id = $current->instance_id; $self->_set_instance_id($current->instance_id); - $self->_find(current_id => $current_id, chronology => 1); + $self->_find(current_id => $current_id, chronology => 1, current => $current); } sub find_draftuser_id @@ -1007,6 +1007,14 @@ sub _find $first_run = 0; } + # If viewing a chronology, at this point the record will only contain the + # versions that the user is allowed to see in accordance with any view + # limits. It could be that they can't see the actual latest version though, + # and that the latest version here is an old version. Check now whether + # they are allowed to see this record at all. + error __"Requested record not found" + if $find{chronology} && $find{current}->current_version_id != $record_ids[-1]; + $self->clear_is_draft; # Fetch and add multi-values diff --git a/t/003_chronology.t b/t/003_chronology.t index 63ed654b0..b67adffd6 100644 --- a/t/003_chronology.t +++ b/t/003_chronology.t @@ -82,6 +82,68 @@ $record->clear; is($changed_integer->{name_short}, 'L1integer1', "Showing change of integer in second edit"); } +# Check view limits. If the user has view limits applied to their account, they +# should not be able to see historical versions within that restriction. +{ + # Firstly set up a limit that only allows them to see the most recent 2 + # versions: + my $rules = GADS::Filter->new( + as_hash => { + rules => [{ + id => $string1->id, + type => 'string', + value => 'Foo2', + operator => 'equal', + }], + }, + ); + my $view_limit = GADS::View->new( + name => 'limit to view', + filter => $rules, + instance_id => 1, + layout => $layout, + schema => $schema, + user => $user, + ); + $view_limit->write; + $user->set_view_limits([$view_limit->id]); + + $layout->clear; + $record->find_chronology_id(1); + my @changed = @{$record->chronology}; + is(@changed, 2, "Number of versions matches access rights"); + + # Next a view limit that does not allow the user to see the latest version + # (only earlier) versions + $rules = GADS::Filter->new( + as_hash => { + rules => [{ + id => $string1->id, + type => 'string', + value => '20', + operator => 'less', + }], + }, + ); + $view_limit = GADS::View->new( + name => 'limit to view', + filter => $rules, + instance_id => 1, + layout => $layout, + schema => $schema, + user => $user, + ); + $view_limit->write; + $user->set_view_limits([$view_limit->id]); + + $layout->clear; + try { $record->find_chronology_id(1) }; + like($@, qr/record not found/, "Cannot see chronology of record with no current access"); + + # Reset + $user->set_view_limits([]); +} + # Check changes as user without permission on integer field { $integer1->set_permissions({$sheet->group->id => []}); @@ -107,5 +169,4 @@ $record->clear; # Second change not shown as integer not visible } -# done_testing();