diff --git a/app/Http/Controllers/Api/V1/NewsletterSubscriptionController.php b/app/Http/Controllers/Api/V1/NewsletterSubscriptionController.php index c077744d..92e58ed0 100644 --- a/app/Http/Controllers/Api/V1/NewsletterSubscriptionController.php +++ b/app/Http/Controllers/Api/V1/NewsletterSubscriptionController.php @@ -6,6 +6,7 @@ use App\Http\Controllers\Controller; use App\Http\Requests\Api\V1\SubscribeNewsletterRequest; use App\Models\NewsletterSubscription; use Illuminate\Http\JsonResponse; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; class NewsletterSubscriptionController extends Controller @@ -14,23 +15,57 @@ class NewsletterSubscriptionController extends Controller { $validated = $request->validated(); - $subscription = NewsletterSubscription::withoutGlobalScopes()->updateOrCreate( - [ - 'portal' => $validated['portal'], - 'email' => mb_strtolower($validated['email']), - ], - [ + $email = mb_strtolower($validated['email']); + + $subscription = DB::transaction(function () use ($request, $validated, $email): NewsletterSubscription { + $subscription = NewsletterSubscription::withoutGlobalScopes() + ->where('portal', $validated['portal']) + ->where('email', $email) + ->lockForUpdate() + ->first(); + + // Nur ein fremder, tatsächlich gesetzter Eigentümer blockiert (409). + // Verwaiste Datensätze (user_id NULL nach Konto-Löschung/Legacy-Import) + // dürfen neu gebunden werden — sonst wäre die portal+email-Kombination + // dauerhaft nicht mehr abonnierbar. + if ($subscription !== null && $subscription->user_id !== null && $subscription->user_id !== $request->user()->id) { + abort(409, 'This email address is already subscribed.'); + } + + if ($subscription === null) { + return NewsletterSubscription::withoutGlobalScopes()->create([ + 'portal' => $validated['portal'], + 'email' => $email, + 'user_id' => $request->user()->id, + 'salutation_key' => $validated['salutation_key'] ?? null, + 'first_name' => $validated['first_name'] ?? null, + 'last_name' => $validated['last_name'] ?? null, + 'ip_address' => $request->ip(), + 'is_confirmed' => false, + 'confirmation_token' => Str::random(32), + 'subscribed_at' => now(), + 'unsubscribed_at' => null, + ]); + } + + $attributes = [ 'user_id' => $request->user()->id, 'salutation_key' => $validated['salutation_key'] ?? null, 'first_name' => $validated['first_name'] ?? null, 'last_name' => $validated['last_name'] ?? null, 'ip_address' => $request->ip(), - 'is_confirmed' => false, - 'confirmation_token' => Str::random(32), 'subscribed_at' => now(), 'unsubscribed_at' => null, - ], - ); + ]; + + if (! $subscription->is_confirmed) { + $attributes['confirmation_token'] = Str::random(32); + } + + $subscription->update($attributes); + + return $subscription; + }); return response()->json([ 'message' => 'Newsletter subscription created.', diff --git a/tests/Feature/Api/V1/CustomerDataApiTest.php b/tests/Feature/Api/V1/CustomerDataApiTest.php index b5817d2f..eb62d9ce 100644 --- a/tests/Feature/Api/V1/CustomerDataApiTest.php +++ b/tests/Feature/Api/V1/CustomerDataApiTest.php @@ -7,10 +7,10 @@ use App\Models\User; use Laravel\Sanctum\Sanctum; use Tests\TestCase; -test('unknown legacy api key requests receive gone response', function () { +test('legacy api keys in query parameters are not accepted but get the migration hint', function () { /** @var TestCase $this */ - // Übernahme 06.07.: importierte Legacy-Keys sind gültige Tokens - // (LegacyApiCompatibilityTest); nur unbekannte Keys bekommen 410. + // Legacy-Keys sind nur noch in Headern zulässig; der Query-Parameter wird + // nicht für die Auth genutzt, liefert aber den 410-Migrationshinweis. $this->getJson('/api/v1/categories?api_key=legacy-key') ->assertStatus(410) ->assertJsonStructure(['message', 'migration_url', 'docs_url']); @@ -72,3 +72,82 @@ test('api user can subscribe an email to newsletter', function () { ->where('portal', 'presseecho') ->exists())->toBeTrue(); }); + +test('resubscribing keeps an existing confirmation intact', function () { + /** @var TestCase $this */ + $user = User::factory()->create(); + $subscription = NewsletterSubscription::withoutGlobalScopes()->create([ + 'portal' => 'presseecho', + 'user_id' => $user->id, + 'email' => 'confirmed@example.com', + 'is_confirmed' => true, + 'confirmation_token' => null, + 'subscribed_at' => now()->subMonth(), + 'unsubscribed_at' => now()->subDay(), + ]); + Sanctum::actingAs($user, ['newsletter:subscribe']); + + $this->postJson('/api/v1/newsletter/subscribe', [ + 'portal' => 'presseecho', + 'email' => 'confirmed@example.com', + 'first_name' => 'Neu', + ])->assertCreated() + ->assertJsonPath('data.is_confirmed', true); + + $subscription->refresh(); + + expect($subscription->is_confirmed)->toBeTrue() + ->and($subscription->confirmation_token)->toBeNull() + ->and($subscription->unsubscribed_at)->toBeNull() + ->and($subscription->first_name)->toBe('Neu'); +}); + +test('an orphaned subscription without owner can be rebound instead of being permanently blocked', function () { + /** @var TestCase $this */ + // user_id NULL entsteht z. B. nach Konto-Löschung (FK nullOnDelete) oder + // durch Legacy-Import. Ohne Rebind wäre portal+email dauerhaft gesperrt. + $subscription = NewsletterSubscription::withoutGlobalScopes()->create([ + 'portal' => 'presseecho', + 'user_id' => null, + 'email' => 'orphan@example.com', + 'is_confirmed' => false, + 'subscribed_at' => now()->subMonth(), + ]); + + $user = User::factory()->create(); + Sanctum::actingAs($user, ['newsletter:subscribe']); + + $this->postJson('/api/v1/newsletter/subscribe', [ + 'portal' => 'presseecho', + 'email' => 'orphan@example.com', + 'first_name' => 'Wieder da', + ])->assertCreated(); + + expect($subscription->fresh()->user_id)->toBe($user->id) + ->and($subscription->fresh()->first_name)->toBe('Wieder da'); +}); + +test('an api user cannot take over another newsletter subscription', function () { + /** @var TestCase $this */ + $owner = User::factory()->create(); + $attacker = User::factory()->create(); + $subscription = NewsletterSubscription::withoutGlobalScopes()->create([ + 'portal' => 'presseecho', + 'user_id' => $owner->id, + 'email' => 'owned@example.com', + 'first_name' => 'Original', + 'is_confirmed' => true, + 'subscribed_at' => now(), + ]); + Sanctum::actingAs($attacker, ['newsletter:subscribe']); + + $this->postJson('/api/v1/newsletter/subscribe', [ + 'portal' => 'presseecho', + 'email' => 'owned@example.com', + 'first_name' => 'Übernommen', + ])->assertConflict(); + + expect($subscription->fresh()->user_id)->toBe($owner->id) + ->and($subscription->fresh()->first_name)->toBe('Original') + ->and($subscription->fresh()->is_confirmed)->toBeTrue(); +});