Skip to content

Commit

Permalink
fix: referrer for login not being GET parameter
Browse files Browse the repository at this point in the history 
Quite bad for SEO, however, Google has not indexed the affected pages (ignored).

Also fixes a flaw in the logic that could lead to a "recursion" due to
consecutive failed login attempts.
  • Loading branch information
@tomudding
tomudding committed Jul 21, 2024
1 parent 2b2b2d7 commit b8de551
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 98 deletions.
12 changes: 4 additions & 8 deletions module/Activity/view/activity/activity/view.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,8 @@ $this->headTitle($this->translate('Activities'));
<?php if (!$isAllowedToSubscribe && $signupList->getOnlyGEWIS()): ?>
<a href="<?= $this->url(
'user/login',
[
'user_type' => 'member',
'redirect_to' => base64_encode($this->serverUrl(true)),
],
['user_type' => 'member'],
['query' => ['redirect_to' => base64_encode($this->serverUrl(true))]],
) ?>"
class="btn btn-primary btn-lg">
<span class="fas fa-user-plus"></span> <?= $this->translate('Log in to subscribe') ?>
Expand Down Expand Up @@ -341,10 +339,8 @@ $this->headTitle($this->translate('Activities'));
<?php endif; ?>
<a href="<?= $this->url(
'user/login',
[
'user_type' => 'member',
'redirect_to' => base64_encode($this->serverUrl(true)),
],
['user_type' => 'member'],
['query' => ['redirect_to' => base64_encode($this->serverUrl(true))]],
) ?>">
<?= $this->translate('Login to view the subscribed members.') ?>
</a>
Expand Down
6 changes: 2 additions & 4 deletions module/Activity/view/partial/signupForm.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,8 @@ function formElementRender(
$this->translate('Do you have a GEWIS account? <a href="%s">Log in to subscribe.</a>'),
$this->url(
'user/login',
[
'user_type' => 'member',
'redirect_to' => base64_encode($this->serverUrl(true)),
],
['user_type' => 'member'],
['query' => ['redirect_to' => base64_encode($this->serverUrl(true))]],
),
) ?>
</strong> <?= $this->translate('Or subscribe without a GEWIS membership: ') ?>
Expand Down
4 changes: 2 additions & 2 deletions module/Application/view/error/403.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ use Laminas\View\Renderer\PhpRenderer;
if ($this->identity() === null): ?>
<p><?= $this->translate('You might be able to view this page by logging in') ?></p>
<a href="<?= $this->url(
'user/login',
['redirect_to' => base64_encode($this->serverUrl(true))],
name: 'user/login',
options: ['query' => ['redirect_to' => base64_encode($this->serverUrl(true))]],
) ?>">
<span class="fas fa-user"></span>
<?= $this->translate('Login') ?>
Expand Down
6 changes: 2 additions & 4 deletions module/Application/view/partial/admin.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,8 @@ use Laminas\View\Renderer\PhpRenderer;
<li>
<a href="<?= $this->url(
'user/login',
[
'user_type' => 'member',
'redirect_to' => base64_encode($this->serverUrl(true)),
],
['user_type' => 'member'],
['query' => ['redirect_to' => base64_encode($this->serverUrl(true))]],
) ?>">
<?= $this->translate('Login') ?>
</a>
Expand Down
5 changes: 4 additions & 1 deletion module/Application/view/partial/main-nav.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ endif; ?>
</a>
<ul class="dropdown-menu">
<li>
<a href="<?= $this->url('user/login', ['redirect_to' => base64_encode($this->serverUrl(true))]) ?>">
<a href="<?= $this->url(
name: 'user/login',
options: ['query' => ['redirect_to' => base64_encode($this->serverUrl(true))]],
) ?>">
<?= $this->translate('Login') ?>
</a>
</li>
Expand Down
6 changes: 2 additions & 4 deletions module/Education/view/education/education/course.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,8 @@ foreach ($documents as $document) {
$this->translate('If you want to view these documents, you can either <a href="%s">login</a> or access these documents on the campus.'),
$this->url(
'user/login',
[
'user_type' => 'member',
'redirect_to' => base64_encode($this->serverUrl(true)),
],
['user_type' => 'member'],
['query' => ['redirect_to' => base64_encode($this->serverUrl(true))]],
),
) ?>
</p>
Expand Down
3 changes: 2 additions & 1 deletion module/Frontpage/view/frontpage/organ/organ.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ function getOrganDescription($organInformation, $lang)
<p>
<a href="<?= $this->url(
'user/login',
['redirect_to' => base64_encode($this->serverUrl(true))],
['user_type' => 'member'],
['query' => ['redirect_to' => base64_encode($this->serverUrl(true))]],
) ?>">
<?= $this->translate('Login') ?>
</a>
Expand Down
3 changes: 1 addition & 2 deletions module/User/config/module.config.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,9 @@
'login' => [
'type' => Segment::class,
'options' => [
'route' => '/login[/:user_type][/to/:redirect_to]',
'route' => '/login[/:user_type]',
'constraints' => [
'user_type' => '(company|member)',
'redirect_to' => '[a-zA-Z0-9\+\=]+',
],
'defaults' => [
'action' => 'login',
Expand Down
61 changes: 20 additions & 41 deletions module/User/src/Controller/UserController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
use Laminas\Mvc\I18n\Translator;
use Laminas\Mvc\Plugin\FlashMessenger\FlashMessenger;
use Laminas\View\Model\ViewModel;
use User\Form\CompanyUserLogin as CompanyLoginForm;
use User\Form\UserLogin as UserLoginForm;
use User\Permissions\NotAllowedException;
use User\Service\AclService;
use User\Service\User as UserService;
Expand Down Expand Up @@ -44,18 +42,19 @@ public function loginAction(): Response|ViewModel
}

$userType = $this->params()->fromRoute('user_type');
$redirectTo = $this->params()->fromRoute('redirect_to');
$redirectTo = $this->params()->fromQuery('redirect_to');

if ('company' === $userType) {
$form = $this->userService->getCompanyUserLoginForm();
} else {
$form = $this->userService->getUserLoginForm();
}

/** @var Request $request */
$request = $this->getRequest();
if ($request->isPost()) {
if ('company' === $userType) {
$form = $this->userService->getCompanyUserLoginForm();
} else {
$form = $this->userService->getUserLoginForm();
}

$form->setData($request->getPost()->toArray());

if ($form->isValid()) {
$data = $form->getData();

Expand All @@ -67,50 +66,30 @@ public function loginAction(): Response|ViewModel

if (null !== $login) {
return $this->redirect()->toUrl(
$this->decodeRedirect(empty($data['redirect']) ? $redirectTo : $data['redirect']),
$this->decodeRedirect($redirectTo),
);
}
}
}

if (null === $redirectTo) {
$redirectTo = base64_encode(
$this->url()->fromRoute(
route: 'home',
options: ['force_canonical' => true],
),
);
}

return new ViewModel(
[
'form' => $this->handleRedirect($userType, $redirectTo),
'form' => $form,
'redirectTo' => $redirectTo,
'userType' => $userType,
],
);
}

private function handleRedirect(
string $userType,
?string $referer,
): CompanyLoginForm|UserLoginForm {
if ('company' === $userType) {
$form = $this->userService->getCompanyUserLoginForm();
} else {
$form = $this->userService->getUserLoginForm();
}

if (null === $form->get('redirect')->getValue()) {
if (null !== $referer) {
$form->get('redirect')->setValue($referer);

return $form;
}

$form->get('redirect')->setValue(
base64_encode(
$this->url()->fromRoute(
route: 'home',
options: ['force_canonical' => true],
),
),
);
}

return $form;
}

/**
* Decode the base64 encoded referer, if it is not valid always return the home page.
*/
Expand Down
8 changes: 0 additions & 8 deletions module/User/src/Form/CompanyUserLogin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use Laminas\Filter\StringTrim;
use Laminas\Form\Element\Csrf;
use Laminas\Form\Element\Email;
use Laminas\Form\Element\Hidden;
use Laminas\Form\Element\Password;
use Laminas\Form\Element\Submit;
use Laminas\Form\Form;
Expand Down Expand Up @@ -56,13 +55,6 @@ public function __construct(
],
);

$this->add(
[
'name' => 'redirect',
'type' => Hidden::class,
],
);

$this->add(
[
'name' => 'security',
Expand Down
8 changes: 0 additions & 8 deletions module/User/src/Form/UserLogin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Laminas\Authentication\Result;
use Laminas\Form\Element\Checkbox;
use Laminas\Form\Element\Csrf;
use Laminas\Form\Element\Hidden;
use Laminas\Form\Element\Password;
use Laminas\Form\Element\Submit;
use Laminas\Form\Element\Text;
Expand Down Expand Up @@ -68,13 +67,6 @@ public function __construct(
],
);

$this->add(
[
'name' => 'redirect',
'type' => Hidden::class,
],
);

$this->add(
[
'name' => 'security',
Expand Down
9 changes: 3 additions & 6 deletions module/User/view/partial/login/company.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use User\Form\CompanyUserLogin as CompanyUserLoginForm;
/**
* @var PhpRenderer|HelperTrait $this
* @var CompanyUserLoginForm $form
* @var string $redirectTo
*/

$form->prepare();
Expand All @@ -17,10 +18,8 @@ $form->setAttribute(
'action',
$this->url(
'user/login',
[
'user_type' => 'company',
'redirect_to' => base64_encode($this->serverUrl(true)),
],
['user_type' => 'company'],
['query' => ['redirect_to' => $redirectTo]],
),
);
$form->setAttribute('method', 'post');
Expand Down Expand Up @@ -63,8 +62,6 @@ $form->setAttribute('class', 'form-horizontal');
</div>
</div>

<?= $this->formInput($form->get('redirect')) ?>

<div class="form-group">
<div class="col-sm-offset-5 col-sm-7">
<?php
Expand Down
10 changes: 3 additions & 7 deletions module/User/view/partial/login/member.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,16 @@ use User\Form\UserLogin as UserLoginForm;
/**
* @var PhpRenderer|HelperTrait $this
* @var UserLoginForm $form
* @var string $redirectTo
*/

$form->prepare();

$form->setAttribute(
'action',
$this->url(
'user/login',
[
'user_type' => 'member',
'redirect_to' => base64_encode($this->serverUrl(true)),
],
['user_type' => 'member'],
['query' => ['redirect_to' => $redirectTo]],
),
);
$form->setAttribute('method', 'post');
Expand Down Expand Up @@ -71,8 +69,6 @@ $form->setAttribute('class', 'form-horizontal');
</div>
</div>

<?= $this->formInput($form->get('redirect')) ?>

<div class="form-group">
<div class="col-sm-offset-5 col-sm-7">
<?php
Expand Down
17 changes: 15 additions & 2 deletions module/User/view/user/user/login.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use User\Form\UserLogin as UserLoginForm;
/**
* @var PhpRenderer|HelperTrait $this
* @var CompanyUserLoginForm|UserLoginForm $form
* @var string $redirectTo
* @var string $userType
*/

Expand Down Expand Up @@ -39,9 +40,21 @@ $isMember = ('member' === $userType);
<h3><?= $this->translate('Login') ?></h3>
</div>
<?php if ($isMember): ?>
<?= $this->partial('partial/login/member.phtml', ['form' => $form]) ?>
<?= $this->partial(
'partial/login/member.phtml',
[
'form' => $form,
'redirectTo' => $redirectTo,
],
) ?>
<?php else: ?>
<?= $this->partial('partial/login/company.phtml', ['form' => $form]) ?>
<?= $this->partial(
'partial/login/company.phtml',
[
'form' => $form,
'redirectTo' => $redirectTo,
],
) ?>
<?php endif; ?>
</div>
</div>
Expand Down

0 comments on commit b8de551

Please sign in to comment.