From b3da7e53fd1959cfbb539e150ca030bac0b16f5d Mon Sep 17 00:00:00 2001 From: sendi0011 Date: Fri, 17 Jul 2026 01:55:24 +0100 Subject: [PATCH 1/3] test(admin): assert admin auth is actually required, not just mocked setup() uses env.mock_all_auths(), which approves any address's require_auth() call unconditionally. Existing admin-suite tests only asserted on resulting state, so a deleted require_admin() call would not fail CI. Add negative tests per privileged entrypoint using env.mock_auths(&[]) so a missing auth check surfaces as a passing call instead of an error, turning the test red. Closes #73 --- contracts/events/src/tests/admin.rs | 113 ++++++++++++++++++++++- contracts/profile/src/tests/admin.rs | 133 +++++++++++++++++++++++++++ 2 files changed, 244 insertions(+), 2 deletions(-) diff --git a/contracts/events/src/tests/admin.rs b/contracts/events/src/tests/admin.rs index 20f2036..1223491 100644 --- a/contracts/events/src/tests/admin.rs +++ b/contracts/events/src/tests/admin.rs @@ -3,8 +3,8 @@ #![cfg(test)] use soroban_sdk::{ - testutils::{BytesN as _, Ledger}, - BytesN, String, + testutils::{Address as _, BytesN as _, Ledger}, + Address, BytesN, String, }; use super::common::setup; @@ -170,3 +170,112 @@ fn migrate_marks_current_version_and_blocks_replay() { .unwrap(); assert_eq!(err, Error::MigrationAlreadyApplied); } + +// AUTH REGRESSION GUARDS (#73) +// +// setup() mocks all auths for every address, so a call succeeding is not +// proof that require_admin() ran — it succeeds identically whether the +// check is present or was deleted. These tests replace the mock with an +// empty auth set so the call can only succeed if the contract explicitly +// requests and receives the admin's authorization. If require_admin() is +// ever removed from one of these entrypoints, the call stops requesting +// auth altogether and runs to completion instead of failing here, +// turning the test red. + +#[test] +fn pause_reverts_without_admin_auth() { + let ctx = setup(250); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_pause(); + assert!(err.is_err(), "pause must require admin auth"); +} + +#[test] +fn unpause_reverts_without_admin_auth() { + let ctx = setup(250); + ctx.client.pause(); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_unpause(); + assert!(err.is_err(), "unpause must require admin auth"); +} + +#[test] +fn set_admin_reverts_without_admin_auth() { + let ctx = setup(250); + let new_admin = Address::generate(&ctx.env); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_set_admin(&new_admin); + assert!(err.is_err(), "set_admin must require admin auth"); +} + +#[test] +fn set_fee_bps_reverts_without_admin_auth() { + let ctx = setup(250); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_set_fee_bps(&300); + assert!(err.is_err(), "set_fee_bps must require admin auth"); +} + +#[test] +fn set_fee_account_reverts_without_admin_auth() { + let ctx = setup(250); + let new_account = Address::generate(&ctx.env); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_set_fee_account(&new_account); + assert!(err.is_err(), "set_fee_account must require admin auth"); +} + +#[test] +fn set_profile_contract_reverts_without_admin_auth() { + let ctx = setup(250); + let new_profile = Address::generate(&ctx.env); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_set_profile_contract(&new_profile); + assert!(err.is_err(), "set_profile_contract must require admin auth"); +} + +#[test] +fn propose_upgrade_reverts_without_admin_auth() { + let ctx = setup(250); + let new_hash: BytesN<32> = BytesN::random(&ctx.env); + let new_version = String::from_str(&ctx.env, "0.3.0"); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_propose_upgrade(&new_hash, &new_version); + assert!(err.is_err(), "propose_upgrade must require admin auth"); +} + +#[test] +fn apply_upgrade_reverts_without_admin_auth() { + let ctx = setup(250); + let new_hash: BytesN<32> = BytesN::random(&ctx.env); + let new_version = String::from_str(&ctx.env, "0.3.0"); + ctx.client.propose_upgrade(&new_hash, &new_version); + + ctx.env.ledger().with_mut(|li| { + li.sequence_number += UPGRADE_TIMELOCK_LEDGERS; + }); + + ctx.env.mock_auths(&[]); + let err = ctx.client.try_apply_upgrade(); + assert!(err.is_err(), "apply_upgrade must require admin auth"); +} + +#[test] +fn cancel_pending_upgrade_reverts_without_admin_auth() { + let ctx = setup(250); + let new_hash: BytesN<32> = BytesN::random(&ctx.env); + let new_version = String::from_str(&ctx.env, "0.3.0"); + ctx.client.propose_upgrade(&new_hash, &new_version); + + ctx.env.mock_auths(&[]); + let err = ctx.client.try_cancel_pending_upgrade(); + assert!(err.is_err(), "cancel_pending_upgrade must require admin auth"); +} + +#[test] +fn migrate_reverts_without_admin_auth() { + let ctx = setup(250); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_migrate(); + assert!(err.is_err(), "migrate must require admin auth"); +} \ No newline at end of file diff --git a/contracts/profile/src/tests/admin.rs b/contracts/profile/src/tests/admin.rs index bcb2b0a..f4a802f 100644 --- a/contracts/profile/src/tests/admin.rs +++ b/contracts/profile/src/tests/admin.rs @@ -250,3 +250,136 @@ fn migrate_marks_version_and_blocks_replay_profile() { .unwrap(); assert_eq!(err, Error::MigrationAlreadyApplied); } + +// ============================================================ +// AUTH REGRESSION GUARDS (#73) +// +// Mirror of the events-contract guards in +// contracts/events/src/tests/admin.rs — see that file for the rationale. +// ============================================================ + +#[test] +fn pause_reverts_without_admin_auth() { + let ctx = setup(); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_pause(); + assert!(err.is_err(), "pause must require admin auth"); +} + +#[test] +fn unpause_reverts_without_admin_auth() { + let ctx = setup(); + ctx.client.pause(); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_unpause(); + assert!(err.is_err(), "unpause must require admin auth"); +} + +#[test] +fn set_admin_reverts_without_admin_auth() { + let ctx = setup(); + let new_admin = Address::generate(&ctx.env); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_set_admin(&new_admin); + assert!(err.is_err(), "set_admin must require admin auth"); +} + +#[test] +fn set_events_contract_reverts_without_admin_auth() { + let ctx = setup(); + let events = Address::generate(&ctx.env); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_set_events_contract(&events); + assert!(err.is_err(), "set_events_contract must require admin auth"); +} + +#[test] +fn propose_events_contract_reverts_without_admin_auth() { + let ctx = setup(); + let events_a = Address::generate(&ctx.env); + ctx.client.set_events_contract(&events_a); + + let events_b = Address::generate(&ctx.env); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_propose_events_contract(&events_b); + assert!(err.is_err(), "propose_events_contract must require admin auth"); +} + +#[test] +fn accept_events_contract_reverts_without_admin_auth() { + let ctx = setup(); + let events_a = Address::generate(&ctx.env); + ctx.client.set_events_contract(&events_a); + let events_b = Address::generate(&ctx.env); + let start = ctx.env.ledger().sequence(); + ctx.client.propose_events_contract(&events_b); + ctx.env.ledger().with_mut(|li| { + li.sequence_number = start + EVENTS_CONTRACT_TIMELOCK_LEDGERS + 1; + }); + + ctx.env.mock_auths(&[]); + let err = ctx.client.try_accept_events_contract(); + assert!(err.is_err(), "accept_events_contract must require admin auth"); +} + +#[test] +fn cancel_pending_events_contract_reverts_without_admin_auth() { + let ctx = setup(); + let events_a = Address::generate(&ctx.env); + ctx.client.set_events_contract(&events_a); + let events_b = Address::generate(&ctx.env); + ctx.client.propose_events_contract(&events_b); + + ctx.env.mock_auths(&[]); + let err = ctx.client.try_cancel_pending_events_contract(); + assert!( + err.is_err(), + "cancel_pending_events_contract must require admin auth" + ); +} + +#[test] +fn propose_upgrade_reverts_without_admin_auth() { + let ctx = setup(); + let new_hash: BytesN<32> = BytesN::random(&ctx.env); + let new_version = String::from_str(&ctx.env, "0.3.0"); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_propose_upgrade(&new_hash, &new_version); + assert!(err.is_err(), "propose_upgrade must require admin auth"); +} + +#[test] +fn apply_upgrade_reverts_without_admin_auth() { + let ctx = setup(); + let new_hash: BytesN<32> = BytesN::random(&ctx.env); + let new_version = String::from_str(&ctx.env, "0.3.0"); + ctx.client.propose_upgrade(&new_hash, &new_version); + + ctx.env.ledger().with_mut(|li| { + li.sequence_number += UPGRADE_TIMELOCK_LEDGERS; + }); + + ctx.env.mock_auths(&[]); + let err = ctx.client.try_apply_upgrade(); + assert!(err.is_err(), "apply_upgrade must require admin auth"); +} + +#[test] +fn cancel_pending_upgrade_reverts_without_admin_auth() { + let ctx = setup(); + let new_hash: BytesN<32> = BytesN::random(&ctx.env); + let new_version = String::from_str(&ctx.env, "0.3.0"); + ctx.client.propose_upgrade(&new_hash, &new_version); + + ctx.env.mock_auths(&[]); + let err = ctx.client.try_cancel_pending_upgrade(); + assert!(err.is_err(), "cancel_pending_upgrade must require admin auth"); +} + +#[test] +fn migrate_reverts_without_admin_auth() { + let ctx = setup(); + ctx.env.mock_auths(&[]); + let err = ctx.client.try_migrate(); + assert!(err.is_err(), "migrate must require admin auth"); +} From 2858ca8538ac74969ae46dd50d74f0e27b43aab0 Mon Sep 17 00:00:00 2001 From: sendi0011 Date: Fri, 17 Jul 2026 02:08:51 +0100 Subject: [PATCH 2/3] fix: update code format to maintain consistency --- contracts/events/src/tests/admin.rs | 7 +++++-- contracts/profile/src/tests/admin.rs | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/contracts/events/src/tests/admin.rs b/contracts/events/src/tests/admin.rs index 1223491..d3a29c2 100644 --- a/contracts/events/src/tests/admin.rs +++ b/contracts/events/src/tests/admin.rs @@ -269,7 +269,10 @@ fn cancel_pending_upgrade_reverts_without_admin_auth() { ctx.env.mock_auths(&[]); let err = ctx.client.try_cancel_pending_upgrade(); - assert!(err.is_err(), "cancel_pending_upgrade must require admin auth"); + assert!( + err.is_err(), + "cancel_pending_upgrade must require admin auth" + ); } #[test] @@ -278,4 +281,4 @@ fn migrate_reverts_without_admin_auth() { ctx.env.mock_auths(&[]); let err = ctx.client.try_migrate(); assert!(err.is_err(), "migrate must require admin auth"); -} \ No newline at end of file +} diff --git a/contracts/profile/src/tests/admin.rs b/contracts/profile/src/tests/admin.rs index f4a802f..d0dd7ac 100644 --- a/contracts/profile/src/tests/admin.rs +++ b/contracts/profile/src/tests/admin.rs @@ -302,7 +302,10 @@ fn propose_events_contract_reverts_without_admin_auth() { let events_b = Address::generate(&ctx.env); ctx.env.mock_auths(&[]); let err = ctx.client.try_propose_events_contract(&events_b); - assert!(err.is_err(), "propose_events_contract must require admin auth"); + assert!( + err.is_err(), + "propose_events_contract must require admin auth" + ); } #[test] @@ -319,7 +322,10 @@ fn accept_events_contract_reverts_without_admin_auth() { ctx.env.mock_auths(&[]); let err = ctx.client.try_accept_events_contract(); - assert!(err.is_err(), "accept_events_contract must require admin auth"); + assert!( + err.is_err(), + "accept_events_contract must require admin auth" + ); } #[test] @@ -373,7 +379,10 @@ fn cancel_pending_upgrade_reverts_without_admin_auth() { ctx.env.mock_auths(&[]); let err = ctx.client.try_cancel_pending_upgrade(); - assert!(err.is_err(), "cancel_pending_upgrade must require admin auth"); + assert!( + err.is_err(), + "cancel_pending_upgrade must require admin auth" + ); } #[test] From ed7174b4f5f0e623223b96088d13c76c59513c7d Mon Sep 17 00:00:00 2001 From: sendi0011 Date: Fri, 17 Jul 2026 20:38:56 +0100 Subject: [PATCH 3/3] test(admin): add accept_admin target-auth coverage and reinforce admin_slash accept_admin does not call require_admin() in either contract -- it authorizes against the pending target address instead. Add a negative test (empty mock set) proving that call fails without any auth, and a positive test proving the auth demanded is specifically the pending target's via env.auths(), for both boundless-events and boundless-profile. Also add admin_slash_demands_admins_auth_specifically to reputation.rs, complementing the existing admin_slash_rejects_non_admin_caller (from #60) by proving the auth demanded on a normal call is specifically the admin's, not just any address mock_all_auths() happens to approve. --- contracts/events/src/tests/admin.rs | 41 +++++++++++++++++++++++ contracts/profile/src/tests/admin.rs | 41 +++++++++++++++++++++++ contracts/profile/src/tests/reputation.rs | 16 +++++++++ 3 files changed, 98 insertions(+) diff --git a/contracts/events/src/tests/admin.rs b/contracts/events/src/tests/admin.rs index d3a29c2..def308a 100644 --- a/contracts/events/src/tests/admin.rs +++ b/contracts/events/src/tests/admin.rs @@ -282,3 +282,44 @@ fn migrate_reverts_without_admin_auth() { let err = ctx.client.try_migrate(); assert!(err.is_err(), "migrate must require admin auth"); } + +// ============================================================ +// ACCEPT_ADMIN — target-auth guard +// +// accept_admin does not call require_admin(); it authorizes against the +// pending target address instead (pending.target.require_auth()). These +// two tests prove that guard independently of the require_admin() guards +// above: the empty-mock test shows *some* auth is demanded, and the +// auths() check shows it is demanded from the pending target +// specifically, not just any address mock_all_auths() happens to cover. +// ============================================================ + +#[test] +fn accept_admin_reverts_without_targets_auth() { + let ctx = setup(250); + let new_admin = Address::generate(&ctx.env); + ctx.client.set_admin(&new_admin); + + ctx.env.mock_auths(&[]); + let err = ctx.client.try_accept_admin(); + assert!( + err.is_err(), + "accept_admin must require the pending target's auth" + ); +} + +#[test] +fn accept_admin_demands_pending_targets_auth_specifically() { + let ctx = setup(250); + let new_admin = Address::generate(&ctx.env); + ctx.client.set_admin(&new_admin); + + ctx.client.accept_admin(); + + let auths = ctx.env.auths(); + let target_required = auths.iter().any(|(addr, _)| *addr == new_admin); + assert!( + target_required, + "accept_admin must demand the pending target's own auth" + ); +} diff --git a/contracts/profile/src/tests/admin.rs b/contracts/profile/src/tests/admin.rs index d0dd7ac..5c1bfd7 100644 --- a/contracts/profile/src/tests/admin.rs +++ b/contracts/profile/src/tests/admin.rs @@ -392,3 +392,44 @@ fn migrate_reverts_without_admin_auth() { let err = ctx.client.try_migrate(); assert!(err.is_err(), "migrate must require admin auth"); } + +// ============================================================ +// ACCEPT_ADMIN — target-auth guard +// +// accept_admin does not call require_admin(); it authorizes against the +// pending target address instead (pending.target.require_auth()). These +// two tests prove that guard independently of the require_admin() guards +// above: the empty-mock test shows *some* auth is demanded, and the +// auths() check shows it is demanded from the pending target +// specifically, not just any address mock_all_auths() happens to cover. +// ============================================================ + +#[test] +fn accept_admin_reverts_without_targets_auth() { + let ctx = setup(); + let new_admin = Address::generate(&ctx.env); + ctx.client.set_admin(&new_admin); + + ctx.env.mock_auths(&[]); + let err = ctx.client.try_accept_admin(); + assert!( + err.is_err(), + "accept_admin must require the pending target's auth" + ); +} + +#[test] +fn accept_admin_demands_pending_targets_auth_specifically() { + let ctx = setup(); + let new_admin = Address::generate(&ctx.env); + ctx.client.set_admin(&new_admin); + + ctx.client.accept_admin(); + + let auths = ctx.env.auths(); + let target_required = auths.iter().any(|(addr, _)| *addr == new_admin); + assert!( + target_required, + "accept_admin must demand the pending target's own auth" + ); +} diff --git a/contracts/profile/src/tests/reputation.rs b/contracts/profile/src/tests/reputation.rs index 8274625..cc47fe3 100644 --- a/contracts/profile/src/tests/reputation.rs +++ b/contracts/profile/src/tests/reputation.rs @@ -418,3 +418,19 @@ fn admin_slash_rejects_non_admin_caller() { .try_admin_slash_reputation(&user, &1, &admin_reason(&ctx), &op_id(&ctx)); assert!(res.is_err(), "non-admin admin_slash must be rejected"); } + +#[test] +fn admin_slash_demands_admins_auth_specifically() { + // Complements admin_slash_rejects_non_admin_caller above (already in + // the codebase since #60): that test proves *some* auth is required; + // this one proves the auth demanded under a normal call is + // specifically the admin's, not just any address mock_all_auths() + // happens to approve. + let (ctx, user) = setup_with_user(); + ctx.client + .admin_slash_reputation(&user, &1, &admin_reason(&ctx), &op_id(&ctx)); + + let auths = ctx.env.auths(); + let admin_required = auths.iter().any(|(addr, _)| *addr == ctx.admin); + assert!(admin_required, "admin_slash must demand the admin's auth"); +}