Skip to content

Commit

Permalink
feat: add detailed error response to iam/cp4d authenticators (#66)
Browse files Browse the repository at this point in the history
* feat: add detailed error response to iam/cp4d authenticators

* address review comments

* more review changes
  • Loading branch information
jorge-ibm authored Aug 14, 2020
1 parent 58a7e05 commit 3485263
Show file tree
Hide file tree
Showing 12 changed files with 295 additions and 41 deletions.
10 changes: 10 additions & 0 deletions v4/core/authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,13 @@ type Authenticator interface {
Authenticate(*http.Request) error
Validate() error
}

// AuthenticationError describes the error returned when authentication fails
type AuthenticationError struct {
Response *DetailedResponse
err error
}

func (e *AuthenticationError) Error() string {
return e.err.Error()
}
10 changes: 7 additions & 3 deletions v4/core/base_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,13 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
return
}

err = service.Options.Authenticator.Authenticate(req)
if err != nil {
err = fmt.Errorf(ERRORMSG_AUTHENTICATE_ERROR, err.Error())
authError := service.Options.Authenticator.Authenticate(req)
if authError != nil {
err = fmt.Errorf(ERRORMSG_AUTHENTICATE_ERROR, authError.Error())
castErr, ok := authError.(*AuthenticationError)
if ok {
detailedResponse = castErr.Response
}
return
}

Expand Down
112 changes: 101 additions & 11 deletions v4/core/base_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func TestGoodResponseJSONExtraFields(t *testing.T) {
Authenticator: &NoAuthAuthenticator{},
}
service, _ := NewBaseService(options)

var foo *Foo
detailedResponse, _ := service.Request(req, &foo)
result, ok := detailedResponse.Result.(*Foo)
Expand Down Expand Up @@ -254,7 +254,7 @@ func TestGoodResponseString(t *testing.T) {
assert.Nil(t, err)
assert.NotNil(t, service.Options.Authenticator)
assert.Equal(t, AUTHTYPE_NOAUTH, service.Options.Authenticator.AuthenticationType())

var responseString *string
detailedResponse, err := service.Request(req, &responseString)
assert.Nil(t, err)
Expand All @@ -264,7 +264,7 @@ func TestGoodResponseString(t *testing.T) {
assert.NotNil(t, detailedResponse.Result)
assert.NotNil(t, responseString)
assert.Equal(t, expectedResponse, *responseString)

resultField, ok := detailedResponse.Result.(*string)
assert.Equal(t, true, ok)
assert.NotNil(t, resultField)
Expand Down Expand Up @@ -330,7 +330,7 @@ func TestGoodResponseJSONDeserFailure(t *testing.T) {
Authenticator: &NoAuthAuthenticator{},
}
service, _ := NewBaseService(options)

var foo *Foo
detailedResponse, err := service.Request(req, &foo)
assert.NotNil(t, detailedResponse)
Expand Down Expand Up @@ -437,7 +437,7 @@ func TestErrorResponseJSON(t *testing.T) {
Authenticator: &NoAuthAuthenticator{},
}
service, _ := NewBaseService(options)

var foo *Foo
response, err := service.Request(req, &foo)
assert.NotNil(t, err)
Expand Down Expand Up @@ -473,7 +473,7 @@ func TestErrorResponseJSONDeserError(t *testing.T) {
Authenticator: &NoAuthAuthenticator{},
}
service, _ := NewBaseService(options)

var foo *Foo
response, err := service.Request(req, &foo)
assert.NotNil(t, err)
Expand Down Expand Up @@ -505,7 +505,7 @@ func TestErrorResponseNotJSON(t *testing.T) {
Authenticator: &NoAuthAuthenticator{},
}
service, _ := NewBaseService(options)

var foo *Foo
response, err := service.Request(req, &foo)
assert.NotNil(t, err)
Expand Down Expand Up @@ -585,7 +585,7 @@ func TestRequestForDefaultUserAgent(t *testing.T) {
Authenticator: authenticator,
}
service, _ := NewBaseService(options)

var foo *Foo
_, _ = service.Request(req, &foo)
}
Expand Down Expand Up @@ -614,7 +614,7 @@ func TestRequestForProvidedUserAgent(t *testing.T) {
headers := http.Header{}
headers.Add("User-Agent", "provided user agent")
service.SetDefaultHeaders(headers)

var foo *Foo
_, _ = service.Request(req, &foo)
}
Expand Down Expand Up @@ -887,11 +887,55 @@ func TestIAMFailure(t *testing.T) {
assert.NotNil(t, service.Options.Authenticator)

var foo *Foo
_, err = service.Request(req, &foo)
detailedResponse, err := service.Request(req, &foo)
assert.NotNil(t, err)
assert.NotNil(t, detailedResponse)
assert.NotNil(t, detailedResponse.GetHeaders())
assert.NotNil(t, detailedResponse.GetRawResult())
statusCode := detailedResponse.GetStatusCode()
assert.Equal(t, http.StatusForbidden, statusCode)
assert.Contains(t, err.Error(), "Sorry you are forbidden")
}

func TestIAMFailureRetryAfter(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Retry-After", "20")
w.WriteHeader(http.StatusTooManyRequests)
_, _ = w.Write([]byte("Sorry rate limit has been exceeded"))
}))
defer server.Close()

builder := NewRequestBuilder("GET")
_, err := builder.ConstructHTTPURL(server.URL, nil, nil)
assert.Nil(t, err)
builder.AddQuery("Version", "2018-22-09")
req, _ := builder.Build()

options := &ServiceOptions{
URL: server.URL,
Authenticator: &IamAuthenticator{
URL: server.URL,
ApiKey: "xxxxx",
},
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service)
assert.NotNil(t, service.Options.Authenticator)

var foo *Foo
detailedResponse, err := service.Request(req, &foo)
assert.NotNil(t, err)
assert.NotNil(t, detailedResponse)
assert.NotNil(t, detailedResponse.GetRawResult())
statusCode := detailedResponse.GetStatusCode()
headers := detailedResponse.GetHeaders()
assert.NotNil(t, headers)
assert.Equal(t, http.StatusTooManyRequests, statusCode)
assert.Contains(t, headers, "Retry-After")
assert.Contains(t, err.Error(), "Sorry rate limit has been exceeded")
}

func TestIAMWithIdSecret(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
Expand Down Expand Up @@ -1052,10 +1096,56 @@ func TestCP4DFail(t *testing.T) {
assert.NotNil(t, service.Options.Authenticator)

var foo *Foo
_, err = service.Request(req, &foo)
detailedResponse, err := service.Request(req, &foo)
assert.NotNil(t, err)
assert.NotNil(t, detailedResponse)
assert.NotNil(t, detailedResponse.GetHeaders())
assert.NotNil(t, detailedResponse.GetRawResult())
statusCode := detailedResponse.GetStatusCode()
assert.Equal(t, http.StatusForbidden, statusCode)
assert.Contains(t, err.Error(), "Sorry you are forbidden")
}

func TestCp4dFailureRetryAfter(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Retry-After", "20")
w.WriteHeader(http.StatusTooManyRequests)
_, _ = w.Write([]byte("Sorry rate limit has been exceeded"))
}))
defer server.Close()

builder := NewRequestBuilder("GET")
_, err := builder.ConstructHTTPURL(server.URL, nil, nil)
assert.Nil(t, err)
builder.AddQuery("Version", "2018-22-09")
req, _ := builder.Build()

options := &ServiceOptions{
URL: server.URL,
Authenticator: &CloudPakForDataAuthenticator{
URL: server.URL,
Username: "bogus",
Password: "bogus",
},
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service)
assert.NotNil(t, service.Options.Authenticator)

var foo *Foo
detailedResponse, err := service.Request(req, &foo)
assert.NotNil(t, err)
assert.NotNil(t, detailedResponse)
assert.NotNil(t, detailedResponse.GetRawResult())
statusCode := detailedResponse.GetStatusCode()
headers := detailedResponse.GetHeaders()
assert.NotNil(t, headers)
assert.Equal(t, http.StatusTooManyRequests, statusCode)
assert.Contains(t, headers, "Retry-After")
assert.Contains(t, err.Error(), "Sorry rate limit has been exceeded")
}

// Test for the deprecated SetURL method.
func TestSetURL(t *testing.T) {
service, err := NewBaseService(
Expand Down
5 changes: 2 additions & 3 deletions v4/core/config_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ var testEnvironment = map[string]string{
"SERVICE3_USERNAME": "my-cp4d-user",
"SERVICE3_PASSWORD": "my-cp4d-password",
"SERVICE3_AUTH_DISABLE_SSL": "false",
"EQUAL_SERVICE_URL": "https://my=host.com/my=service/api",
"EQUAL_SERVICE_APIKEY": "===my=iam=apikey===",
"EQUAL_SERVICE_URL": "https://my=host.com/my=service/api",
"EQUAL_SERVICE_APIKEY": "===my=iam=apikey===",
}

// Set the environment variables described in our map.
Expand Down Expand Up @@ -179,7 +179,6 @@ func TestGetServicePropertiesFromEnvironment(t *testing.T) {
assert.NotNil(t, props)
assert.Equal(t, "https://my=host.com/my=service/api", props[PROPNAME_SVC_URL])
assert.Equal(t, "===my=iam=apikey===", props[PROPNAME_APIKEY])


props, err = getServiceProperties("not_a_service")
assert.Nil(t, err)
Expand Down
17 changes: 13 additions & 4 deletions v4/core/cp4d_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,20 @@ func (authenticator *CloudPakForDataAuthenticator) requestToken() (*cp4dTokenSer
}

if resp.StatusCode < 200 || resp.StatusCode >= 300 {
if resp != nil {
buff := new(bytes.Buffer)
_, _ = buff.ReadFrom(resp.Body)
return nil, fmt.Errorf(buff.String())
buff := new(bytes.Buffer)
_, _ = buff.ReadFrom(resp.Body)
// Start to populate the DetailedResponse.
detailedResponse := &DetailedResponse{
StatusCode: resp.StatusCode,
Headers: resp.Header,
RawResult: buff.Bytes(),
}
// Return a AuthenticationError in place of a generic "error"
authError := &AuthenticationError{
Response: detailedResponse,
err: fmt.Errorf(buff.String()),
}
return nil, authError
}

tokenResponse := &cp4dTokenServerResponse{}
Expand Down
73 changes: 70 additions & 3 deletions v4/core/cp4d_authenticator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,37 @@ func TestCp4dConfigErrors(t *testing.T) {
assert.NotNil(t, err)
}

func TestCp4dAuthenticateFail(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusUnauthorized)
_, _ = w.Write([]byte("Sorry you are not authorized"))
}))
defer server.Close()

authenticator, err := NewCloudPakForDataAuthenticator(server.URL, "mookie", "betts", false, nil)
assert.Nil(t, err)
assert.NotNil(t, authenticator)
assert.Equal(t, authenticator.AuthenticationType(), AUTHTYPE_CP4D)

// Create a new Request object.
builder, err := NewRequestBuilder("GET").ConstructHTTPURL("https://localhost/placeholder/url", nil, nil)
assert.Nil(t, err)

request, err := builder.Build()
assert.Nil(t, err)
assert.NotNil(t, request)

err = authenticator.Authenticate(request)
// Validate the resulting error is a valid
assert.NotNil(t, err)
authErr, ok := err.(*AuthenticationError)
assert.True(t, ok)
assert.NotNil(t, authErr)
assert.EqualValues(t, authErr, err)
// The casted error should match the original error message
assert.Equal(t, err.Error(), authErr.Error())
}

func TestCp4dGetTokenSuccess(t *testing.T) {
firstCall := true
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -336,6 +367,9 @@ func TestCp4dBackgroundTokenRefreshFailure(t *testing.T) {
_, err = authenticator.getToken()
assert.NotNil(t, err)
assert.Equal(t, "Error while trying to parse access token!", err.Error())
// We don't expect a AuthenticateError to be returned, so casting should fail
_, ok := err.(*AuthenticationError)
assert.False(t, ok)
}

func TestCp4dDisableSSL(t *testing.T) {
Expand Down Expand Up @@ -435,8 +469,23 @@ func TestGetTokenFailure(t *testing.T) {
Password: "snow",
}

var expectedResponse = []byte("Sorry you are forbidden")

_, err := authenticator.getToken()
assert.NotNil(t, err)
assert.Equal(t, "Sorry you are forbidden", err.Error())
// We expect an AuthenticationError to be returned, so cast the returned error
authError, ok := err.(*AuthenticationError)
assert.True(t, ok)
assert.NotNil(t, authError)
assert.NotNil(t, authError.Error())
assert.NotNil(t, authError.Response)
rawResult := authError.Response.GetRawResult()
assert.NotNil(t, rawResult)
assert.Equal(t, expectedResponse, rawResult)
statusCode := authError.Response.GetStatusCode()
assert.Equal(t, "Sorry you are forbidden", authError.Error())
assert.Equal(t, http.StatusForbidden, statusCode)
}

func TestNewCloudPakForDataAuthenticatorFromMap(t *testing.T) {
Expand Down Expand Up @@ -486,7 +535,7 @@ func TestCp4dGetTokenTimeoutError(t *testing.T) {
fmt.Fprintf(w, `{
"username":"hello",
"role":"user",
"permissions":[
"permissions":[
"administrator",
"deployment_admin"
],
Expand Down Expand Up @@ -542,8 +591,12 @@ func TestCp4dGetTokenTimeoutError(t *testing.T) {
// Set the client timeout to something very low
authenticator.Client.Timeout = time.Second * 2
token, err = authenticator.getToken()
assert.NotNil(t, err)
assert.Equal(t, "", token)
assert.NotNil(t, err)
assert.NotNil(t, err.Error())
// We don't expect a AuthenticateError to be returned, so casting should fail
_, ok := err.(*AuthenticationError)
assert.False(t, ok)
}

func TestCp4dGetTokenServerError(t *testing.T) {
Expand All @@ -554,7 +607,7 @@ func TestCp4dGetTokenServerError(t *testing.T) {
fmt.Fprintf(w, `{
"username":"hello",
"role":"user",
"permissions":[
"permissions":[
"administrator",
"deployment_admin"
],
Expand Down Expand Up @@ -585,9 +638,23 @@ func TestCp4dGetTokenServerError(t *testing.T) {
token)
assert.NotNil(t, authenticator.tokenData)

var expectedResponse = []byte("Gateway Timeout")

// Force expiration and verify that we got a server error
authenticator.tokenData.Expiration = GetCurrentTime() - 3600
token, err = authenticator.getToken()
assert.NotNil(t, err)
// We expect an AuthenticationError to be returned, so cast the returned error
authError, ok := err.(*AuthenticationError)
assert.True(t, ok)
assert.NotNil(t, authError)
assert.NotNil(t, authError.Response)
assert.NotNil(t, authError.Error())
rawResult := authError.Response.GetRawResult()
statusCode := authError.Response.GetStatusCode()
assert.Equal(t, "Gateway Timeout", authError.Error())
assert.Equal(t, expectedResponse, rawResult)
assert.NotNil(t, rawResult)
assert.Equal(t, http.StatusGatewayTimeout, statusCode)
assert.Equal(t, "", token)
}
Loading

0 comments on commit 3485263

Please sign in to comment.