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

Align authentication errors with azcore.ResponseError #16777

Merged
merged 8 commits into from
Jan 11, 2022
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
11 changes: 5 additions & 6 deletions sdk/azidentity/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
# Release History

## 0.12.1 (Unreleased)

### Features Added
## 0.13.0 (2022-01-11)

### Breaking Changes

* Replaced `AuthenticationFailedError.RawResponse()` with a field having the same name
* Unexported `CredentialUnavailableError`
* Instances of `ChainedTokenCredential` will now skip looping through the list of source credentials and re-use the first successful credential on subsequent calls to `GetToken`.
* If `ChainedTokenCredentialOptions.RetrySources` is true, `ChainedTokenCredential` will continue to try all of the originally provided credentials each time the `GetToken` method is called.
* `ChainedTokenCredential.successfulCredential` will contain a reference to the last successful credential.
* `DefaultAzureCredenial` will also re-use the first successful credential on subsequent calls to `GetToken`.
* `DefaultAzureCredential.chain.successfulCredential` will also contain a reference to the last successful credential.

### Bugs Fixed

### Other Changes
* `ManagedIdentityCredential` no longer probes IMDS before requesting a token
from it. Also, an error response from IMDS no longer disables a credential
instance. Following an error, a credential instance will continue to send
requests to IMDS as necessary.
* Adopted MSAL for user and service principal authentication
* Updated `azcore` requirement to 0.21.0

## 0.12.0 (2021-11-02)
### Breaking Changes
Expand Down
9 changes: 4 additions & 5 deletions sdk/azidentity/chained_token_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,21 +54,20 @@ func NewChainedTokenCredential(sources []azcore.TokenCredential, options *Chaine
// ctx: Context controlling the request lifetime.
// opts: Options for the token request, in particular the desired scope of the access token.
func (c *ChainedTokenCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (token *azcore.AccessToken, err error) {
var errList []CredentialUnavailableError

if c.successfulCredential != nil && !c.retrySources {
return c.successfulCredential.GetToken(ctx, opts)
}
var errList []credentialUnavailableError
for _, cred := range c.sources {
token, err = cred.GetToken(ctx, opts)
var credErr CredentialUnavailableError
var credErr credentialUnavailableError
if errors.As(err, &credErr) {
errList = append(errList, credErr)
} else if err != nil {
var authFailed AuthenticationFailedError
if errors.As(err, &authFailed) {
err = fmt.Errorf("Authentication failed:\n%s\n%s"+createChainedErrorMessage(errList), err)
authErr := newAuthenticationFailedError(err, authFailed.RawResponse())
authErr := newAuthenticationFailedError(err, authFailed.RawResponse)
return nil, authErr
}
return nil, err
Expand All @@ -86,7 +85,7 @@ func (c *ChainedTokenCredential) GetToken(ctx context.Context, opts policy.Token
return nil, credErr
}

func createChainedErrorMessage(errList []CredentialUnavailableError) string {
func createChainedErrorMessage(errList []credentialUnavailableError) string {
msg := ""
for _, err := range errList {
msg += err.Error()
Expand Down
4 changes: 2 additions & 2 deletions sdk/azidentity/client_certificate_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func TestClientCertificateCredential_InvalidCertLive(t *testing.T) {
if !errors.As(err, &e) {
t.Fatal("expected AuthenticationFailedError")
}
if e.RawResponse() == nil {
t.Fatal("expected RawResponse() to return a non-nil *http.Response")
if e.RawResponse == nil {
t.Fatal("expected a non-nil RawResponse")
}
}
4 changes: 2 additions & 2 deletions sdk/azidentity/client_secret_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestClientSecretCredential_InvalidSecretLive(t *testing.T) {
if !errors.As(err, &e) {
t.Fatal("expected AuthenticationFailedError")
}
if e.RawResponse() == nil {
t.Fatal("expected RawResponse() to return a non-nil *http.Response")
if e.RawResponse == nil {
t.Fatal("expected a non-nil RawResponse")
}
}
8 changes: 4 additions & 4 deletions sdk/azidentity/environment_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ func TestEnvironmentCredential_InvalidClientSecretLive(t *testing.T) {
if !errors.As(err, &e) {
t.Fatal("expected AuthenticationFailedError")
}
if e.RawResponse() == nil {
t.Fatal("expected RawResponse() to return a non-nil *http.Response")
if e.RawResponse == nil {
t.Fatal("expected a non-nil RawResponse")
}
}

Expand Down Expand Up @@ -254,7 +254,7 @@ func TestEnvironmentCredential_InvalidPasswordLive(t *testing.T) {
if !errors.As(err, &e) {
t.Fatal("expected AuthenticationFailedError")
}
if e.RawResponse() == nil {
t.Fatal("expected RawResponse() to return a non-nil *http.Response")
if e.RawResponse == nil {
t.Fatal("expected a non-nil RawResponse")
}
}
78 changes: 42 additions & 36 deletions sdk/azidentity/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,66 +4,78 @@
package azidentity

import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"

"github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo"
msal "github.com/AzureAD/microsoft-authentication-library-for-go/apps/errors"
)

// AuthenticationFailedError indicates an authentication request has failed.
type AuthenticationFailedError interface {
errorinfo.NonRetriable
RawResponse() *http.Response
authenticationFailed()
}
type AuthenticationFailedError struct {
err error

type authenticationFailedError struct {
error
resp *http.Response
// RawResponse is the HTTP response motivating the error, if available.
RawResponse *http.Response
}

func newAuthenticationFailedError(err error, resp *http.Response) AuthenticationFailedError {
if resp == nil {
var e msal.CallErr
if errors.As(err, &e) {
return authenticationFailedError{err, e.Resp}
return AuthenticationFailedError{err: e, RawResponse: e.Resp}
}
}
return authenticationFailedError{err, resp}
return AuthenticationFailedError{err: err, RawResponse: resp}
}

// NonRetriable indicates that this error should not be retried.
func (authenticationFailedError) NonRetriable() {
// marker method
// Error implements the error interface for type ResponseError.
// Note that the message contents are not contractual and can change over time.
func (e AuthenticationFailedError) Error() string {
if e.RawResponse == nil {
return e.err.Error()
}
msg := &bytes.Buffer{}
fmt.Fprintf(msg, "%s %s://%s%s\n", e.RawResponse.Request.Method, e.RawResponse.Request.URL.Scheme, e.RawResponse.Request.URL.Host, e.RawResponse.Request.URL.Path)
fmt.Fprintln(msg, "--------------------------------------------------------------------------------")
fmt.Fprintf(msg, "RESPONSE %s\n", e.RawResponse.Status)
fmt.Fprintln(msg, "--------------------------------------------------------------------------------")
body, err := io.ReadAll(e.RawResponse.Body)
e.RawResponse.Body.Close()
if err != nil {
fmt.Fprintf(msg, "Error reading response body: %v", err)
} else if len(body) > 0 {
e.RawResponse.Body = io.NopCloser(bytes.NewReader(body))
if err := json.Indent(msg, body, "", " "); err != nil {
// failed to pretty-print so just dump it verbatim
fmt.Fprint(msg, string(body))
}
} else {
fmt.Fprint(msg, "Response contained no body")
}
fmt.Fprintln(msg, "\n--------------------------------------------------------------------------------")
return msg.String()
}

// AuthenticationFailed indicates that an authentication attempt failed
func (authenticationFailedError) authenticationFailed() {
// NonRetriable indicates that this error should not be retried.
func (AuthenticationFailedError) NonRetriable() {
// marker method
}

// RawResponse returns the HTTP response motivating the error, if available.
func (e authenticationFailedError) RawResponse() *http.Response {
return e.resp
}

var _ AuthenticationFailedError = (*authenticationFailedError)(nil)
var _ errorinfo.NonRetriable = (*authenticationFailedError)(nil)

// CredentialUnavailableError indicates a credential can't attempt authenticate
// because it lacks required data or state.
type CredentialUnavailableError interface {
errorinfo.NonRetriable
credentialUnavailable()
}
var _ errorinfo.NonRetriable = (*AuthenticationFailedError)(nil)

// credentialUnavailableError indicates a credential can't attempt
// authentication because it lacks required data or state.
type credentialUnavailableError struct {
credType string
message string
}

func newCredentialUnavailableError(credType, message string) CredentialUnavailableError {
func newCredentialUnavailableError(credType, message string) credentialUnavailableError {
return credentialUnavailableError{credType: credType, message: message}
}

Expand All @@ -76,10 +88,4 @@ func (e credentialUnavailableError) NonRetriable() {
// marker method
}

// CredentialUnavailable indicates that the credential didn't attempt to authenticate
func (e credentialUnavailableError) credentialUnavailable() {
// marker method
}

var _ CredentialUnavailableError = (*credentialUnavailableError)(nil)
var _ errorinfo.NonRetriable = (*credentialUnavailableError)(nil)
6 changes: 2 additions & 4 deletions sdk/azidentity/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ module github.com/Azure/azure-sdk-for-go/sdk/azidentity

go 1.16

replace github.com/Azure/azure-sdk-for-go/sdk/azcore => ../azcore

require (
github.com/Azure/azure-sdk-for-go/sdk/azcore v0.20.0
github.com/Azure/azure-sdk-for-go/sdk/internal v0.8.2
github.com/Azure/azure-sdk-for-go/sdk/azcore v0.21.0
github.com/Azure/azure-sdk-for-go/sdk/internal v0.8.3
github.com/AzureAD/microsoft-authentication-library-for-go v0.4.0
github.com/davecgh/go-spew v1.1.1 // indirect
golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897
Expand Down
7 changes: 4 additions & 3 deletions sdk/azidentity/go.sum
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
github.com/Azure/azure-sdk-for-go/sdk/internal v0.8.1/go.mod h1:KLF4gFr6DcKFZwSuH8w8yEK6DpFl3LP5rhdvAb7Yz5I=
github.com/Azure/azure-sdk-for-go/sdk/internal v0.8.2 h1:rImM7Yjz9yUgpdxp3A4cZLm1JZuo4XbtIp2LrUZnwYw=
github.com/Azure/azure-sdk-for-go/sdk/internal v0.8.2/go.mod h1:KLF4gFr6DcKFZwSuH8w8yEK6DpFl3LP5rhdvAb7Yz5I=
github.com/Azure/azure-sdk-for-go/sdk/azcore v0.21.0 h1:8wVJL0HUP5yDFXvotdewORTw7Yu88JbreWN/mobSvsQ=
github.com/Azure/azure-sdk-for-go/sdk/azcore v0.21.0/go.mod h1:fBF9PQNqB8scdgpZ3ufzaLntG0AG7C1WjPMsiFOmfHM=
github.com/Azure/azure-sdk-for-go/sdk/internal v0.8.3 h1:E+m3SkZCN0Bf5q7YdTs5lSm2CYY3CK4spn5OmUIiQtk=
github.com/Azure/azure-sdk-for-go/sdk/internal v0.8.3/go.mod h1:KLF4gFr6DcKFZwSuH8w8yEK6DpFl3LP5rhdvAb7Yz5I=
github.com/AzureAD/microsoft-authentication-library-for-go v0.4.0 h1:WVsrXCnHlDDX8ls+tootqRE87/hL9S/g4ewig9RsD/c=
github.com/AzureAD/microsoft-authentication-library-for-go v0.4.0/go.mod h1:Vt9sXTKwMyGcOxSmLDMnGPgqsUg7m8pe215qMLrDXw4=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down
4 changes: 2 additions & 2 deletions sdk/azidentity/live_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func TestMain(m *testing.M) {
// TODO: reset matcher
case recording.RecordingMode:
// remove default sanitizers such as the OAuth response sanitizer
err := recording.ResetSanitizers(nil)
err := recording.ResetProxy(nil)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -151,7 +151,7 @@ func TestMain(m *testing.M) {
}
}
defer func() {
err := recording.ResetSanitizers(nil)
err := recording.ResetProxy(nil)
if err != nil {
panic(err)
}
Expand Down
10 changes: 5 additions & 5 deletions sdk/azidentity/managed_identity_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,12 +395,12 @@ func TestManagedIdentityCredential_GetTokenIMDS400(t *testing.T) {
if err != nil {
t.Fatal(err)
}
// cred should return CredentialUnavailableError when IMDS responds 400 to a token request
var expected CredentialUnavailableError
// cred should return credentialUnavailableError when IMDS responds 400 to a token request
var expected credentialUnavailableError
for i := 0; i < 3; i++ {
_, err = cred.GetToken(context.Background(), policy.TokenRequestOptions{Scopes: []string{liveTestScope}})
if !errors.As(err, &expected) {
t.Fatalf(`expected CredentialUnavailableError, got %T: "%s"`, err, err.Error())
t.Fatalf(`expected credentialUnavailableError, got %T: "%s"`, err, err.Error())
}
}
}
Expand Down Expand Up @@ -731,9 +731,9 @@ func TestManagedIdentityCredential_IMDSTimeoutExceeded(t *testing.T) {
}
cred.client.imdsTimeout = time.Nanosecond
tk, err := cred.GetToken(context.Background(), policy.TokenRequestOptions{Scopes: []string{liveTestScope}})
var expected CredentialUnavailableError
var expected credentialUnavailableError
if !errors.As(err, &expected) {
t.Fatalf(`expected CredentialUnavailableError, got %T: "%v"`, err, err)
t.Fatalf(`expected credentialUnavailableError, got %T: "%v"`, err, err)
}
if tk != nil {
t.Fatal("GetToken returned a token and an error")
Expand Down
4 changes: 2 additions & 2 deletions sdk/azidentity/username_password_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestUsernamePasswordCredential_InvalidPasswordLive(t *testing.T) {
if !errors.As(err, &e) {
t.Fatal("expected AuthenticationFailedError")
}
if e.RawResponse() == nil {
t.Fatal("expected RawResponse() to return a non-nil *http.Response")
if e.RawResponse == nil {
t.Fatal("expected a non-nil RawResponse")
}
}
2 changes: 1 addition & 1 deletion sdk/azidentity/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ const (
component = "azidentity"

// Version is the semantic version (see http://semver.org) of this module.
version = "v0.12.1"
version = "v0.13.0"
)