Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add reCAPTCHA support and update captcha provider handling #647

Merged
merged 10 commits into from
Dec 18, 2024
Merged
3 changes: 3 additions & 0 deletions api/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ STRIPE_TEST_DEFAULT_PRICING_YEARLY=
H_CAPTCHA_SITE_KEY=
H_CAPTCHA_SECRET_KEY=

RE_CAPTCHA_SITE_KEY=
RE_CAPTCHA_SECRET_KEY=
Comment on lines +75 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Documentation needed for reCAPTCHA environment variables

The new reCAPTCHA environment variables are not documented in the environment variables configuration file (docs/configuration/environment-variables.mdx). Please add the following to the Configuration Environment Variables table:

  • RE_CAPTCHA_SITE_KEY: Site key for reCAPTCHA integration
  • RE_CAPTCHA_SECRET_KEY: Secret key for reCAPTCHA integration
🔗 Analysis chain

Verify documentation for new reCAPTCHA configuration

The environment variables are correctly added. Please ensure these new variables are documented in:

  1. Installation guide
  2. Configuration documentation
  3. Deployment checklist
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if documentation exists for new variables
fd -e md -e mdx | xargs rg -l "RE_CAPTCHA_(SITE|SECRET)_KEY"

Length of output: 62


Script:

#!/bin/bash
# Let's first check if there are any markdown files in docs or similar directories
fd -e md -e mdx

# Also check for README files that might contain documentation
fd README

Length of output: 907


Script:

#!/bin/bash
# Let's check the environment variables documentation and installation guides
cat docs/configuration/environment-variables.mdx
cat docs/deployment/local-deployment.mdx

Length of output: 5373


MUX_WORKSPACE_ID=
MUX_API_TOKEN=

Expand Down
9 changes: 7 additions & 2 deletions api/app/Http/Requests/AnswerFormRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use App\Rules\StorageFile;
use App\Rules\ValidHCaptcha;
use App\Rules\ValidPhoneInputRule;
use App\Rules\ValidReCaptcha;
use App\Rules\ValidUrl;
use App\Service\Forms\FormLogicPropertyResolver;
use Illuminate\Foundation\Http\FormRequest;
Expand Down Expand Up @@ -116,9 +117,13 @@ public function rules()
$this->requestRules[$propertyId] = $rules;
}

// Validate hCaptcha
// Validate Captcha
if ($this->form->use_captcha) {
$this->requestRules['h-captcha-response'] = [new ValidHCaptcha()];
if ($this->form->captcha_provider === 'recaptcha') {
$this->requestRules['g-recaptcha-response'] = [new ValidReCaptcha()];
} elseif ($this->form->captcha_provider === 'hcaptcha') {
$this->requestRules['h-captcha-response'] = [new ValidHCaptcha()];
}
Comment on lines +122 to +126
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add default case handling for captcha validation.

While the current implementation handles both reCAPTCHA and hCaptcha, it lacks handling for unexpected captcha_provider values. This could lead to a security issue where no captcha validation is applied even when use_captcha is true.

Consider adding a default case:

 if ($this->form->use_captcha) {
     if ($this->form->captcha_provider === 'recaptcha') {
         $this->requestRules['g-recaptcha-response'] = [new ValidReCaptcha()];
     } elseif ($this->form->captcha_provider === 'hcaptcha') {
         $this->requestRules['h-captcha-response'] = [new ValidHCaptcha()];
+    } else {
+        // Default to hCaptcha if provider is invalid
+        $this->requestRules['h-captcha-response'] = [new ValidHCaptcha()];
     }
 }

Committable suggestion skipped: line range outside the PR's diff.

}

// Validate submission_id for edit mode
Expand Down
1 change: 1 addition & 0 deletions api/app/Http/Requests/UserFormRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public function rules()
'can_be_indexed' => 'boolean',
'password' => 'sometimes|nullable',
'use_captcha' => 'boolean',
'captcha_provider' => ['sometimes', Rule::in(['recaptcha', 'hcaptcha'])],

// Custom SEO
'seo_meta' => 'nullable|array',
Expand Down
1 change: 1 addition & 0 deletions api/app/Models/Forms/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class Form extends Model implements CachableAttributes
'submitted_text',
'redirect_url',
'use_captcha',
'captcha_provider',
'closes_at',
'closed_text',
'max_submissions_count',
Expand Down
51 changes: 51 additions & 0 deletions api/app/Rules/ValidReCaptcha.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

namespace App\Rules;

use Closure;
use Illuminate\Contracts\Validation\ImplicitRule;
use Illuminate\Support\Facades\Http;

class ValidReCaptcha implements ImplicitRule
{
public const RECAPTCHA_VERIFY_URL = 'https://www.google.com/recaptcha/api/siteverify';

private $error = 'Invalid CAPTCHA. Please prove you\'re not a bot.';

/**
* Determine if the validation rule passes.
*
* @param string $attribute
* @param mixed $value
* @return bool
*/
public function passes($attribute, $value)
{
if (empty($value)) {
$this->error = 'Please complete the captcha.';

return false;
}

return Http::asForm()->post(self::RECAPTCHA_VERIFY_URL, [
'secret' => config('services.re_captcha.secret_key'),
'response' => $value,
])->json('success');
}
Comment on lines +30 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance reCAPTCHA verification security and reliability

The verification request should include additional security measures and error handling:

  1. Include the user's IP address in the verification request (recommended by Google)
  2. Add timeout and error handling for the HTTP request
     return Http::asForm()->post(self::RECAPTCHA_VERIFY_URL, [
         'secret' => config('services.re_captcha.secret_key'),
         'response' => $value,
+        'remoteip' => request()->ip(),
-    ])->json('success');
+    ])
+    ->timeout(5)
+    ->throw(function($response, $e) {
+        $this->error = 'CAPTCHA verification failed. Please try again.';
+        return false;
+    })
+    ->json('success');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return Http::asForm()->post(self::RECAPTCHA_VERIFY_URL, [
'secret' => config('services.re_captcha.secret_key'),
'response' => $value,
])->json('success');
}
return Http::asForm()->post(self::RECAPTCHA_VERIFY_URL, [
'secret' => config('services.re_captcha.secret_key'),
'response' => $value,
'remoteip' => request()->ip(),
])
->timeout(5)
->throw(function($response, $e) {
$this->error = 'CAPTCHA verification failed. Please try again.';
return false;
})
->json('success');
}

public function validate(string $attribute, mixed $value, Closure $fail): void
{
if (!$this->passes($attribute, $value)) {
$fail($this->message());
}
}

/**
* Get the validation error message.
*
* @return string
*/
public function message()
{
return $this->error;
}
}
5 changes: 5 additions & 0 deletions api/config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@
'secret_key' => env('H_CAPTCHA_SECRET_KEY'),
],

're_captcha' => [
'site_key' => env('RE_CAPTCHA_SITE_KEY'),
'secret_key' => env('RE_CAPTCHA_SECRET_KEY'),
],

'canny' => [
'api_key' => env('CANNY_API_KEY'),
],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class () extends Migration {
public function up()
{
Schema::table('forms', function (Blueprint $table) {
$table->string('captcha_provider')->default('hcaptcha')->after('use_captcha');
});
}

public function down()
{
Schema::table('forms', function (Blueprint $table) {
$table->dropColumn('captcha_provider');
});
}
};
23 changes: 22 additions & 1 deletion api/tests/Feature/Forms/FormCaptchaTest.php
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
<?php

it('create form with captcha and raise validation issue', function () {
it('create form with hcaptcha and raise validation issue', function () {
$user = $this->actingAsUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace, [
'use_captcha' => true,
'captcha_provider' => 'hcaptcha',
]);

$this->postJson(route('forms.answer', $form->slug), [])
Expand All @@ -18,3 +19,23 @@
],
]);
});

it('create form with recaptcha and raise validation issue', function () {
$user = $this->actingAsUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace, [
'use_captcha' => true,
'captcha_provider' => 'recaptcha',
]);

$this->postJson(route('forms.answer', $form->slug), [])
->assertStatus(422)
->assertJson([
'message' => 'Please complete the captcha.',
'errors' => [
'g-recaptcha-response' => [
'Please complete the captcha.',
],
],
]);
});
1 change: 1 addition & 0 deletions client/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ NUXT_PUBLIC_CRISP_WEBSITE_ID=
NUXT_PUBLIC_ENV=
NUXT_PUBLIC_GOOGLE_ANALYTICS_CODE=
NUXT_PUBLIC_H_CAPTCHA_SITE_KEY=
NUXT_PUBLIC_RE_CAPTCHA_SITE_KEY=
NUXT_API_SECRET=secret
179 changes: 179 additions & 0 deletions client/components/forms/components/CaptchaInput.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
<template>
<div>
<div v-if="showCaptcha">
<RecaptchaV2
v-if="provider === 'recaptcha'"
:key="`recaptcha-${componentKey}`"
ref="captchaRef"
:sitekey="recaptchaSiteKey"
:theme="darkMode ? 'dark' : 'light'"
:language="language"
@verify="onCaptchaVerify"
@expired="onCaptchaExpired"
@opened="onCaptchaOpen"
@closed="onCaptchaClose"
/>
<HCaptchaV2
v-else
:key="`hcaptcha-${componentKey}`"
ref="captchaRef"
:sitekey="hCaptchaSiteKey"
:theme="darkMode ? 'dark' : 'light'"
:language="language"
@verify="onCaptchaVerify"
@expired="onCaptchaExpired"
@opened="onCaptchaOpen"
@closed="onCaptchaClose"
/>
</div>
<has-error
:form="form"
:field-id="formFieldName"
/>
</div>
</template>

<script setup>
import HCaptchaV2 from './HCaptchaV2.vue'
import RecaptchaV2 from './RecaptchaV2.vue'

const props = defineProps({
provider: {
type: String,
required: true,
validator: (value) => ['recaptcha', 'hcaptcha'].includes(value)
},
form: {
type: Object,
required: true
},
language: {
type: String,
required: true
},
darkMode: {
type: Boolean,
default: false
}
})

const config = useRuntimeConfig()
const recaptchaSiteKey = config.public.recaptchaSiteKey
const hCaptchaSiteKey = config.public.hCaptchaSiteKey

const captchaRef = ref(null)
const isIframe = ref(false)
const showCaptcha = ref(true)
const componentKey = ref(0)

const formFieldName = computed(() => props.provider === 'recaptcha' ? 'g-recaptcha-response' : 'h-captcha-response')

// Watch for provider changes to reset the form field
watch(() => props.provider, async (newProvider, oldProvider) => {
if (newProvider !== oldProvider) {
// Clear old provider's value
if (oldProvider === 'recaptcha') {
props.form['g-recaptcha-response'] = null
} else if (oldProvider === 'hcaptcha') {
props.form['h-captcha-response'] = null
}

// Force remount by toggling visibility and incrementing key
showCaptcha.value = false

// Wait longer to ensure complete cleanup
await new Promise(resolve => setTimeout(resolve, 1000))

componentKey.value++
await nextTick()

// Wait again before showing new captcha
await new Promise(resolve => setTimeout(resolve, 1000))

showCaptcha.value = true
}
})

onMounted(() => {
isIframe.value = window.self !== window.top
})

// Add a ref to track if captcha was completed
const wasCaptchaCompleted = ref(false)

// Handle captcha verification
const onCaptchaVerify = (token) => {
wasCaptchaCompleted.value = true
props.form[formFieldName.value] = token
// Also set the DOM element value for compatibility with existing code
if (import.meta.client) {
const element = document.getElementsByName(formFieldName.value)[0]
if (element) element.value = token
}
}

// Handle captcha expiration
const onCaptchaExpired = () => {
wasCaptchaCompleted.value = false
props.form[formFieldName.value] = null
// Also clear the DOM element value for compatibility with existing code
if (import.meta.client) {
const element = document.getElementsByName(formFieldName.value)[0]
if (element) element.value = ''
}
}

// Handle iframe resizing
const resizeIframe = (height) => {
if (!isIframe.value) return

try {
window.parentIFrame?.size(height)
} catch (e) {
// Silently handle error
}
}

// Handle captcha open/close for iframe resizing
const onCaptchaOpen = () => {
resizeIframe(500)
// Ensure the captcha is visible by scrolling to it
if (import.meta.client) {
nextTick(() => {
const captchaElement = captchaRef.value?.$el
if (captchaElement) {
captchaElement.scrollIntoView({ behavior: 'smooth', block: 'center' })
}
})
}
}

const onCaptchaClose = () => {
resizeIframe(0)
}

// Method to reset captcha - can be called from parent
defineExpose({
reset: () => {
// Only do a full reset if the captcha was previously completed
if (captchaRef.value) {
if (wasCaptchaCompleted.value) {
wasCaptchaCompleted.value = false
captchaRef.value.reset()
}
}
}
})
</script>

<style>
.fade-enter-active,
.fade-leave-active {
transition: opacity 0.2s ease;
}

.fade-enter-from,
.fade-leave-to {
opacity: 0;
}
</style>
Loading
Loading