Require access to current record for historical (#860)

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 ($@)

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

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();