Skip to content

Commit d45fc1b

Browse files
committed
refactor: Move alternative login code to a new service
And use it in LoginController Signed-off-by: Carl Schwan <carl@carlschwan.eu>
1 parent 2bb23b2 commit d45fc1b

7 files changed

Lines changed: 196 additions & 244 deletions

File tree

build/psalm-baseline.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3110,7 +3110,6 @@
31103110
</file>
31113111
<file src="core/Controller/LoginController.php">
31123112
<DeprecatedMethod>
3113-
<code><![CDATA[deleteUserValue]]></code>
31143113
<code><![CDATA[getDelay]]></code>
31153114
</DeprecatedMethod>
31163115
</file>

core/Controller/LoginController.php

Lines changed: 47 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
namespace OC\Core\Controller;
1212

1313
use OC\AppFramework\Http\Request;
14+
use OC\Authentication\Login\AlternativeLoginService;
1415
use OC\Authentication\Login\Chain;
1516
use OC\Authentication\Login\LoginData;
1617
use OC\Authentication\WebAuthn\Manager as WebAuthnManager;
1718
use OC\User\Session;
18-
use OC_App;
1919
use OCA\User_LDAP\Configuration;
2020
use OCA\User_LDAP\Helper;
2121
use OCP\App\IAppManager;
@@ -30,9 +30,10 @@
3030
use OCP\AppFramework\Http\Attribute\UseSession;
3131
use OCP\AppFramework\Http\DataResponse;
3232
use OCP\AppFramework\Http\RedirectResponse;
33-
use OCP\AppFramework\Http\Response;
3433
use OCP\AppFramework\Http\TemplateResponse;
3534
use OCP\AppFramework\Services\IInitialState;
35+
use OCP\Authentication\IAlternativeLogin;
36+
use OCP\Config\IUserConfig;
3637
use OCP\Defaults;
3738
use OCP\IConfig;
3839
use OCP\IL10N;
@@ -48,41 +49,40 @@
4849
use OCP\Util;
4950

5051
class LoginController extends Controller {
51-
public const LOGIN_MSG_INVALIDPASSWORD = 'invalidpassword';
52-
public const LOGIN_MSG_USERDISABLED = 'userdisabled';
53-
public const LOGIN_MSG_CSRFCHECKFAILED = 'csrfCheckFailed';
54-
public const LOGIN_MSG_INVALID_ORIGIN = 'invalidOrigin';
52+
public const string LOGIN_MSG_INVALIDPASSWORD = 'invalidpassword';
53+
public const string LOGIN_MSG_USERDISABLED = 'userdisabled';
54+
public const string LOGIN_MSG_CSRFCHECKFAILED = 'csrfCheckFailed';
55+
public const string LOGIN_MSG_INVALID_ORIGIN = 'invalidOrigin';
5556

5657
public function __construct(
57-
?string $appName,
58+
string $appName,
5859
IRequest $request,
59-
private IUserManager $userManager,
60-
private IConfig $config,
61-
private ISession $session,
62-
private Session $userSession,
63-
private IURLGenerator $urlGenerator,
64-
private Defaults $defaults,
65-
private IThrottler $throttler,
66-
private IInitialState $initialState,
67-
private WebAuthnManager $webAuthnManager,
68-
private IManager $manager,
69-
private IL10N $l10n,
70-
private IAppManager $appManager,
60+
private readonly IUserManager $userManager,
61+
private readonly IConfig $config,
62+
private readonly IUserConfig $userConfig,
63+
private readonly ISession $session,
64+
private readonly Session $userSession,
65+
private readonly IURLGenerator $urlGenerator,
66+
private readonly Defaults $defaults,
67+
private readonly IThrottler $throttler,
68+
private readonly IInitialState $initialState,
69+
private readonly WebAuthnManager $webAuthnManager,
70+
private readonly IManager $manager,
71+
private readonly IL10N $l10n,
72+
private readonly IAppManager $appManager,
73+
private readonly AlternativeLoginService $alternativeLoginService,
7174
) {
7275
parent::__construct($appName, $request);
7376
}
7477

75-
/**
76-
* @return RedirectResponse
77-
*/
7878
#[NoAdminRequired]
7979
#[UseSession]
8080
#[FrontpageRoute(verb: 'GET', url: '/logout')]
81-
public function logout() {
81+
public function logout(): RedirectResponse {
8282
$loginToken = $this->request->getCookie('nc_token');
8383
$uid = $this->userSession->getUser()?->getUID();
8484
if ($loginToken !== null && $uid !== null) {
85-
$this->config->deleteUserValue($uid, 'login_token', $loginToken);
85+
$this->userConfig->deleteUserConfig($uid, 'login_token', $loginToken);
8686
}
8787
$this->userSession->logout();
8888

@@ -107,22 +107,17 @@ public function logout() {
107107
return $response;
108108
}
109109

110-
/**
111-
* @param string $user
112-
* @param string $redirect_url
113-
*
114-
* @return TemplateResponse|RedirectResponse
115-
*/
116110
#[NoCSRFRequired]
117111
#[PublicPage]
118112
#[UseSession]
119113
#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
120114
#[FrontpageRoute(verb: 'GET', url: '/login')]
121-
public function showLoginForm(?string $user = null, ?string $redirect_url = null): Response {
115+
public function showLoginForm(?string $user = null, ?string $redirect_url = null): TemplateResponse|RedirectResponse {
122116
if ($this->userSession->isLoggedIn()) {
123117
return new RedirectResponse($this->urlGenerator->linkToDefaultPageUrl());
124118
}
125119

120+
/** @var array|string $loginMessages */
126121
$loginMessages = $this->session->get('loginMessages');
127122
if (!$this->manager->isFairUseOfFreePushService()) {
128123
if (!is_array($loginMessages)) {
@@ -153,13 +148,14 @@ public function showLoginForm(?string $user = null, ?string $redirect_url = null
153148
$this->config->getSystemValueInt('remember_login_cookie_lifetime', 60 * 60 * 24 * 15) > 0
154149
);
155150

156-
if (!empty($redirect_url)) {
151+
if ($redirect_url !== null && $redirect_url !== '') {
157152
[$url, ] = explode('?', $redirect_url);
158153
if ($url !== $this->urlGenerator->linkToRoute('core.login.logout')) {
159154
$this->initialState->provideInitialState('loginRedirectUrl', $redirect_url);
160155
}
161156
}
162157

158+
/** @psalm-suppress */
163159
$this->initialState->provideInitialState(
164160
'loginThrottleDelay',
165161
$this->throttler->getDelay($this->request->getRemoteAddress())
@@ -185,7 +181,11 @@ public function showLoginForm(?string $user = null, ?string $redirect_url = null
185181
Util::addHeader('meta', ['name' => 'referrer', 'content' => 'same-origin']);
186182

187183
$parameters = [
188-
'alt_login' => OC_App::getAlternativeLogIns(),
184+
'alt_login' => array_map(static fn (IAlternativeLogin $alternativeLogin): array => [
185+
'name' => $alternativeLogin->getLabel(),
186+
'href' => $alternativeLogin->getLink(),
187+
'class' => $alternativeLogin->getClass(),
188+
], $this->alternativeLoginService->getAlternativeLogins()),
189189
'pageTitle' => $this->l10n->t('Login'),
190190
];
191191

@@ -203,8 +203,6 @@ public function showLoginForm(?string $user = null, ?string $redirect_url = null
203203

204204
/**
205205
* Sets the password reset state
206-
*
207-
* @param string $username
208206
*/
209207
private function setPasswordResetInitialState(?string $username): void {
210208
if ($username !== null && $username !== '') {
@@ -227,15 +225,14 @@ private function setPasswordResetInitialState(?string $username): void {
227225
}
228226

229227
/**
230-
* Sets the initial state of whether or not a user is allowed to login with their email
228+
* Sets the initial state of whether a user is allowed to log in with their email
231229
* initial state is passed in the array of 1 for email allowed and 0 for not allowed
232230
*/
233231
private function setEmailStates(): void {
234-
$emailStates = []; // true: can login with email, false otherwise - default to true
232+
$emailStates = []; // true: can log in with email, false otherwise - default to true
235233

236234
// check if user_ldap is enabled, and the required classes exist
237-
if ($this->appManager->isAppLoaded('user_ldap')
238-
&& class_exists(Helper::class)) {
235+
if ($this->appManager->isAppLoaded('user_ldap') && class_exists(Helper::class)) {
239236
$helper = Server::get(Helper::class);
240237
$allPrefixes = $helper->getServerConfigurationPrefixes();
241238
// check each LDAP server the user is connected too
@@ -248,15 +245,10 @@ private function setEmailStates(): void {
248245
}
249246

250247
/**
251-
* @param string|null $passwordLink
252-
* @param IUser|null $user
253-
*
254248
* Users may not change their passwords if:
255249
* - The account is disabled
256250
* - The backend doesn't support password resets
257251
* - The password reset function is disabled
258-
*
259-
* @return bool
260252
*/
261253
private function canResetPassword(?string $passwordLink, ?IUser $user): bool {
262254
if ($passwordLink === 'disabled') {
@@ -366,29 +358,23 @@ public function tryLogin(
366358
);
367359
}
368360

369-
if ($result->getRedirectUrl() !== null) {
370-
return new RedirectResponse($result->getRedirectUrl());
361+
$redirectUrl = $result->getRedirectUrl();
362+
if ($redirectUrl !== null) {
363+
return new RedirectResponse($redirectUrl);
371364
}
372365
return $this->generateRedirect($redirect_url);
373366
}
374367

375368
/**
376369
* Creates a login failed response.
377-
*
378-
* @param string $user
379-
* @param string $originalUser
380-
* @param string $redirect_url
381-
* @param string $loginMessage
382-
*
383-
* @return RedirectResponse
384370
*/
385371
private function createLoginFailedResponse(
386-
$user,
387-
$originalUser,
388-
$redirect_url,
372+
?string $user,
373+
string $originalUser,
374+
?string $redirect_url,
389375
string $loginMessage,
390376
bool $throttle = true,
391-
) {
377+
): RedirectResponse {
392378
// Read current user and append if possible we need to
393379
// return the unmodified user otherwise we will leak the login name
394380
$args = $user !== null ? ['user' => $originalUser, 'direct' => 1] : [];
@@ -399,7 +385,7 @@ private function createLoginFailedResponse(
399385
$this->urlGenerator->linkToRoute('core.login.showLoginForm', $args)
400386
);
401387
if ($throttle) {
402-
$response->throttle(['user' => substr($user, 0, 64)]);
388+
$response->throttle(['user' => substr($user ?? '', 0, 64)]);
403389
}
404390
$this->session->set('loginMessages', [
405391
[$loginMessage], []
@@ -428,6 +414,9 @@ private function createLoginFailedResponse(
428414
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
429415
public function confirmPassword(string $password): DataResponse {
430416
$loginName = $this->userSession->getLoginName();
417+
if ($loginName === null) {
418+
throw new \RuntimeException('Login name not set');
419+
}
431420
$loginResult = $this->userManager->checkPassword($loginName, $password);
432421
if ($loginResult === false) {
433422
$response = new DataResponse([], Http::STATUS_FORBIDDEN);

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,6 +1263,7 @@
12631263
'OC\\Authentication\\LoginCredentials\\Credentials' => $baseDir . '/lib/private/Authentication/LoginCredentials/Credentials.php',
12641264
'OC\\Authentication\\LoginCredentials\\Store' => $baseDir . '/lib/private/Authentication/LoginCredentials/Store.php',
12651265
'OC\\Authentication\\Login\\ALoginCommand' => $baseDir . '/lib/private/Authentication/Login/ALoginCommand.php',
1266+
'OC\\Authentication\\Login\\AlternativeLoginService' => $baseDir . '/lib/private/Authentication/Login/AlternativeLoginService.php',
12661267
'OC\\Authentication\\Login\\Chain' => $baseDir . '/lib/private/Authentication/Login/Chain.php',
12671268
'OC\\Authentication\\Login\\ClearLostPasswordTokensCommand' => $baseDir . '/lib/private/Authentication/Login/ClearLostPasswordTokensCommand.php',
12681269
'OC\\Authentication\\Login\\CompleteLoginCommand' => $baseDir . '/lib/private/Authentication/Login/CompleteLoginCommand.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
13041304
'OC\\Authentication\\LoginCredentials\\Credentials' => __DIR__ . '/../../..' . '/lib/private/Authentication/LoginCredentials/Credentials.php',
13051305
'OC\\Authentication\\LoginCredentials\\Store' => __DIR__ . '/../../..' . '/lib/private/Authentication/LoginCredentials/Store.php',
13061306
'OC\\Authentication\\Login\\ALoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/ALoginCommand.php',
1307+
'OC\\Authentication\\Login\\AlternativeLoginService' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/AlternativeLoginService.php',
13071308
'OC\\Authentication\\Login\\Chain' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/Chain.php',
13081309
'OC\\Authentication\\Login\\ClearLostPasswordTokensCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/ClearLostPasswordTokensCommand.php',
13091310
'OC\\Authentication\\Login\\CompleteLoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/CompleteLoginCommand.php',
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH
7+
* SPDX-FileContributor: Carl Schwan
8+
* SPDX-License-Identifier: AGPL-3.0-or-later
9+
*/
10+
11+
namespace OC\Authentication\Login;
12+
13+
use OC\AppFramework\Bootstrap\Coordinator;
14+
use OCP\Authentication\IAlternativeLogin;
15+
use OCP\Authentication\IAlternativeLoginProvider;
16+
use Psr\Container\ContainerExceptionInterface;
17+
use Psr\Container\ContainerInterface;
18+
use Psr\Log\LoggerInterface;
19+
use Throwable;
20+
21+
class AlternativeLoginService {
22+
public function __construct(
23+
private readonly Coordinator $coordinator,
24+
private readonly LoggerInterface $logger,
25+
private readonly ContainerInterface $container,
26+
) {
27+
28+
}
29+
30+
/**
31+
* @return list<IAlternativeLogin>
32+
*/
33+
public function getAlternativeLogins(): array {
34+
$result = [];
35+
36+
foreach ($this->coordinator->getRegistrationContext()->getAlternativeLoginProviders() as $registration) {
37+
if (!in_array(IAlternativeLoginProvider::class, class_implements($registration->getService()), true)) {
38+
$this->logger->error('Alternative login option {option} does not implement {interface} and is therefore ignored.', [
39+
'option' => $registration->getService(),
40+
'interface' => IAlternativeLoginProvider::class,
41+
'app' => $registration->getAppId(),
42+
]);
43+
continue;
44+
}
45+
46+
try {
47+
/** @var IAlternativeLoginProvider $provider */
48+
$provider = $this->container->get($registration->getService());
49+
} catch (ContainerExceptionInterface $e) {
50+
$this->logger->error('Alternative login option {option} can not be initialized.',
51+
[
52+
'exception' => $e,
53+
'option' => $registration->getService(),
54+
'app' => $registration->getAppId(),
55+
]);
56+
continue;
57+
}
58+
59+
foreach ($provider->getAlternativeLogins() as $alternativeLogin) {
60+
try {
61+
$alternativeLogin->load();
62+
63+
$result[] = $alternativeLogin;
64+
} catch (Throwable $e) {
65+
$this->logger->error('Alternative login option {option} had an error while loading.',
66+
[
67+
'exception' => $e,
68+
'option' => $registration->getService(),
69+
'app' => $registration->getAppId(),
70+
]);
71+
}
72+
}
73+
}
74+
75+
foreach ($this->coordinator->getRegistrationContext()->getAlternativeLogins() as $registration) {
76+
if (!in_array(IAlternativeLogin::class, class_implements($registration->getService()), true)) {
77+
$this->logger->error('Alternative login option {option} does not implement {interface} and is therefore ignored.', [
78+
'option' => $registration->getService(),
79+
'interface' => IAlternativeLogin::class,
80+
'app' => $registration->getAppId(),
81+
]);
82+
continue;
83+
}
84+
85+
try {
86+
/** @var IAlternativeLogin $provider */
87+
$provider = $this->container->get($registration->getService());
88+
} catch (ContainerExceptionInterface $e) {
89+
$this->logger->error('Alternative login option {option} can not be initialized.',
90+
[
91+
'exception' => $e,
92+
'option' => $registration->getService(),
93+
'app' => $registration->getAppId(),
94+
]);
95+
}
96+
97+
try {
98+
$provider->load();
99+
100+
$result[] = $provider;
101+
} catch (Throwable $e) {
102+
$this->logger->error('Alternative login option {option} had an error while loading.',
103+
[
104+
'exception' => $e,
105+
'option' => $registration->getService(),
106+
'app' => $registration->getAppId(),
107+
]);
108+
}
109+
}
110+
111+
return $result;
112+
}
113+
}

0 commit comments

Comments
 (0)