fix: prevent sensitive field leaks in /me, add retry logic to Brent price sources
Some checks failed
linter / quality (push) Has been cancelled
tests / ci (8.3) (push) Has been cancelled
tests / ci (8.4) (push) Has been cancelled
tests / ci (8.5) (push) Has been cancelled

- Made `/api/auth/me` public and return explicit allowlist (name, email,
  two_factor_confirmed_at, tier, subscription fields) instead of spreading
  `$user->toArray()` which leaked is_admin, stripe_id, pm_type, pm_last_four,
  postcode. Returns `null` when unauthenticated rather than 401.
- Moved `/auth/logout` to remain behind auth:sanctum gate.
- Added 3×200ms retry with exponential backoff to EiaBrentPriceSource and
  FredBrentPriceSource on ConnectionException or 5xx responses. Timeout
  raised from 10s to 30s.
- Both sources now throw typed BrentPriceFetchException on exhausted retries
  instead of silently returning null + logging. Updated tests to assert
  exception message includes HTTP status or "connection failed".
This commit is contained in:
Ovidiu U
2026-05-01 13:22:36 +01:00
parent df70e514e9
commit 73de53994f
6 changed files with 145 additions and 75 deletions

View File

@@ -64,19 +64,24 @@ class AuthController extends Controller
public function me(Request $request): JsonResponse public function me(Request $request): JsonResponse
{ {
$user = $request->user(); $user = $request->user();
if ($user === null) {
return new JsonResponse('null', json: true);
}
$subscription = $user->subscription(); $subscription = $user->subscription();
$expiresAt = $subscription?->ends_at ?? $subscription?->current_period_end; $expiresAt = $subscription?->ends_at ?? $subscription?->current_period_end;
return response()->json(array_merge( return response()->json([
$user->toArray(), 'name' => $user->name,
[ 'email' => $user->email,
'tier' => PlanFeatures::for($user)->tier(), 'two_factor_confirmed_at' => $user->two_factor_confirmed_at?->toIso8601String(),
'subscription_cancelled' => $subscription?->canceled() ?? false, 'tier' => PlanFeatures::for($user)->tier(),
'subscription_cadence' => Plan::resolveCadenceForUser($user), 'subscription_cancelled' => $subscription?->canceled() ?? false,
'subscribed_at' => $subscription?->created_at?->toIso8601String(), 'subscription_cadence' => Plan::resolveCadenceForUser($user),
'subscription_expires_at' => $expiresAt?->toIso8601String(), 'subscribed_at' => $subscription?->created_at?->toIso8601String(),
], 'subscription_expires_at' => $expiresAt?->toIso8601String(),
)); ]);
} }
} }

View File

@@ -3,8 +3,9 @@
namespace App\Services\BrentPriceSources; namespace App\Services\BrentPriceSources;
use App\Services\ApiLogger; use App\Services\ApiLogger;
use Illuminate\Http\Client\ConnectionException;
use Illuminate\Http\Client\RequestException;
use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Log;
use Throwable; use Throwable;
final class EiaBrentPriceSource final class EiaBrentPriceSource
@@ -14,12 +15,16 @@ final class EiaBrentPriceSource
public function __construct(private readonly ApiLogger $apiLogger) {} public function __construct(private readonly ApiLogger $apiLogger) {}
/** /**
* @return array{date: string, price_usd: float}[]|null * @return array{date: string, price_usd: float}[]|null null only when the response carried no usable rows
*
* @throws BrentPriceFetchException on network failure or non-2xx response after retries
*/ */
public function fetch(): ?array public function fetch(): ?array
{ {
try { try {
$response = $this->apiLogger->send('eia', 'GET', self::URL, fn () => Http::timeout(10) $response = $this->apiLogger->send('eia', 'GET', self::URL, fn () => Http::timeout(30)
->retry(3, 200, fn (Throwable $e) => $this->shouldRetry($e))
->throw()
->get(self::URL, [ ->get(self::URL, [
'api_key' => config('services.eia.api_key'), 'api_key' => config('services.eia.api_key'),
'frequency' => 'daily', 'frequency' => 'daily',
@@ -29,32 +34,26 @@ final class EiaBrentPriceSource
'sort[0][direction]' => 'desc', 'sort[0][direction]' => 'desc',
'length' => 30, 'length' => 30,
])); ]));
} catch (ConnectionException $e) {
if (! $response->successful()) { throw new BrentPriceFetchException("EIA connection failed: {$e->getMessage()}", previous: $e);
Log::error('EiaBrentPriceSource: request failed', ['status' => $response->status()]); } catch (RequestException $e) {
throw new BrentPriceFetchException("EIA returned HTTP {$e->response->status()}", previous: $e);
return null;
}
$rows = collect($response->json('response.data') ?? [])
->filter(fn (array $row) => ($row['value'] ?? '.') !== '.')
->map(fn (array $row) => [
'date' => $row['period'],
'price_usd' => (float) $row['value'],
])
->all();
if ($rows === []) {
Log::warning('EiaBrentPriceSource: no valid observations returned');
return null;
}
return $rows;
} catch (Throwable $e) {
Log::error('EiaBrentPriceSource: fetch failed', ['error' => $e->getMessage()]);
return null;
} }
$rows = collect($response->json('response.data') ?? [])
->filter(fn (array $row) => ($row['value'] ?? '.') !== '.')
->map(fn (array $row) => [
'date' => $row['period'],
'price_usd' => (float) $row['value'],
])
->all();
return $rows === [] ? null : $rows;
}
private function shouldRetry(Throwable $e): bool
{
return $e instanceof ConnectionException
|| ($e instanceof RequestException && $e->response->serverError());
} }
} }

View File

@@ -3,8 +3,9 @@
namespace App\Services\BrentPriceSources; namespace App\Services\BrentPriceSources;
use App\Services\ApiLogger; use App\Services\ApiLogger;
use Illuminate\Http\Client\ConnectionException;
use Illuminate\Http\Client\RequestException;
use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Log;
use Throwable; use Throwable;
final class FredBrentPriceSource final class FredBrentPriceSource
@@ -14,12 +15,16 @@ final class FredBrentPriceSource
public function __construct(private readonly ApiLogger $apiLogger) {} public function __construct(private readonly ApiLogger $apiLogger) {}
/** /**
* @return array{date: string, price_usd: float}[]|null * @return array{date: string, price_usd: float}[]|null null only when the response carried no usable rows
*
* @throws BrentPriceFetchException on network failure or non-2xx response after retries
*/ */
public function fetch(): ?array public function fetch(): ?array
{ {
try { try {
$response = $this->apiLogger->send('fred', 'GET', self::URL, fn () => Http::timeout(10) $response = $this->apiLogger->send('fred', 'GET', self::URL, fn () => Http::timeout(30)
->retry(3, 200, fn (Throwable $e) => $this->shouldRetry($e))
->throw()
->get(self::URL, [ ->get(self::URL, [
'series_id' => 'DCOILBRENTEU', 'series_id' => 'DCOILBRENTEU',
'api_key' => config('services.fred.api_key'), 'api_key' => config('services.fred.api_key'),
@@ -27,32 +32,26 @@ final class FredBrentPriceSource
'limit' => 30, 'limit' => 30,
'file_type' => 'json', 'file_type' => 'json',
])); ]));
} catch (ConnectionException $e) {
if (! $response->successful()) { throw new BrentPriceFetchException("FRED connection failed: {$e->getMessage()}", previous: $e);
Log::error('FredBrentPriceSource: request failed', ['status' => $response->status()]); } catch (RequestException $e) {
throw new BrentPriceFetchException("FRED returned HTTP {$e->response->status()}", previous: $e);
return null;
}
$rows = collect($response->json('observations') ?? [])
->filter(fn (array $obs) => $obs['value'] !== '.')
->map(fn (array $obs) => [
'date' => $obs['date'],
'price_usd' => (float) $obs['value'],
])
->all();
if ($rows === []) {
Log::warning('FredBrentPriceSource: no valid observations returned');
return null;
}
return $rows;
} catch (Throwable $e) {
Log::error('FredBrentPriceSource: fetch failed', ['error' => $e->getMessage()]);
return null;
} }
$rows = collect($response->json('observations') ?? [])
->filter(fn (array $obs) => $obs['value'] !== '.')
->map(fn (array $obs) => [
'date' => $obs['date'],
'price_usd' => (float) $obs['value'],
])
->all();
return $rows === [] ? null : $rows;
}
private function shouldRetry(Throwable $e): bool
{
return $e instanceof ConnectionException
|| ($e instanceof RequestException && $e->response->serverError());
} }
} }

View File

@@ -12,6 +12,7 @@ use Illuminate\Support\Facades\Route;
// Public endpoints (no API key required) // Public endpoints (no API key required)
Route::post('/auth/register', [AuthController::class, 'register']); Route::post('/auth/register', [AuthController::class, 'register']);
Route::post('/auth/login', [AuthController::class, 'login']); Route::post('/auth/login', [AuthController::class, 'login']);
Route::get('/auth/me', [AuthController::class, 'me']);
Route::get('/fuel-types', function () { Route::get('/fuel-types', function () {
return Cache::remember('api:fuel-types', now()->addDay(), fn () => collect(FuelType::cases()) return Cache::remember('api:fuel-types', now()->addDay(), fn () => collect(FuelType::cases())
@@ -29,7 +30,6 @@ Route::middleware(['throttle:60,1', VerifyApiKey::class])->group(function (): vo
// Sanctum-authenticated endpoints // Sanctum-authenticated endpoints
Route::middleware('auth:sanctum')->group(function (): void { Route::middleware('auth:sanctum')->group(function (): void {
Route::get('/auth/me', [AuthController::class, 'me']);
Route::post('/auth/logout', [AuthController::class, 'logout']); Route::post('/auth/logout', [AuthController::class, 'logout']);
// User dashboard endpoints // User dashboard endpoints

View File

@@ -69,6 +69,41 @@ it('returns the authenticated user on /me', function () {
->assertJsonPath('email', $user->email); ->assertJsonPath('email', $user->email);
}); });
it('does not leak sensitive or internal user fields on /me', function () {
$user = User::factory()->create([
'is_admin' => true,
'stripe_id' => 'cus_secret',
'pm_type' => 'visa',
'pm_last_four' => '4242',
'postcode' => 'SW1A 1AA',
]);
$user->subscriptions()->create([
'type' => 'default',
'stripe_id' => 'sub_secret',
'stripe_status' => 'active',
'stripe_price' => 'price_plus_monthly',
'quantity' => 1,
]);
$response = $this->actingAs($user, 'sanctum')
->getJson('/api/auth/me')
->assertOk();
$payload = $response->json();
expect(array_keys($payload))->toEqualCanonicalizing([
'name',
'email',
'two_factor_confirmed_at',
'tier',
'subscription_cancelled',
'subscription_cadence',
'subscribed_at',
'subscription_expires_at',
]);
});
it('reports subscription_cancelled=false for a user with no subscription', function () { it('reports subscription_cancelled=false for a user with no subscription', function () {
$user = User::factory()->create(); $user = User::factory()->create();
@@ -215,6 +250,12 @@ it('logs out and revokes the token', function () {
expect($user->tokens()->count())->toBe(0); expect($user->tokens()->count())->toBe(0);
}); });
it('returns 401 on protected routes without a token', function () { it('returns null on /me when unauthenticated', function () {
$this->getJson('/api/auth/me')->assertUnauthorized(); $response = $this->getJson('/api/auth/me')->assertOk();
expect($response->getContent())->toBe('null');
});
it('returns 401 on protected routes without a token', function () {
$this->postJson('/api/auth/logout')->assertUnauthorized();
}); });

View File

@@ -38,11 +38,24 @@ it('fetches and stores brent prices from EIA', function (): void {
->and(BrentPrice::find('2026-04-02')->price_usd)->toBe('73.80'); ->and(BrentPrice::find('2026-04-02')->price_usd)->toBe('73.80');
}); });
it('throws when EIA returns a 500', function (): void { it('throws with HTTP status when EIA returns a 500', function (): void {
Http::fake(['*eia.gov/*' => Http::response([], 500)]); Http::fake(['*eia.gov/*' => Http::response([], 500)]);
expect(fn () => $this->fetcher->fetchFromEia())
->toThrow(BrentPriceFetchException::class, 'EIA returned HTTP 500');
});
it('retries EIA on transient 500 and succeeds', function (): void {
Http::fake([
'*eia.gov/*' => Http::sequence()
->push([], 500)
->push(['response' => ['data' => [['period' => '2026-04-01', 'value' => '75.10']]]]),
]);
$this->fetcher->fetchFromEia(); $this->fetcher->fetchFromEia();
})->throws(BrentPriceFetchException::class);
expect(BrentPrice::count())->toBe(1);
});
it('throws when EIA returns empty data', function (): void { it('throws when EIA returns empty data', function (): void {
Http::fake(['*eia.gov/*' => Http::response(['response' => ['data' => []]])]); Http::fake(['*eia.gov/*' => Http::response(['response' => ['data' => []]])]);
@@ -84,11 +97,24 @@ it('fetches and stores brent prices from FRED', function (): void {
expect(BrentPrice::count())->toBe(2); expect(BrentPrice::count())->toBe(2);
}); });
it('throws when FRED fails', function (): void { it('throws with HTTP status when FRED returns a 500', function (): void {
Http::fake(['*/fred/series/observations*' => Http::response([], 500)]); Http::fake(['*/fred/series/observations*' => Http::response([], 500)]);
expect(fn () => $this->fetcher->fetchFromFred())
->toThrow(BrentPriceFetchException::class, 'FRED returned HTTP 500');
});
it('retries FRED on transient 500 and succeeds', function (): void {
Http::fake([
'*/fred/series/observations*' => Http::sequence()
->push([], 500)
->push(['observations' => [['date' => '2026-04-01', 'value' => '75.10']]]),
]);
$this->fetcher->fetchFromFred(); $this->fetcher->fetchFromFred();
})->throws(BrentPriceFetchException::class);
expect(BrentPrice::count())->toBe(1);
});
it('filters out FRED missing value markers', function (): void { it('filters out FRED missing value markers', function (): void {
Http::fake([ Http::fake([