From 851b0b5adb52f5721e9d7504a4238095a3c1cc45 Mon Sep 17 00:00:00 2001
From: mohamed ez-zarghili <ezzarghili@users.noreply.github.com>
Date: Wed, 26 Dec 2018 11:34:09 +0000
Subject: [PATCH] Cleaning api for first release and use time.Duration where
 relevant (#16)

---
 README.md         | 42 +++++++++++++++++++--------------
 recaptcha.go      | 35 ++++++++++++++-------------
 recaptcha_test.go | 60 +++++++++++++++++++++++------------------------
 3 files changed, 74 insertions(+), 63 deletions(-)

diff --git a/README.md b/README.md
index 74d805a..b8d764f 100644
--- a/README.md
+++ b/README.md
@@ -8,20 +8,22 @@ Google reCAPTCHA v2 & v3 form submittion verification in golang
 
 The API has changed form last version hence the new major version change.  
 Old API is still available using the package `gopkg.in/ezzarghili/recaptcha-go.v2` although it does not provide all options available in this version  
-As always install the package in your environment by using a stable API version, see latest version in release page.  
+As always install the package in your environment by using a stable API version, see latest version in release page.
 
 ```bash
 go get -u gopkg.in/ezzarghili/recaptcha-go.v3
 ```
-### recaptcha v2 API 
+
+### recaptcha v2 API
+
 ```go
 import "gopkg.in/ezzarghili/recaptcha-go.v3"
 func main(){
-    captcha := recaptcha.NewReCAPTCHA(recaptchaSecret, recaptcha.V2, timeout) // for v2 API get your secret from https://www.google.com/recaptcha/admin 
+    captcha := recaptcha.NewReCAPTCHA(recaptchaSecret, recaptcha.V2, 10*time.Second) // for v2 API get your secret from https://www.google.com/recaptcha/admin
 }
 ```
 
-Now everytime you need to verify a V2 API client with no special options request use  
+Now everytime you need to verify a V2 API client with no special options request use
 
 ```go
 err := captcha.Verify(recaptchaResponse)
@@ -30,15 +32,17 @@ if err != nil {
 }
 // proceed
 ```
+
 For specific options use the `VerifyWithOptions` method  
 Available options for the v2 api are:
 
 ```go
-	Hostname       string
-	ApkPackageName string
-	ResponseTime   float64
-	RemoteIP       string
+  Hostname       string
+  ApkPackageName string
+  ResponseTime   time.Duration
+  RemoteIP       string
 ```
+
 Other v3 options are ignored and method will return `nil` when succeeded
 
 ```go
@@ -49,16 +53,18 @@ if err != nil {
 // proceed
 ```
 
-### recaptcha v3 API 
+### recaptcha v3 API
+
 ```go
 import "github.com/ezzarghili/recaptcha-go.v3"
 func main(){
-    captcha := recaptcha.NewReCAPTCHA(recaptchaSecret, recaptcha.V3, timeout) // for v3 API use https://g.co/recaptcha/v3 (apperently the same admin UI at the time of writing)
+    captcha := recaptcha.NewReCAPTCHA(recaptchaSecret, recaptcha.V3, 10*time.Second) // for v3 API use https://g.co/recaptcha/v3 (apperently the same admin UI at the time of writing)
 }
 ```
 
 Now everytime you need to verify a V3 API client with no special options request use  
 Note that as recaptcha v3 use score for challenge validation, if no treshold option is set the **default** value is `0.5`
+
 ```go
 err := captcha.Verify(recaptchaResponse)
 if err != nil {
@@ -66,16 +72,17 @@ if err != nil {
 }
 // proceed
 ```
+
 For specific options use the `VerifyWithOptions` method  
 Availavle options for the v3 api are:
 
 ```go
-	Treshold       float32 
-	Action         string  
-	Hostname       string
-	ApkPackageName string
-	ResponseTime   float64
-	RemoteIP       string
+   Treshold       float32
+   Action         string
+   Hostname       string
+   ApkPackageName string
+   ResponseTime   time.Duration
+   RemoteIP       string
 ```
 
 ```go
@@ -95,10 +102,11 @@ Use the `error` to check for issues with the secret, connection with the server,
 This version made timeout explcit to make sure users have the possiblity to set the underling http client timeout suitable for their implemetation.
 
 ### Run Tests
+
 Use the standard go means of running test.
 You can also check examples of usable in the tests.
 
-```
+```bash
 go test
 ```
 
diff --git a/recaptcha.go b/recaptcha.go
index 8260d35..925d3fc 100644
--- a/recaptcha.go
+++ b/recaptcha.go
@@ -19,7 +19,8 @@ const (
 	V2 VERSION = iota
 	// V3 recaptcha api v3, more details can be found here : https://developers.google.com/recaptcha/docs/v3
 	V3
-	DEFAULT_TRESHOLD float32 = 0.5
+	// DefaultTreshold Default minimin score when using V3 api
+	DefaultTreshold float32 = 0.5
 )
 
 type reCHAPTCHARequest struct {
@@ -43,6 +44,7 @@ type netClient interface {
 	PostForm(url string, formValues url.Values) (resp *http.Response, err error)
 }
 
+// custom clock so we can mock in tests
 type clock interface {
 	Since(t time.Time) time.Duration
 }
@@ -56,23 +58,23 @@ func (realClock) Since(t time.Time) time.Duration {
 
 // ReCAPTCHA recpatcha holder struct, make adding mocking code simpler
 type ReCAPTCHA struct {
-	Client        netClient
+	client        netClient
 	Secret        string
 	ReCAPTCHALink string
 	Version       VERSION
-	Timeout       uint
+	Timeout       time.Duration
 	horloge       clock
 }
 
-// NewReCAPTCHA Create new ReCAPTCHA with the v2 reCAPTCHA secret optained from https://www.google.com/recaptcha/admin
-// or  https://www.google.com/recaptcha/admin
-func NewReCAPTCHA(ReCAPTCHASecret string, version VERSION, timeout uint) (ReCAPTCHA, error) {
+// NewReCAPTCHA new ReCAPTCHA instance if version is set to V2 uses recatpcha v2 API, get your secret from https://www.google.com/recaptcha/admin
+//  if version is set to V2 uses recatpcha v2 API, get your secret from https://g.co/recaptcha/v3
+func NewReCAPTCHA(ReCAPTCHASecret string, version VERSION, timeout time.Duration) (ReCAPTCHA, error) {
 	if ReCAPTCHASecret == "" {
 		return ReCAPTCHA{}, fmt.Errorf("recaptcha secret cannot be blank")
 	}
 	return ReCAPTCHA{
-		Client: &http.Client{
-			Timeout: time.Duration(timeout) * time.Second,
+		client: &http.Client{
+			Timeout: timeout,
 		},
 		horloge:       &realClock{},
 		Secret:        ReCAPTCHASecret,
@@ -82,7 +84,7 @@ func NewReCAPTCHA(ReCAPTCHASecret string, version VERSION, timeout uint) (ReCAPT
 	}, nil
 }
 
-// Verify returns (true, nil) if  no error the client answered the challenge correctly and have correct remoteIP
+// Verify returns `nil` if no error and the client solved the challenge correctly
 func (r *ReCAPTCHA) Verify(challengeResponse string) error {
 	body := reCHAPTCHARequest{Secret: r.Secret, Response: challengeResponse}
 	return r.confirm(body, VerifyOption{})
@@ -94,11 +96,12 @@ type VerifyOption struct {
 	Action         string  // ignored in v2 recaptcha
 	Hostname       string
 	ApkPackageName string
-	ResponseTime   float64
+	ResponseTime   time.Duration
 	RemoteIP       string
 }
 
-// VerifyWithOptions returns (true, nil) if  no error the client answered the challenge correctly and have correct remoteIP
+// VerifyWithOptions returns `nil` if no error and the client solved the challenge correctly and all options are natching
+// `Treshold` and `Action` are ignored when using V2 version
 func (r *ReCAPTCHA) VerifyWithOptions(challengeResponse string, options VerifyOption) error {
 	var body reCHAPTCHARequest
 	if options.RemoteIP == "" {
@@ -117,7 +120,7 @@ func (r *ReCAPTCHA) confirm(recaptcha reCHAPTCHARequest, options VerifyOption) (
 	} else {
 		formValues = url.Values{"secret": {recaptcha.Secret}, "response": {recaptcha.Response}}
 	}
-	response, err := r.Client.PostForm(r.ReCAPTCHALink, formValues)
+	response, err := r.client.PostForm(r.ReCAPTCHALink, formValues)
 	if err != nil {
 		Err = fmt.Errorf("error posting to recaptcha endpoint: '%s'", err)
 		return
@@ -146,9 +149,9 @@ func (r *ReCAPTCHA) confirm(recaptcha reCHAPTCHARequest, options VerifyOption) (
 	}
 
 	if options.ResponseTime != 0 {
-		duration := r.horloge.Since(result.ChallengeTS).Seconds()
+		duration := r.horloge.Since(result.ChallengeTS)
 		if options.ResponseTime < duration {
-			Err = fmt.Errorf("time spent in resolving challenge '%f', while expecting maximum '%f'", duration, options.ResponseTime)
+			Err = fmt.Errorf("time spent in resolving challenge '%fs', while expecting maximum '%fs'", duration.Seconds(), options.ResponseTime.Seconds())
 			return
 		}
 	}
@@ -161,8 +164,8 @@ func (r *ReCAPTCHA) confirm(recaptcha reCHAPTCHARequest, options VerifyOption) (
 			Err = fmt.Errorf("received score '%f', while expecting minimum '%f'", result.Score, options.Treshold)
 			return
 		}
-		if options.Treshold == 0 && DEFAULT_TRESHOLD >= result.Score {
-			Err = fmt.Errorf("received score '%f', while expecting minimum '%f'", result.Score, DEFAULT_TRESHOLD)
+		if options.Treshold == 0 && DefaultTreshold >= result.Score {
+			Err = fmt.Errorf("received score '%f', while expecting minimum '%f'", result.Score, DefaultTreshold)
 			return
 		}
 	}
diff --git a/recaptcha_test.go b/recaptcha_test.go
index bc1d910..b00cf66 100644
--- a/recaptcha_test.go
+++ b/recaptcha_test.go
@@ -19,11 +19,11 @@ type ReCaptchaSuite struct{}
 var _ = Suite(&ReCaptchaSuite{})
 
 func (s *ReCaptchaSuite) TestNewReCAPTCHA(c *C) {
-	captcha, err := NewReCAPTCHA("my secret", V2, 10)
+	captcha, err := NewReCAPTCHA("my secret", V2, 10*time.Second)
 	c.Assert(err, IsNil)
 	c.Check(captcha.Secret, Equals, "my secret")
 	c.Check(captcha.Version, Equals, V2)
-	c.Check(captcha.Timeout, Equals, (uint)(10))
+	c.Check(captcha.Timeout, Equals, 10*time.Second)
 	c.Check(captcha.ReCAPTCHALink, Equals, reCAPTCHALink)
 
 	captcha, err = NewReCAPTCHA("", V2, 10)
@@ -54,7 +54,7 @@ func (*mockUnavailableClient) PostForm(url string, formValues url.Values) (resp
 
 func (s *ReCaptchaSuite) TestConfirm(c *C) {
 	captcha := ReCAPTCHA{
-		Client: &mockInvalidClient{},
+		client: &mockInvalidClient{},
 	}
 	body := reCHAPTCHARequest{Secret: "", Response: ""}
 
@@ -62,7 +62,7 @@ func (s *ReCaptchaSuite) TestConfirm(c *C) {
 	c.Assert(err, NotNil)
 	c.Check(err, ErrorMatches, "invalid response body json:.*")
 
-	captcha.Client = &mockUnavailableClient{}
+	captcha.client = &mockUnavailableClient{}
 	err = captcha.confirm(body, VerifyOption{})
 	c.Assert(err, NotNil)
 	c.Check(err, ErrorMatches, "error posting to recaptcha endpoint:.*")
@@ -88,7 +88,7 @@ func (*mockInvalidSolutionClient) PostForm(url string, formValues url.Values) (r
 
 func (s *ReCaptchaSuite) TestVerifyInvalidSolutionNoRemoteIp(c *C) {
 	captcha := ReCAPTCHA{
-		Client: &mockInvalidSolutionClient{},
+		client: &mockInvalidSolutionClient{},
 	}
 
 	err := captcha.Verify("mycode")
@@ -130,13 +130,13 @@ func (*mockFailedClientNoOptions) PostForm(url string, formValues url.Values) (r
 }
 func (s *ReCaptchaSuite) TestVerifyWithoutOptions(c *C) {
 	captcha := ReCAPTCHA{
-		Client: &mockSuccessClientNoOptions{},
+		client: &mockSuccessClientNoOptions{},
 	}
 
 	err := captcha.Verify("mycode")
 	c.Assert(err, IsNil)
 
-	captcha.Client = &mockFailedClientNoOptions{}
+	captcha.client = &mockFailedClientNoOptions{}
 	err = captcha.Verify("mycode")
 	c.Assert(err, NotNil)
 	c.Check(err, ErrorMatches, "remote error codes:.*")
@@ -176,13 +176,13 @@ func (*mockFailClientWithRemoteIPOption) PostForm(url string, formValues url.Val
 }
 func (s *ReCaptchaSuite) TestVerifyWithRemoteIPOption(c *C) {
 	captcha := ReCAPTCHA{
-		Client: &mockSuccessClientWithRemoteIPOption{},
+		client: &mockSuccessClientWithRemoteIPOption{},
 	}
 
 	err := captcha.VerifyWithOptions("mycode", VerifyOption{RemoteIP: "123.123.123.123"})
 	c.Assert(err, IsNil)
 
-	captcha.Client = &mockFailClientWithRemoteIPOption{}
+	captcha.client = &mockFailClientWithRemoteIPOption{}
 	err = captcha.VerifyWithOptions("mycode", VerifyOption{RemoteIP: "123.123.123.123"})
 	c.Assert(err, NotNil)
 	c.Check(err, ErrorMatches, "invalid challenge solution or remote IP")
@@ -222,13 +222,13 @@ func (*mockFailClientWithHostnameOption) PostForm(url string, formValues url.Val
 }
 func (s *ReCaptchaSuite) TestVerifyWithHostnameOption(c *C) {
 	captcha := ReCAPTCHA{
-		Client: &mockSuccessClientWithHostnameOption{},
+		client: &mockSuccessClientWithHostnameOption{},
 	}
 
 	err := captcha.VerifyWithOptions("mycode", VerifyOption{Hostname: "test.com"})
 	c.Assert(err, IsNil)
 
-	captcha.Client = &mockFailClientWithHostnameOption{}
+	captcha.client = &mockFailClientWithHostnameOption{}
 	err = captcha.VerifyWithOptions("mycode", VerifyOption{Hostname: "test.com"})
 	c.Assert(err, NotNil)
 	c.Check(err, ErrorMatches, "invalid response hostname 'test2.com', while expecting 'test.com'")
@@ -248,17 +248,17 @@ func (*mockClockOverRespenseTime) Since(t time.Time) time.Duration {
 
 func (s *ReCaptchaSuite) TestVerifyWithResponseOption(c *C) {
 	captcha := ReCAPTCHA{
-		Client:  &mockSuccessClientNoOptions{},
+		client:  &mockSuccessClientNoOptions{},
 		horloge: &mockClockWithinRespenseTime{},
 	}
 
-	err := captcha.VerifyWithOptions("mycode", VerifyOption{ResponseTime: 5})
+	err := captcha.VerifyWithOptions("mycode", VerifyOption{ResponseTime: 5 * time.Second})
 	c.Assert(err, IsNil)
 
 	captcha.horloge = &mockClockOverRespenseTime{}
-	err = captcha.VerifyWithOptions("mycode", VerifyOption{ResponseTime: 5})
+	err = captcha.VerifyWithOptions("mycode", VerifyOption{ResponseTime: 5 * time.Second})
 	c.Assert(err, NotNil)
-	c.Check(err, ErrorMatches, "time spent in resolving challenge '8.000000', while expecting maximum '5.000000'")
+	c.Check(err, ErrorMatches, "time spent in resolving challenge '8.000000s', while expecting maximum '5.000000s'")
 
 }
 
@@ -295,13 +295,13 @@ func (*mockFailClientWithApkPackageNameOption) PostForm(url string, formValues u
 }
 func (s *ReCaptchaSuite) TestVerifyWithApkPackageNameOption(c *C) {
 	captcha := ReCAPTCHA{
-		Client: &mockSuccessClientWithApkPackageNameOption{},
+		client: &mockSuccessClientWithApkPackageNameOption{},
 	}
 
 	err := captcha.VerifyWithOptions("mycode", VerifyOption{ApkPackageName: "com.test.app"})
 	c.Assert(err, IsNil)
 
-	captcha.Client = &mockFailClientWithApkPackageNameOption{}
+	captcha.client = &mockFailClientWithApkPackageNameOption{}
 	err = captcha.VerifyWithOptions("mycode", VerifyOption{ApkPackageName: "com.test.app"})
 	c.Assert(err, NotNil)
 	c.Check(err, ErrorMatches, "invalid response ApkPackageName 'com.test.app2', while expecting 'com.test.app'")
@@ -344,14 +344,14 @@ func (*mockV3FailClientWithActionOption) PostForm(url string, formValues url.Val
 }
 func (s *ReCaptchaSuite) TestV3VerifyWithActionOption(c *C) {
 	captcha := ReCAPTCHA{
-		Client:  &mockV3SuccessClientWithActionOption{},
+		client:  &mockV3SuccessClientWithActionOption{},
 		Version: V3,
 	}
 
 	err := captcha.VerifyWithOptions("mycode", VerifyOption{Action: "homepage"})
 	c.Assert(err, IsNil)
 
-	captcha.Client = &mockV3FailClientWithActionOption{}
+	captcha.client = &mockV3FailClientWithActionOption{}
 	err = captcha.VerifyWithOptions("mycode", VerifyOption{Action: "homepage"})
 	c.Assert(err, NotNil)
 	c.Check(err, ErrorMatches, "invalid response action 'homepage2', while expecting 'homepage'")
@@ -391,14 +391,14 @@ func (*mockV3FailClientWithTresholdOption) PostForm(url string, formValues url.V
 }
 func (s *ReCaptchaSuite) TestV3VerifyWithTresholdOption(c *C) {
 	captcha := ReCAPTCHA{
-		Client:  &mockV3SuccessClientWithTresholdOption{},
+		client:  &mockV3SuccessClientWithTresholdOption{},
 		Version: V3,
 	}
 
 	err := captcha.VerifyWithOptions("mycode", VerifyOption{Treshold: 0.6})
 	c.Assert(err, IsNil)
 
-	captcha.Client = &mockV3FailClientWithTresholdOption{}
+	captcha.client = &mockV3FailClientWithTresholdOption{}
 	err = captcha.VerifyWithOptions("mycode", VerifyOption{Treshold: 0.6})
 	c.Assert(err, NotNil)
 	c.Check(err, ErrorMatches, "received score '0.230000', while expecting minimum '0.600000'")
@@ -424,7 +424,7 @@ func (*mockV2SuccessClientWithV3IgnoreOptions) PostForm(url string, formValues u
 }
 func (s *ReCaptchaSuite) TestV2VerifyWithV3IgnoreOptions(c *C) {
 	captcha := ReCAPTCHA{
-		Client:  &mockV3SuccessClientWithTresholdOption{},
+		client:  &mockV3SuccessClientWithTresholdOption{},
 		Version: V2,
 	}
 	err := captcha.VerifyWithOptions("mycode", VerifyOption{Action: "homepage", Treshold: 0.5})
@@ -509,7 +509,7 @@ func (s *ReCaptchaSuite) TestNewReCAPTCHA(c *C) {
 
 func (s *ReCaptchaSuite) TestVerifyV2WithoutClientIP(c *C) {
 	captcha := ReCAPTCHA{
-		Client:  &mockSuccessClient{},
+		client:  &mockSuccessClient{},
 		Version: V2,
 	}
 
@@ -517,7 +517,7 @@ func (s *ReCaptchaSuite) TestVerifyV2WithoutClientIP(c *C) {
 	c.Assert(err, IsNil)
 	c.Check(success, Equals, true)
 
-	captcha.Client = &mockFailedClient{}
+	captcha.client = &mockFailedClient{}
 	success, err = captcha.Verify("mycode")
 	c.Assert(err, IsNil)
 	c.Check(success, Equals, false)
@@ -525,7 +525,7 @@ func (s *ReCaptchaSuite) TestVerifyV2WithoutClientIP(c *C) {
 
 func (s *ReCaptchaSuite) TestV3VerifyWithCHostnameOption(c *C) {
 	captcha := ReCAPTCHA{
-		Client:  &mockSuccessClientHostnameOption{},
+		client:  &mockSuccessClientHostnameOption{},
 		Version: V3,
 	}
 
@@ -533,7 +533,7 @@ func (s *ReCaptchaSuite) TestV3VerifyWithCHostnameOption(c *C) {
 	c.Assert(err, IsNil)
 	c.Check(success, Equals, true)
 
-	captcha.Client = &mockFailedClientHostnameOption{}
+	captcha.client = &mockFailedClientHostnameOption{}
 	success, err = captcha.VerifyWithOptions("mycode", VerifyOption{Hostname: "valid.com"})
 	c.Assert(err, NotNil)
 	c.Check(success, Equals, false)
@@ -542,14 +542,14 @@ func (s *ReCaptchaSuite) TestV3VerifyWithCHostnameOption(c *C) {
 /*
 func (s *ReCaptchaSuite) TestVerifyWithClientIP(c *C) {
 	captcha := ReCAPTCHA{
-		Client: &mockSuccessClient{},
+		client: &mockSuccessClient{},
 	}
 
 	success, err := captcha.Verify("mycode", "127.0.0.1")
 	c.Assert(err, IsNil)
 	c.Check(success, Equals, true)
 
-	captcha.Client = &mockFailedClient{}
+	captcha.client = &mockFailedClient{}
 	success, err = captcha.Verify("mycode", "127.0.0.1")
 	c.Assert(err, IsNil)
 	c.Check(success, Equals, false)
@@ -558,7 +558,7 @@ func (s *ReCaptchaSuite) TestVerifyWithClientIP(c *C) {
 func (s *ReCaptchaSuite) TestConfirm(c *C) {
 	// check that an invalid json body errors
 	captcha := ReCAPTCHA{
-		Client: &mockInvalidClient{},
+		client: &mockInvalidClient{},
 	}
 	body := reCHAPTCHARequest{Secret: "", Response: ""}
 
@@ -566,7 +566,7 @@ func (s *ReCaptchaSuite) TestConfirm(c *C) {
 	c.Assert(err, NotNil)
 	c.Check(success, Equals, false)
 
-	captcha.Client = &mockUnavailableClient{}
+	captcha.client = &mockUnavailableClient{}
 	success, err = captcha.confirm(body)
 	c.Assert(err, NotNil)
 	c.Check(success, Equals, false)