Kaynağa Gözat

Show guest console captures only to admins

The screenshot route requires the admin acl, so the img tags pointing at it
were broken images for everyone else -- ten of them on the VMs page, plus ten
dead links. They are behind $can_edit now, along with the link to the full size
capture, which the untiled view has gained for consistency.

A logged out visitor still gets the useful part: every guest, named, with its
power state. That is a status board, and it is safe to publish.

The test for this renders the template in all three views a reader can actually
be in -- tiled and logged out, untiled and logged out, untiled and admin -- and
asserts on the output, rather than grepping the source for guards. The template
has enough branches that reading it is not the same as knowing what comes out
of it, which is how the previous version of this test managed to assert
something that was not true of the untiled admin view.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U3k2hhSZpcsoe6uBmhF9nB
George Baugh 2 ay önce
ebeveyn
işleme
934649dfbe

+ 40 - 20
t/Trog-DataSource-Virt.t

@@ -245,28 +245,48 @@ subtest 'the guest routes avoid the router\'s own query keys' => sub {
     like( $form, qr/name="guest"/, 'it posts the guest under a name that survives routing' );
 };
 
-subtest 'guest controls are admin only' => sub {
-    my $form = Path::Tiny->new("$FindBin::Bin/../www/templates/html/components/forms/guests.tx")->slurp_utf8;
+subtest 'guests show nothing an unprivileged viewer cannot use' => sub {
+    require Text::Xslate;
+
+    # Rendered rather than grepped: what matters is what a given viewer ends up
+    # looking at, and the template has enough branches that reading it is not
+    # the same as knowing.
+    my $tx = Text::Xslate->new(
+        path     => ["$FindBin::Bin/../www/templates/html/components"],
+        function => { render_it => sub { $_[0] } },
+    );
 
-    # Powering off, snapshotting and destroying a guest must never render for
-    # someone who cannot do them.  The route checks the acl too, but a button
-    # that does nothing is its own kind of bug, and these pages are meant to be
-    # safe to show to unauthenticated visitors.
-    foreach my $action (qw{poweron poweroff snapshot destroy}) {
-        my ($before) = $form =~ m/(.*)value="\Q$action\E"/s;
-        ok( defined $before, "the $action button is in the template" ) or next;
-
-        # Everything up to the button must have opened a $can_edit guard and
-        # not yet closed it.
-        my $opens  = () = $before =~ m/:\s*if\s*\(\s*\$can_edit\s*\)/g;
-        ok( $opens > 0, "the $action button sits behind a \$can_edit guard" );
-    }
+    my %post = (
+        id      => 'uuid-alpha', title  => 'alpha',  state   => 'running', is_active => 1,
+        preview => '/guest/screenshot/hv-1/alpha',   domain  => 'alpha',
+        hypervisor => 'hv-1', hypervisor_title => 'spec-hv', vcpus => 2,
+        addpost => 0, unreachable => 0,
+    );
+
+    my %views = (
+        'tiled, logged out'   => { tiled => 1, can_edit => 0 },
+        'untiled, logged out' => { tiled => 0, can_edit => 0 },
+        'untiled, admin'      => { tiled => 0, can_edit => 1 },
+    );
+
+    foreach my $view ( sort keys %views ) {
+        my $out = $tx->render( 'forms/guests.tx', { post => \%post, style => '', route => '/vm', %{ $views{$view} } } );
+        my $admin = $views{$view}{can_edit};
+
+        # The guest is always named -- that is the point of the page.
+        like( $out, qr/alpha/, "$view: the guest is listed" );
 
-    # The tiled view is the one a logged out visitor gets, and it has no
-    # controls at all.
-    my ($tiled) = $form =~ m/:\s*if\s*\(\s*\$tiled\s*\)\s*\{(.*?):\s*\}\s*else/s;
-    ok( defined $tiled, 'the template has a tiled branch' );
-    unlike( $tiled, qr{/guest/act}, 'which offers no actions' ) if defined $tiled;
+        # The screenshot route requires the admin acl, so showing the img to
+        # anyone else is a broken image and nothing else.
+        is( scalar( () = $out =~ m/<img /g ), $admin ? 1 : 0, "$view: console capture shown only to an admin" );
+        is( scalar( () = $out =~ m{href="/guest/screenshot}g ), $admin ? 1 : 0, "$view: and so is the link to it" );
+
+        # Powering off, snapshotting and destroying likewise.
+        is( scalar( () = $out =~ m{action="/guest/act"}g ), $admin ? 1 : 0, "$view: controls shown only to an admin" );
+        foreach my $action (qw{poweroff snapshot destroy}) {
+            is( scalar( () = $out =~ m/value="\Q$action\E"/g ), $admin ? 1 : 0, "$view: no $action button" ) unless $admin;
+        }
+    }
 };
 
 subtest 'a hypervisor does not show its connection uri to everyone' => sub {

+ 17 - 9
www/templates/html/components/forms/guests.tx

@@ -7,21 +7,29 @@
             </div>
         : } else {
             : if ( $tiled ) {
-                <a href="<: $post.preview :>" class="undecorated" title="<: $post.title :> (<: $post.state :>)">
-                    <img src="<: $post.preview :>" class="responsive-img guest-screenshot" alt="console of <: $post.title :>" loading="lazy" />
-                    <p class="midtitle">
-                        <: $post.title :>
-                        <span class="guest-state guest-<: $post.is_active ? 'up' : 'down' :>"><: $post.state :></span>
-                    </p>
-                </a>
+                <p class="midtitle">
+                    <: $post.title :>
+                    <span class="guest-state guest-<: $post.is_active ? 'up' : 'down' :>"><: $post.state :></span>
+                </p>
+                : # A console capture is admin-only: the route that serves it
+                : # requires the admin acl, so anyone else gets a broken image.
+                : if ( $can_edit ) {
+                    <a href="<: $post.preview :>" class="undecorated" title="<: $post.title :> (<: $post.state :>)">
+                        <img src="<: $post.preview :>" class="responsive-img guest-screenshot" alt="console of <: $post.title :>" loading="lazy" />
+                    </a>
+                : }
             : } else {
                 <h3 class='blogtitles'>
                     <: $post.title :>
                     <span class="guest-state guest-<: $post.is_active ? 'up' : 'down' :>"><: $post.state :></span>
                 </h3>
                 <div class="postData responsive-text" id="postData-<: $post.id :>">
-                    <img src="<: $post.preview :>" class="responsive-img guest-screenshot" alt="console of <: $post.title :>" loading="lazy" />
-                    <br />
+                    : if ( $can_edit ) {
+                        <a href="<: $post.preview :>" class="undecorated" title="console of <: $post.title :>">
+                            <img src="<: $post.preview :>" class="responsive-img guest-screenshot" alt="console of <: $post.title :>" loading="lazy" />
+                        </a>
+                        <br />
+                    : }
                     on <: $post.hypervisor_title :> &mdash; <: $post.vcpus :> vCPU
                     : if ( $can_edit ) {
                     <br />