Complex classes like LdapService often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use LdapService, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 19 | class LdapService |
||
| 20 | { |
||
| 21 | protected $ldap; |
||
| 22 | protected $ldapConnection; |
||
| 23 | protected $config; |
||
| 24 | protected $userRepo; |
||
| 25 | protected $enabled; |
||
| 26 | |||
| 27 | /** |
||
| 28 | * LdapService constructor. |
||
| 29 | * |
||
| 30 | * @param Ldap $ldap |
||
| 31 | * @param \Integrations\Models\UserRepo $userRepo |
||
| 32 | */ |
||
| 33 | public function __construct(Access\Ldap $ldap, UserRepo $userRepo) |
||
| 40 | |||
| 41 | /** |
||
| 42 | * Check if groups should be synced. |
||
| 43 | * |
||
| 44 | * @return bool |
||
| 45 | */ |
||
| 46 | public function shouldSyncGroups() |
||
| 50 | |||
| 51 | /** |
||
| 52 | * Search for attributes for a specific user on the ldap |
||
| 53 | * |
||
| 54 | * @param string $userName |
||
| 55 | * @param array $attributes |
||
| 56 | * @return null|array |
||
| 57 | * @throws LdapException |
||
| 58 | */ |
||
| 59 | private function getUserWithAttributes($userName, $attributes) |
||
| 77 | |||
| 78 | /** |
||
| 79 | * Get the details of a user from LDAP using the given username. |
||
| 80 | * User found via configurable user filter. |
||
| 81 | * |
||
| 82 | * @param $userName |
||
| 83 | * @return array|null |
||
| 84 | * @throws LdapException |
||
| 85 | */ |
||
| 86 | public function getUserDetails($userName) |
||
| 105 | |||
| 106 | /** |
||
| 107 | * Get a property from an LDAP user response fetch. |
||
| 108 | * Handles properties potentially being part of an array. |
||
| 109 | * |
||
| 110 | * @param array $userDetails |
||
| 111 | * @param string $propertyKey |
||
| 112 | * @param $defaultValue |
||
| 113 | * @return mixed |
||
| 114 | */ |
||
| 115 | protected function getUserResponseProperty(array $userDetails, string $propertyKey, $defaultValue) |
||
| 123 | |||
| 124 | /** |
||
| 125 | * @param Authenticatable $user |
||
| 126 | * @param string $username |
||
| 127 | * @param string $password |
||
| 128 | * @return bool |
||
| 129 | * @throws LdapException |
||
| 130 | */ |
||
| 131 | public function validateUserCredentials(Authenticatable $user, $username, $password) |
||
| 151 | |||
| 152 | /** |
||
| 153 | * Bind the system user to the LDAP connection using the given credentials |
||
| 154 | * otherwise anonymous access is attempted. |
||
| 155 | * |
||
| 156 | * @param $connection |
||
| 157 | * @throws LdapException |
||
| 158 | */ |
||
| 159 | protected function bindSystemUser($connection) |
||
| 175 | |||
| 176 | /** |
||
| 177 | * Get the connection to the LDAP server. |
||
| 178 | * Creates a new connection if one does not exist. |
||
| 179 | * |
||
| 180 | * @return resource |
||
| 181 | * @throws LdapException |
||
| 182 | */ |
||
| 183 | protected function getConnection() |
||
| 226 | |||
| 227 | /** |
||
| 228 | * Build a filter string by injecting common variables. |
||
| 229 | * |
||
| 230 | * @param string $filterString |
||
| 231 | * @param array $attrs |
||
| 232 | * @return string |
||
| 233 | */ |
||
| 234 | protected function buildFilter($filterString, array $attrs) |
||
| 243 | |||
| 244 | /** |
||
| 245 | * Get the groups a user is a part of on ldap |
||
| 246 | * |
||
| 247 | * @param string $userName |
||
| 248 | * @return array |
||
| 249 | * @throws LdapException |
||
| 250 | */ |
||
| 251 | public function getUserGroups($userName) |
||
| 264 | |||
| 265 | /** |
||
| 266 | * Get the parent groups of an array of groups |
||
| 267 | * |
||
| 268 | * @param array $groupsArray |
||
| 269 | * @param array $checked |
||
| 270 | * @return array |
||
| 271 | * @throws LdapException |
||
| 272 | */ |
||
| 273 | private function getGroupsRecursive($groupsArray, $checked) |
||
| 293 | |||
| 294 | /** |
||
| 295 | * Get the parent groups of a single group |
||
| 296 | * |
||
| 297 | * @param string $groupName |
||
| 298 | * @return array |
||
| 299 | * @throws LdapException |
||
| 300 | */ |
||
| 301 | private function getGroupGroups($groupName) |
||
| 321 | |||
| 322 | /** |
||
| 323 | * Filter out LDAP CN and DN language in a ldap search return |
||
| 324 | * Gets the base CN (common name) of the string |
||
| 325 | * |
||
| 326 | * @param array $userGroupSearchResponse |
||
| 327 | * @return array |
||
| 328 | */ |
||
| 329 | protected function groupFilter(array $userGroupSearchResponse) |
||
| 348 | |||
| 349 | /** |
||
| 350 | * Sync the LDAP groups to the user roles for the current user |
||
| 351 | * |
||
| 352 | * @param \App\Models\User $user |
||
| 353 | * @param string $username |
||
| 354 | * @throws LdapException |
||
| 355 | */ |
||
| 356 | public function syncGroups(User $user, string $username) |
||
| 371 | |||
| 372 | /** |
||
| 373 | * Match an array of group names from LDAP to App system roles. |
||
| 374 | * Formats LDAP group names to be lower-case and hyphenated. |
||
| 375 | * |
||
| 376 | * @param array $groupNames |
||
| 377 | * @return \Illuminate\Support\Collection |
||
| 378 | */ |
||
| 379 | protected function matchLdapGroupsToSystemsRoles(array $groupNames) |
||
| 402 | |||
| 403 | /** |
||
| 404 | * Check a role against an array of group names to see if it matches. |
||
| 405 | * Checked against role 'external_auth_id' if set otherwise the name of the role. |
||
| 406 | * |
||
| 407 | * @param \Porteiro\Models\Role $role |
||
| 408 | * @param array $groupNames |
||
| 409 | * @return bool |
||
| 410 | */ |
||
| 411 | protected function roleMatchesGroupNames(Role $role, array $groupNames) |
||
| 426 | } |
||
| 427 |
If you access a property on an interface, you most likely code against a concrete implementation of the interface.
Available Fixes
Adding an additional type check:
Changing the type hint: