Skip to content

Commit a350416

Browse files
committed
fix(login): remove duplicate loginName/user_autofocus block that clobbered LDAP resolution
A second if/else block after the alt_login assignment was unconditionally overwriting $parameters['loginName'] with the raw $user input, silently discarding the getUserName() resolution done in the first block for LDAP accounts whose internal username is a UUID. The first block already handles all cases correctly; the duplicate was dead code with a functional side-effect. Adds a regression test (testShowLoginFormLdapUsernameResolutionNotClobbered) that passes an internal UUID as $user and asserts loginName is the resolved display name, not the raw UUID. Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
1 parent d2607b7 commit a350416

2 files changed

Lines changed: 38 additions & 8 deletions

File tree

core/Controller/LoginController.php

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -193,14 +193,6 @@ public function showLoginForm($user, $redirect_url, $remember_login) {
193193
$parameters['rememberLoginAllowed'] = OC_Util::rememberLoginAllowed();
194194
$parameters['rememberLoginState'] = !empty($remember_login) ? $remember_login : 0;
195195

196-
if ($user !== null && $user !== '') {
197-
$parameters['loginName'] = $user;
198-
$parameters['user_autofocus'] = false;
199-
} else {
200-
$parameters['loginName'] = '';
201-
$parameters['user_autofocus'] = true;
202-
}
203-
204196
/**
205197
* If redirect_url is not empty and remember_login is null and
206198
* user not logged in and check if the string

tests/Core/Controller/LoginControllerTest.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,44 @@ public function testShowLoginFormWithPasswordResetOptionAlwaysTrueWhenNoLostPass
388388
$this->assertEquals($expectedResponse, $this->loginController->showLoginForm('LdapUser', '', ''));
389389
}
390390

391+
/**
392+
* LDAP accounts with a UUID internal username must have loginName resolved to the
393+
* human-readable display name returned by getUserName(). A duplicate loginName block
394+
* that existed after the alt_login assignment was clobbering this resolved value back
395+
* to the raw internal username — this test is the regression guard for that removal.
396+
*/
397+
public function testShowLoginFormLdapUsernameResolutionNotClobbered() {
398+
$this->userSession
399+
->expects($this->once())
400+
->method('isLoggedIn')
401+
->willReturn(false);
402+
$this->config
403+
->expects($this->exactly(3))
404+
->method('getSystemValue')
405+
->willReturnMap([
406+
['lost_password_link', false],
407+
['login.alternatives', '', ''],
408+
['strict_login_enforced', false],
409+
]);
410+
$user = $this->createMock(IUser::class);
411+
$user->method('getUserName')->willReturn('john.doe');
412+
$this->userManager
413+
->expects($this->once())
414+
->method('get')
415+
->with('some-internal-uuid-1234')
416+
->willReturn($user);
417+
418+
$this->licenseManager->method('getLicenseMessageFor')
419+
->willReturn([
420+
'license_state' => ILicenseManager::LICENSE_STATE_MISSING
421+
]);
422+
423+
$response = $this->loginController->showLoginForm('some-internal-uuid-1234', '', '');
424+
$params = $response->getParams();
425+
$this->assertSame('john.doe', $params['loginName'], 'LDAP username must be resolved to getUserName(), not the raw internal ID');
426+
$this->assertFalse($params['user_autofocus']);
427+
}
428+
391429
/**
392430
* Verify user enumeration fix: a non-existent user and an LDAP user (canChangePassword=false)
393431
* must both yield canResetPassword=true when lost_password_link is empty, making the

0 commit comments

Comments
 (0)