newsletter: Bestätigung/Ownership beim Re-Abo korrigieren (M7)
updateOrCreate ersetzt durch eine Ownership-Prüfung: ein fremder, gesetzter Eigentümer wird abgelehnt (409), bestätigte Abos werden nicht still ent-bestätigt. Verwaiste Datensätze (user_id NULL nach Konto- Löschung/Legacy-Import) dürfen neu gebunden werden, statt dauerhaft gesperrt zu bleiben. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
fe1525b9ff
commit
5a492926dd
2 changed files with 127 additions and 13 deletions
|
|
@ -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.',
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue