3 Commits
Author SHA1 Message Date
Thomas VincentandTheWitness 055c5ef5fa security: reject LDAP logins that skip a domain bind (#8009)
* hardening: validate the login realm and escape the LDAP username per context

domains_login_process() now rejects a realm get_auth_realms() does not list and
fails closed on one that is listed but has no domain behind it, and the Ldap
class escapes the username for the DN and the search filter separately rather
than relying on a filter-only blocklist.

- an unlisted realm sent with a non-empty password used to match no branch at
  all and returned an empty user with no error, which auth_login.php reads as
  an authenticated user and provisions from the template account

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* security: reject LDAP logins that skip a domain bind

Posted realms outside user_domains no longer fall through to template or guest login.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(ldap): align escaping contract with username lookup

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: cover LDAP domain login through OpenLDAP

The e2e stack now binds a real directory user and checks that a forged realm cannot skip that bind.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(ldap): address Copilot review feedback

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: substitute LDAP filter placeholders in one pass

LDAP_ESCAPE_FILTER leaves angle brackets intact, so a looped str_replace can rescan one value as another key.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Change realm retrieval function from gnrv to gfrv

* tests: add gfrv() stub to harnesses that exercise gfrv()-calling code

domains_login_process() and api_aggregate_convert_template() call gfrv() for
realm/aggregate_template_id, but their test harnesses only stubbed gnrv(),
so Pest failed with 'Call to undefined function gfrv()'.

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-09-12 20:49:55 -04:00
Sean Mancini 96033267cb refactor(tests): reorganize unit tests into domain category folders (#7733)
* refactor(tests): reorganize unit tests into domain category folders

- Consolidate per-advisory GHSA regression tests into Security/ subfolders by vuln class
- Group all 186 test files into domain folders: DataCollection/, Ui/, Core/,
  Database/, Installer/, Scripts/, Security/, Plugin/
- Strip GitHub issue/GHSA numbers from filenames; keep IDs in test descriptions
- Fix all __DIR__ relative paths to match new folder nesting depth
- Update CI workflow path triggers and phpunit XML configs for new file locations
- Update .github/copilot-instructions.md with unit test organization guidelines
- Add tests/vendor/ to .gitignore

* add gaurds

* fix up guard for unit tests

* fix redeclaration of cacti_log in tests

* fix updates to unit tests

* fix test patjhs

* fix: update test __DIR__ paths for reorganized test structure

* fix: conditionally define read_user_setting to prevent redeclaration in tests

* fix: conditionally define functions to prevent redeclaration in tests

- Wrap clean_up_lines, raise_message, cacti_debug_backtrace, and get_debug_prefix
  with function_exists() checks to prevent conflicts when test stubs are loaded
  before lib/functions.php in the same PHP process

* fix: add unit test bootstrap to prevent function redeclaration errors

- Create tests/bootstrap-unit.php to load core libraries before tests
- Ensures lib/functions.php loads before test stubs define stub functions
- Test stubs use function_exists() checks, so they detect pre-loaded functions
- Update phpunit.xml to use bootstrap-unit.php for Unit testsuite
- Fixes 'Cannot redeclare read_user_setting()' errors when tests run in same process

* fix: load test stubs before global.php in unit test bootstrap

- Reorder bootstrap to load CactiStubs.php and UnitStubs.php first
- Ensures stub functions like __() are available when global.php loads
- global.php calls get_cacti_version_text() which needs __() for translations
- Fixes 'Call to undefined function __()' error during bootstrap

* fix: remove function conflicts from test stubs

- Remove stub definitions for functions already in lib/functions.php:
  cacti_sizeof, cacti_log, cacti_strtolower, read_config_option,
  read_user_setting, cacti_count
- Keep translation function stubs (__(), __esc()) needed by global.php
- Update bootstrap to load lib/functions.php after stubs
- This prevents function redeclaration errors while maintaining test isolation

* fix: prevent constant and variable redeclaration in bootstrap

- Remove explicit PHP_TESTING and CACTI_TEST_BOOTSTRAP defines from bootstrap
  (CactiStubs.php now handles this with guards)
- Add guards to CactiStubs.php to prevent redefinition of PHP_TESTING
- Only set CACTI_TEST_BOOTSTRAP env vars if not already set
- Fixes PHP warnings about 'Constant X already defined'

* fix: remove redundant lib/functions.php load from bootstrap

include/global.php already loads lib/functions.php at lines 287-288.
The bootstrap was trying to load it again after global.php, but global.php
needs format_cacti_version() to be available on lines 293-294 to define
CACTI_VERSION and CACTI_VERSION_FULL constants.

Solution: Let global.php handle loading lib/functions.php itself, and don't
load it separately in the bootstrap.

* fix: remove more conflicting function stubs from test helpers

- Remove stubs for functions also defined in lib/functions.php:
  raise_message, clean_up_lines, cacti_debug_backtrace, get_debug_prefix
- Remove POLLER_ID constant stub (defined unconditionally by global.php)
- Keep only translation functions and test-specific helpers
- This prevents function/constant redeclaration errors

* fix: minimize test stubs to only translation functions

Remove ALL helper functions and constants from UnitStubs.php that are:
- Defined in lib/functions.php (raise_message, clean_up_lines, etc.)
- Defined in lib/html_utility.php (srv, set_request_var)
- Defined in global_constants.php (POLLER_VERBOSITY_*, MESSAGE_LEVEL_*)
- Defined in global.php (POLLER_ID)

Keep ONLY translation functions (__(), __esc()) that are:
- Called by global.php during initialization
- Not defined elsewhere
- Required for proper bootstrap sequence

This eliminates all constant and function redeclaration errors.

* fix: remove __esc stub - already defined in global_languages.php

Keep only __() translation function stub, which is needed by global.php
and not defined elsewhere in Cacti core.

* fix: handle no-arg case in __() translation stub

The __() function may be called with just a string and no format arguments.
Previously, vsprintf() would fail with 'arguments array must contain 2 items'.
Now, if no args are provided, return the text as-is.
2026-08-16 19:23:31 -07:00
Thomas VincentandTheWitness aa93dc247d fix(aggregate): restore item-override saves and bulk-action input names (#7338)
* fix(aggregate): restore item-override saves and bulk-action input names

The form_save() gate dropped save_component_item during the develop
refactor, so item-override saves redirected away unsaved, and the
migrate/combine confirmation dialogs posted their input as
title_format while the handlers read aggregate_template_id and
aggregate_name.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: relax aggregate form-wiring regex assertions

Match the gate and extra-array checks without pinning term order or sub-array key position.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(aggregate): surface error when aggregate template is missing

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(aggregate): catch title_format extra input in any key position

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: add space before return-type colon

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(aggregate): validate request input on convert and combine paths

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: refresh security baselines

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(aggregate): verify convert-template guard writes nothing on bad template

tests/Unit/AggregateFormFieldNamesTest.php only regex-matches the guard
clause in source; this runs api_aggregate_convert_template() against a
real sqlite-backed connection and confirms a missing/unmatched
aggregate_template_id leaves aggregate_graphs and aggregate_graphs_items
untouched.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci(hardening): allow test-fixture superglobal writes in aggregate guard test

tests/integration/AggregateConvertTemplateGuardIntegrationTest.php sets
$_REQUEST directly to simulate request state before calling the real
api_aggregate_convert_template(), which reads aggregate_template_id via
gnrv(). Same pattern already used in tests/HandOff/CactiDispatchHandOffTest.php
and tests/HandOff/CactiSettingsHandOffTest.php.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(aggregate): restore db globals after convert-template guard test

issue7338_wire_default_connection() overwrote $database_hostname/
$database_port/$database_default and added a fake PDO to
$database_sessions without ever resetting them, so any later test in the
same Pest process that hit a db_* helper resolved against the stale
sqlite handle instead of its own fixture -- which crashed the whole
suite before the HandOff tests ran.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Change function to retrieve aggregate template ID

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-08-05 10:30:04 -04:00