Skip to content

Commit f782905

Browse files
fixup! Add a login chain to reduce the complexity of LoginController::tryLogin
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
1 parent 7e6760e commit f782905

File tree

2 files changed

+132
-349
lines changed

2 files changed

+132
-349
lines changed

core/Controller/LoginController.php

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -235,12 +235,8 @@ private function setPasswordResetParameters(?string $user,
235235
return $parameters;
236236
}
237237

238-
/**
239-
* @param string $redirectUrl
240-
* @return RedirectResponse
241-
*/
242-
private function generateRedirect($redirectUrl) {
243-
if (!is_null($redirectUrl) && $this->userSession->isLoggedIn()) {
238+
private function generateRedirect(?string $redirectUrl): RedirectResponse {
239+
if ($redirectUrl !== null && $this->userSession->isLoggedIn()) {
244240
$location = $this->urlGenerator->getAbsoluteURL(urldecode($redirectUrl));
245241
// Deny the redirect if the URL contains a @
246242
// This prevents unvalidated redirects like ?redirect_url=:user@domain.com
@@ -264,36 +260,40 @@ private function generateRedirect($redirectUrl) {
264260
* @param string $timezone_offset
265261
* @return RedirectResponse
266262
*/
267-
public function tryLogin(string $user, string $password, $redirect_url, string $timezone = '', string $timezone_offset = '') {
263+
public function tryLogin(string $user,
264+
string $password,
265+
string $redirect_url = null,
266+
string $timezone = '',
267+
string $timezone_offset = ''): RedirectResponse {
268268
// If the user is already logged in and the CSRF check does not pass then
269269
// simply redirect the user to the correct page as required. This is the
270270
// case when an user has already logged-in, in another tab.
271271
if(!$this->request->passesCSRFCheck()) {
272272
return $this->generateRedirect($redirect_url);
273273
}
274274

275-
$result = $this->loginChain->process(new LoginData(
275+
$data = new LoginData(
276276
$this->request,
277277
$user,
278278
$password,
279279
$redirect_url,
280280
$timezone,
281281
$timezone_offset
282-
));
282+
);
283+
$result = $this->loginChain->process($data);
283284
if (!$result->isSuccess()) {
284-
if ($result->getRedirectUrl() !== null) {
285-
return new RedirectResponse($result->getRedirectUrl());
286-
}
287-
288-
return $this->generateRedirect($redirect_url);
285+
return $this->createLoginFailedResponse(
286+
$data->getUsername(),
287+
$user,
288+
$redirect_url,
289+
$result->getErrorMessage()
290+
);
289291
}
290292

291-
return $this->createLoginFailedResponse(
292-
$loginData->getUsername(),
293-
$user,
294-
$redirect_url,
295-
$result->getErrorMessage()
296-
);
293+
if ($result->getRedirectUrl() !== null) {
294+
return new RedirectResponse($result->getRedirectUrl());
295+
}
296+
return $this->generateRedirect($redirect_url);
297297
}
298298

299299
/**
@@ -309,8 +309,8 @@ private function createLoginFailedResponse(
309309
$user, $originalUser, $redirect_url, string $loginMessage) {
310310
// Read current user and append if possible we need to
311311
// return the unmodified user otherwise we will leak the login name
312-
$args = !is_null($user) ? ['user' => $originalUser] : [];
313-
if (!is_null($redirect_url)) {
312+
$args = $user !== null ? ['user' => $originalUser] : [];
313+
if ($redirect_url !== null) {
314314
$args['redirect_url'] = $redirect_url;
315315
}
316316
$response = new RedirectResponse(

0 commit comments

Comments
 (0)