Add auxiliary authentication header policy in core pipeline

[MaryGao]

Background

Add a policy for external tokens to x-ms-authorization-auxiliary header in core lib. This header will be used when creating a cross-tenant application we may need to handle authentication requests for resources that are in different tenants. You can learn more here. Here I collect two use cases:

• Create a virtual network peering between virtual networks across tenants (see here)
• Share images across tenants (see here)

Usecase - create a virtual network peering across tenants

We have two subscriptions cross two tenants:

subscriptionA = "75d6dc7b-9a8d-4f94-81ce-8a9437f3ce2c" in tenantA
subscriptionB = "92f95d8f-3c67-4124-91c7-8cf07cdbf241" in tenantB 

Prepare the app register and grant permission in both subscriptions, please note we'll have one app register with two service principals in two tenants.

# Create app registration named `appRegisterB` which allows to be used in any orgnaizational directory located in `tenantB`

# Create a service principal for `appRegisterB` in `tenantA` by login url: [https://login.microsoftonline.com/{tenant-id}/adminconsent?client_id={client-id}](https://login.microsoftonline.com/%7Btenant-id%7D/adminconsent?client_id=%7Bclient-id%7D)

# Add roles for `appRegisterB` in both `subscriptionA` and `subscriptionB`

Prepare the virtual network in both subscriptions:

# Switch to subscription A
az account set -s $subscriptionA 

# Create resource group A
az group create --name myResourceGroupA --location eastus

# Create virtual network A
az network vnet create --name myVnetA --resource-group myResourceGroupA --location eastus --address-prefix 10.0.0.0/16

# Switch to subscription B
az account set -s $subscriptionB 

# Create resource group B
az group create --name myResourceGroupB --location eastus

# Create virtual network B
az network vnet create --name myVnetB --resource-group myResourceGroupB --location eastus --address-prefix 10.1.0.0/16

Create a virtual network peering between these two virtual networks. We could build this peer relationship from myVnetA to myVnetB, or from myVnetB to myVnetA.

If we build a client under subscriptionB then we could create this peer from myVnetB to myVnetA with below headers:

Header name	Description	Example value
Authorization	Primary token, token got from credentialB	Bearer <primary-token>
x-ms-authorization-auxiliary	Auxiliary tokens, token got from credentialA	Bearer <auxiliary-token1>

  const tenantA = "c029c2bd-5f77-48fd-b9b8-6dbc7c475125";
  const tenantB = "72f988bf-86f1-41af-91ab-2d7cd011db47";
  const subscriptionB = "92f95d8f-3c67-4124-91c7-8cf07cdbf241";
  const myResourceGroupB = "myResourceGroupB";
  const myVnetB = "myVnetB";
  const virtualNetworkPeeringName = "myVnetA";
  const virtualNetworkPeeringParameters: VirtualNetworkPeering = {
    allowForwardedTraffic: false,
    allowGatewayTransit: false,
    allowVirtualNetworkAccess: true,
    remoteVirtualNetwork: {
      id:
        "/subscriptions/75d6dc7b-9a8d-4f94-81ce-8a9437f3ce2c/resourceGroups/myResourceGroupA/providers/Microsoft.Network/virtualNetworks/myVnetA"
    },
    useRemoteGateways: false
  };

[Preferred] Option 1: Provide an extra policy auxiliaryAuthenticationHeaderPolicy

Provide a new policy auxiliaryAuthenticationHeaderPolicy in core, then customer code could leverage that policy to add auxilary header.

async function createPeeringWithPolicy() {
  const credentialA = new DefaultAzureCredential({tenantId: tenantA});
  const credentialB = new DefaultAzureCredential({tenantId: tenantB});
  const client = new NetworkManagementClient(credentialB, subscriptionB,
    {
      // Add the extra policy when building client
      additionalPolicies: [{
        policy: auxiliaryAuthenticationHeaderPolicy({
          credentials: [credentialA],
          scopes: "https://management.core.windows.net//.default"
        }),
        position: "perRetry",
      }]
    });
  const result = await client.virtualNetworkPeerings.beginCreateOrUpdateAndWait(
    myResourceGroupB,
    myVnetB,
    virtualNetworkPeeringName,
    virtualNetworkPeeringParameters,  
  );
  console.log(result);
}

Option 2: Add auxiliaryTenants as a client option

Similar the implementation in Go, we could provide an option in CommonClientOptions.

/**
 * Auxiliary tenant ids which will be used to get token from
 */
auxiliaryTenants?: string[];

And then enhance the current bearerTokenAuthenticationPolicy logic to detect if we have the auxiliaryTenants provided, if yes we could automatically get tokens and add x-ms-authorization-auxiliary header in request. And the customer code would be like:

async function createPeeringWithParam() {  
  const credential = new ClientSecretCredential(tenantB, env.clientB, env.secretB, {
    // We would also add allowed tenant list into current credential so that we could get relevant tenant tokens
    additionallyAllowedTenants: [tenantA]
  });
  const client = new NetworkManagementClient(credential, subscriptionB, {
      // If the parameter is provided the bearer policy would append the extra header
      auxiliaryTenants: [tenantA]
    });
  const result = await client.virtualNetworkPeerings.beginCreateOrUpdateAndWait(
    myResourceGroupB,
    myVnetB,
    virtualNetworkPeeringName,
    virtualNetworkPeeringParameters
  );
  console.log(result);
}

Option 3: Add auxiliaryCredentials option in BearerTokenAuthenticationPolicyOptions

Instead of providing new policy we could add a new option in BearerTokenAuthenticationPolicyOptions in original bearerTokenAuthenticationPolicy. Then in that policy we could detect if the parameter auxiliaryCredentials is provided, if yes append the header accordingly.

/**
 * Provide the auxiliary credentials to get tokens in header x-ms-authorization-auxiliary
 */
auxiliaryCredentials: TokenCredential[];

But it would be more complex from customer side, because we add bearer policy by default so we have to remove that one first and then re-add a new one.

async function createPeeringWithNewBearerPolicy() {
  const credentialA = new ClientSecretCredential(tenantA, clientB, secretB);
  const credentialB = new ClientSecretCredential(tenantB, clientB, secretB);
  const client = new NetworkManagementClient(credentialB, subscriptionB);
  // Build a new policy with auxiliaryCredentials provide
  const customizedBearerPolicy = bearerTokenAuthenticationPolicy({
    credential: credentialB,
    scopes: "https://management.core.windows.net//.default",
    auxiliaryCredentials: [credentialA]
  });
  // Remove the original one
  client.pipeline.removePolicy({
    name: bearerTokenAuthenticationPolicyName
  });
  // Add our new policy
  client.pipeline.addPolicy(customizedBearerPolicy);
  const result = await client.virtualNetworkPeerings.beginCreateOrUpdateAndWait(
    myResourceGroupB,
    myVnetB,
    virtualNetworkPeeringName,
    virtualNetworkPeeringParameters
  );
  console.log(result);
}

Simply speaking I prefer the option 1, you could know more here.

Reference

Java: Azure/azure-sdk-for-java#14336
Python: Azure/azure-sdk-for-python#24585
Go: Azure/azure-sdk-for-go#19309
.Net: Azure/azure-sdk-for-net#35097 // Only add sample, didn't implement in core

[azure-sdk]

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-core-rest-pipeline

[xirzec]

This is an interesting feature, but I feel like this PR is a bit rushed, especially for an issue that is over a year old.

When we make changes to any core package, I think we need to have a higher bar for code quality and ensure that every PR has a detailed description of the scenario it is enabling with enough context that anyone can understand the change.

Remember that all surface area exported by core packages has far reaching support implications. We may have to maintain this code for many years and new developers to the team will have to be able to read and understand this functionality without the benefit of our current context and experience.

[xirzec]

again, what if we have more than three tokens? This will currently append all of them.

[MaryGao]

I commented here, #25270 (comment).

@xiangyan99 How do you think?

[xiangyan99]

If there are more than three tokens, it means there are some configuration issues. As a client library, I don't think we want to decide which tokens to ignore on behalf of users and try to swallow the issue. Instead, I prefer to let the request fail and ask users to check the configuration. @xirzec your ideas?

[MaryGao]

Regarding the design: #25270 (comment)

Comparations for these three options

According to my understanding the usage of x-ms-authorization-auxiliary header is not commonly used, and mostly these cases are complex because the resources are located in multi-tenants . And in these cases SDK's customers may be aware of the necessity of extra header x-ms-authorization-auxiliary, so I think giving them more flexibility would be better. @xiangyan99 Please correct me if I'm wrong.

So I would prefer the option 1, the inputted parameter TokenCredential[] would be more flexible compared with the option 2 of auxiliary tenants(auxiliaryTenants?: string[]). Because customers could have more ways to get token and no need to the same as the primary credential way.

Compared with option 3, the implementation code would be lower coupling with current bearer policy logic and the integration code would be more seamless.

@xirzec How do you think?

[xirzec]

First off, thank you so much for writing out such a clear and detailed PR description. I understand the scenario much better now, especially after comparing the code examples of what consumers would see in each option.

@xirzec How do you think?

I agree with you that Option 1 is probably the least invasive option and a good starting point. We can always implement something like Option 2 later as a convenience if we get feedback that Option 1 is not discoverable, or the pattern becomes more widely adopted across services. Having to replace an existing policy in Option 3 feels too invasive to me.

@mpodwysocki @willmtemple @KarishmaGhiya - do y'all have any thoughts/preferences?

[xirzec]

Looking good! Minor corrections and style suggestions. 👍

[xirzec]

Looks great! Thank you for contributing this. 👍