mirror/snipe-it
3
0
Fork 0
mirror of https://github.com/snipe/snipe-it.git synced 2026-08-18 11:15:42 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #19438 from grokability/#19206-ldap-department-sync-on-first-login

Browse Source 
LDAP: Fixed #19206 - ldap department not syncing on first login
This commit is contained in:
snipe
2026-08-07 13:19:17 +01:00
committed by GitHub
parent 5c5c3eaa4f db2cfe6cfd
commit 1515a3e835
4 changed files with 379 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
app/Http/Controllers/Auth/LoginController.php
View File

@ -205,16 +205,22 @@ class LoginController extends Controller
Log::debug('Local user '.$request->input('username').' exists in database. Updating existing user against LDAP.');
$ldap_attr = Ldap::parseAndMapLdapAttributes($ldap_user);
$settings = Setting::getSettings();
$user->password = $user->noPassword();
if (Setting::getSettings()->ldap_pw_sync == '1') {
if ($settings->ldap_pw_sync == '1') {
$user->password = bcrypt($request->input('password'));
}
$user->last_login = \Carbon::now();
$user->email = $ldap_attr['email'];
$user->first_name = $ldap_attr['firstname'];
$user->last_name = $ldap_attr['lastname']; // FIXME (or TODO?) - do we need to map additional fields that we now support? E.g. country, phone, etc.
// Refresh every mapped field from the LDAP payload. Shared
// with Ldap::createUserFromLdap so the field list lives in
// one place. Bulk sync via snipe-it:ldap-sync remains the
// canonical path for the fields that need a re-bind
// (manager, active_flag, etc.).
Ldap::applyLdapAttributesToUser($user, $ldap_attr);
$user->saveQuietly();
} // End if(!user)

170
app/Models/Ldap.php
View File

@ -97,7 +97,7 @@ class Ldap extends Model
ldap_set_option($connection, LDAP_OPT_NETWORK_TIMEOUT, 20);
if ($ldap_use_tls == '1') {
if (!ldap_start_tls($connection)) {
if (! ldap_start_tls($connection)) {
throw new Exception('STARTTLS Failed.');
}
}
@ -313,27 +313,45 @@ class Ldap extends Model
*/
public static function parseAndMapLdapAttributes($ldapattributes)
{
// Get LDAP attribute config
$ldap_result_username = Setting::getSettings()->ldap_username_field;
$ldap_result_emp_num = Setting::getSettings()->ldap_emp_num;
$ldap_result_last_name = Setting::getSettings()->ldap_lname_field;
$ldap_result_first_name = Setting::getSettings()->ldap_fname_field;
$ldap_result_email = Setting::getSettings()->ldap_email;
$ldap_result_phone = Setting::getSettings()->ldap_phone;
$ldap_result_jobtitle = Setting::getSettings()->ldap_jobtitle;
$ldap_result_country = Setting::getSettings()->ldap_country;
$ldap_result_location = Setting::getSettings()->ldap_location;
$ldap_result_dept = Setting::getSettings()->ldap_dept;
$ldap_result_manager = Setting::getSettings()->ldap_manager;
// Get LDAP user data
// Get LDAP attribute config. The settings column names here are
// the same ones LdapSync's $ldap_map reads, so this parser and
// the bulk-sync command see identical field lookups.
$settings = Setting::getSettings();
$ldap_result_username = $settings->ldap_username_field;
$ldap_result_emp_num = $settings->ldap_emp_num;
$ldap_result_last_name = $settings->ldap_lname_field;
$ldap_result_first_name = $settings->ldap_fname_field;
$ldap_result_display_name = $settings->ldap_display_name;
$ldap_result_email = $settings->ldap_email;
$ldap_result_phone = $settings->ldap_phone_field;
$ldap_result_mobile = $settings->ldap_mobile;
$ldap_result_jobtitle = $settings->ldap_jobtitle;
$ldap_result_address = $settings->ldap_address;
$ldap_result_city = $settings->ldap_city;
$ldap_result_state = $settings->ldap_state;
$ldap_result_zip = $settings->ldap_zip;
$ldap_result_country = $settings->ldap_country;
$ldap_result_location = $settings->ldap_location;
$ldap_result_dept = $settings->ldap_dept;
$ldap_result_manager = $settings->ldap_manager;
// Get LDAP user data. Kept in the same shape LdapSync's per-user
// $item array uses so the two paths stay comparable when this
// one grows.
$item = [];
$item['username'] = $ldapattributes[$ldap_result_username][0] ?? '';
$item['employee_number'] = $ldapattributes[$ldap_result_emp_num][0] ?? '';
$item['lastname'] = $ldapattributes[$ldap_result_last_name][0] ?? '';
$item['firstname'] = $ldapattributes[$ldap_result_first_name][0] ?? '';
$item['display_name'] = $ldapattributes[$ldap_result_display_name][0] ?? '';
$item['email'] = $ldapattributes[$ldap_result_email][0] ?? '';
$item['telephone'] = $ldapattributes[$ldap_result_phone][0] ?? '';
$item['mobile'] = $ldapattributes[$ldap_result_mobile][0] ?? '';
$item['jobtitle'] = $ldapattributes[$ldap_result_jobtitle][0] ?? '';
$item['address'] = $ldapattributes[$ldap_result_address][0] ?? '';
$item['city'] = $ldapattributes[$ldap_result_city][0] ?? '';
$item['state'] = $ldapattributes[$ldap_result_state][0] ?? '';
$item['zip'] = $ldapattributes[$ldap_result_zip][0] ?? '';
$item['country'] = $ldapattributes[$ldap_result_country][0] ?? '';
$item['department'] = $ldapattributes[$ldap_result_dept][0] ?? '';
$item['manager'] = $ldapattributes[$ldap_result_manager][0] ?? '';
@ -343,6 +361,78 @@ class Ldap extends Model
return $item;
}
/**
* Copy the parseAndMapLdapAttributes() output onto a User row.
* Called by both createUserFromLdap (first login, new user) and
* LoginController::loginViaLdap (existing user re-login), so the
* mapping list lives in exactly one place.
*
* Each optional field is gated on its LDAP mapping being non-blank
* so unset mappings don't overwrite existing values with empty
* strings. Department and Location are firstOrCreate'd only when
* both the mapping is set and the LDAP payload actually carried a
* value, so a blank attribute doesn't accrete a nameless row.
*
* Manager is intentionally out of scope: LdapSync's manager
* resolution needs an admin re-bind + LDAP re-query to translate
* the DN into a Snipe-IT user id, and that's best done in bulk.
* ldap_import users get their manager populated on the next
* `snipe-it:ldap-sync` run.
*/
public static function applyLdapAttributesToUser(User $user, array $ldapAttr): void
{
$settings = Setting::getSettings();
// Always-written identity fields. These have no per-field gate
// because Snipe-IT considers username / first name / last name /
// email load-bearing for every user — if a mapping's blank the
// LDAP payload just gives us an empty string, matching the
// pre-fix behavior on the create path.
$user->username = $ldapAttr['username'];
$user->first_name = $ldapAttr['firstname'];
$user->last_name = $ldapAttr['lastname'];
$user->email = $ldapAttr['email'];
if ($settings->ldap_display_name != '') {
$user->display_name = $ldapAttr['display_name'];
}
if ($settings->ldap_emp_num != '') {
$user->employee_num = e($ldapAttr['employee_number']);
}
if ($settings->ldap_phone_field != '') {
$user->phone = $ldapAttr['telephone'];
}
if ($settings->ldap_mobile != '') {
$user->mobile = $ldapAttr['mobile'];
}
if ($settings->ldap_jobtitle != '') {
$user->jobtitle = $ldapAttr['jobtitle'];
}
if ($settings->ldap_address != '') {
$user->address = $ldapAttr['address'];
}
if ($settings->ldap_city != '') {
$user->city = $ldapAttr['city'];
}
if ($settings->ldap_state != '') {
$user->state = $ldapAttr['state'];
}
if ($settings->ldap_zip != '') {
$user->zip = $ldapAttr['zip'];
}
if ($settings->ldap_country != '') {
$user->country = $ldapAttr['country'];
}
if ($settings->ldap_dept != '' && $ldapAttr['department'] !== '') {
$department = Department::firstOrCreate(['name' => $ldapAttr['department']]);
$user->department_id = $department->id;
}
if ($settings->ldap_location != '' && $ldapAttr['location'] !== '') {
$location = Location::firstOrCreate(['name' => $ldapAttr['location']]);
$user->location_id = $location->id;
}
}
/**
* Create user from LDAP attributes
*
@ -356,33 +446,43 @@ class Ldap extends Model
{
$item = self::parseAndMapLdapAttributes($ldapatttibutes);
// Create user from LDAP data
if (! empty($item['username'])) {
$user = new User;
$user->first_name = $item['firstname'];
$user->last_name = $item['lastname'];
$user->username = $item['username'];
$user->email = $item['email'];
$user->locale = $item['locale'];
$user->password = $user->noPassword();
if (empty($item['username'])) {
return false;
}
if (Setting::getSettings()->ldap_pw_sync == '1') {
$user->password = bcrypt($password);
}
$settings = Setting::getSettings();
$user->activated = 1;
$user->ldap_import = 1;
$user->notes = 'Imported on first login from LDAP';
$user = new User;
self::applyLdapAttributesToUser($user, $item);
if ($user->save()) {
return $user;
} else {
Log::debug('Could not create user.'.$user->getErrors());
throw new Exception('Could not create user: '.$user->getErrors());
$user->locale = $item['locale'];
$user->password = $user->noPassword();
if ($settings->ldap_pw_sync == '1') {
$user->password = bcrypt($password);
}
$user->activated = 1;
$user->ldap_import = 1;
$user->notes = 'Imported on first login from LDAP';
if (! $user->save()) {
Log::debug('Could not create user.'.$user->getErrors());
throw new Exception('Could not create user: '.$user->getErrors());
}
// Attach the configured Default Permissions Group to newly-
// created LDAP users so first-login users land with the same
// baseline permissions bulk-synced users get. Matches
// LdapSync::handle()'s post-save group attachment. Skipped when
// the setting points at a deleted group.
if ($settings->ldap_default_group) {
$default = Group::find($settings->ldap_default_group);
if ($default !== null && ! $user->groups()->where('group_id', $default->id)->exists()) {
$user->groups()->attach($default->id);
}
}
return false;
return $user;
}
/**

8
phpstan-baseline.neon
View File

@ -8041,11 +8041,17 @@ parameters:
path: app/Models/Labels/Tapes/Generic/Tape_53mm.php
-
message: '#^Access to an undefined property App\\Models\\Setting\:\:\$ldap_phone\.$#'
message: '#^Access to an undefined property App\\Models\\User\:\:\$display_name\.$#'
identifier: property.notFound
count: 1
path: app/Models/Ldap.php
-
message: '#^Call to an undefined method Illuminate\\Database\\Eloquent\\Relations\\Relation\:\:attach\(\)\.$#'
identifier: method.notFound
count: 1
path: app/Models/Ldap.php
-
message: '#^Called ''env'' outside of the config directory which returns null when the config is cached, use ''config''\.$#'
identifier: larastan.noEnvCallsOutsideOfConfig

227
tests/Unit/LdapCreateUserFromLdapTest.php Normal file
View File

@ -0,0 +1,227 @@
<?php
namespace Tests\Unit;
use App\Models\Department;
use App\Models\Group;
use App\Models\Ldap;
use App\Models\Location;
use App\Models\User;
use Illuminate\Support\Facades\Hash;
use PHPUnit\Framework\Attributes\Group as PhpUnitGroup;
use Tests\TestCase;
/**
* FD/#19206 regression coverage. Ldap::createUserFromLdap runs when a
* user logs in via LDAP for the first time and no local User row
* exists yet. Before this coverage landed the method only wrote
* first_name / last_name / username / email / locale and skipped the
* default permissions group, so first-login users came up short on
* their mapped fields and unassigned to the configured Default
* Permissions Group even though bulk `snipe-it:ldap-sync` set both.
*/
#[PhpUnitGroup('ldap')]
class LdapCreateUserFromLdapTest extends TestCase
{
/**
* Full synthetic LDAP payload keyed on the same attribute names the
* setting mappings in configureLdapMappings() point at. Overrides
* let a single test case flip one field without redeclaring the
* whole payload.
*/
private function ldapAttributes(array $overrides = []): array
{
return array_merge([
'samaccountname' => ['jsmith'],
'sn' => ['Smith'],
'givenname' => ['Jane'],
'displayname' => ['Jane Smith'],
'mail' => ['jane@example.com'],
'employeenumber' => ['E1234'],
'telephonenumber' => ['555-0100'],
'mobile' => ['555-0200'],
'title' => ['Widget Wrangler'],
'streetaddress' => ['1 Main St'],
'l' => ['Springfield'],
'st' => ['IL'],
'postalcode' => ['62704'],
'c' => ['US'],
'department' => ['Widgets'],
'physicaldeliveryofficename' => ['HQ'],
], $overrides);
}
private function configureLdapMappings(): void
{
$this->settings->enableLdap();
$this->settings->set([
'ldap_username_field' => 'samaccountname',
'ldap_lname_field' => 'sn',
'ldap_fname_field' => 'givenname',
'ldap_display_name' => 'displayname',
'ldap_email' => 'mail',
'ldap_emp_num' => 'employeenumber',
'ldap_phone_field' => 'telephonenumber',
'ldap_mobile' => 'mobile',
'ldap_jobtitle' => 'title',
'ldap_address' => 'streetaddress',
'ldap_city' => 'l',
'ldap_state' => 'st',
'ldap_zip' => 'postalcode',
'ldap_country' => 'c',
'ldap_dept' => 'department',
'ldap_location' => 'physicaldeliveryofficename',
]);
}
public function test_populates_every_configured_scalar_field(): void
{
$this->configureLdapMappings();
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'pw');
$this->assertInstanceOf(User::class, $user);
$this->assertSame('jsmith', $user->username);
$this->assertSame('Jane', $user->first_name);
$this->assertSame('Smith', $user->last_name);
$this->assertSame('Jane Smith', $user->display_name);
$this->assertSame('jane@example.com', $user->email);
$this->assertSame('E1234', $user->employee_num);
$this->assertSame('555-0100', $user->phone);
$this->assertSame('555-0200', $user->mobile);
$this->assertSame('Widget Wrangler', $user->jobtitle);
$this->assertSame('1 Main St', $user->address);
$this->assertSame('Springfield', $user->city);
$this->assertSame('IL', $user->state);
$this->assertSame('62704', $user->zip);
$this->assertSame('US', $user->country);
$this->assertSame(1, (int) $user->activated);
$this->assertSame(1, (int) $user->ldap_import);
}
public function test_creates_department_from_ldap_value(): void
{
$this->configureLdapMappings();
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'pw');
$this->assertNotNull($user->department_id);
$this->assertSame('Widgets', Department::find($user->department_id)->name);
}
public function test_reuses_existing_department_by_name(): void
{
$this->configureLdapMappings();
$existing = Department::factory()->create(['name' => 'Widgets']);
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'pw');
$this->assertSame($existing->id, $user->department_id);
}
public function test_creates_location_from_ldap_value(): void
{
$this->configureLdapMappings();
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'pw');
$this->assertNotNull($user->location_id);
$this->assertSame('HQ', Location::find($user->location_id)->name);
}
public function test_reuses_existing_location_by_name(): void
{
$this->configureLdapMappings();
$existing = Location::factory()->create(['name' => 'HQ']);
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'pw');
$this->assertSame($existing->id, $user->location_id);
}
public function test_skips_field_when_setting_mapping_is_blank(): void
{
$this->configureLdapMappings();
$this->settings->set(['ldap_phone_field' => '']);
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'pw');
$this->assertNull($user->phone);
}
public function test_skips_department_when_ldap_dept_mapping_blank(): void
{
$this->configureLdapMappings();
$this->settings->set(['ldap_dept' => '']);
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'pw');
$this->assertNull($user->department_id);
$this->assertDatabaseMissing('departments', ['name' => 'Widgets']);
}
public function test_skips_department_when_ldap_value_is_missing(): void
{
$this->configureLdapMappings();
// Mapping IS configured, but the LDAP payload for this user
// simply doesn't carry the department attribute. A blank
// "Department" row is worse than no row.
$user = Ldap::createUserFromLdap(
$this->ldapAttributes(['department' => []]),
'pw',
);
$this->assertNull($user->department_id);
$this->assertDatabaseMissing('departments', ['name' => '']);
}
public function test_attaches_default_permissions_group(): void
{
$this->configureLdapMappings();
$group = Group::factory()->create();
$this->settings->set(['ldap_default_group' => $group->id]);
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'pw');
$this->assertTrue($user->groups()->where('group_id', $group->id)->exists());
}
public function test_does_not_attach_default_permissions_group_when_group_deleted(): void
{
$this->configureLdapMappings();
$this->settings->set(['ldap_default_group' => 99999]);
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'pw');
$this->assertSame(0, $user->groups()->count());
}
public function test_returns_false_when_ldap_username_missing(): void
{
$this->configureLdapMappings();
$this->assertFalse(
Ldap::createUserFromLdap($this->ldapAttributes(['samaccountname' => []]), 'pw'),
);
}
public function test_sets_bcrypted_password_when_ldap_pw_sync_enabled(): void
{
$this->configureLdapMappings();
$this->settings->set(['ldap_pw_sync' => 1]);
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'secret-password');
$this->assertTrue(Hash::check('secret-password', $user->password));
}
public function test_password_is_unusable_when_ldap_pw_sync_disabled(): void
{
$this->configureLdapMappings();
$this->settings->set(['ldap_pw_sync' => 0]);
$user = Ldap::createUserFromLdap($this->ldapAttributes(), 'secret-password');
$this->assertFalse(Hash::check('secret-password', $user->password));
}
}