Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add detailed error response to iam/cp4d authenticators #66

Merged
merged 3 commits into from
Aug 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After initializing "err" on this line, you'd need to do a type assertion to cast "authErr" to be an AuthenticationError instance (we'll use castErr in this example), then set detailedResponse = castErr.Response.

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this new test to test the error returned from the Authenticate method

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