Google reCAPTCHA Validator: Iteration III

Question

This is the third iteration of my reCAPTCHA validator. Iteration II can be found at: Google reCAPTCHA Validator: Iteration II

It does almost everything required to do reCAPTCHA validation on anything .NET.

I removed all the XML docs to shorten it up a bit.

GitHub commit of the version in this thread (with all the docs added that I omitted here): ReCaptchaValidator.cs

The only comments I do not wish to hear of are any comments relating to my use of fully-qualifying static/const members. I do this primarily to remind myself of where they originate.

using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Linq;
using System.Net;
using System.Web;
using System.Web.Script.Serialization;

namespace Evbpc.Framework.Integrations.Google
{
    public class ReCaptchaValidator
    {
        private const string _headScriptInclude = "<script src='https://www.google.com/recaptcha/api.js'></script>";
        private const string _bodyDivInclude = "<div class=\"g-recaptcha %EXTRACLASSES%\" data-sitekey=\"%SITEKEY%\"></div>";
        private const string _reCaptchaFormCode = "g-recaptcha-response";
        private const string _googleApiEndpoint = "https://www.google.com/recaptcha/api/siteverify";

        private readonly string _reCaptchaSecret;
        private readonly string _reCaptchaSiteKey;
        private readonly List<string> _extraClasses = new List<string>();

        public string HeadScriptInclude => ReCaptchaValidator._headScriptInclude;
        public List<string> ExtraClasses => _extraClasses;
        public string BodyDivInclude => GetBodyDivContent(this.ExtraClasses);

        public ReCaptchaValidator(string reCaptchaSecret, string reCaptchaSiteKey)
        {
            _reCaptchaSecret = reCaptchaSecret;
            _reCaptchaSiteKey = reCaptchaSiteKey;
        }

        public ReCaptchaResponse Validate(NameValueCollection form, string remoteIp = null)
        {
            string reCaptchaSecret = _reCaptchaSecret;
            string reCaptchaResponse = form[ReCaptchaValidator._reCaptchaFormCode];

            using (WebClient client = new WebClient())
            {
                NameValueCollection postParameters = new NameValueCollection() { { "secret", reCaptchaSecret }, { "response", reCaptchaResponse } };

                if (remoteIp != null)
                {
                    postParameters.Add("remoteip", remoteIp);
                }

                byte[] response = client.UploadValues(ReCaptchaValidator._googleApiEndpoint, postParameters);

                string reCaptchaResult = System.Text.Encoding.UTF8.GetString(response);

                ReCaptchaResponse result = new ReCaptchaResponse();

                if (result.ParseJson(reCaptchaResult))
                {
                    return result;
                }

                return null;
            }
        }

        public string GetBodyDivContent(List<string> extraClasses)
        {
            string result = ReCaptchaValidator._bodyDivInclude;

            result = result.Replace("%SITEKEY%", _reCaptchaSiteKey);

            if (extraClasses != null)
            {
                result = result.Replace("%EXTRACLASSES%", string.Join(" ", extraClasses));
            }

            return result;
        }
    }

    public class ReCaptchaResponse
    {
        private bool _success = false;
        private ReCaptchaErrors _errors = ReCaptchaErrors.None;

        public bool Success => _success; 
        public ReCaptchaErrors Errors => _errors;

        public bool ParseJson(string jsonResponse)
        {
            JavaScriptSerializer jss = new JavaScriptSerializer();
            dynamic deserializedJson = jss.DeserializeObject(jsonResponse);

            if (deserializedJson.ContainsKey("success"))
            {
                _success = deserializedJson["success"];
                _errors = ReCaptchaErrors.None;

                if (deserializedJson.ContainsKey("error-codes"))
                {
                    foreach (string error in deserializedJson["error-codes"])
                    {
                        // Our `ReCaptchaErrors` enum contains the exact same strings as the returned `error` text would be, with the following transformations:
                        // 1. The words are transformed to PascalCase;
                        // 2. The dashes are stripped;
                        string[] errorWords = error.Split('-');

                        string errorEnumName = "";
                        foreach (string errorWord in errorWords)
                        {
                            errorEnumName += errorWord[0].ToString().ToUpper() + errorWord.Substring(1);
                        }

                        if (Enum.IsDefined(typeof(ReCaptchaErrors), errorEnumName))
                        {
                            _errors = _errors | (ReCaptchaErrors)Enum.Parse(typeof(ReCaptchaErrors), errorEnumName);
                        }
                        else
                        {
                            return false;
                        }
                    }
                }
            }
            else
            {
                return false;
            }

            return true;
        }
    }

    [Flags]
    public enum ReCaptchaErrors
    {
        None = 0x00,
        MissingInputSecret = 1 << 0,
        InvalidInputSecret = 1 << 1,
        MissingInputResponse = 1 << 2,
        InvalidInputResponse = 1 << 3,
    }
}

Yes, this is the first question I have asked here with all the braces included.

This is part of an open-source library I have which, as usual, anyone is free to use.

Answer 1

The strings "success" and "error-codes" should be const string.

---

Why is the construction of postParameters inside using (WebClient client = new WebClient())? On that same topic: considering client isn't used after it has filled response, why keep it open? Basically, only response = client.UploadValues(ReCaptchaValidator._googleApiEndpoint, postParameters); should be inside using (WebClient client = new WebClient()) (with byte[] response defined outside of it).

You also don't need the empty argument at the end of NameValueCollection().

---

Can remoteIp be an empty string? Otherwise, why not use string.IsNullOrEmpty instead of if (remoteIp != null)?

---

There's no need to do this:

string result = ReCaptchaValidator._bodyDivInclude;
result = result.Replace("%SITEKEY%", _reCaptchaSiteKey);

Just do it in one line:

string result = ReCaptchaValidator._bodyDivInclude.Replace("%SITEKEY%", _reCaptchaSiteKey);

---

I'm not a fan of an enum with a plural as a name: ReCaptchaErrors. Especially since you then named the field _errors, which suggests a collection. Same for Errors.

---

Why do you use JavaScriptSerializer and not Json.NET?

---

You can reduce the indentation by re-thinkign your if-else blocks:

if (!deserializedJson.ContainsKey("success"))
{
    return false;
}

_success = deserializedJson["success"];
_errors = ReCaptchaErrors.None;

if (!deserializedJson.ContainsKey("error-codes"))
{
    return true;
}

---

It's customary to use string.Empty instead of "": string errorEnumName = "";.

---

Move errorWord[0].ToString().ToUpper() + errorWord.Substring(1); to a method called Capitalize and rewrite it slightly:

private string Capitalize(string s)
{
    if (string.IsNullOrEmpty(s))
    {
        return string.Empty;
    }
    return char.ToUpper(s[0]) + s.Substring(1);
}

That way your for loop can become:

string errorEnumName = errorWords.Aggregate(string.Empty, (current, errorWord) => current + Capitalize(errorWord));

---

I would consider converting deserializedJson to a case insensitive Dictionary<K, V> to avoid the foreach. Also, that way you can use TryGetValue.

Answer 2

Just one quick comment:

ReCaptchaResponse result = new ReCaptchaResponse();

if (result.ParseJson(reCaptchaResult))
{
    return result;
}
return null;

This would work better as static method following the familiar TryParse pattern:

ReCaptchaResponse result;
if (ReCaptchaResponse.TryParseJson(reCaptchaResult, out result))
{
    return result;
}
return null;

Answer 3

Some changes I made (after noticing a few issues with it):

• ExtraClasses has no need to be a field and a property; replaced both with public List<string> ExtraClasses { get; } = new List<string>();.
• Since GetBodyDivContent is not static, I made removed extraClasses from the parameter, and made it directly used in the method.
• I extracted a lot of Validate out to other protected methods.
• I replaced ParseJson with TryParseJson as per RobH's answer.

Version as of this answer: ReCaptchaValidator.cs