Skip to content

Commit

Permalink
feat: add detailed error response to iam/cp4d authenticators
Browse files Browse the repository at this point in the history
  • Loading branch information
jorge-ibm committed Aug 11, 2020
1 parent 58a7e05 commit 8147adc
Show file tree
Hide file tree
Showing 10 changed files with 324 additions and 121 deletions.
12 changes: 11 additions & 1 deletion v4/core/authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ import (
// Authenticator describes the set of methods implemented by each authenticator.
type Authenticator interface {
AuthenticationType() string
Authenticate(*http.Request) error
Authenticate(*http.Request) *AuthenticationError
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()
}
8 changes: 4 additions & 4 deletions v4/core/base_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,10 @@ 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())
return
authError := service.Options.Authenticator.Authenticate(req)
if authError != nil {
err = fmt.Errorf(ERRORMSG_AUTHENTICATE_ERROR, authError.Error())
return authError.Response, err
}

// Invoke the request.
Expand Down
116 changes: 105 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,57 @@ 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.NotNil(t, statusCode)
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, statusCode)
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 +1098,58 @@ 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.NotNil(t, statusCode)
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, statusCode)
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
2 changes: 1 addition & 1 deletion v4/core/basic_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (BasicAuthenticator) AuthenticationType() string {
//
// Authorization: Basic <encoded username and password>
//
func (this *BasicAuthenticator) Authenticate(request *http.Request) error {
func (this *BasicAuthenticator) Authenticate(request *http.Request) *AuthenticationError {
request.SetBasicAuth(this.Username, this.Password)
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion v4/core/bearer_token_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (BearerTokenAuthenticator) AuthenticationType() string {
//
// Authorization: Bearer <bearer-token>
//
func (this *BearerTokenAuthenticator) Authenticate(request *http.Request) error {
func (this *BearerTokenAuthenticator) Authenticate(request *http.Request) *AuthenticationError {
request.Header.Set("Authorization", fmt.Sprintf(`Bearer %s`, this.BearerToken))
return nil
}
Expand Down
52 changes: 37 additions & 15 deletions v4/core/cp4d_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (authenticator CloudPakForDataAuthenticator) Validate() error {
//
// Authorization: Bearer <bearer-token>
//
func (authenticator *CloudPakForDataAuthenticator) Authenticate(request *http.Request) error {
func (authenticator *CloudPakForDataAuthenticator) Authenticate(request *http.Request) *AuthenticationError {
token, err := authenticator.getToken()
if err != nil {
return err
Expand All @@ -150,7 +150,7 @@ func (authenticator *CloudPakForDataAuthenticator) Authenticate(request *http.Re
// getToken: returns an access token to be used in an Authorization header.
// Whenever a new token is needed (when a token doesn't yet exist, needs to be refreshed,
// or the existing token has expired), a new access token is fetched from the token server.
func (authenticator *CloudPakForDataAuthenticator) getToken() (string, error) {
func (authenticator *CloudPakForDataAuthenticator) getToken() (string, *AuthenticationError) {
if authenticator.tokenData == nil || !authenticator.tokenData.isTokenValid() {
// synchronously request the token
err := authenticator.synchronizedRequestToken()
Expand All @@ -159,7 +159,7 @@ func (authenticator *CloudPakForDataAuthenticator) getToken() (string, error) {
}
} else if authenticator.tokenData.needsRefresh() {
// If refresh needed, kick off a go routine in the background to get a new token
ch := make(chan error)
ch := make(chan *AuthenticationError)
go func() {
ch <- authenticator.getTokenData()
}()
Expand All @@ -172,9 +172,13 @@ func (authenticator *CloudPakForDataAuthenticator) getToken() (string, error) {
}
}

authError := &AuthenticationError{}

// return an error if the access token is not valid or was not fetched
if authenticator.tokenData == nil || authenticator.tokenData.AccessToken == "" {
return "", fmt.Errorf("Error while trying to get access token")
errorMsg := fmt.Errorf("Error while trying to get access token")
authError.err = errorMsg
return "", authError
}

return authenticator.tokenData.AccessToken, nil
Expand All @@ -183,7 +187,7 @@ func (authenticator *CloudPakForDataAuthenticator) getToken() (string, error) {
// synchronizedRequestToken: synchronously checks if the current token in cache
// is valid. If token is not valid or does not exist, it will fetch a new token
// and set the tokenRefreshTime
func (authenticator *CloudPakForDataAuthenticator) synchronizedRequestToken() error {
func (authenticator *CloudPakForDataAuthenticator) synchronizedRequestToken() *AuthenticationError {
cp4dRequestTokenMutex.Lock()
defer cp4dRequestTokenMutex.Unlock()
// if cached token is still valid, then just continue to use it
Expand All @@ -197,32 +201,39 @@ func (authenticator *CloudPakForDataAuthenticator) synchronizedRequestToken() er
// getTokenData: requests a new token from the access server and
// unmarshals the token information to the tokenData cache. Returns
// an error if the token was unable to be fetched, otherwise returns nil
func (authenticator *CloudPakForDataAuthenticator) getTokenData() error {
tokenResponse, err := authenticator.requestToken()
if err != nil {
return err
func (authenticator *CloudPakForDataAuthenticator) getTokenData() *AuthenticationError {
var err error
tokenResponse, authErr := authenticator.requestToken()
if authErr != nil {
return authErr
}

newAuthenticationError := &AuthenticationError{}

authenticator.tokenData, err = newCp4dTokenData(tokenResponse)
if err != nil {
return err
newAuthenticationError.err = err
return newAuthenticationError
}

return nil
}

// requestToken: fetches a new access token from the token server.
func (authenticator *CloudPakForDataAuthenticator) requestToken() (*cp4dTokenServerResponse, error) {
func (authenticator *CloudPakForDataAuthenticator) requestToken() (*cp4dTokenServerResponse, *AuthenticationError) {
// If the user-specified URL does not end with the required path,
// then add it now.
url := authenticator.URL
if !strings.HasSuffix(url, PRE_AUTH_PATH) {
url = fmt.Sprintf("%s%s", url, PRE_AUTH_PATH)
}

authError := &AuthenticationError{}

builder, err := NewRequestBuilder(GET).ConstructHTTPURL(url, nil, nil)
if err != nil {
return nil, err
authError.err = err
return nil, authError
}

// Add user-defined headers to request.
Expand All @@ -232,7 +243,8 @@ func (authenticator *CloudPakForDataAuthenticator) requestToken() (*cp4dTokenSer

req, err := builder.Build()
if err != nil {
return nil, err
authError.err = err
return nil, authError
}

req.SetBasicAuth(authenticator.Username, authenticator.Password)
Expand All @@ -254,14 +266,24 @@ func (authenticator *CloudPakForDataAuthenticator) requestToken() (*cp4dTokenSer

resp, err := authenticator.Client.Do(req)
if err != nil {
return nil, err
authError.err = err
return nil, authError
}

// Start to populate the DetailedResponse.
detailedResponse := &DetailedResponse{
StatusCode: resp.StatusCode,
Headers: resp.Header,
}

if resp.StatusCode < 200 || resp.StatusCode >= 300 {
if resp != nil {
buff := new(bytes.Buffer)
_, _ = buff.ReadFrom(resp.Body)
return nil, fmt.Errorf(buff.String())
authError.err = fmt.Errorf(buff.String())
detailedResponse.RawResult = buff.Bytes()
authError.Response = detailedResponse
return nil, authError
}
}

Expand Down
Loading

0 comments on commit 8147adc

Please sign in to comment.