From 075d38e562949e9972bb70108d814bafe6e323b0 Mon Sep 17 00:00:00 2001
From: PGMacDesign <patrickmacdowell@gmail.com>
Date: Fri, 25 Sep 2026 00:02:25 -0700
Subject: [PATCH] Stop sitters from authorizing or deauthorizing sitters

authorize_sitters and deauthorize_sitters never checked whether the
session was a sitter session. Someone logged in with an empire's sitter
password could grant sitter access on that empire to any other empire,
and could expire every sitter the owner had authorized.

Every other method that controls access to the account already refuses
sitters with error 1015: change_password, view_profile and edit_profile
(which is where the sitter password itself is set), and
enable/disable_self_destruct. Sitters cannot change who holds the sitter
password, so they should not be able to change the sitter list either.
Use the same guard in both methods, in the $session->is_sitter form that
Entertainment.pm uses, ahead of the captcha check as Embassy's
accept_invite does.

Both methods act on the session's own empire_id and ignore any empire_id
option, so a sitter reaches them for someone else's empire only through a
sitter-password session. An empire sitting through a sitter
authorization calls them as itself, is not a sitter there, and is
unaffected.

The regression test logs in with the sitter password, solves the
captcha, and checks that both calls return 1015 and leave the list
unchanged, and that the owner can still deauthorize. Against the
previous code the sitter's authorize_sitters succeeded and
deauthorize_sitters expired the owner's sitter.
---
 lib/Lacuna/RPC/Empire.pm                 |  6 +++
 t/bugs/0043_SittersCannotManageSitters.t | 50 ++++++++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 t/bugs/0043_SittersCannotManageSitters.t

diff --git a/lib/Lacuna/RPC/Empire.pm b/lib/Lacuna/RPC/Empire.pm
index f996f13c..bb15cf33 100644
--- a/lib/Lacuna/RPC/Empire.pm
+++ b/lib/Lacuna/RPC/Empire.pm
@@ -1235,6 +1235,9 @@ sub authorize_sitters
 {
     my ($self, $session_id, $opts) = @_;
     my $session  = $self->get_session({session_id => $session_id});
+    if ($session->is_sitter) {
+        confess [1015, 'Sitters cannot manage sitters.'];
+    }
     $session->check_captcha;
 
     my $baby = $session->current_empire;
@@ -1309,6 +1312,9 @@ sub deauthorize_sitters {
     my ($self, $session_id, $opts) = @_;
 
     my $session  = $self->get_session({session_id => $session_id});
+    if ($session->is_sitter) {
+        confess [1015, 'Sitters cannot manage sitters.'];
+    }
     my $baby = $session->current_empire;
 
     my $baby_id = $session->empire_id;
diff --git a/t/bugs/0043_SittersCannotManageSitters.t b/t/bugs/0043_SittersCannotManageSitters.t
new file mode 100644
index 00000000..c6e389d6
--- /dev/null
+++ b/t/bugs/0043_SittersCannotManageSitters.t
@@ -0,0 +1,50 @@
+use lib '..','../../lib';
+use Test::More tests => 6;
+use 5.010;
+
+use strict;
+use warnings;
+
+use TestHelper;
+TestHelper->clear_all_test_empires;
+
+# BUG someone logged in with an empire's sitter password could authorize more sitters on
+# that empire, or remove the ones its owner had authorized. Sitters already cannot change
+# the password or the sitter password, so they should not manage the sitter list either.
+
+my $owner = TestHelper->new->generate_test_empire;
+my $empire = $owner->empire;
+$empire->sitter_password('testsitter');
+$empire->update;
+
+my $other = TestHelper->new(empire_name => 'TLE Test Sitter')->generate_test_empire;
+my $other_id = $other->empire->id;
+
+my $result = $owner->post('empire', 'login', [$owner->empire_name, 'testsitter', 'Anonymous']);
+my $sitter_session_id = $result->{result}{session_id};
+ok($sitter_session_id, 'logged in with the sitter password');
+
+# Solve the captcha so that, without the guard, authorize_sitters would go through.
+Lacuna->cache->set('captcha', $sitter_session_id, { guid => 1111, solution => 1111 }, 60 * 15);
+$owner->post('captcha', 'solve', [$sitter_session_id, 1111, 1111]);
+
+$result = $owner->post('empire', 'authorize_sitters', [$sitter_session_id, { empires => [$other_id] }]);
+is($result->{error}{code}, 1015, 'a sitter cannot authorize sitters');
+is($empire->sitters->count, 0, 'and no sitter was added');
+
+my %key  = (baby_id => $empire->id, sitter_id => $other_id);
+my $auths = Lacuna->db->resultset('SitterAuths');
+my $auth  = $auths->find(\%key) // $auths->new(\%key);
+$auth->reauthorise;
+$auth->update_or_insert;
+
+$result = $owner->post('empire', 'deauthorize_sitters', [$sitter_session_id, { deauthorize_all => 1 }]);
+is($result->{error}{code}, 1015, 'a sitter cannot deauthorize sitters');
+ok($auth->get_from_storage->expiry > DateTime->now, 'and the owner\'s sitter is still authorized');
+
+$result = $owner->post('empire', 'deauthorize_sitters', [$owner->session->id, { empires => [$other_id] }]);
+is($result->{error}, undef, 'the owner can still deauthorize sitters');
+
+END {
+    TestHelper->clear_all_test_empires;
+}
-- 
2.51.2