Skip to content

Commit

Permalink
azcore: Remove CAE support (Azure#20497)
Browse files Browse the repository at this point in the history
  • Loading branch information
chlowell authored Mar 29, 2023
1 parent 1cd56ad commit 9020972
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 54 deletions.
3 changes: 3 additions & 0 deletions sdk/azcore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
* Added `ShouldRetry` to `policy.RetryOptions` for finer-grained control over when to retry.

### Breaking Changes
> These changes affect only code written against a beta version such as v1.5.0-beta.1
* Removed `Claims` and `TenantID` fields from `policy.TokenRequestOptions`
* Removed CAE support for ARM clients

### Bugs Fixed
* Added non-conformant LRO terminal states `Cancelled` and `Completed`.
Expand Down
48 changes: 1 addition & 47 deletions sdk/azcore/arm/runtime/policy_bearer_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package runtime

import (
"context"
"encoding/base64"
"fmt"
"net/http"
"strings"
Expand Down Expand Up @@ -64,28 +63,11 @@ func NewBearerTokenPolicy(cred azcore.TokenCredential, opts *armpolicy.BearerTok
p.scopes = make([]string, len(opts.Scopes))
copy(p.scopes, opts.Scopes)
p.btp = azruntime.NewBearerTokenPolicy(cred, opts.Scopes, &azpolicy.BearerTokenOptions{
AuthorizationHandler: azpolicy.AuthorizationHandler{
OnChallenge: p.onChallenge,
OnRequest: p.onRequest,
},
AuthorizationHandler: azpolicy.AuthorizationHandler{OnRequest: p.onRequest},
})
return p
}

func (b *BearerTokenPolicy) onChallenge(req *azpolicy.Request, res *http.Response, authNZ func(azpolicy.TokenRequestOptions) error) error {
challenge := res.Header.Get(shared.HeaderWWWAuthenticate)
claims, err := parseChallenge(challenge)
if err != nil {
// the challenge contains claims we can't parse
return err
} else if claims != "" {
// request a new token having the specified claims, send the request again
return authNZ(azpolicy.TokenRequestOptions{Claims: claims, Scopes: b.scopes})
}
// auth challenge didn't include claims, so this is a simple authorization failure
return azruntime.NewResponseError(res)
}

// onRequest authorizes requests with one or more bearer tokens
func (b *BearerTokenPolicy) onRequest(req *azpolicy.Request, authNZ func(azpolicy.TokenRequestOptions) error) error {
// authorize the request with a token for the primary tenant
Expand Down Expand Up @@ -115,31 +97,3 @@ func (b *BearerTokenPolicy) onRequest(req *azpolicy.Request, authNZ func(azpolic
func (b *BearerTokenPolicy) Do(req *azpolicy.Request) (*http.Response, error) {
return b.btp.Do(req)
}

// parseChallenge parses claims from an authentication challenge issued by ARM so a client can request a token
// that will satisfy conditional access policies. It returns a non-nil error when the given value contains
// claims it can't parse. If the value contains no claims, it returns an empty string and a nil error.
func parseChallenge(wwwAuthenticate string) (string, error) {
claims := ""
var err error
for _, param := range strings.Split(wwwAuthenticate, ",") {
if _, after, found := strings.Cut(param, "claims="); found {
if claims != "" {
// The header contains multiple challenges, at least two of which specify claims. The specs allow this
// but it's unclear what a client should do in this case and there's as yet no concrete example of it.
err = fmt.Errorf("found multiple claims challenges in %q", wwwAuthenticate)
break
}
// trim stuff that would get an error from RawURLEncoding; claims may or may not be padded
claims = strings.Trim(after, `\"=`)
// we don't return this error because it's something unhelpful like "illegal base64 data at input byte 42"
if b, decErr := base64.RawURLEncoding.DecodeString(claims); decErr == nil {
claims = string(b)
} else {
err = fmt.Errorf("failed to parse claims from %q", wwwAuthenticate)
break
}
}
}
return claims, err
}
8 changes: 5 additions & 3 deletions sdk/azcore/arm/runtime/policy_bearer_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ func TestAuxiliaryTenants(t *testing.T) {
}

func TestBearerTokenPolicyChallengeParsing(t *testing.T) {
t.Skip("unskip this test after adding back CAE support")
for _, test := range []struct {
challenge, desc, expectedClaims string
err error
Expand Down Expand Up @@ -261,9 +262,10 @@ func TestBearerTokenPolicyChallengeParsing(t *testing.T) {
cred := mockCredential{
getTokenImpl: func(ctx context.Context, actual azpolicy.TokenRequestOptions) (azcore.AccessToken, error) {
calls += 1
if calls == 2 && test.expectedClaims != "" {
require.Equal(t, test.expectedClaims, actual.Claims)
}
// TODO: uncomment after restoring TokenRequestOptions.Claims
// if calls == 2 && test.expectedClaims != "" {
// require.Equal(t, test.expectedClaims, actual.Claims)
// }
return azcore.AccessToken{Token: "...", ExpiresOn: time.Now().Add(time.Hour).UTC()}, nil
},
}
Expand Down
4 changes: 0 additions & 4 deletions sdk/azcore/internal/exported/exported.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ type AccessToken struct {
// TokenRequestOptions contain specific parameter that may be used by credentials types when attempting to get a token.
// Exported as policy.TokenRequestOptions.
type TokenRequestOptions struct {
// Claims are any additional claims required for the token to satisfy a conditional access policy, such as a
// service may return in a claims challenge following an authorization failure. If a service returned the
// claims value base64 encoded, it must be decoded before setting this field.
Claims string
// Scopes contains the list of permission scopes required for the token.
Scopes []string

Expand Down

0 comments on commit 9020972

Please sign in to comment.