Skip to content

Commit

Permalink
Merge pull request #5442 from jrjohnson/configurable-ldap-preffered-n…
Browse files Browse the repository at this point in the history 
…ames

Add configurable preferred names in ldap sync
  • Loading branch information
@michaelchadwick
michaelchadwick committed May 8, 2024
2 parents 97782e0 + 9028853 commit 8d9f12c
Show file tree
Hide file tree
Showing 6 changed files with 231 additions and 22 deletions.
22 changes: 12 additions & 10 deletions docs/authentication_and_users.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,15 @@ sudo -u apache bin/console --env=prod ilios:maintenance:set-config-value ldap_di
sudo -u apache bin/console --env=prod ilios:maintenance:set-config-value ldap_directory_username_property <your campus value>
```

| Property Name | Description | Example Value |
|------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------|
| ldap_directory_url | URL to connect to your LDAP server including protocol | ldaps://directory.campus.edu |
| ldap_directory_user | The user we will use to authenticate | uid=Ilios,ou=applications,dc=campus,dc=edu |
| ldap_directory_password | The bind password for your user | 123GoLdap! |
| ldap_directory_search_base | What scope in the directory we should user for users | ou=people,dc=campus,dc=edu |
| ldap_directory_campus_id_property | In the returned data for a user what property is unique and can be used to populate the campusId field in Ilios | eduIDNumber |
| ldap_directory_username_property | In the returned data for a user what property contains the username that links to the **cas**, **ldap**, or **shibboleth** authentication service | eduPersonPrincipalName |
| ldap_directory_first_name_property | In the returned data for a user what property contains the first name. If this isn't provided it will default to **givenName** | givenName |
| ldap_directory_last_name_property | In the returned data for a user what property contains the last name. If this isn't provided it will default to **sn** | sn |
| Property Name | Description | Example Value |
|----------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------|
| ldap_directory_url | URL to connect to your LDAP server including protocol | ldaps://directory.campus.edu |
| ldap_directory_user | The user we will use to authenticate | uid=Ilios,ou=applications,dc=campus,dc=edu |
| ldap_directory_password | The bind password for your user | 123GoLdap! |
| ldap_directory_search_base | What scope in the directory we should user for users | ou=people,dc=campus,dc=edu |
| ldap_directory_campus_id_property | In the returned data for a user what property is unique and can be used to populate the campusId field in Ilios | eduIDNumber |
| ldap_directory_username_property | In the returned data for a user what property contains the username that links to the **cas**, **ldap**, or **shibboleth** authentication service | eduPersonPrincipalName |
| ldap_directory_first_name_property | In the returned data for a user what property contains the first name. If this isn't provided it will default to **givenName** | givenName |
| ldap_directory_last_name_property | In the returned data for a user what property contains the last name. If this isn't provided it will default to **sn** | sn |
| ldap_directory_preferred_first_name_property | In the returned data for a user what property contains the users preferred first name | preferredFirstName |
| ldap_directory_preferred_last_name_property | In the returned data for a user what property contains the users preferred last name | preferredLastName |
14 changes: 8 additions & 6 deletions src/Command/SyncAllUsersCommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,22 +125,24 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->pendingUserUpdateRepository->update($pendingUpdate, false);
}
}
$computedFirstName = $recordArray['preferredFirstName'] ?? $recordArray['firstName'];
$computedLastName = $recordArray['preferredLastName'] ?? $recordArray['lastName'];

if ($fixSmallThings && $user->getFirstName() != $recordArray['firstName']) {
if ($fixSmallThings && $user->getFirstName() != $computedFirstName) {
$update = true;
$output->writeln(
' <comment>[I] Updating first name from "' . $user->getFirstName() .
'" to "' . $recordArray['firstName'] . '".</comment>'
'" to "' . $computedFirstName . '".</comment>'
);
$user->setFirstName($recordArray['firstName']);
$user->setFirstName($computedFirstName);
}
if ($fixSmallThings && $user->getLastName() != $recordArray['lastName']) {
if ($fixSmallThings && $user->getLastName() != $computedLastName) {
$update = true;
$output->writeln(
' <comment>[I] Updating last name from "' . $user->getLastName() .
'" to "' . $recordArray['lastName'] . '".</comment>'
'" to "' . $computedLastName . '".</comment>'
);
$user->setLastName($recordArray['lastName']);
$user->setLastName($computedLastName);
}
if ($fixSmallThings && $user->getDisplayName() != $recordArray['displayName']) {
$update = true;
Expand Down
12 changes: 6 additions & 6 deletions src/Command/SyncUserCommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
'Display Name',
'Pronouns',
'Email',
'Phone Number'
'Phone Number',
])
->setRows([
[
Expand All @@ -87,12 +87,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int
[
'Directory User',
$userRecord['campusId'],
$userRecord['firstName'],
$userRecord['lastName'],
$userRecord['preferredFirstName'] ?? $userRecord['firstName'],
$userRecord['preferredLastName'] ?? $userRecord['lastName'],
$userRecord['displayName'],
$userRecord['pronouns'],
$userRecord['email'],
$userRecord['telephoneNumber']
$userRecord['telephoneNumber'],
]
])
;
Expand All @@ -107,8 +107,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
);

if ($helper->ask($input, $output, $question)) {
$user->setFirstName($userRecord['firstName']);
$user->setLastName($userRecord['lastName']);
$user->setFirstName($userRecord['preferredFirstName'] ?? $userRecord['firstName']);
$user->setLastName($userRecord['preferredLastName'] ?? $userRecord['lastName']);
$user->setDisplayName($userRecord['displayName']);
$user->setPronouns($userRecord['pronouns']);
$user->setEmail($userRecord['email']);
Expand Down
8 changes: 8 additions & 0 deletions src/Service/LdapManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ public function search(string $filter): array
$ldapPronounsProperty = $this->config->get('ldap_directory_pronouns_property');
$ldapFirstNameProperty = $this->config->get('ldap_directory_first_name_property') ?? 'givenName';
$ldapLastNameProperty = $this->config->get('ldap_directory_last_name_property') ?? 'sn';
$ldapPreferredFirstNameProperty = $this->config->get('ldap_directory_preferred_first_name_property');
$ldapPreferredLastNameProperty = $this->config->get('ldap_directory_preferred_last_name_property');

$rhett = [];
try {
Expand All @@ -52,6 +54,12 @@ public function search(string $filter): array
$ldapDisplayNameProperty => 'displayName',
$ldapPronounsProperty => 'pronouns',
];
if ($ldapPreferredFirstNameProperty) {
$attributes[$ldapPreferredFirstNameProperty] = 'preferredFirstName';
}
if ($ldapPreferredLastNameProperty) {
$attributes[$ldapPreferredLastNameProperty] = 'preferredLastName';
}

foreach ($results as $userData) {
$values = [];
Expand Down
126 changes: 126 additions & 0 deletions tests/Command/SyncAllUsersCommandTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,69 @@ public function testExecuteWithFirstNameChange(): void
);
}

public function testExecuteWithPreferredFirstNameChange(): void
{
$this->userRepository->shouldReceive('getAllCampusIds')
->with(false, false)->andReturn(['abc']);
$fakeDirectoryUser = [
'firstName' => 'new legal first',
'preferredFirstName' => 'new-first',
'lastName' => 'last',
'email' => 'email',
'displayName' => 'display',
'pronouns' => 'pronouns',
'telephoneNumber' => 'phone',
'campusId' => 'abc',
'username' => 'abc123',
];
$this->directory
->shouldReceive('findByCampusIds')
->with(['abc'])
->andReturn([$fakeDirectoryUser]);
$authentication = m::mock(AuthenticationInterface::class);
$authentication->shouldReceive('getUsername')->andReturn('abc123');

$user = m::mock(UserInterface::class);
$user->shouldReceive('getId')->andReturn(42);
$user->shouldReceive('getFirstAndLastName')->andReturn('first last');
$user->shouldReceive('getFirstName')->andReturn('first');
$user->shouldReceive('getLastName')->andReturn('last');
$user->shouldReceive('getEmail')->andReturn('email');
$user->shouldReceive('getDisplayName')->andReturn('display');
$user->shouldReceive('getPronouns')->andReturn('pronouns');
$user->shouldReceive('getPhone')->andReturn('phone');
$user->shouldReceive('getCampusId')->andReturn('abc');
$user->shouldReceive('getAuthentication')->andReturn($authentication);
$user->shouldReceive('setExamined')->with(true);
$user->shouldReceive('setFirstName')->with('new-first');

$this->userRepository
->shouldReceive('findBy')
->with(['campusId' => 'abc', 'enabled' => true])
->andReturn([$user])
->once();
$this->userRepository
->shouldReceive('findBy')
->with(m::hasKey('examined'), m::any())->andReturn([])
->andReturn([])
->once();
$this->userRepository->shouldReceive('update')->with($user, false)->once();

$this->em->shouldReceive('flush')->twice();
$this->em->shouldReceive('clear')->once();
$this->commandTester->execute([]);

$output = $this->commandTester->getDisplay();
$this->assertMatchesRegularExpression(
'/Updating first name from "first" to "new-first"./',
$output
);
$this->assertMatchesRegularExpression(
'/Completed sync process 1 users found in the directory; 1 users updated./',
$output
);
}

public function testExecuteWithLastNameChange(): void
{
$this->userRepository->shouldReceive('getAllCampusIds')
Expand Down Expand Up @@ -257,6 +320,69 @@ public function testExecuteWithLastNameChange(): void
);
}

public function testExecuteWithPreferredLastNameChange(): void
{
$this->userRepository->shouldReceive('getAllCampusIds')
->with(false, false)->andReturn(['abc']);
$fakeDirectoryUser = [
'firstName' => 'first',
'lastName' => 'new legal last',
'preferredLastName' => 'new-last',
'email' => 'email',
'displayName' => 'display',
'pronouns' => 'pronouns',
'telephoneNumber' => 'phone',
'campusId' => 'abc',
'username' => 'abc123',
];
$this->directory
->shouldReceive('findByCampusIds')
->with(['abc'])
->andReturn([$fakeDirectoryUser]);
$authentication = m::mock(AuthenticationInterface::class);
$authentication->shouldReceive('getUsername')->andReturn('abc123');

$user = m::mock(UserInterface::class);
$user->shouldReceive('getId')->andReturn(42);
$user->shouldReceive('getFirstAndLastName')->andReturn('first last');
$user->shouldReceive('getFirstName')->andReturn('first');
$user->shouldReceive('getLastName')->andReturn('last');
$user->shouldReceive('getEmail')->andReturn('email');
$user->shouldReceive('getDisplayName')->andReturn('display');
$user->shouldReceive('getPronouns')->andReturn('pronouns');
$user->shouldReceive('getPhone')->andReturn('phone');
$user->shouldReceive('getCampusId')->andReturn('abc');
$user->shouldReceive('getAuthentication')->andReturn($authentication);
$user->shouldReceive('setExamined')->with(true);
$user->shouldReceive('setLastName')->with('new-last');

$this->userRepository
->shouldReceive('findBy')
->with(['campusId' => 'abc', 'enabled' => true])
->andReturn([$user])
->once();
$this->userRepository
->shouldReceive('findBy')
->with(m::hasKey('examined'), m::any())->andReturn([])
->andReturn([])
->once();
$this->userRepository->shouldReceive('update')->with($user, false)->once();

$this->em->shouldReceive('flush')->twice();
$this->em->shouldReceive('clear')->once();
$this->commandTester->execute([]);

$output = $this->commandTester->getDisplay();
$this->assertMatchesRegularExpression(
'/Updating last name from "last" to "new-last"./',
$output
);
$this->assertMatchesRegularExpression(
'/Completed sync process 1 users found in the directory; 1 users updated./',
$output
);
}

public function testExecuteWithPhoneNumberChange(): void
{
$this->userRepository->shouldReceive('getAllCampusIds')
Expand Down
71 changes: 71 additions & 0 deletions tests/Command/SyncUserCommandTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,77 @@ public function testExecute(): void
$output
);
}

public function testExecuteWithPreferredNames(): void
{
$authentication = m::mock(AuthenticationInterface::class);
$authentication->shouldReceive('setUsername')->with('username');

$pendingUpdate = m::mock(PendingUserUpdateInterface::class);
$user = m::mock(UserInterface::class);
$user->shouldReceive('getFirstName')->andReturn('old-first');
$user->shouldReceive('getPendingUserUpdates')->andReturn(new ArrayCollection([$pendingUpdate]));
$user->shouldReceive('getLastName')->andReturn('old-last');
$user->shouldReceive('getEmail')->andReturn('old-email');
$user->shouldReceive('getDisplayName')->andReturn('old-display');
$user->shouldReceive('getPronouns')->andReturn('old-pronouns');
$user->shouldReceive('getPhone')->andReturn('old-phone');
$user->shouldReceive('getCampusId')->andReturn('abc');
$user->shouldReceive('getAuthentication')->andReturn($authentication);
$user->shouldReceive('setFirstName')->with('preferred-first');
$user->shouldReceive('setLastName')->with('preferred-last');
$user->shouldReceive('setEmail')->with('email');
$user->shouldReceive('setDisplayName')->with('display');
$user->shouldReceive('setPronouns')->with('pronouns');
$user->shouldReceive('setPhone')->with('phone');

$this->userRepository->shouldReceive('findOneBy')->with(['id' => 1])->andReturn($user);
$this->userRepository->shouldReceive('update')->with($user);
$this->authenticationRepository->shouldReceive('update')->with($authentication, false);
$this->pendingUserUpdateRepository->shouldReceive('delete')->with($pendingUpdate)->once();
$fakeDirectoryUser = [
'firstName' => 'first',
'lastName' => 'last',
'preferredFirstName' => 'preferred-first',
'preferredLastName' => 'preferred-last',
'email' => 'email',
'displayName' => 'display',
'pronouns' => 'pronouns',
'telephoneNumber' => 'phone',
'campusId' => 'abc',
'username' => 'username'
];
$this->directory->shouldReceive('findByCampusId')->with('abc')->andReturn($fakeDirectoryUser);
$this->commandTester->setInputs(['Yes']);

$this->commandTester->execute([
'userId' => '1'
]);

$output = $this->commandTester->getDisplay();
$this->assertMatchesRegularExpression(
'/Ilios User\s+\| ' .
'abc\s+\| ' .
'old-first\s+\| ' .
'old-last\s+\| ' .
'old-display\s+\| ' .
'old-pronouns\s+\| ' .
'old-email\s+\| ' .
'old-phone/',
$output
);
$this->assertMatchesRegularExpression(
'/Directory User\s+\| ' .
'abc\s+\| ' .
'preferred-first\s+\| ' .
'preferred-last\s+\| ' .
'display\s+\| ' .
'pronouns\s+\| ' .
'email\s+\| ' .
'phone/',
$output
);
}
public function testEmptyPronouns(): void
{
$authentication = m::mock(AuthenticationInterface::class);
Expand Down

0 comments on commit 8d9f12c

Please sign in to comment.