matt/opnform-host-nginx
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Email spam security (#641)

Browse Source 
* Add hCaptcha on register page

* register page captcha test cases

* Refactor integration validation rules to include form context

- Updated the `getValidationRules` method in various integration handlers (Discord, Email, Google Sheets, Slack, Webhook, Zapier) to accept an optional `Form` parameter, allowing for context-aware validation.
- Enhanced the `EmailIntegration` handler to enforce restrictions based on user plans, ensuring free users can only create one email integration per form and can only send to a single email address.
- Added a new test suite for `EmailIntegration` to validate the new restrictions and ensure proper functionality for both free and pro users.
- Introduced loading state management in the `IntegrationModal` component to improve user experience during save operations.

These changes improve the flexibility and user experience of form integrations, particularly for email handling.

* for self-hosted ignore emil validation for spam

* fix pint

* ignore register throttle for testing env

* support new migration for mysql also

* Register page captcha enable if captcha key set

* fix test case

* fix test case

* fix test case

* fix pint

* Refactor RegisterController middleware and update TestCase setup

- Removed environment check for throttling middleware in RegisterController, ensuring consistent rate limiting for the registration endpoint.
- Updated TestCase to disable throttle middleware during tests, allowing for more flexible testing scenarios without rate limiting interference.

* Enhance hCaptcha integration in tests and configuration

- Added hCaptcha site and secret keys to phpunit.xml for testing purposes.
- Updated RegisterTest to configure hCaptcha secret key dynamically, ensuring proper token validation in production environment.

These changes improve the testing setup for hCaptcha, facilitating more accurate simulation of production conditions.

---------

Co-authored-by: Julien Nahum <julien@nahum.net>
This commit is contained in:
Chirag Chhatrala
2024-12-18 17:46:27 +05:30
committed by GitHub
parent c1ee072b71
commit 7365479c83
18 changed files with 375 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
api/app/Http/Controllers/Auth/RegisterController.php
View File

@@ -12,6 +12,7 @@ use Illuminate\Foundation\Auth\RegistersUsers;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Validator;
use Illuminate\Validation\Rule;
use App\Rules\ValidHCaptcha;
class RegisterController extends Controller
{
@@ -27,6 +28,9 @@ class RegisterController extends Controller
public function __construct()
{
$this->middleware('guest');
$this->middleware('throttle:5,1')->only('register'); // 5 attempts per minute
$this->middleware('throttle:30,60')->only('register'); // 30 attempts per hour
}
/**
@@ -56,7 +60,7 @@ class RegisterController extends Controller
*/
protected function validator(array $data)
{
return Validator::make($data, [
$rules = [
'name' => 'required|max:255',
'email' => 'required|email:filter|max:255|unique:users|indisposable',
'password' => 'required|min:6|confirmed',
@@ -64,8 +68,14 @@ class RegisterController extends Controller
'agree_terms' => ['required', Rule::in([true])],
'appsumo_license' => ['nullable'],
'invite_token' => ['nullable', 'string'],
'utm_data' => ['nullable', 'array']
], [
'utm_data' => ['nullable', 'array'],
];
if (config('services.h_captcha.secret_key')) {
$rules['h-captcha-response'] = [new ValidHCaptcha()];
}
return Validator::make($data, $rules, [
'agree_terms' => 'Please agree with the terms and conditions.',
]);
}
@@ -84,6 +94,7 @@ class RegisterController extends Controller
'password' => bcrypt($data['password']),
'hear_about_us' => $data['hear_about_us'],
'utm_data' => array_key_exists('utm_data', $data) ? $data['utm_data'] : null,
'meta' => ['registration_ip' => request()->ip()],
]);
// Add relation with user

5
api/app/Http/Requests/Integration/FormIntegrationsRequest.php
View File

@@ -2,6 +2,7 @@
namespace App\Http\Requests\Integration;
use App\Models\Forms\Form;
use App\Models\Integration\FormIntegration;
use App\Rules\IntegrationLogicRule;
use Illuminate\Foundation\Http\FormRequest;
@@ -14,9 +15,11 @@ class FormIntegrationsRequest extends FormRequest
public array $integrationRules = [];
private ?string $integrationClassName = null;
private ?Form $form = null;
public function __construct(Request $request)
{
$this->form = Form::findOrFail(request()->route('id'));
if ($request->integration_id) {
// Load integration class, and get rules
$integration = FormIntegration::getIntegration($request->integration_id);
@@ -77,7 +80,7 @@ class FormIntegrationsRequest extends FormRequest
private function loadIntegrationRules()
{
foreach ($this->integrationClassName::getValidationRules() as $key => $value) {
foreach ($this->integrationClassName::getValidationRules($this->form) as $key => $value) {
$this->integrationRules['settings.' . $key] = $value;
}
}

2
api/app/Integrations/Handlers/AbstractIntegrationHandler.php
View File

@@ -94,7 +94,7 @@ abstract class AbstractIntegrationHandler
Http::throw()->post($this->getWebhookUrl(), $this->getWebhookData());
}
abstract public static function getValidationRules(): array;
abstract public static function getValidationRules(?Form $form): array;
public static function isOAuthRequired(): bool
{

3
api/app/Integrations/Handlers/DiscordIntegration.php
View File

@@ -2,6 +2,7 @@
namespace App\Integrations\Handlers;
use App\Models\Forms\Form;
use App\Open\MentionParser;
use App\Service\Forms\FormSubmissionFormatter;
use Illuminate\Support\Arr;
@@ -9,7 +10,7 @@ use Vinkla\Hashids\Facades\Hashids;
class DiscordIntegration extends AbstractIntegrationHandler
{
public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [
'discord_webhook_url' => 'required|url|starts_with:https://discord.com/api/webhooks',

34
api/app/Integrations/Handlers/EmailIntegration.php
View File

@@ -2,20 +2,23 @@
namespace App\Integrations\Handlers;
use App\Models\Forms\Form;
use App\Models\Integration\FormIntegration;
use App\Notifications\Forms\FormEmailNotification;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Notification;
use App\Open\MentionParser;
use App\Service\Forms\FormSubmissionFormatter;
use Illuminate\Validation\ValidationException;
class EmailIntegration extends AbstractEmailIntegrationHandler
{
public const RISKY_USERS_LIMIT = 120;
public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [
'send_to' => 'required',
$rules = [
'send_to' => ['required'],
'sender_name' => 'required',
'sender_email' => 'email|nullable',
'subject' => 'required',
@@ -24,6 +27,31 @@ class EmailIntegration extends AbstractEmailIntegrationHandler
'include_hidden_fields_submission_data' => ['nullable', 'boolean'],
'reply_to' => 'nullable',
];
if ($form->is_pro || config('app.self_hosted')) {
return $rules;
}
// Free plan users can only send to a single email address (avoid spam)
$rules['send_to'][] = function ($attribute, $value, $fail) use ($form) {
if (count(explode("\n", trim($value))) > 1 || count(explode(',', $value)) > 1) {
$fail('You can only send to a single email address on the free plan. Please upgrade to the Pro plan to create a new integration.');
}
};
// Free plan users can only have a single email integration per form (avoid spam)
if (!request()->route('integrationid')) {
$existingEmailIntegrations = FormIntegration::where('form_id', $form->id)
->where('integration_id', 'email')
->count();
if ($existingEmailIntegrations > 0) {
throw ValidationException::withMessages([
'settings.send_to' => ['Free users are limited to 1 email integration per form.']
]);
}
}
return $rules;
}
protected function shouldRun(): bool

7
api/app/Integrations/Handlers/GoogleSheetsIntegration.php
View File

@@ -4,6 +4,7 @@ namespace App\Integrations\Handlers;
use App\Events\Forms\FormSubmitted;
use App\Integrations\Google\Google;
use App\Models\Forms\Form;
use App\Models\Integration\FormIntegration;
use Exception;
use Illuminate\Support\Facades\Log;
@@ -22,11 +23,9 @@ class GoogleSheetsIntegration extends AbstractIntegrationHandler
$this->client = new Google($formIntegration);
}
public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [
];
return [];
}
public static function isOAuthRequired(): bool

3
api/app/Integrations/Handlers/SlackIntegration.php
View File

@@ -2,6 +2,7 @@
namespace App\Integrations\Handlers;
use App\Models\Forms\Form;
use App\Open\MentionParser;
use App\Service\Forms\FormSubmissionFormatter;
use Illuminate\Support\Arr;
@@ -9,7 +10,7 @@ use Vinkla\Hashids\Facades\Hashids;
class SlackIntegration extends AbstractIntegrationHandler
{
public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [
'slack_webhook_url' => 'required|url|starts_with:https://hooks.slack.com/',

4
api/app/Integrations/Handlers/WebhookIntegration.php
View File

@@ -2,9 +2,11 @@
namespace App\Integrations\Handlers;
use App\Models\Forms\Form;
class WebhookIntegration extends AbstractIntegrationHandler
{
public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [
'webhook_url' => 'required|url'

3
api/app/Integrations/Handlers/ZapierIntegration.php
View File

@@ -3,6 +3,7 @@
namespace App\Integrations\Handlers;
use App\Events\Forms\FormSubmitted;
use App\Models\Forms\Form;
use App\Models\Integration\FormIntegration;
use Exception;
@@ -16,7 +17,7 @@ class ZapierIntegration extends AbstractIntegrationHandler
parent::__construct($event, $formIntegration, $integration);
}
public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [];
}

3
api/app/Models/User.php
View File

@@ -33,6 +33,7 @@ class User extends Authenticatable implements JWTSubject
'password',
'hear_about_us',
'utm_data',
'meta'
];
/**
@@ -44,6 +45,7 @@ class User extends Authenticatable implements JWTSubject
'password',
'remember_token',
'hear_about_us',
'meta'
];
/**
@@ -56,6 +58,7 @@ class User extends Authenticatable implements JWTSubject
return [
'email_verified_at' => 'datetime',
'utm_data' => 'array',
'meta' => 'array',
];
}

35
api/database/migrations/2024_12_10_094605_add_meta_to_users_table.php Normal file
View File

@@ -0,0 +1,35 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Query\Expression;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;
return new class () extends Migration {
/**
* Run the migrations.
*/
public function up(): void
{
$driver = DB::getDriverName();
Schema::table('users', function (Blueprint $table) use ($driver) {
if ($driver === 'mysql') {
$table->json('meta')->default(new Expression('(JSON_OBJECT())'));
} else {
$table->json('meta')->default('{}');
}
});
}
/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::table('users', function (Blueprint $table) {
$table->dropColumn('meta');
});
}
};

2
api/phpunit.xml
View File

@@ -27,6 +27,8 @@
<env name="JWT_SECRET" value="9K6whOetAFaokQgSIdbMQZuJuDV5uS2Y"/>
<env name="STRIPE_KEY" value="TEST_KEY"/>
<env name="STRIPE_SECRET" value="TEST_SECRET"/>
<env name="H_CAPTCHA_SITE_KEY" value="TEST_SITE_KEY"/>
<env name="H_CAPTCHA_SECRET_KEY" value="TEST_SECRET"/>
</php>
<source>
<include>

175
api/tests/Feature/Integrations/Email/EmailIntegrationTest.php Normal file
View File

@@ -0,0 +1,175 @@
<?php
use App\Models\Integration\FormIntegration;
test('free user can create one email integration', function () {
$user = $this->actingAsUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);
// First email integration should succeed
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => 'test@example.com',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);
$response->assertSuccessful();
expect(FormIntegration::where('form_id', $form->id)->count())->toBe(1);
// Second email integration should fail
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => 'another@example.com',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);
$response->assertStatus(422)
->assertJson([
'errors' => [
'settings.send_to' => ['Free users are limited to 1 email integration per form.']
]
]);
});
test('pro user can create multiple email integrations', function () {
$user = $this->actingAsProUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);
// First email integration
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => 'test@example.com',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);
$response->assertSuccessful();
// Second email integration should also succeed for pro users
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => 'another@example.com',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);
$response->assertSuccessful();
expect(FormIntegration::where('form_id', $form->id)->count())->toBe(2);
});
test('free user cannot add multiple emails', function () {
$user = $this->actingAsUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => "test@example.com\nanother@example.com",
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);
$response->assertStatus(422)
->assertJsonValidationErrors(['settings.send_to'])
->assertJson([
'errors' => [
'settings.send_to' => ['You can only send to a single email address on the free plan. Please upgrade to the Pro plan to create a new integration.']
]
]);
});
test('pro user can add multiple emails', function () {
$user = $this->actingAsProUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => "test@example.com\nanother@example.com\nthird@example.com",
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);
$response->assertSuccessful();
$integration = FormIntegration::where('form_id', $form->id)->first();
expect($integration)->not->toBeNull();
expect($integration->data->send_to)->toContain('test@example.com');
expect($integration->data->send_to)->toContain('another@example.com');
expect($integration->data->send_to)->toContain('third@example.com');
});
test('free user can update their single email integration', function () {
$user = $this->actingAsUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);
// Create initial integration
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => 'test@example.com',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);
$response->assertSuccessful();
$integrationId = $response->json('form_integration.id');
// Update the integration
$response = $this->putJson(route('open.forms.integration.update', [$form, $integrationId]), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => 'updated@example.com',
'sender_name' => 'Updated Sender',
'subject' => 'Updated Subject',
'email_content' => 'Updated Content',
'include_submission_data' => true
]
]);
$response->assertSuccessful();
$integration = FormIntegration::find($integrationId);
expect($integration->data->send_to)->toBe('updated@example.com');
expect($integration->data->sender_name)->toBe('Updated Sender');
});

42
api/tests/Feature/RegisterTest.php
View File

@@ -1,8 +1,15 @@
<?php
use App\Models\User;
use App\Rules\ValidHCaptcha;
use Illuminate\Support\Facades\Http;
it('can register', function () {
Http::fake([
ValidHCaptcha::H_CAPTCHA_VERIFY_URL => Http::response(['success' => true])
]);
$this->postJson('/register', [
'name' => 'Test User',
'email' => 'test@test.app',
@@ -10,13 +17,15 @@ it('can register', function () {
'password' => 'secret',
'password_confirmation' => 'secret',
'agree_terms' => true,
'h-captcha-response' => 'test-token', // Mock token for testing
])
->assertSuccessful()
->assertJsonStructure(['id', 'name', 'email']);
$this->assertDatabaseHas('users', [
'name' => 'Test User',
'email' => 'test@test.app',
]);
$user = User::where('email', 'test@test.app')->first();
expect($user)->not->toBeNull();
expect($user->meta)->toHaveKey('registration_ip');
expect($user->meta['registration_ip'])->toBe(request()->ip());
});
it('cannot register with existing email', function () {
@@ -27,12 +36,17 @@ it('cannot register with existing email', function () {
'email' => 'test@test.app',
'password' => 'secret',
'password_confirmation' => 'secret',
'h-captcha-response' => 'test-token',
])
->assertStatus(422)
->assertJsonValidationErrors(['email']);
});
it('cannot register with disposable email', function () {
Http::fake([
ValidHCaptcha::H_CAPTCHA_VERIFY_URL => Http::response(['success' => true])
]);
// Select random email
$email = [
'dumliyupse@gufum.com',
@@ -48,6 +62,7 @@ it('cannot register with disposable email', function () {
'password' => 'secret',
'password_confirmation' => 'secret',
'agree_terms' => true,
'h-captcha-response' => 'test-token',
])
->assertStatus(422)
->assertJsonValidationErrors(['email'])
@@ -60,3 +75,22 @@ it('cannot register with disposable email', function () {
],
]);
});
it('requires hcaptcha token in production', function () {
config(['services.h_captcha.secret_key' => 'test-key']);
Http::fake([
ValidHCaptcha::H_CAPTCHA_VERIFY_URL => Http::response(['success' => true])
]);
$this->postJson('/register', [
'name' => 'Test User',
'email' => 'test@test.app',
'hear_about_us' => 'google',
'password' => 'secret',
'password_confirmation' => 'secret',
'agree_terms' => true,
])
->assertStatus(422)
->assertJsonValidationErrors(['h-captcha-response']);
});

10
api/tests/Feature/UserManagementTest.php
View File

@@ -2,10 +2,15 @@
use App\Models\UserInvite;
use Carbon\Carbon;
use App\Rules\ValidHCaptcha;
use Illuminate\Support\Facades\Http;
beforeEach(function () {
$this->user = $this->actingAsProUser();
$this->workspace = $this->createUserWorkspace($this->user);
Http::fake([
ValidHCaptcha::H_CAPTCHA_VERIFY_URL => Http::response(['success' => true])
]);
});
@@ -31,6 +36,7 @@ it('can register with invite token', function () {
'password_confirmation' => 'secret',
'agree_terms' => true,
'invite_token' => $token,
'h-captcha-response' => 'test-token',
]);
$response->assertSuccessful();
expect($this->workspace->users()->count())->toBe(2);
@@ -59,6 +65,7 @@ it('cannot register with expired invite token', function () {
'password_confirmation' => 'secret',
'agree_terms' => true,
'invite_token' => $token,
'h-captcha-response' => 'test-token',
]);
$response->assertStatus(400)->assertJson([
'message' => 'Invite token has expired.',
@@ -88,6 +95,7 @@ it('cannot re-register with accepted invite token', function () {
'password_confirmation' => 'secret',
'agree_terms' => true,
'invite_token' => $token,
'h-captcha-response' => 'test-token',
]);
$response->assertSuccessful();
expect($this->workspace->users()->count())->toBe(2);
@@ -104,6 +112,7 @@ it('cannot re-register with accepted invite token', function () {
'password_confirmation' => 'secret',
'agree_terms' => true,
'invite_token' => $token,
'h-captcha-response' => 'test-token',
]);
$response->assertStatus(422)->assertJson([
@@ -138,6 +147,7 @@ it('can cancel user invite', function () {
'password_confirmation' => 'secret',
'agree_terms' => true,
'invite_token' => $token,
'h-captcha-response' => 'test-token',
]);
$response->assertStatus(400)->assertJson([
'message' => 'Invite token is invalid.',

9
api/tests/TestCase.php
View File

@@ -4,10 +4,19 @@ namespace Tests;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
use Illuminate\Routing\Middleware\ThrottleRequests;
abstract class TestCase extends BaseTestCase
{
use CreatesApplication;
use RefreshDatabase;
use TestHelpers;
protected function setUp(): void
{
parent::setUp();
$this->withoutMiddleware(
ThrottleRequests::class
);
}
}