Code analysis: potential PHP 8+ and lifecycle regressions

A static analysis audit of the plugin codebase to identify potential type-safety regressions (similar to the $CFG_GLPI / setup.php issue) and edge cases under PHP 8.0+.

---

1. Unsafe core string functions (TypeErrors on null/integer values)

In PHP 8.0+, core functions such as strpos(), strstr(), explode(), and strlen() throw fatal TypeError exceptions if they receive parameters of incorrect types (like null or int), whereas PHP 7.x silently cast them or emitted minor notices.

Identified risk areas:

A. User-Agent Checking without existence checks in src/Exclude.php

• Code:

  !empty($exclude[Exclude::CLIENTAGENT]) &&
  strpos($_SERVER['HTTP_USER_AGENT'], $exclude[Exclude::CLIENTAGENT]) !== false
  

• Risk: If a CLI script, API request, or automated script triggers Exclude::isExcluded() without sending an HTTP_USER_AGENT header, $_SERVER['HTTP_USER_AGENT'] is undefined (null). Under PHP 8.1+, passing null as the haystack to strpos() throws a fatal TypeError.
• Fix: Guard with ?? '':

  strpos($_SERVER['HTTP_USER_AGENT'] ?? '', $exclude[Exclude::CLIENTAGENT]) !== false
  

B. Nullable auth context in src/Config/ConfigEntity.php

• Code:

  public function getRequestedAuthnContextArray(): array
  {
      if (strstr($this->fields[ConfigEntity::AUTHN_CONTEXT], ':')) {
          return explode(':', $this->fields[ConfigEntity::AUTHN_CONTEXT]);
      } else {
          return [$this->fields[ConfigEntity::AUTHN_CONTEXT]];
      }
  }
  

• Risk: If the requested_authn_context column in the database contains null (e.g. from raw DB edits or incomplete template generation), strstr(null, ':') throws a fatal TypeError under PHP 8.0+.
• Fix: Cast to string or check nullability before calling:

  $context = (string)($this->fields[ConfigEntity::AUTHN_CONTEXT] ?? '');
  if (str_contains($context, ':')) {
      return explode(':', $context);
  }
  

C. Insecure keys in $_POST Loop in src/LoginFlow.php

• Code:

  foreach ($_POST as $key => $value) {
      if (strstr($key, 'login_name') && !empty($_POST[$key])) {
  

• Risk: If a malicious client posts a payload containing integer keys (e.g. <input name="123" />), PHP converts the key in $_POST to integer 123. Passing integer 123 to strstr() throws a TypeError: strstr(): Argument #1 ($haystack) must be of type string, int given.
• Fix: Force string casting or use str_contains():

  if (str_contains((string)$key, 'login_name') && !empty($_POST[$key])) {
  

---

2. Global state dependencies ($CFG_GLPI early-boot access)

Early-boot entry points (setup.php and hook.php) run before the GLPI configuration state is guaranteed to be loaded.

Identified risk areas:

A. Session Cleanup in CLI Context src/CronTask.php

• Code: GLPI cron tasks execute in a CLI context where HTTP headers and cookie contexts are unavailable.
• Risk: If any cron execution paths attempt to resolve web URLs or session attributes using $CFG_GLPI['url_base'] without verification, they will fail. (Our analysis shows this is currently handled cleanly by restricting direct session access inside the cron, but should remain a priority check for new developers).

---

3. Recommended defensive rules

To prevent regressions in future code updates, we recommend the following strict developer practices for samlsso:

1. Never pass variables directly to string/array functions without checking nullability (?? '' or casting (string)).
2. Never assume global variables (like $CFG_GLPI or $_SESSION) are active in files that run at inclusion time (setup.php, hook.php).
3. Always use modern PHP 8 alternative functions like str_contains instead of strstr or strpos where possible, as they make intent clearer and can be wrapped with explicit type checking.