diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index 19199682d06d..e77b8073e151 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/sdk/azcore/CHANGELOG.md @@ -14,6 +14,7 @@ * Include error text instead of error type in traces when the transport returns an error. * Fixed an issue that could cause an HTTP/2 request to hang when the TCP connection becomes unresponsive. * Block key and SAS authentication for non TLS protected endpoints. +* Passing a `nil` credential value will no longer cause a panic. Instead, the authentication is skipped. ### Other Changes diff --git a/sdk/azcore/runtime/policy_bearer_token.go b/sdk/azcore/runtime/policy_bearer_token.go index 101c637626a0..1ead28a24ab3 100644 --- a/sdk/azcore/runtime/policy_bearer_token.go +++ b/sdk/azcore/runtime/policy_bearer_token.go @@ -72,9 +72,17 @@ func (b *BearerTokenPolicy) authenticateAndAuthorize(req *policy.Request) func(p // Do authorizes a request with a bearer token func (b *BearerTokenPolicy) Do(req *policy.Request) (*http.Response, error) { + // skip adding the authorization header if no TokenCredential was provided. + // this prevents a panic that might be hard to diagnose and allows testing + // against http endpoints that don't require authentication. + if b.cred == nil { + return req.Next() + } + if err := checkHTTPSForAuth(req); err != nil { return nil, err } + var err error if b.authzHandler.OnRequest != nil { err = b.authzHandler.OnRequest(req, b.authenticateAndAuthorize(req)) diff --git a/sdk/azcore/runtime/policy_bearer_token_test.go b/sdk/azcore/runtime/policy_bearer_token_test.go index 3546c17042ef..72445305f988 100644 --- a/sdk/azcore/runtime/policy_bearer_token_test.go +++ b/sdk/azcore/runtime/policy_bearer_token_test.go @@ -246,3 +246,15 @@ func TestCheckHTTPSForAuth(t *testing.T) { require.NoError(t, err) require.NoError(t, checkHTTPSForAuth(req)) } + +func TestBearerTokenPolicy_NilCredential(t *testing.T) { + policy := NewBearerTokenPolicy(nil, nil, nil) + pl := exported.NewPipeline(shared.TransportFunc(func(req *http.Request) (*http.Response, error) { + require.Zero(t, req.Header.Get(shared.HeaderAuthorization)) + return &http.Response{}, nil + }), policy) + req, err := NewRequest(context.Background(), "GET", "http://contoso.com") + require.NoError(t, err) + _, err = pl.Do(req) + require.NoError(t, err) +} diff --git a/sdk/azcore/runtime/policy_key_credential.go b/sdk/azcore/runtime/policy_key_credential.go index 02601c6cbd50..6f577fa7a9e4 100644 --- a/sdk/azcore/runtime/policy_key_credential.go +++ b/sdk/azcore/runtime/policy_key_credential.go @@ -40,13 +40,18 @@ func NewKeyCredentialPolicy(cred *exported.KeyCredential, header string, options // Do implementes the Do method on the [policy.Polilcy] interface. func (k *KeyCredentialPolicy) Do(req *policy.Request) (*http.Response, error) { - if err := checkHTTPSForAuth(req); err != nil { - return nil, err + // skip adding the authorization header if no KeyCredential was provided. + // this prevents a panic that might be hard to diagnose and allows testing + // against http endpoints that don't require authentication. + if k.cred != nil { + if err := checkHTTPSForAuth(req); err != nil { + return nil, err + } + val := exported.KeyCredentialGet(k.cred) + if k.prefix != "" { + val = k.prefix + val + } + req.Raw().Header.Add(k.header, val) } - val := exported.KeyCredentialGet(k.cred) - if k.prefix != "" { - val = k.prefix + val - } - req.Raw().Header.Add(k.header, val) return req.Next() } diff --git a/sdk/azcore/runtime/policy_key_credential_test.go b/sdk/azcore/runtime/policy_key_credential_test.go index 5937b91245f0..231e90543141 100644 --- a/sdk/azcore/runtime/policy_key_credential_test.go +++ b/sdk/azcore/runtime/policy_key_credential_test.go @@ -65,3 +65,20 @@ func TestKeyCredentialPolicy_RequiresHTTPS(t *testing.T) { _, err = pl.Do(req) require.Error(t, err) } + +func TestKeyCredentialPolicy_NilCredential(t *testing.T) { + const headerName = "fake-auth" + policy := NewKeyCredentialPolicy(nil, headerName, nil) + require.NotNil(t, policy) + + pl := exported.NewPipeline(shared.TransportFunc(func(req *http.Request) (*http.Response, error) { + require.Zero(t, req.Header.Get(headerName)) + return &http.Response{}, nil + }), policy) + + req, err := NewRequest(context.Background(), http.MethodGet, "http://contoso.com") + require.NoError(t, err) + + _, err = pl.Do(req) + require.NoError(t, err) +} diff --git a/sdk/azcore/runtime/policy_sas_credential.go b/sdk/azcore/runtime/policy_sas_credential.go index 5b88f0ddf60d..ebe2b7772ba7 100644 --- a/sdk/azcore/runtime/policy_sas_credential.go +++ b/sdk/azcore/runtime/policy_sas_credential.go @@ -34,9 +34,14 @@ func NewSASCredentialPolicy(cred *exported.SASCredential, header string, options // Do implementes the Do method on the [policy.Polilcy] interface. func (k *SASCredentialPolicy) Do(req *policy.Request) (*http.Response, error) { - if err := checkHTTPSForAuth(req); err != nil { - return nil, err + // skip adding the authorization header if no SASCredential was provided. + // this prevents a panic that might be hard to diagnose and allows testing + // against http endpoints that don't require authentication. + if k.cred != nil { + if err := checkHTTPSForAuth(req); err != nil { + return nil, err + } + req.Raw().Header.Add(k.header, exported.SASCredentialGet(k.cred)) } - req.Raw().Header.Add(k.header, exported.SASCredentialGet(k.cred)) return req.Next() } diff --git a/sdk/azcore/runtime/policy_sas_credential_test.go b/sdk/azcore/runtime/policy_sas_credential_test.go index 5de7beece66b..e0aaaa266333 100644 --- a/sdk/azcore/runtime/policy_sas_credential_test.go +++ b/sdk/azcore/runtime/policy_sas_credential_test.go @@ -49,3 +49,20 @@ func TestSASCredentialPolicy_RequiresHTTPS(t *testing.T) { _, err = pl.Do(req) require.Error(t, err) } + +func TestSASCredentialPolicy_NilCredential(t *testing.T) { + const headerName = "fake-auth" + policy := NewSASCredentialPolicy(nil, headerName, nil) + require.NotNil(t, policy) + + pl := exported.NewPipeline(shared.TransportFunc(func(req *http.Request) (*http.Response, error) { + require.Zero(t, req.Header.Get(headerName)) + return &http.Response{}, nil + }), policy) + + req, err := NewRequest(context.Background(), http.MethodGet, "http://contoso.com") + require.NoError(t, err) + + _, err = pl.Do(req) + require.NoError(t, err) +}