Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.08k stars 1.2k forks source link

Unable to use TokenCredentials instances from the azure/identity library in ServiceClientCredential based SDK's for Sovereign Azure Clouds #15945

Closed prashantchari closed 3 years ago

prashantchari commented 3 years ago

On Azure China, Trying to use the TokenCredentials instances from the identity library that work with ServiceClientCredentials instances throw the error -

{
  "stack": "AuthenticationError: invalid_resource(status code 400).\nMore details:\nAADSTS500011: The resource principal named https://management.azure.com was not found in the tenant named <tenant>. This can happen if the application has not been installed by the administrator of the tenant or consented to by any user in the tenant. You might have sent your authentication request to the wrong tenant.\r\nTrace ID: b633abc6-dde5-485e-a83a-380a25ad5500\r\nCorrelation ID: 619bec33-dc10-4740-ad78-e728d25f2ee9\r\nTimestamp: 2021-06-23 21:00:14Z\n    at IdentityClient.<anonymous> (/app/node_modules/@azure/identity/dist/index.js:345:31)\n    at Generator.next (<anonymous>)\n    at fulfilled (/app/node_modules/tslib/tslib.js:114:62)\n    at processTicksAndRejections (internal/process/task_queues.js:95:5)",
  "message": "invalid_resource(status code 400).\nMore details:\nAADSTS500011: The resource principal named https://management.azure.com was not found in the tenant named a<tenant>. This can happen if the application has not been installed by the administrator of the tenant or consented to by any user in the tenant. You might have sent your authentication request to the wrong tenant.\r\nTrace ID: b633abc6-dde5-485e-a83a-380a25ad5500\r\nCorrelation ID: 619bec33-dc10-4740-ad78-e728d25f2ee9\r\nTimestamp: 2021-06-23 21:00:14Z",
  "statusCode": 400,
  "errorResponse": {
    "error": "invalid_resource",
    "errorDescription": "AADSTS500011: The resource principal named https://management.azure.com was not found in the tenant named <tenant>. This can happen if the application has not been installed by the administrator of the tenant or consented to by any user in the tenant. You might have sent your authentication request to the wrong tenant.\r\nTrace ID: b633abc6-dde5-485e-a83a-380a25ad5500\r\nCorrelation ID: 619bec33-dc10-4740-ad78-e728d25f2ee9\r\nTimestamp: 2021-06-23 21:00:14Z",
    "correlationId": "619bec33-dc10-4740-ad78-e728d25f2ee9",
    "errorCodes": [
      500011
    ],
    "timestamp": "2021-06-23 21:00:14Z",
    "traceId": "b633abc6-dde5-485e-a83a-380a25ad5500"
  },
  "name": "AuthenticationError"
}

Looking into the code, it looks like SDK's that use the serviceclientcredentials instances default to public azure scopes in https://github.com/Azure/ms-rest-js/blob/de6aa5157603639001785b4a43afa5f325381dbd/lib/serviceClient.ts#L189 in AzureIdentityCredentialAdapter. It is my understanding that this is causing the auth flow to fail.

Example usage:

new IotDpsClient( creds as any, subscriptionId, { baseUri } );

where credentials is a ChainedTokenCredential object with authority https://login.chinacloudapi.cn, baseUri points to https://management.chinacloudapi.cn, and IoTDpsClient is from https://www.npmjs.com/package/@azure/arm-deviceprovisioningservices.

Am I using this correctly ?

voidfoo commented 3 years ago

Maybe a workaround is to create an instance of AzureIdentityCredentialAdapter with the TokenCredential and the correct scope then pass it to IosDpsClient?

prashantchari commented 3 years ago

Absolutely, I'm hoping the ServiceClient can be fixed to take care of this, since it's already trying to handle interop between the token/serviceclient credentials. I can definitely give the workaround a try in the mean time.

ramya-rao-a commented 3 years ago

Thanks for reporting @prashantchari!

Can you share how you got around this problem when using the older @azure/ms-rest-nodeauth with @azure/arm-deviceprovisioningservices?

@sadasant This issue is for when using the @azure/identity package with one of the management plane libraries, but am guessing should affect our other data plane libraries as well.

sadasant commented 3 years ago

Hello team! I’m Daniel. I’m here to help.

Identity offers the possibility to specify authority hosts through the authorityHost property in the credential options. We offer an object containing these authorities, AzureAuthorityHosts, which has one property that should be useful: AzureAuthorityHosts.AzureChina.

Would something similar to this code work for you?

import { AzureAuthorityHosts, EnvironmentCredential } from “@azure/identity”;

const credential = new EnvironmentCredential({ authorityHost: AzureAuthorityHosts.AzureChina });

const client = new IotDpsClient(credential, subscriptionId, { baseUri });

We’re working on improving our documentation, and in the following weeks, we will have a sample showcasing how to use this feature. In the meantime, please let us know if this approach works for you. Your time is appreciated!

voidfoo commented 3 years ago

I could be wrong but I think the problem is the scope not the auth host. TokenCredential does not carry info about scope so when ms-rest-js creates the adapter for the token credential passed in it won't know what scope to use.

sadasant commented 3 years ago

@voidfoo thank you! I see your point now. Give us some time to think on a good answer.

sadasant commented 3 years ago

@voidfoo , @prashantchari since the solution to this is likely going to need a public API change from our part, please give us a couple of days to coordinate an appropriate response. Thank you for reporting this problem to us.

sadasant commented 3 years ago

@voidfoo , @prashantchari We’re fully booked these days. It will take us some time to come up with a good answer. In the mean time, would it work for you to have your own copy of the AzureIdentityCredentialAdapter? You could change the scope in this copy of yours, and once we arrive on a solution, it should be easy to replace yours with ours. The code I’m referring to:


import { ServiceClientCredentials, WebResource, TokenResponse } from "ms-rest-js";
import { TokenCredential } from "@azure/core-auth";

export class AzureIdentityCredentialAdapter implements ServiceClientCredentials {
  private azureTokenCredential: TokenCredential;
  private scopes: string | string[];
  constructor(
    azureTokenCredential: TokenCredential,
    scopes: string | string[] = "https://<your-scope-endpoint>/.default"
  ) {
    this.azureTokenCredential = azureTokenCredential;
    this.scopes = scopes;
  }

  public async getToken(): Promise<TokenResponse> {
    const accessToken = await this.azureTokenCredential.getToken(this.scopes);
    if (accessToken !== null) {
      const result: TokenResponse = {
        accessToken: accessToken.token,
        tokenType: "Bearer",
        expiresOn: accessToken.expiresOnTimestamp,
      };
      return result;
    } else {
      throw new Error("Could find token for scope");
    }
  }

  public async signRequest(webResource: WebResource) {
    const tokenResponse = await this.getToken();
    webResource.headers.set(
      "authorization",
      `${tokenResponse.tokenType} ${tokenResponse.accessToken}`
    );
    return Promise.resolve(webResource);
  }
}

With an AzureIdentityCredentialAdapter, you can wrap an instance of your TokenCredential with new AzureIdentityCredentialAdapter(myTokenCredentialInstance), and pass it to the IotDpsClient. It should work, and it will give you full control while we come up with a solution.

Would that be a good workaround in the short term? We appreciate your time and feedback.

prashantchari commented 3 years ago

Hey @sadasant , Thanks for the update. It looks like there was a push to enable tokencredentials as a first class citizen on the library I was looking at - azure/arm-deviceprovisioningservices.

I'll scan on my side to see if I'm using SDKs that do not yet work on tokencredentials directly and look into the feasibility of having the above workaround for the affected services. Is it a reasonable assumption that all of the azure SDKs should eventually move to not requiring the adapter/working with tokencredentials directly at some point ?

sadasant commented 3 years ago

@prashantchari I’m happy to read that! - That is correct, the goal is to have all of the Azure SDKs use TokenCredentials.

sadasant commented 3 years ago

@prashantchari If your concerns are resolved, please feel free to close this issue. In any case, please let us know if there’s anything else we can do. Thank you for your time using the Azure SDKs!

prashantchari commented 3 years ago

Uggh. I realized the sdk's just moved the problem around. This is misleading. Also, It appears to be very hard to avoid this problem given that the function signature for the SDK's that operate on ServiceClientCredentials take in TokenCredentials now, and internally errors out because of this issue. @sadasant , do you happen to have an issue that tracks the fixing of the larger change / adapter , so I know when to remove it's usage at my end ?

sadasant commented 3 years ago

@prashantchari hello again! Thank you for giving us more information. Let’s use this issue to track the problem you’re seeing. I will need some days to route this problem internally properly. I’ll answer back next week, as soon as possible.

ramya-rao-a commented 3 years ago

It appears to be very hard to avoid this problem given that the function signature for the SDK's that operate on ServiceClientCredentials take in TokenCredentials now,

@prashantchari While the constructor signature for the SDKs now take in TokenCredentials, they support the older ServiceClientCredentials too.

See https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/deviceprovisioningservices/arm-deviceprovisioningservices/src/iotDpsClient.ts#L36

image

constructor(credentials: msRest.ServiceClientCredentials, has changed to constructor(credentials: msRest.ServiceClientCredentials | TokenCredential

So, you should be able to use the modified adapter that @sadasant shared above to create a ServiceClientCredentials and pass that to the client constructor. You will need to set the scopes to your-baseUri/.default. Am guessing your baseUri is already set to https://management.chinacloudapi.cn.

If you can confirm that this works for you, we can then make a change to the ServiceClient constructor at https://github.com/Azure/ms-rest-js/blob/c47191b4255534771dc320960c39a25645823800/lib/serviceClient.ts#L190 as follows

image

ramya-rao-a commented 3 years ago

In conclusion

import { AzureAuthorityHosts, EnvironmentCredential } from “@azure/identity”;

const credential = new EnvironmentCredential({ authorityHost: AzureAuthorityHosts.AzureChina });

If you can confirm that this works for you, we can get working on a fix soon

prashantchari commented 3 years ago

Hi @ramya-rao-a , Correct, I am passing in the base uri as you mentioned above, I think the solution works for me.

ramya-rao-a commented 3 years ago

Thanks for the confirmation @prashantchari

@sadasant Can you make a PR to make the required changes to @azure/ms-rest-js?

sadasant commented 3 years ago

We have released a new version for @azure/ms-rest-js that should fix this issue. Please let us know if you can test it, and if it solves this issue for you.

ghost commented 3 years ago

Hi @prashantchari. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.

ghost commented 3 years ago

Hi @prashantchari, since you haven’t asked that we “/unresolve” the issue, we’ll close this out. If you believe further discussion is needed, please add a comment “/unresolve” to reopen the issue.