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

Added optional recaptcha v3 support #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yohanesgultom
Copy link

Added optional recaptcha support to protect registration and login form from bots

@nafiesl
Copy link
Owner

nafiesl commented Sep 25, 2021

Hello, @yohanesgultom thanks for contributing.

OK, it looks like we need 2 env variables: RECAPTCHA_SITE_KEY and RECAPTCHA_SECRET_KEY.

I have no experience of using Google Recaptcha, could you possibly help to write "How to test instruction" on the description (something like this #50)?

Or maybe you could send me a tutorial link for setup google Recaptcha?

Thanks.

@yohanesgultom
Copy link
Author

Hi @nafiesl . Thanks for your response.

This is the link to a complete official guide on Google Recaptcha setup https://developers.google.com/recaptcha/docs/v3.

In a nutshell, we just need to:

  1. Create google account
  2. Register our website in https://www.google.com/recaptcha/admin
  3. Copy paste the Site Key and Secret Key to the .env file

Let me know if you need more information.

Copy link
Owner

@nafiesl nafiesl left a comment

Choose a reason for hiding this comment

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

Hi @yohanesgultom, I apologize for my very late check on this PR. I left some comments for you. Please have a look when you have time.

Thanks!

Comment on lines +9 to +17
/**
* Create a new rule instance.
*
* @return void
*/
public function __construct()
{
//
}
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this unused method.

Suggested change
/**
* Create a new rule instance.
*
* @return void
*/
public function __construct()
{
//
}

Comment on lines +19 to +25
/**
* Determine if the validation rule passes.
*
* @param string $attribute
* @param mixed $value
* @return bool
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this docblock.

Suggested change
/**
* Determine if the validation rule passes.
*
* @param string $attribute
* @param mixed $value
* @return bool
*/

Comment on lines +40 to +44
$response_keys = json_decode($response, true);
if (!$response_keys['success']) {
$this->error_codes = $response_keys['error-codes'];
}
return $response_keys['success'];
Copy link
Owner

@nafiesl nafiesl Jul 31, 2023

Choose a reason for hiding this comment

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

Please use camelCase for variable names.

Suggested change
$response_keys = json_decode($response, true);
if (!$response_keys['success']) {
$this->error_codes = $response_keys['error-codes'];
}
return $response_keys['success'];
$responseKeys = json_decode($response, true);
if (!$responseKeys['success']) {
$this->error_codes = $responseKeys['error-codes'];
}
return $responseKeys['success'];

Comment on lines +48 to +52
/**
* Get the validation error message.
*
* @return string
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this docblock.

Suggested change
/**
* Get the validation error message.
*
* @return string
*/

Comment on lines +53 to +61
public function message()
{
$msg = __('validation.g_recaptcha_response.failed');
if (!empty($this->error_codes)) {
$msg = implode(', ', $this->error_codes);
}
return $msg;
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

I am not seeing this validation message showing anywhere in the views, maybe you need to add something like these on the views/components/recaptcha-v3.blade.php:

@if ($errors->has('g-recaptcha-response'))
    <div class="form-group has-error">
        <div class="col-md-8 col-md-offset-4">
            <span class="help-block">
                <strong>{{ $errors->first('g-recaptcha-response') }}</strong>
            </span>
        </div>
    </div>
@endif

Comment on lines +9 to +17
/**
* Create a new component instance.
*
* @return void
*/
public function __construct()
{
//
}
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove unused methods.

Suggested change
/**
* Create a new component instance.
*
* @return void
*/
public function __construct()
{
//
}

Comment on lines +19 to +23
/**
* Get the view / contents that represent the component.
*
* @return \Illuminate\Contracts\View\View|\Closure|string
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this docblock.

Suggested change
/**
* Get the view / contents that represent the component.
*
* @return \Illuminate\Contracts\View\View|\Closure|string
*/

Comment on lines +251 to +254
'recaptcha' => [
'site_key' => env('RECAPTCHA_SITE_KEY'),
'secret_key' => env('RECAPTCHA_SECRET_KEY'),
],
Copy link
Owner

Choose a reason for hiding this comment

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

Please move this config definition into config/services.php.

@@ -0,0 +1,15 @@
@if (!empty(config('app.recaptcha.site_key')))
Copy link
Owner

Choose a reason for hiding this comment

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

Very nice, I agree with this.


use Illuminate\Contracts\Validation\Rule;

class RecaptchaV3 implements Rule
Copy link
Owner

Choose a reason for hiding this comment

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

I do not see you adding this validation rule anywhere. I think you need to add this rule to the registration and login controller. Please add this rule to them.

Note:
Make sure you use the !empty(config('app.recaptcha.site_key')) check. So if our users not filling the site_key, they won't get errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants