Incorrect return type hint

Created on 22 August 2024, 4 months ago
Updated 9 September 2024, 3 months ago

Problem/Motivation

sanitizeUserDataResponse() can return NULL, however getUserDataByIdentifier() doesn't say that it can return NULL.

  /**
   * Fetch user data from server by Identifier.
   *
   * @param string $identifier
   *   User identifier.
   *
   * @return \Symfony\Component\Ldap\Entry|false
   *
   *   This should go into LdapUserProcessor or LdapUserManager, leaning toward
   *   the former.
   */
  public function getUserDataByIdentifier(string $identifier) {
    if (!$this->checkAvailability()) {
      return FALSE;
    }

    // Try to retrieve the user from the cache.
    $cache = $this->cache->get('ldap_servers:user_data:' . $identifier);
    if ($cache && $cache->data) {
      return $cache->data;
    }

    $ldap_entry = $this->queryAllBaseDnLdapForUsername($identifier);
    if ($ldap_entry) {
      $ldap_entry = $this->sanitizeUserDataResponse($ldap_entry, $identifier);
      $cache_expiry = 5 * 60 + time();
      $cache_tags = ['ldap', 'ldap_servers', 'ldap_servers.user_data'];
      $this->cache->set('ldap_servers:user_data:' . $identifier, $ldap_entry, $cache_expiry, $cache_tags);
    }

    return $ldap_entry;
  }
  /**
   * Sanitize user data response.
   *
   * @param \Symfony\Component\Ldap\Entry $entry
   *   LDAP entry.
   * @param string $drupal_username
   *   Drupal username.
   *
   * @return \Symfony\Component\Ldap\Entry|null
   *   LDAP Entry.
   */
  public function sanitizeUserDataResponse(Entry $entry, string $drupal_username): ?Entry {
    if ($this->server->get('bind_method') === 'anon_user') {
      return $entry;
    }

    // Filter out results with spaces added before or after, which are
    // considered OK by LDAP but are no good for us. Some setups have multiple
    // $nameAttribute per entry, so we loop through all possible options.
    $attribute_values = $entry->getAttribute($this->server->getAuthenticationNameAttribute(), FALSE);
    if ($attribute_values) {
      foreach ($attribute_values as $attribute_value) {
        if (mb_strtolower(trim($attribute_value)) === mb_strtolower($drupal_username)) {
          return $entry;
        }
      }
    }
    return NULL;
  }

Debug Report

Proposed resolution

Fix the return type hint to be \Symfony\Component\Ldap\Entry|null|false

Remaining tasks

Fix the return type hint.

Also, ideally, all of these functions would throw an exception if the LDAP server is unavailable, rather than returning a falsy value. It would be easier to have bubbling exceptions and catch them.

User interface changes

API changes

Data model changes

πŸ“Œ Task
Status

Active

Version

4.0

Component

Code

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Production build 0.71.5 2024