Improve the error handling to include the status code and error details

CHANGELOG.md

@@ -0,0 +1,9 @@
+# v0.2.0
+
+* Adds support for structured error responses from the Conjur v5 server, using the struct `conjurapi.ConjurError`. This is a backwards incompatible change.
+* All API methods accept fully qualified object ids in v5 mode. This is a backwards compatible bug fix.
+* API methods which do not work in v4 mode return an appropriate error message. This is a backwards compatible bug fix.
+
+# v0.1.0
+
+* Initial version

conjurapi/authn.go

@@ -1,8 +1,10 @@
 package conjurapi
 
 import (
-	"net/http"
 	"encoding/base64"
+	"fmt"
+	"net/http"
+
 	"github.com/cyberark/conjur-api-go/conjurapi/authn"
 	"github.com/cyberark/conjur-api-go/conjurapi/wrapper"
 	"github.com/cyberark/conjur-api-go/conjurapi/wrapper_v4"
@@ -38,13 +40,20 @@ func (c *Client) createAuthRequest(req *http.Request) (error) {
 		return err
 	}
 
-	wrapper.SetRequestAuthorization(req, base64.StdEncoding.EncodeToString(c.authToken.Raw()))
+	req.Header.Set(
+		"Authorization",
+		fmt.Sprintf("Token token=\"%s\"", base64.StdEncoding.EncodeToString(c.authToken.Raw())),
+	)
 
 	return nil
 }
 
 func (c *Client) Authenticate(loginPair authn.LoginPair) ([]byte, error) {
-	req, err := wrapper.AuthenticateRequest(c.config.ApplianceURL, c.config.Account, loginPair)
+	var (
+		req *http.Request
+		err error
+	)
+
 	if c.config.V4 {
 		req, err = wrapper_v4.AuthenticateRequest(c.config.ApplianceURL, loginPair)
 	} else {
@@ -60,7 +69,6 @@ func (c *Client) Authenticate(loginPair authn.LoginPair) ([]byte, error) {
 		return nil, err
 	}
 
-
 	if c.config.V4 {
 		return wrapper_v4.AuthenticateResponse(resp)
 	} else {

conjurapi/client.go

@@ -7,6 +7,8 @@ import (
 	"crypto/x509"
 	"crypto/tls"
 	"os"
+	"strings"
+
 	"github.com/bgentry/go-netrc/netrc"
 	"github.com/cyberark/conjur-api-go/conjurapi/authn"
 )
@@ -117,6 +119,17 @@ func (c *Client) SubmitRequest(req *http.Request) (resp *http.Response, err erro
 	return
 }
 
+func makeFullId(account, kind, id string) (string) {
+	tokens := strings.SplitN(id, ":", 3)
+	switch len(tokens) {
+		case 1:
+			tokens = []string{ account, kind, tokens[0] }
+		case 2: 
+			tokens = []string{ account, tokens[0], tokens[1] }
+	}
+	return strings.Join(tokens, ":")
+}
+
 func newClientWithAuthenticator(config Config, authenticator Authenticator) (*Client, error) {
 	var (
 		err error

conjurapi/config_test.go

@@ -32,7 +32,7 @@ func TestConfig_IsValid(t *testing.T) {
 	})
 }
 
-func TestLoadFromEnv(t *testing.T) {
+func TestConfig_LoadFromEnv(t *testing.T) {
 	Convey("Given configuration and authentication credentials in env", t, func() {
 		e := ClearEnv()
 		defer e.RestoreEnv()

conjurapi/policy.go

@@ -1,12 +1,25 @@
 package conjurapi
 
 import (
+	"fmt"
 	"io"
+	"net/http"
+
 	"github.com/cyberark/conjur-api-go/conjurapi/wrapper"
 )
 
-func (c *Client) LoadPolicy(policyIdentifier string, policy io.Reader) ([]byte, error) {
-	req, err := wrapper.LoadPolicyRequest(c.config.ApplianceURL, c.config.Account, policyIdentifier, policy)
+func (c *Client) LoadPolicy(policyId string, policy io.Reader) (map[string]interface{}, error) {
+	var (
+		req *http.Request
+		err error
+	)
+
+	if c.config.V4 {
+		err = fmt.Errorf("LoadPolicy is not supported for Conjur V4")
+	} else {
+		req, err = wrapper.LoadPolicyRequest(c.config.ApplianceURL, makeFullId(c.config.Account, "policy", policyId), policy)
+	}
+
 	if err != nil {
 		return nil, err
 	}

conjurapi/policy_test.go

@@ -7,10 +7,11 @@ import (
 	"fmt"
 	"strings"
 	"github.com/cyberark/conjur-api-go/conjurapi/authn"
+	"github.com/cyberark/conjur-api-go/conjurapi/response"
 )
 
 func TestClient_LoadPolicy(t *testing.T) {
-	Convey("Given valid configuration and login credentials", t, func() {
+	Convey("V5", t, func() {
 		config := &Config{}
 		config.mergeEnv()
 
@@ -32,7 +33,7 @@ func TestClient_LoadPolicy(t *testing.T) {
 			)
 
 			So(err, ShouldBeNil)
-			So(string(resp), ShouldContainSubstring, `{"created_roles":{"cucumber:user:alice":`)
+			So(resp["created_roles"], ShouldNotBeNil)
 		})
 
 		Convey("Given invalid login credentials", func() {
@@ -45,11 +46,41 @@ func TestClient_LoadPolicy(t *testing.T) {
 				resp, err := conjur.LoadPolicy("root", strings.NewReader(""))
 
 				So(err, ShouldNotBeNil)
-				So(string(resp), ShouldEqual, "")
-				So(err.Error(), ShouldContainSubstring, "401")
+				So(resp, ShouldBeNil)
+				So(err, ShouldHaveSameTypeAs, &response.ConjurError{})
+				conjurError := err.(*response.ConjurError)
+				So(conjurError.Code, ShouldEqual, 401)
 			})
 
 		})
 	})
+	Convey("V4", t, func() {
+		config := &Config{
+			ApplianceURL: os.Getenv("CONJUR_V4_APPLIANCE_URL"),
+			SSLCert:      os.Getenv("CONJUR_V4_SSL_CERTIFICATE"),
+			Account:      os.Getenv("CONJUR_V4_ACCOUNT"),
+			V4:           true,
+		}
 
+		login := os.Getenv("CONJUR_V4_AUTHN_LOGIN")
+		api_key := os.Getenv("CONJUR_V4_AUTHN_API_KEY")
+
+		Convey("Policy loading is not supported", func() {
+			variable_identifier := "alice"
+			policy := fmt.Sprintf(`
+- !user %s
+`, variable_identifier)
+
+			conjur, err := NewClientFromKey(*config, authn.LoginPair{login, api_key})
+			So(err, ShouldBeNil)
+
+			_, err = conjur.LoadPolicy(
+				"root",
+				strings.NewReader(policy),
+			)
+
+			So(err, ShouldNotBeNil)
+			So(err.Error(), ShouldEqual, "LoadPolicy is not supported for Conjur V4")
+		})
+	})
 }

conjurapi/response/error.go

@@ -0,0 +1,45 @@
+package response
+
+import (
+  "encoding/json"
+  "io/ioutil"
+  "net/http"
+  "strings"
+)
+
+type ConjurError struct {
+  Code    int
+  Message string
+  Details *ConjurErrorDetails `json:"error"`
+}
+
+type ConjurErrorDetails struct {
+  Message string
+  Code    string
+  Target  string
+  Details map[string]interface{}
+}
+
+func NewConjurError(resp *http.Response) (error) {
+  defer resp.Body.Close()
+  body, err := ioutil.ReadAll(resp.Body)
+  if err != nil {
+    return err
+  }
+
+  cerr := ConjurError{}
+  cerr.Code = resp.StatusCode
+  err = json.Unmarshal(body, &cerr)
+  if err != nil {
+    cerr.Message = strings.TrimSpace(string(body))
+  }
+  return &cerr
+}
+
+func (self *ConjurError) Error() string {
+  if self.Details != nil && self.Details.Message != "" {
+    return self.Details.Message
+  } else {
+    return self.Message
+  }
+}

conjurapi/response/response.go

@@ -0,0 +1,46 @@
+package response
+
+import (
+  "encoding/json"
+  "io/ioutil"
+  "net/http"
+)
+
+func readBody(resp *http.Response) ([]byte, error) {
+  defer resp.Body.Close()
+
+  responseText, err := ioutil.ReadAll(resp.Body)
+  if err != nil {
+    return nil, err
+  }
+
+  return responseText, err
+}
+
+func SecretDataResponse(resp *http.Response) ([]byte, error) {
+  if resp.StatusCode < 300 {
+    return readBody(resp)
+  } else {
+    return nil, NewConjurError(resp)   
+  }  
+}
+
+func JSONResponse(resp *http.Response, obj interface{}) (error) {
+  if resp.StatusCode < 300 {
+    body, err := readBody(resp)
+    if err != nil {
+      return err
+    }
+    return json.Unmarshal(body, obj)
+  } else {
+    return NewConjurError(resp)   
+  }  
+}
+
+func EmptyResponse(resp *http.Response) (error) {
+  if resp.StatusCode < 300 {
+    return nil
+  } else {
+    return NewConjurError(resp)   
+  }    
+}

conjurapi/variable.go

@@ -1,21 +1,23 @@
 package conjurapi
 
 import (
-	"github.com/cyberark/conjur-api-go/conjurapi/wrapper"
+	"fmt"
 	"net/http"
+
+	"github.com/cyberark/conjur-api-go/conjurapi/wrapper"
 	"github.com/cyberark/conjur-api-go/conjurapi/wrapper_v4"
 )
 
-func (c *Client) RetrieveSecret(variableIdentifier string) ([]byte, error) {
+func (c *Client) RetrieveSecret(variableId string) ([]byte, error) {
 	var (
 		req *http.Request
 		err error
 	)
 
 	if c.config.V4 {
-		req, err = wrapper_v4.RetrieveSecretRequest(c.config.ApplianceURL, variableIdentifier)
+		req, err = wrapper_v4.RetrieveSecretRequest(c.config.ApplianceURL, variableId)
 	} else {
-		req, err = wrapper.RetrieveSecretRequest(c.config.ApplianceURL, c.config.Account, variableIdentifier)
+		req, err = wrapper.RetrieveSecretRequest(c.config.ApplianceURL, makeFullId(c.config.Account, "variable", variableId))
 	}
 
 	if err != nil {
@@ -28,14 +30,24 @@ func (c *Client) RetrieveSecret(variableIdentifier string) ([]byte, error) {
 	}
 
 	if c.config.V4 {
-		return wrapper_v4.RetrieveSecretResponse(variableIdentifier, resp)
+		return wrapper_v4.RetrieveSecretResponse(resp)
 	} else {
-		return wrapper.RetrieveSecretResponse(variableIdentifier, resp)
+		return wrapper.RetrieveSecretResponse(resp)
 	}
 }
 
-func (c *Client) AddSecret(variableIdentifier string, secretValue string) error {
-	req, err := wrapper.AddSecretRequest(c.config.ApplianceURL, c.config.Account, variableIdentifier, secretValue)
+func (c *Client) AddSecret(variableId string, secretValue string) error {
+	var (
+		req *http.Request
+		err error
+	)
+
+	if c.config.V4 {
+		err = fmt.Errorf("AddSecret is not supported for Conjur V4")
+	} else {
+		req, err = wrapper.AddSecretRequest(c.config.ApplianceURL, makeFullId(c.config.Account, "variable", variableId), secretValue)
+	}
+
 	if err != nil {
 		return err
 	}
@@ -45,5 +57,5 @@ func (c *Client) AddSecret(variableIdentifier string, secretValue string) error
 		return err
 	}
 
-	return wrapper.AddSecretResponse(variableIdentifier, resp)
+	return wrapper.AddSecretResponse(resp)
 }