Skip to content

Commit 98c65c3

Browse files
authored
Merge pull request #530 from dbarzin/dev
Dev
2 parents bbfbb1f + a461eee commit 98c65c3

File tree

5 files changed

+72
-82
lines changed

5 files changed

+72
-82
lines changed

app/Http/Controllers/Auth/LoginController.php

Lines changed: 66 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
use App\Models\User;
77
use Illuminate\Foundation\Auth\AuthenticatesUsers;
88
use Illuminate\Http\Request;
9+
use Illuminate\Support\Facades\Hash;
910
use Illuminate\Support\Facades\Log;
11+
use Illuminate\Support\Str;
1012
use LdapRecord\Auth\BindException;
1113
use LdapRecord\Container;
1214
use LdapRecord\Models\Entry as LdapEntry;
@@ -15,19 +17,8 @@ class LoginController extends Controller
1517
{
1618
use AuthenticatesUsers;
1719

18-
/**
19-
* Where to redirect users after login.
20-
*
21-
* @var string
22-
*/
23-
protected $redirectTo = '/home';
24-
25-
/**
26-
* Login username to be used by the controller.
27-
*
28-
* @var string
29-
*/
30-
protected $username;
20+
protected string $redirectTo = '/home';
21+
protected string $username;
3122

3223
public function __construct()
3324
{
@@ -36,45 +27,49 @@ public function __construct()
3627
}
3728

3829
/**
39-
* Determine the field used for login (email or login).
30+
* N'utiliser QUE le champ 'login' comme identifiant.
4031
*/
41-
public function findUsername()
32+
public function findUsername(): string
4233
{
43-
$login = request()->input('login');
34+
$login = trim((string) request()->input('login', ''));
35+
// On merge pour que credentials($request) prenne bien 'login'
36+
request()->merge(['login' => $login]);
4437

45-
$fieldType = filter_var($login, FILTER_VALIDATE_EMAIL) ? 'email' : 'login';
46-
47-
request()->merge([$fieldType => $login]);
48-
49-
return $fieldType;
38+
return 'login';
5039
}
5140

52-
/**
53-
* Expose username property to the framework.
54-
*/
55-
public function username()
41+
public function username(): string
5642
{
5743
return $this->username;
5844
}
5945

6046
/**
61-
* Attempt an LDAP bind for the given app username + password using LDAPRecord v2.
62-
* Returns the corresponding LDAP user on success, or null on failure.
47+
* LDAP bind (LDAPRecord v2)
6348
*/
6449
protected function ldapBindAndGetUser(string $appUsername, string $password): ?LdapEntry
6550
{
51+
if ($appUsername === '' || $password === '') {
52+
Log::debug('LDAP skipped: empty identifier or password.');
53+
return null;
54+
}
55+
6656
try {
6757
$query = LdapEntry::query();
6858

6959
// Optionnel : restreindre à une OU si configuré
70-
$base = config('app.ldap_users_base_dn', config('app.ldap_users_base_dn'));
71-
if ($base) {
60+
$base = trim((string) config('app.ldap_users_base_dn'));
61+
if ($base !== '') {
7262
$query->in($base);
7363
}
7464

75-
// Filtre de localisation : OR sur les attributs pertinents
76-
$attrs = array_filter(array_map('trim', explode(',', config('app.ldap_login_attributes'))));
65+
// Attributs de login à tester côté LDAP (uid, sAMAccountName, etc.)
66+
$attrs = array_values(array_filter(array_map('trim', explode(',', (string) config('app.ldap_login_attributes')))));
67+
if (empty($attrs)) {
68+
Log::warning('LDAP login aborted: app.ldap_login_attributes is empty.');
69+
return null;
70+
}
7771

72+
// Filtre OR sur les attributs configurés
7873
$first = true;
7974
foreach ($attrs as $attr) {
8075
if ($first) {
@@ -85,27 +80,35 @@ protected function ldapBindAndGetUser(string $appUsername, string $password): ?L
8580
}
8681
}
8782

88-
\Log::debug('LDAP dn: ' . $query->getDn() . ' query: ' . $query->getQuery());
89-
90-
/** @var LdapEntry|null $ldapUser */
91-
$ldapUser = $query->first();
92-
if (! $ldapUser) {
93-
\Log::debug('LDAP user not found !');
83+
// Collision guard
84+
$results = $query->limit(2)->get();
85+
if ($results->count() === 0) {
86+
Log::debug('LDAP user not found for identifier.', ['identifier' => $appUsername]);
87+
return null;
88+
}
89+
if ($results->count() > 1) {
90+
Log::warning('LDAP identifier collision: multiple entries match.', [
91+
'identifier' => $appUsername,
92+
'attributes' => $attrs,
93+
]);
9494
return null;
9595
}
9696

97+
/** @var LdapEntry $ldapUser */
98+
$ldapUser = $results->first();
99+
97100
$connection = Container::getConnection();
98101
$dn = $ldapUser->getDn();
99102

100-
if ($connection->auth()->attempt($dn, $password, true)) {
103+
if ($dn && $connection->auth()->attempt($dn, $password, true)) {
101104
return $ldapUser;
102105
}
103106

104107
return null;
105108
} catch (BindException $e) {
106109
Log::warning('LDAP bind failed', [
107110
'error' => $e->getMessage(),
108-
'diagnostic' => $e->getDetailedError()->getDiagnosticMessage(),
111+
'diagnostic' => $e->getDetailedError()?->getDiagnosticMessage(),
109112
]);
110113
return null;
111114
} catch (\Throwable $e) {
@@ -115,68 +118,55 @@ protected function ldapBindAndGetUser(string $appUsername, string $password): ?L
115118
}
116119

117120
/**
118-
* Override Laravel's default login attempt to add LDAPRecord support, toggled by .env
119-
*
120-
* Priority:
121-
* - If LDAP_ENABLED=true => try LDAP; on success, log the mapped local user in.
122-
* - If LDAP fails and LDAP_FALLBACK_LOCAL=true => try local DB credentials.
123-
* - If LDAP_ENABLED=false => only local DB credentials.
121+
* Login avec LDAP optionnel + fallback local (UNIQUEMENT via 'login').
124122
*/
125-
protected function attemptLogin(Request $request)
123+
protected function attemptLogin(Request $request): bool
126124
{
127-
$useLdap = config('app.ldap_enabled');
128-
$fallbackLocal = config('app.ldap_fallback_local');
129-
$autoProvision = config('app.ldap_auto_provision');
125+
$useLdap = (bool) config('app.ldap_enabled');
126+
$fallbackLocal = (bool) config('app.ldap_fallback_local');
127+
$autoProvision = (bool) config('app.ldap_auto_provision');
130128

131-
$credentials = $request->only($this->username(), 'password');
132-
$identifier = $credentials[$this->username()] ?? '';
133-
$password = $credentials['password'] ?? '';
129+
$credentials = $request->only($this->username(), 'password'); // ['login' => ..., 'password' => ...]
130+
$identifier = (string) ($credentials[$this->username()] ?? '');
131+
$password = (string) ($credentials['password'] ?? '');
132+
$remember = $request->boolean('remember');
134133

135134
if ($useLdap) {
136135
$ldapUser = $this->ldapBindAndGetUser($identifier, $password);
137136

138137
if ($ldapUser) {
139-
// Map / locate local application user
140-
$local = User::query()
141-
->when(filter_var($identifier, FILTER_VALIDATE_EMAIL), function ($q) use ($identifier) {
142-
return $q->where('email', $identifier);
143-
}, function ($q) use ($identifier) {
144-
return $q->where('login', $identifier);
145-
})
146-
->first();
147-
148-
if (! $local && $autoProvision) {
149-
// Minimal safe provisioning – adapt attributes to your schema
138+
// Mapping local UNIQUEMENT par 'login'
139+
$local = User::where('login', $identifier)->first();
140+
141+
if (!$local && $autoProvision) {
150142
$local = User::create([
151-
'name' => $ldapUser->getFirstAttribute('cn') ?? $identifier,
152-
'email' => $ldapUser->getFirstAttribute('mail') ?? 'user@localhost.local',
153-
'login' => $identifier,
154-
'role' => 5, // Auditee
155-
// Store a random password so DB auth is not accidentally usable unless you set one explicitly
156-
'password' => bcrypt(str()->random(32)),
143+
'name' => $ldapUser->getFirstAttribute('cn') ?: $identifier,
144+
'email' => $ldapUser->getFirstAttribute('mail') ?: 'user@localhost.local',
145+
'login' => $identifier,
146+
'role' => 5,
147+
'password' => Hash::make(Str::random(32)), // inutilisable en local par défaut
157148
]);
158149
}
159150

160151
if ($local) {
161-
$remember = $request->boolean('remember');
162152
$this->guard()->login($local, $remember);
163153
return true;
164154
}
165155

166-
// LDAP OK but no mapped local user and no auto-provision
156+
// LDAP OK mais pas d'utilisateur local et pas d’auto-provision
167157
return false;
168158
}
169159

170-
// LDAP failed – optionally fall back to local DB auth
171-
if (! $fallbackLocal) {
160+
// LDAP KO → éventuel fallback local (toujours via 'login')
161+
if (!$fallbackLocal) {
172162
return false;
173163
}
174164
}
175165

176-
// Local database auth path (default Laravel)
166+
// Auth locale (Laravel) — utilisera ['login' => ..., 'password' => ...]
177167
return $this->guard()->attempt(
178-
$this->credentials($request),
179-
$request->filled('remember')
168+
$this->credentials($request), // credentials() retournera login + password car username() = 'login'
169+
$remember
180170
);
181171
}
182172
}

app/Http/Controllers/UserController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,8 @@ public function update(Request $request, User $user)
155155
}
156156

157157
// Update user information
158-
$user->name = $request->input('name');
159158
$user->login = $request->input('login');
159+
$user->name = $request->input('name');
160160
$user->email = $request->input('email');
161161
if ($this->isAdmin()) {
162162
$user->role = $request->input('role');

resources/lang/en/cruds.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@
286286
'add' => 'Add User',
287287
'show' => 'Show user',
288288
'fields' => [
289-
'login' => 'UserId',
289+
'login' => 'Login',
290290
'name' => 'Name',
291291
'title' => 'Title',
292292
'role' => 'Role',

resources/lang/fr/cruds.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@
284284
'edit' => 'Modifier un utilisateur',
285285
'add' => 'Ajouter un utilisateur',
286286
'fields' => [
287-
'login' => 'UserId',
287+
'login' => 'Login',
288288
'name' => 'Nom',
289289
'title' => 'Titre',
290290
'role' => 'Role',

resources/views/users/edit.blade.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
<label>{{ trans('cruds.user.fields.login') }}</label>
1616
</div>
1717
<div class="cell-3">
18-
<input type="text" class="input {{ $errors->has('login') ? 'is-danger' : ''}}" name="login" value="{{ $errors->has('login') ? old('login') : $user->login }}" size='40'>
18+
<input type="text" class="input {{ $errors->has('login') ? 'is-danger' : ''}}" name="login" value="{{ old('login', $user->login) }}" size='40'>
1919
</div>
2020
</div>
2121

@@ -25,7 +25,7 @@
2525
<label>{{ trans('cruds.user.fields.name') }}</label>
2626
</div>
2727
<div class="cell-3">
28-
<input type="text" class="input {{ $errors->has('name') ? 'is-danger' : ''}}" name="name" value="{{ $errors->has('name') ? old('name') : $user->name }}" size='80'>
28+
<input type="text" class="input {{ $errors->has('name') ? 'is-danger' : ''}}" name="name" value="{{ old('name', $user->name) }}" size='80'>
2929
</div>
3030
</div>
3131

@@ -34,7 +34,7 @@
3434
<label>{{ trans('cruds.user.fields.title') }}</label>
3535
</div>
3636
<div class="cell-3">
37-
<input type="text" class="input {{ $errors->has('title') ? 'is-danger' : ''}}" name="title" value="{{ $errors->has('title') ? old('title') : $user->title }}" size='80'>
37+
<input type="text" class="input {{ $errors->has('title') ? 'is-danger' : ''}}" name="title" value="{{ old('title', $user->title) }}" size='80'>
3838
</div>
3939
</div>
4040

0 commit comments

Comments
 (0)