Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Identity] Wrapper for packages that use @azure/ms-rest-nodeauth #11359

Closed
ramya-rao-a opened this issue Sep 21, 2020 · 8 comments
Closed

[Identity] Wrapper for packages that use @azure/ms-rest-nodeauth #11359

ramya-rao-a opened this issue Sep 21, 2020 · 8 comments
Assignees
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library.
Milestone

Comments

@ramya-rao-a
Copy link
Contributor

Most of the JS packages for Azure services use the credentials from the @azure/ms-rest-nodeauth. These follow the TokenClientCredentials interface.

Now the latest docs around Identity will lead people to the @azure/identity which is also the place where active development is taking place.

In order to take advantage of the new features in the @azure/identity package, it would be good to have a wrapper/helper method that would take a credential from the new package and return something that follows the interface TokenClientCredentials from the older package

@jongio did mention that he has something along these lines. If so, can we have a pointer to it from the readme of the @azure/identity package?

cc @jonathandturner

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 21, 2020
@ramya-rao-a ramya-rao-a added Azure.Identity Client This issue points to a problem in the data-plane of the library. and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Sep 21, 2020
@ramya-rao-a ramya-rao-a added this to the MQ-2020 milestone Sep 21, 2020
@ramya-rao-a
Copy link
Contributor Author

@ramya-rao-a ramya-rao-a assigned sadasant and unassigned daviwil Nov 16, 2020
@ramya-rao-a
Copy link
Contributor Author

ramya-rao-a commented Nov 18, 2020

Assuming we know which "scope" to use, we have a couple of options where we update the v4 code generator to provide a solution here. This scope would be the same for all ARM packages, but would be different for the rest like batch, graph, service fabric etc.

  • Update the v4 code generator to update the readme that show the use of ms-rest-nodeauth to instead use the code snippet that
    • Uses @azure/identity to create a credential
    • Call getToken() on it with the right scope
    • Then use Authenticating with existing token to create a ServiceClientCredentials that can then be used with the package in question
  • Do the above, but move the last step into the generated package i.e. update the v4 code generator to add a constructor overload that takes the access token instead of ServiceClientCredentials. Internally, this can create the credentials using the code snippet shared in Authenticating with existing token.
  • Update the v4 code generator to add a constructor overload that takes a TokenCredential instead of ServiceClientCredentials. Internally, this can create the credentials using the code snippet shared in Jon's adapter. This requires the generated code to take dependency on @azure/core-auth

@joheredi, Thoughts?

cc @daviwil

Once we are done with the above, the Authenticate with the Azure management modules for JavaScript will need to be updated as well

@joheredi
Copy link
Member

joheredi commented Nov 18, 2020

Option 1 is of course the cheaper option, however we may end up with a snippet that is not easy to follow.

Option 3 makes sense, as it shouldn't be too difficult to generate the overload and adapter. Also the user experience will be better.

@ramya-rao-a what's the timeframe for this?

@jongio
Copy link
Member

jongio commented Nov 18, 2020

Yeah, i recommend creating the adapter - please steal the existing code.

@jongio
Copy link
Member

jongio commented Nov 18, 2020

Heads up , removed space from file name, new path: https://github.com/jongio/azidext/blob/master/js/azureIdentityCredentialAdapter.ts

@ramya-rao-a
Copy link
Contributor Author

@joheredi If Option 3 is doable, then doing so now during the MQ milestone would be preferable

@ramya-rao-a
Copy link
Contributor Author

@joheredi Can you log an issue in the code gen repo, link it here and close this issue?

@joheredi
Copy link
Member

joheredi commented Dec 8, 2020

Logged Azure/autorest.typescript#815 which will be the main tracking issue

@joheredi joheredi closed this as completed Dec 8, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

No branches or pull requests

5 participants