Skip to content

Commit

Permalink
Remove azcore multitenant auth API (#20572)
Browse files Browse the repository at this point in the history
  • Loading branch information
chlowell authored Apr 7, 2023
1 parent 0a42300 commit a554ef0
Show file tree
Hide file tree
Showing 17 changed files with 22 additions and 58 deletions.
5 changes: 3 additions & 2 deletions sdk/azcore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@

### Breaking Changes
> These changes affect only code written against a beta version such as v1.5.0-beta.1
* Removed `TokenRequestOptions.Claims`
* Removed CAE support for ARM clients
> These features will return in v1.6.0-beta.1.
* Removed `TokenRequestOptions.Claims` and `.TenantID`
* Removed ARM client support for CAE and cross-tenant auth.

### Bugs Fixed
* Added non-conformant LRO terminal states `Cancelled` and `Completed`.
Expand Down
6 changes: 0 additions & 6 deletions sdk/azcore/arm/policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ import (

// BearerTokenOptions configures the bearer token policy's behavior.
type BearerTokenOptions struct {
// AuxiliaryTenants are additional tenant IDs for authenticating cross-tenant requests.
// The policy will add a token from each of these tenants to every request. The
// authenticating user or service principal must be a guest in these tenants, and the
// policy's credential must support multitenant authentication.
AuxiliaryTenants []string

// Scopes contains the list of permission scopes required for the token.
Scopes []string
}
Expand Down
3 changes: 1 addition & 2 deletions sdk/azcore/arm/runtime/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ func NewPipeline(module, version string, cred azcore.TokenCredential, plOpts azr
return azruntime.Pipeline{}, err
}
authPolicy := NewBearerTokenPolicy(cred, &armpolicy.BearerTokenOptions{
AuxiliaryTenants: options.AuxiliaryTenants,
Scopes: []string{conf.Audience + "/.default"},
Scopes: []string{conf.Audience + "/.default"},
})
perRetry := make([]azpolicy.Policy, len(plOpts.PerRetry), len(plOpts.PerRetry)+1)
copy(perRetry, plOpts.PerRetry)
Expand Down
18 changes: 0 additions & 18 deletions sdk/azcore/arm/runtime/policy_bearer_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"net/http"
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy"
Expand All @@ -27,19 +26,6 @@ type acquiringResourceState struct {
tenant string
}

// acquire acquires or updates the resource; only one
// thread/goroutine at a time ever calls this function
func acquire(state acquiringResourceState) (newResource azcore.AccessToken, newExpiration time.Time, err error) {
tk, err := state.p.cred.GetToken(state.ctx, azpolicy.TokenRequestOptions{
Scopes: state.p.scopes,
TenantID: state.tenant,
})
if err != nil {
return azcore.AccessToken{}, time.Time{}, err
}
return tk, tk.ExpiresOn, nil
}

// BearerTokenPolicy authorizes requests with bearer tokens acquired from a TokenCredential.
type BearerTokenPolicy struct {
auxResources map[string]*temporal.Resource[azcore.AccessToken, acquiringResourceState]
Expand All @@ -56,10 +42,6 @@ func NewBearerTokenPolicy(cred azcore.TokenCredential, opts *armpolicy.BearerTok
opts = &armpolicy.BearerTokenOptions{}
}
p := &BearerTokenPolicy{cred: cred}
p.auxResources = make(map[string]*temporal.Resource[azcore.AccessToken, acquiringResourceState], len(opts.AuxiliaryTenants))
for _, t := range opts.AuxiliaryTenants {
p.auxResources[t] = temporal.NewResource(acquire)
}
p.scopes = make([]string, len(opts.Scopes))
copy(p.scopes, opts.Scopes)
p.btp = azruntime.NewBearerTokenPolicy(cred, opts.Scopes, &azpolicy.BearerTokenOptions{
Expand Down
9 changes: 5 additions & 4 deletions sdk/azcore/arm/runtime/policy_bearer_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ func TestBearerPolicy_GetTokenFailsNoDeadlock(t *testing.T) {
}

func TestAuxiliaryTenants(t *testing.T) {
t.Skip("unskip this test after restoring cross-tenant auth support")
srv, close := mock.NewTLSServer()
defer close()
srv.SetResponse(mock.WithStatusCode(http.StatusOK))
Expand All @@ -176,13 +177,13 @@ func TestAuxiliaryTenants(t *testing.T) {
getTokenImpl: func(ctx context.Context, options azpolicy.TokenRequestOptions) (azcore.AccessToken, error) {
require.False(t, expectCache, "client should have used a cached token instead of requesting another")
tenant := primary
if options.TenantID != "" {
tenant = options.TenantID
}
// if options.TenantID != "" {
// tenant = options.TenantID
// }
return azcore.AccessToken{Token: tenant, ExpiresOn: time.Now().Add(time.Hour).UTC()}, nil
},
},
&armpolicy.BearerTokenOptions{AuxiliaryTenants: auxTenants, Scopes: []string{scope}},
&armpolicy.BearerTokenOptions{ /*AuxiliaryTenants: auxTenants,*/ Scopes: []string{scope}},
)
pipeline := newTestPipeline(&azpolicy.ClientOptions{Transport: srv, PerRetryPolicies: []azpolicy.Policy{b}})
expected := strings.Split(shared.BearerTokenPrefix+strings.Join(auxTenants, ","+shared.BearerTokenPrefix), ",")
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 @@ -53,10 +53,6 @@ type AccessToken struct {
type TokenRequestOptions struct {
// Scopes contains the list of permission scopes required for the token.
Scopes []string

// TenantID identifies the tenant from which to request the token. azidentity credentials authenticate in
// their configured default tenants when this field isn't set.
TenantID string
}

// TokenCredential represents a credential capable of providing an OAuth token.
Expand Down
3 changes: 2 additions & 1 deletion sdk/azidentity/azidentity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ func Test_NonHTTPSAuthorityHost(t *testing.T) {
}

func TestAdditionallyAllowedTenants(t *testing.T) {
t.Skip("unskip this test after restoring TokenRequestOptions.TenantID")
af := filepath.Join(t.TempDir(), t.Name()+credNameWorkloadIdentity)
if err := os.WriteFile(af, []byte("assertion"), os.ModePerm); err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -320,7 +321,7 @@ func TestAdditionallyAllowedTenants(t *testing.T) {
err: true,
},
} {
tro := policy.TokenRequestOptions{Scopes: []string{liveTestScope}, TenantID: test.tenant}
tro := policy.TokenRequestOptions{Scopes: []string{liveTestScope}}
for _, subtest := range []struct {
ctor func(azcore.ClientOptions) (azcore.TokenCredential, error)
env map[string]string
Expand Down
2 changes: 1 addition & 1 deletion sdk/azidentity/azure_cli_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (c *AzureCLICredential) GetToken(ctx context.Context, opts policy.TokenRequ
}

func (c *AzureCLICredential) requestToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
b, err := c.tokenProvider(ctx, opts.Scopes[0], opts.TenantID)
b, err := c.tokenProvider(ctx, opts.Scopes[0], "")
if err != nil {
return azcore.AccessToken{}, err
}
Expand Down
1 change: 1 addition & 0 deletions sdk/azidentity/azure_cli_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func TestAzureCLICredential_GetTokenInvalidToken(t *testing.T) {
}

func TestAzureCLICredential_TenantID(t *testing.T) {
t.Skip("unskip this test after restoring TokenRequestOptions.TenantID")
expected := "expected-tenant-id"
called := false
options := AzureCLICredentialOptions{
Expand Down
4 changes: 2 additions & 2 deletions sdk/azidentity/client_assertion_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ func (c *ClientAssertionCredential) GetToken(ctx context.Context, opts policy.To
}

func (c *ClientAssertionCredential) silentAuth(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes, confidential.WithTenantID(opts.TenantID))
ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}

func (c *ClientAssertionCredential) requestToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenByCredential(ctx, opts.Scopes, confidential.WithTenantID(opts.TenantID))
ar, err := c.client.AcquireTokenByCredential(ctx, opts.Scopes)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}

Expand Down
4 changes: 2 additions & 2 deletions sdk/azidentity/client_certificate_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ func (c *ClientCertificateCredential) GetToken(ctx context.Context, opts policy.
}

func (c *ClientCertificateCredential) silentAuth(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes, confidential.WithTenantID(opts.TenantID))
ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}

func (c *ClientCertificateCredential) requestToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenByCredential(ctx, opts.Scopes, confidential.WithTenantID(opts.TenantID))
ar, err := c.client.AcquireTokenByCredential(ctx, opts.Scopes)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}

Expand Down
4 changes: 2 additions & 2 deletions sdk/azidentity/client_secret_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ func (c *ClientSecretCredential) GetToken(ctx context.Context, opts policy.Token
}

func (c *ClientSecretCredential) silentAuth(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes, confidential.WithTenantID(opts.TenantID))
ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}

func (c *ClientSecretCredential) requestToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenByCredential(ctx, opts.Scopes, confidential.WithTenantID(opts.TenantID))
ar, err := c.client.AcquireTokenByCredential(ctx, opts.Scopes)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}

Expand Down
3 changes: 1 addition & 2 deletions sdk/azidentity/device_code_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (c *DeviceCodeCredential) GetToken(ctx context.Context, opts policy.TokenRe
}

func (c *DeviceCodeCredential) requestToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
dc, err := c.client.AcquireTokenByDeviceCode(ctx, opts.Scopes, public.WithTenantID(opts.TenantID))
dc, err := c.client.AcquireTokenByDeviceCode(ctx, opts.Scopes)
if err != nil {
return azcore.AccessToken{}, err
}
Expand All @@ -123,7 +123,6 @@ func (c *DeviceCodeCredential) requestToken(ctx context.Context, opts policy.Tok
func (c *DeviceCodeCredential) silentAuth(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes,
public.WithSilentAccount(c.account),
public.WithTenantID(opts.TenantID),
)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}
Expand Down
2 changes: 0 additions & 2 deletions sdk/azidentity/interactive_browser_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ func (c *InteractiveBrowserCredential) requestToken(ctx context.Context, opts po
ar, err := c.client.AcquireTokenInteractive(ctx, opts.Scopes,
public.WithLoginHint(c.options.LoginHint),
public.WithRedirectURI(c.options.RedirectURL),
public.WithTenantID(opts.TenantID),
)
if err == nil {
c.account = ar.Account
Expand All @@ -95,7 +94,6 @@ func (c *InteractiveBrowserCredential) requestToken(ctx context.Context, opts po
func (c *InteractiveBrowserCredential) silentAuth(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes,
public.WithSilentAccount(c.account),
public.WithTenantID(opts.TenantID),
)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/azidentity/on_behalf_of_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (o *OnBehalfOfCredential) GetToken(ctx context.Context, opts policy.TokenRe
}

func (o *OnBehalfOfCredential) requestToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := o.client.AcquireTokenOnBehalfOf(ctx, o.assertion, opts.Scopes, confidential.WithTenantID(opts.TenantID))
ar, err := o.client.AcquireTokenOnBehalfOf(ctx, o.assertion, opts.Scopes)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}

Expand Down
7 changes: 0 additions & 7 deletions sdk/azidentity/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,6 @@ func (s *syncer) GetToken(ctx context.Context, opts policy.TokenRequestOptions)
return at, errors.New(s.name + ".GetToken() requires at least one scope")
}
// we don't resolve the tenant for managed identities because they can acquire tokens only from their home tenants
if s.name != credNameManagedIdentity {
tenant, err := s.resolveTenant(opts.TenantID)
if err != nil {
return at, err
}
opts.TenantID = tenant
}
auth := false
s.cond.L.Lock()
defer s.cond.L.Unlock()
Expand Down
3 changes: 1 addition & 2 deletions sdk/azidentity/username_password_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (c *UsernamePasswordCredential) GetToken(ctx context.Context, opts policy.T
}

func (c *UsernamePasswordCredential) requestToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenByUsernamePassword(ctx, opts.Scopes, c.username, c.password, public.WithTenantID(opts.TenantID))
ar, err := c.client.AcquireTokenByUsernamePassword(ctx, opts.Scopes, c.username, c.password)
if err == nil {
c.account = ar.Account
}
Expand All @@ -70,7 +70,6 @@ func (c *UsernamePasswordCredential) requestToken(ctx context.Context, opts poli
func (c *UsernamePasswordCredential) silentAuth(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes,
public.WithSilentAccount(c.account),
public.WithTenantID(opts.TenantID),
)
return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
}
Expand Down

0 comments on commit a554ef0

Please sign in to comment.