Skip to content

Commit 0f759bf

Browse files
marienfressinaudmarienfressinaud
committed
chore: Refactor the my/Sessions controller
1 parent 29a68f3 commit 0f759bf

File tree

7 files changed

+88
-56
lines changed

7 files changed

+88
-56
lines changed

‎src/auth/Access.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ public static function can(?models\User $user, string $action, object $subject):
3737
$access_class = NotesAccess::class;
3838
} elseif ($subject instanceof models\Importation) {
3939
$access_class = ImportationsAccess::class;
40+
} elseif ($subject instanceof models\Session) {
41+
$access_class = SessionsAccess::class;
4042
} else {
4143
throw new \InvalidArgumentException("{$subject_class} subject is not supported");
4244
}

‎src/auth/SessionsAccess.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
namespace App\auth;
4+
5+
use App\models;
6+
7+
/**
8+
* @author Marien Fressinaud <dev@marienfressinaud.fr>
9+
* @license http://www.gnu.org/licenses/agpl-3.0.en.html AGPL
10+
*/
11+
class SessionsAccess
12+
{
13+
public static function canDelete(?models\User $user, models\Session $session): bool
14+
{
15+
return (
16+
$user &&
17+
$user->id === $session->user_id
18+
);
19+
}
20+
}

‎src/controllers/my/Sessions.php‎

Lines changed: 39 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22

33
namespace App\controllers\my;
44

5-
use Minz\Request;
6-
use Minz\Response;
75
use App\auth;
86
use App\controllers\BaseController;
7+
use App\forms;
98
use App\models;
9+
use Minz\Request;
10+
use Minz\Response;
1011

1112
/**
1213
* @author Marien Fressinaud <dev@marienfressinaud.fr>
@@ -17,27 +18,23 @@ class Sessions extends BaseController
1718
/**
1819
* List the sessions of the current user
1920
*
20-
* @response 302 /login?redirect_to=/my/sessions
21-
* If the user is not connected.
22-
* @response 302 /my/security/confirmation?from=/my/sessions
23-
* If the password is not confirmed.
2421
* @response 200
2522
* On success.
23+
*
24+
* @throws auth\MissingCurrentUserError
25+
* If the user is not connected.
26+
* @throws auth\PasswordNotConfirmedError
27+
* If the password is not confirmed.
2628
*/
2729
public function index(Request $request): Response
2830
{
29-
$user = $this->requireCurrentUser(redirect_after_login: \Minz\Url::for('sessions'));
31+
$user = auth\CurrentUser::require();
3032

31-
$session = auth\CurrentUser::session();
33+
auth\CurrentUser::requireConfirmedPassword();
3234

35+
$session = auth\CurrentUser::session();
3336
assert($session !== null);
3437

35-
if (!$session->isPasswordConfirmed()) {
36-
return Response::redirect('password confirmation', [
37-
'from' => \Minz\Url::for('sessions'),
38-
]);
39-
}
40-
4138
$sessions = models\Session::listBy([
4239
'user_id' => $user->id,
4340
], 'created_at DESC');
@@ -53,53 +50,50 @@ public function index(Request $request): Response
5350
*
5451
* @request_param string id
5552
*
56-
* @response 302 /login?redirect_to=/my/sessions
57-
* If the user is not connected.
58-
* @response 302 /my/security/confirmation?from=/my/sessions
59-
* If the password is not confirmed.
60-
* @response 404
61-
* If the session doesn't exist.
6253
* @response 302 /my/sessions
54+
* @flash error
6355
* If the CSRF token is invalid.
6456
* @response 302 /my/sessions
6557
* On success.
58+
*
59+
* @throws auth\MissingCurrentUserError
60+
* If the user is not connected.
61+
* @throws auth\PasswordNotConfirmedError
62+
* If the password is not confirmed.
63+
* @throws \Minz\Errors\MissingRecordError
64+
* If the session doesn't exist.
65+
* @throws auth\AccessDeniedError
66+
* If the user cannot delete the session.
6667
*/
6768
public function delete(Request $request): Response
6869
{
69-
$user = $this->requireCurrentUser(redirect_after_login: \Minz\Url::for('sessions'));
70-
71-
$current_session = auth\CurrentUser::session();
70+
$user = auth\CurrentUser::require();
7271

73-
assert($current_session !== null);
72+
auth\CurrentUser::requireConfirmedPassword();
7473

75-
if (!$current_session->isPasswordConfirmed()) {
76-
return Response::redirect('password confirmation', [
77-
'from' => \Minz\Url::for('sessions'),
78-
]);
79-
}
74+
$session = models\Session::requireFromRequest($request);
8075

81-
$session_id = $request->parameters->getString('id', '');
82-
$csrf = $request->parameters->getString('csrf', '');
76+
auth\Access::require($user, 'delete', $session);
8377

84-
$session = models\Session::findBy([
85-
'id' => $session_id,
86-
'user_id' => $user->id,
87-
]);
78+
$form = new forms\security\DeleteSession();
79+
$form->handleRequest($request);
8880

89-
if (!$session) {
90-
return Response::notFound('not_found.phtml');
81+
if (!$form->validate()) {
82+
\Minz\Flash::set('error', $form->error('@base'));
83+
return Response::redirect('sessions');
9184
}
9285

9386
$response = Response::redirect('sessions');
9487

95-
if (\App\Csrf::validate($csrf)) {
96-
if ($session->id === $current_session->id) {
97-
auth\CurrentUser::deleteSession();
98-
$response->removeCookie('session_token');
99-
$response->removeCookie('flusio_session_token');
100-
} else {
101-
$session->remove();
102-
}
88+
$current_session = auth\CurrentUser::session();
89+
assert($current_session !== null);
90+
91+
if ($session->id === $current_session->id) {
92+
auth\CurrentUser::deleteSession();
93+
$response->removeCookie('session_token');
94+
$response->removeCookie('flusio_session_token');
95+
} else {
96+
$session->remove();
10397
}
10498

10599
return $response;

‎src/forms/security/DeleteSession.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
namespace App\forms\security;
4+
5+
use App\forms\BaseForm;
6+
7+
/**
8+
* @author Marien Fressinaud <dev@marienfressinaud.fr>
9+
* @license http://www.gnu.org/licenses/agpl-3.0.en.html AGPL
10+
*/
11+
class DeleteSession extends BaseForm
12+
{
13+
}

‎src/models/Session.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ class Session
1818
{
1919
use dao\Session;
2020
use Database\Recordable;
21+
use Database\Resource;
2122

2223
public const SCOPES = ['browser', 'api'];
2324

‎src/views/my/sessions/index.phtml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@
4848

4949
<?php if ($session->id !== $current_session->id): ?>
5050
<form method="post" action="<?= url('delete session', ['id' => $session->id]) ?>">
51-
<input type="hidden" name="csrf" value="<?= csrf_token() ?>" />
52-
5351
<small>
5452
<button class="button--smaller" type="submit">
5553
<?= _('disconnect') ?>
5654
</button>
5755
</small>
56+
57+
<input type="hidden" name="csrf_token" value="<?= csrf_token('security\\DeleteSession') ?>" />
5858
</form>
5959
<?php endif; ?>
6060
</div>

‎tests/controllers/my/SessionsTest.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33
namespace App\controllers\my;
44

55
use App\auth;
6+
use App\forms;
67
use App\models;
78
use tests\factories\SessionFactory;
89
use tests\factories\UserFactory;
910

1011
class SessionsTest extends \PHPUnit\Framework\TestCase
1112
{
1213
use \Minz\Tests\ApplicationHelper;
14+
use \Minz\Tests\CsrfHelper;
1315
use \Minz\Tests\InitializerHelper;
1416
use \Minz\Tests\ResponseAsserts;
1517
use \Minz\Tests\TimeHelper;
@@ -45,7 +47,7 @@ public function testIndexRedirectsIfPasswordIsNotConfirmed(): void
4547

4648
$response = $this->appRun('GET', '/my/sessions');
4749

48-
$this->assertResponseCode($response, 302, '/my/security/confirmation?from=%2Fmy%2Fsessions');
50+
$this->assertResponseCode($response, 302, '/my/security/confirmation?redirect_to=%2Fmy%2Fsessions');
4951
}
5052

5153
public function testIndexRedirectsIfUserIsNotConnected(): void
@@ -69,7 +71,7 @@ public function testDeleteRemovesTheSessionAndRedirects(): void
6971
]);
7072

7173
$response = $this->appRun('POST', "/my/sessions/{$session->id}/deletion", [
72-
'csrf' => \App\Csrf::generate(),
74+
'csrf_token' => $this->csrfToken(forms\security\DeleteSession::class),
7375
]);
7476

7577
$this->assertResponseCode($response, 302, '/my/sessions');
@@ -90,7 +92,7 @@ public function testDeleteLogsOutIfGivenSessionIsCurrentSession(): void
9092
$this->assertNotNull($session);
9193

9294
$response = $this->appRun('POST', "/my/sessions/{$session->id}/deletion", [
93-
'csrf' => \App\Csrf::generate(),
95+
'csrf_token' => $this->csrfToken(forms\security\DeleteSession::class),
9496
]);
9597

9698
$this->assertResponseCode($response, 302, '/my/sessions');
@@ -116,7 +118,7 @@ public function testDeleteDoesNotRemoveSessionIfCsrfIsInvalid(): void
116118
]);
117119

118120
$response = $this->appRun('POST', "/my/sessions/{$session->id}/deletion", [
119-
'csrf' => 'not the token',
121+
'csrf_token' => 'not the token',
120122
]);
121123

122124
$this->assertResponseCode($response, 302, '/my/sessions');
@@ -131,10 +133,10 @@ public function testDeleteRedirectsIfUserIsNotConnected(): void
131133
]);
132134

133135
$response = $this->appRun('POST', "/my/sessions/{$session->id}/deletion", [
134-
'csrf' => \App\Csrf::generate(),
136+
'csrf_token' => $this->csrfToken(forms\security\DeleteSession::class),
135137
]);
136138

137-
$this->assertResponseCode($response, 302, '/login?redirect_to=%2Fmy%2Fsessions');
139+
$this->assertResponseCode($response, 302, '/login?redirect_to=%2F');
138140
$this->assertTrue(models\Session::exists($session->id));
139141
}
140142

@@ -152,10 +154,10 @@ public function testDeleteRedirectsIfPasswordIsNotConfirmed(): void
152154
]);
153155

154156
$response = $this->appRun('POST', "/my/sessions/{$session->id}/deletion", [
155-
'csrf' => \App\Csrf::generate(),
157+
'csrf_token' => $this->csrfToken(forms\security\DeleteSession::class),
156158
]);
157159

158-
$this->assertResponseCode($response, 302, '/my/security/confirmation?from=%2Fmy%2Fsessions');
160+
$this->assertResponseCode($response, 302, '/my/security/confirmation?redirect_to=%2F');
159161
$this->assertTrue(models\Session::exists($session->id));
160162
}
161163

@@ -173,7 +175,7 @@ public function testDeleteFailsIfSessionDoesNotExist(): void
173175
]);
174176

175177
$response = $this->appRun('POST', "/my/sessions/not-an-id/deletion", [
176-
'csrf' => \App\Csrf::generate(),
178+
'csrf_token' => $this->csrfToken(forms\security\DeleteSession::class),
177179
]);
178180

179181
$this->assertResponseCode($response, 404);

0 commit comments

Comments
 (0)