first draft of recaptcha validation #2
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2 +/- ##
==========================================
+ Coverage 75% 87.5% +12.5%
==========================================
Files 1 2 +1
Lines 8 32 +24
Branches 1 6 +5
==========================================
+ Hits 6 28 +22
- Misses 2 4 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This looks like a good start. I think we can simplify into a single class so explore that and then let's get a unit test or two on your new validator class and then an E2E test.
https://developers.google.com/recaptcha/docs/faq#id-like-to-run-automated-tests-with-recaptcha-v2-what-should-i-do
package.json
Outdated
| @@ -21,9 +21,11 @@ | |||
| "@nestjs/websockets": "^4.5.8", | |||
| "class-transformer": "^0.1.9", | |||
| "class-validator": "^0.8.1", | |||
| "multer": "^1.3.0", | |||
There was a problem hiding this comment.
What are we using multer for?
| secret: process.env.CAPTCHA_SECRET, | ||
| verbose: true | ||
| }); | ||
| return new Promise((resolve, reject) => { |
There was a problem hiding this comment.
This is perfect, I think we can move this into a custom validation class and this is the implementation of the validate function.
There was a problem hiding this comment.
Not sure where one of my comments went, I'm doing something wrong apparently
| import { MetadataStorageAndValidation } from './MetadataStorage'; | ||
| import { getFromContainer } from './container'; | ||
|
|
||
| export function IsCaptcha(validationOptions?: ValidationOptions) { |
There was a problem hiding this comment.
I think we can simplify this by creating a custom validation class that implements ValidatorConstraintInterface, this is documented in the README. You already have the validation function figured out but I think we can accomplish it without having to duplicate the files in our project.
| resolve({}); | ||
| }); | ||
| } else { | ||
| const Recaptcha = require('recaptcha-verify'); |
There was a problem hiding this comment.
rather than require this should probably be an import statement at the top I think import * as Recaptcha from 'recaptcha-verify'; should work.
| @@ -1,4 +1,4 @@ | |||
| import { Controller, Get, Post, Body } from '@nestjs/common'; | |||
| import { Controller, Get, Post, Body, BadRequestException } from '@nestjs/common'; | |||
There was a problem hiding this comment.
Is BadRequestException being used?
No description provided.