HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Simple Google ReCaptcha validation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
recaptchasimplevalidationgoogle

Problem

Linked:
Google reCAPTCHA Validator: Iteration II

I have also created a simple Google Recaptcha Validation class to handle verification.

I used some code from CodingFusion's post Google New reCaptcha I am not a robot using asp .net, but I have altered it so that it fit my use case, and to make it extendable and reusable.

Can I clean it up further?

```
public class ReCaptchaValidator
{
private readonly string _ReCaptchaSecret;
private readonly string _ReCaptchaSiteKey;
public List ErrorCodes { get; set; }

public ReCaptchaValidator(string reCaptchaSecret)
{
_ReCaptchaSecret = reCaptchaSecret;
this.ErrorCodes = new List();
}
public ReCaptchaValidator(string reCaptchaSecret, string reCaptchaSiteKey)
{
_ReCaptchaSecret = reCaptchaSecret;
_ReCaptchaSiteKey = reCaptchaSiteKey;
this.ErrorCodes = new List();
}

public bool ValidateCaptcha(HttpRequest request)
{
var sb = new StringBuilder();
sb.Append("https://www.google.com/recaptcha/api/siteverify?secret=");
sb.Append(_ReCaptchaSecret);
sb.Append("&response=");
sb.Append(request.Form["g-recaptcha-response"]);

//client ip address
sb.Append("&remoteip=");
sb.Append(GetUserIp(request));

//make the api call and determine validity
using (var client = new WebClient())
{
var uri = sb.ToString();
var json = client.DownloadString(uri);
var serializer = new DataContractJsonSerializer(typeof(RecaptchaApiResponse));
var ms = new MemoryStream(Encoding.Unicode.GetBytes(json));
var result = serializer.ReadObject(ms) as RecaptchaApiResponse;

if (result == null)
{
return false;
}
else if (result.ErrorCodes != null)
{
foreach(var code in result.ErrorCodes)
{

Solution

private readonly string _ReCaptchaSecret;
private readonly string _ReCaptchaSiteKey;


private fields are either lowerCamelCase or _lowerCamelCase, both conventions are common.

public ReCaptchaValidator(string reCaptchaSecret)
{
    _ReCaptchaSecret = reCaptchaSecret;
    this.ErrorCodes = new List();
}
public ReCaptchaValidator(string reCaptchaSecret, string reCaptchaSiteKey)
{
    _ReCaptchaSecret = reCaptchaSecret;
    _ReCaptchaSiteKey = reCaptchaSiteKey;
    this.ErrorCodes = new List();
}


Don't repeat work in your constructors: use chaining instead.

public ReCaptchaValidator(string reCaptchaSecret)
{
    _ReCaptchaSecret = reCaptchaSecret;
    this.ErrorCodes = new List();
}

public ReCaptchaValidator(string reCaptchaSecret, string reCaptchaSiteKey) 
    : this (reCaptchaSecret)
{
    _ReCaptchaSiteKey = reCaptchaSiteKey;
}


With C# 6, you can also use a property initializer to.. initialize.. your property.

public List ErrorCodes { get; set; }

public ReCaptchaValidator(string reCaptchaSecret)
{
    _ReCaptchaSecret = reCaptchaSecret;
    this.ErrorCodes = new List();
}


becomes

public List ErrorCodes { get; set; } = new List();

public ReCaptchaValidator(string reCaptchaSecret)
{
    _ReCaptchaSecret = reCaptchaSecret;
}


I would also advise to use a private setter for your ErrorCodes like this:

public List ErrorCodes { get; private set; } = new List();


Likewise, with C# 6, you might as well make it a readonly property:

public List ErrorCodes { get; } = new List();


StringBuilders see their most use when you're concatenating strings in a loop. In this scenario I would use a formatted string which turns this:

var sb = new StringBuilder();
sb.Append("https://www.google.com/recaptcha/api/siteverify?secret=");
sb.Append(_ReCaptchaSecret);
sb.Append("&response=");
sb.Append(request.Form["g-recaptcha-response"]);
sb.Append("&remoteip=");
sb.Append(GetUserIp(request));


into this:

var uri = string.Format("https://www.google.com/recaptcha/api/siteverify?secret={0}&response={1}&remoteip={2}",
    _ReCaptchaSecret, 
    request.Form["g-recaptcha-response"], 
    GetUserIp(request));


Or with C# 6:

var uri = $"https://www.google.com/recaptcha/api/siteverify?secret={_ReCaptchaSecret}&response={(request.Form["g-recaptcha-response"])}&remoteip={GetUserIp(request)}";


else if (!string.IsNullOrEmpty(request.UserHostAddress))


I prefer string.IsNullOrWhiteSpace because only rarely whitespace characters are considered good input.

Code Snippets

private readonly string _ReCaptchaSecret;
private readonly string _ReCaptchaSiteKey;
public ReCaptchaValidator(string reCaptchaSecret)
{
    _ReCaptchaSecret = reCaptchaSecret;
    this.ErrorCodes = new List<string>();
}
public ReCaptchaValidator(string reCaptchaSecret, string reCaptchaSiteKey)
{
    _ReCaptchaSecret = reCaptchaSecret;
    _ReCaptchaSiteKey = reCaptchaSiteKey;
    this.ErrorCodes = new List<string>();
}
public ReCaptchaValidator(string reCaptchaSecret)
{
    _ReCaptchaSecret = reCaptchaSecret;
    this.ErrorCodes = new List<string>();
}

public ReCaptchaValidator(string reCaptchaSecret, string reCaptchaSiteKey) 
    : this (reCaptchaSecret)
{
    _ReCaptchaSiteKey = reCaptchaSiteKey;
}
public List<string> ErrorCodes { get; set; }

public ReCaptchaValidator(string reCaptchaSecret)
{
    _ReCaptchaSecret = reCaptchaSecret;
    this.ErrorCodes = new List<string>();
}
public List<string> ErrorCodes { get; set; } = new List<string>();

public ReCaptchaValidator(string reCaptchaSecret)
{
    _ReCaptchaSecret = reCaptchaSecret;
}

Context

StackExchange Code Review Q#100632, answer score: 10

Revisions (0)

No revisions yet.