fix(auth): update refresh token behavior to be reusable within TTL and add tests for token functionality
This commit is contained in:
+3
-3
@@ -220,13 +220,13 @@ Refresh expired JWT using refresh token.
|
|||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
> **چرخش (rotation):** هر refresh token **یکبارمصرف** است. با هر فراخوانی، توکن ارائهشده باطل میشود و یک جفت `access_token` + `refresh_token` تازه صادر میگردد. توکن قبلی دیگر کار نمیکند (`401`). کاربر **معلق** (`status != 1`) نمیتواند refresh کند.
|
> **قابل استفادهی مجدد:** refresh token تا انقضای TTL خود معتبر است و در پاسخ **بدون تغییر** برگردانده میشود (یکبارمصرف/چرخشی **نیست** — سایت عمومی در هر render سمتسرور refresh میزند و نمیتواند توکن چرخشیافته را ذخیره کند). کاربر **معلق** (`status != 1`) نمیتواند refresh کند (`401`).
|
||||||
|
|
||||||
### Response `200`
|
### Response `200`
|
||||||
```json
|
```json
|
||||||
{
|
{
|
||||||
"access_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9...",
|
"access_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9...",
|
||||||
"refresh_token": "<توکن جدید — با قبلی فرق دارد>",
|
"refresh_token": "<همان توکن ارسالی — بدون تغییر>",
|
||||||
"token_type": "Bearer",
|
"token_type": "Bearer",
|
||||||
"expires_in": 900,
|
"expires_in": 900,
|
||||||
"refresh_token_expires_in": 2592000
|
"refresh_token_expires_in": 2592000
|
||||||
@@ -236,7 +236,7 @@ Refresh expired JWT using refresh token.
|
|||||||
### Errors
|
### Errors
|
||||||
| Code | HTTP | Description |
|
| Code | HTTP | Description |
|
||||||
|------|------|-------------|
|
|------|------|-------------|
|
||||||
| `ERR_AUTH_001` | 401 | Invalid/expired/already-rotated refresh token, or suspended user |
|
| `ERR_AUTH_001` | 401 | Invalid/expired refresh token, or suspended user (`status != 1`) |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -60,7 +60,7 @@ _None outstanding._
|
|||||||
| ✅M4 | IDOR write: ClinicService `createItem`/`updateItem` bind staff via global `staffRepo->findByUuid()`, no entity-scope check (cross-tenant staff binding) | src/ClinicService/Controller/ClinicServiceController.php:157,195 | security-idor | **DONE** — staff must match tenant (entity_type/id) → 422. `tests/ClinicService/ServiceItemStaffOwnershipTest` |
|
| ✅M4 | IDOR write: ClinicService `createItem`/`updateItem` bind staff via global `staffRepo->findByUuid()`, no entity-scope check (cross-tenant staff binding) | src/ClinicService/Controller/ClinicServiceController.php:157,195 | security-idor | **DONE** — staff must match tenant (entity_type/id) → 422. `tests/ClinicService/ServiceItemStaffOwnershipTest` |
|
||||||
| ✅M5 | IDOR read: AppointmentSettings list endpoints leak any doctor's config — `listOverrides`, `listHolidays`, `availableLocations` (no ownership on {doctorUuid}) | src/Appointment/Controller/AppointmentSettingsController.php:150,256,349 | security-idor | **DONE** — ownership added to listOverrides/listHolidays/availableLocations. `tests/Appointment/AppointmentSettingsListOwnershipTest` |
|
| ✅M5 | IDOR read: AppointmentSettings list endpoints leak any doctor's config — `listOverrides`, `listHolidays`, `availableLocations` (no ownership on {doctorUuid}) | src/Appointment/Controller/AppointmentSettingsController.php:150,256,349 | security-idor | **DONE** — ownership added to listOverrides/listHolidays/availableLocations. `tests/Appointment/AppointmentSettingsListOwnershipTest` |
|
||||||
| ✅M6 | OTP send-code has no per-mobile/per-uuid cap, only per-IP (5/hr) → SMS flood from rotating IPs | src/Auth/Controller/AuthController.php:136-153 · OtpService.php:59-77 · rate_limiter.yaml:4-7 | security-ratelimit | **DONE** — per-mobile limiter (5/hr) added alongside per-IP. `tests/Auth/SendCodeMobileRateLimitTest` (6 reqs / 6 IPs → 6th 429). |
|
| ✅M6 | OTP send-code has no per-mobile/per-uuid cap, only per-IP (5/hr) → SMS flood from rotating IPs | src/Auth/Controller/AuthController.php:136-153 · OtpService.php:59-77 · rate_limiter.yaml:4-7 | security-ratelimit | **DONE** — per-mobile limiter (5/hr) added alongside per-IP. `tests/Auth/SendCodeMobileRateLimitTest` (6 reqs / 6 IPs → 6th 429). |
|
||||||
| ✅M7 | Refresh token not rotated on use (same raw token 30d), never re-checks user status | src/Auth/Controller/AuthController.php:477-480 · TokenService.php:33-45 | security-auth | **DONE** — single-use rotation (revoke old + issue new) + suspended-user (status!=1) rejected. `tests/Auth/RefreshTokenRotationTest`. |
|
| ✅M7 | Refresh token not rotated on use (same raw token 30d), never re-checks user status | src/Auth/Controller/AuthController.php:477-480 · TokenService.php:33-45 | security-auth | **DONE (revised → Path A)** — single-use rotation **removed** (it broke nobat724's refresh-per-render: a Server Component can't persist a rotated token). Token now reusable within TTL; **kept the suspended-user (status!=1) 401 check** (the real gap). `tests/Auth/RefreshTokenTest`. Admin SPA was unaffected either way (authStore persists rotated tokens). |
|
||||||
| ✅M8 | N+1: secretary list lazy-loads secretary/doctor ManyToOne per row | src/Secretary/Controller/SecretaryController.php:192,213 | perf-nplus1 | **DONE** — fetch-join secretary/doctor/clinic. `tests/Secretary/SecretaryListNPlusOneTest` |
|
| ✅M8 | N+1: secretary list lazy-loads secretary/doctor ManyToOne per row | src/Secretary/Controller/SecretaryController.php:192,213 | perf-nplus1 | **DONE** — fetch-join secretary/doctor/clinic. `tests/Secretary/SecretaryListNPlusOneTest` |
|
||||||
| ✅M9 | N+1: billing claims lazy `items` + `insuranceRepo->find()` per claim in enrichClaims | src/Billing/Controller/BillingController.php:~49,68 | perf-nplus1 | **DONE** — fetch-join items (Paginator) + batch insurance names. `tests/Billing/ClaimsListNPlusOneTest` |
|
| ✅M9 | N+1: billing claims lazy `items` + `insuranceRepo->find()` per claim in enrichClaims | src/Billing/Controller/BillingController.php:~49,68 | perf-nplus1 | **DONE** — fetch-join items (Paginator) + batch insurance names. `tests/Billing/ClaimsListNPlusOneTest` |
|
||||||
| ✅M10 | Unbounded list: `listMine` settlements `findByUser` no limit | src/Settlement/Controller/SettlementController.php:193 | perf-pagination | **DONE** — page/limit + countByUser + data.meta. `tests/Settlement/SettlementListPaginationTest` |
|
| ✅M10 | Unbounded list: `listMine` settlements `findByUser` no limit | src/Settlement/Controller/SettlementController.php:193 | perf-pagination | **DONE** — page/limit + countByUser + data.meta. `tests/Settlement/SettlementListPaginationTest` |
|
||||||
|
|||||||
@@ -481,11 +481,15 @@ class AuthController extends BaseController
|
|||||||
return $this->error(ErrorCodes::ERR_AUTH_001, ErrorCodes::message(ErrorCodes::ERR_AUTH_001), 401);
|
return $this->error(ErrorCodes::ERR_AUTH_001, ErrorCodes::message(ErrorCodes::ERR_AUTH_001), 401);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Rotate: the presented refresh token is single-use. Revoke it and issue a
|
// The refresh token is NOT single-use: it stays valid for its TTL and is
|
||||||
// fresh access + refresh pair, so a stolen token can't be reused.
|
// returned unchanged. (The public site refreshes on every server render
|
||||||
$this->tokenService->revokeRefreshToken($refreshToken);
|
// and cannot persist a rotated token from a Server Component, so rotation
|
||||||
|
// would log users out — see nobat724_front adapt-backend-audit-api prompt.)
|
||||||
|
// We keep the re-check above so a suspended user (status != 1) cannot refresh.
|
||||||
|
$tokens = $this->tokenService->issueTokens($user);
|
||||||
|
$tokens['refresh_token'] = $result['rawToken'];
|
||||||
|
|
||||||
return new JsonResponse($this->tokenService->issueTokens($user));
|
return new JsonResponse($tokens);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[OA\Get(
|
#[OA\Get(
|
||||||
|
|||||||
@@ -32,7 +32,9 @@ abstract class ApiTestCase extends WebTestCase
|
|||||||
*/
|
*/
|
||||||
protected function createUser(array $roles = ['ROLE_USER'], ?string $mobile = null): User
|
protected function createUser(array $roles = ['ROLE_USER'], ?string $mobile = null): User
|
||||||
{
|
{
|
||||||
$mobile ??= '0912' . str_pad((string) random_int(0, 9_999_999), 7, '0', STR_PAD_LEFT);
|
// 9 random digits after 09 (full ^09\d{9}$ space) — db_test is never reset,
|
||||||
|
// so a narrower space eventually collides on the unique mobile.
|
||||||
|
$mobile ??= '09' . str_pad((string) random_int(0, 999_999_999), 9, '0', STR_PAD_LEFT);
|
||||||
$user = new User($mobile);
|
$user = new User($mobile);
|
||||||
$user->setRoles($roles);
|
$user->setRoles($roles);
|
||||||
$user->setStatus(1);
|
$user->setStatus(1);
|
||||||
|
|||||||
@@ -0,0 +1,62 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
namespace App\Tests\Auth;
|
||||||
|
|
||||||
|
use App\Auth\Entity\User;
|
||||||
|
use App\Auth\Service\TokenService;
|
||||||
|
use App\Tests\ApiTestCase;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Refresh tokens are reusable within their TTL (not single-use rotated — the
|
||||||
|
* public site refreshes per server-render and can't persist a rotated token), but
|
||||||
|
* a suspended user (status != 1) cannot refresh.
|
||||||
|
*/
|
||||||
|
class RefreshTokenTest extends ApiTestCase
|
||||||
|
{
|
||||||
|
private function issueRefresh(User $user): string
|
||||||
|
{
|
||||||
|
return static::getContainer()->get(TokenService::class)->issueTokens($user)['refresh_token'];
|
||||||
|
}
|
||||||
|
|
||||||
|
private function refresh(string $token): array
|
||||||
|
{
|
||||||
|
$this->client->request(
|
||||||
|
'POST',
|
||||||
|
'/oauth/token/refresh',
|
||||||
|
server: ['CONTENT_TYPE' => 'application/json'],
|
||||||
|
content: json_encode(['refresh_token' => $token]),
|
||||||
|
);
|
||||||
|
|
||||||
|
return json_decode($this->client->getResponse()->getContent(), true) ?? [];
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testTokenIsReusableAndReturnedUnchanged(): void
|
||||||
|
{
|
||||||
|
$this->client->disableReboot();
|
||||||
|
$user = $this->createUser();
|
||||||
|
$token = $this->issueRefresh($user);
|
||||||
|
|
||||||
|
$body = $this->refresh($token);
|
||||||
|
$this->assertSame(200, $this->responseCode());
|
||||||
|
$this->assertNotEmpty($body['access_token']);
|
||||||
|
// not rotated → the same refresh token comes back and stays valid
|
||||||
|
$this->assertSame($token, $body['refresh_token']);
|
||||||
|
|
||||||
|
// reusable: the same token works again (no single-use revocation)
|
||||||
|
$this->refresh($token);
|
||||||
|
$this->assertSame(200, $this->responseCode());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSuspendedUserCannotRefresh(): void
|
||||||
|
{
|
||||||
|
$this->client->disableReboot();
|
||||||
|
$user = $this->createUser();
|
||||||
|
$token = $this->issueRefresh($user);
|
||||||
|
|
||||||
|
$user->setStatus(0);
|
||||||
|
$this->em->flush();
|
||||||
|
|
||||||
|
$this->refresh($token);
|
||||||
|
$this->assertSame(401, $this->responseCode());
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user