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.09k stars 1.2k forks source link

SimpleTokenCache (java) for node js azure Identity #29387

Closed ghost closed 6 months ago

ghost commented 7 months ago

I'm interested in a feature similar to the SimpleTokenCache class from the Azure SDK for Java in Node.js for managing authentication tokens efficiently. Currently, Node.js does not seem to have an equivalent feature within the Azure SDK for JavaScript that provides simple token caching capabilities. Having such a class would greatly simplify the process of token management by automatically handling the caching, refreshing, and retrieval of tokens. This functionality is particularly useful for long-running applications or services that frequently interact with Azure services, requiring frequent token renewals. I believe integrating a SimpleTokenCache class into the Node.js Azure SDK could significantly improve developer productivity and application performance. Could this be considered for inclusion in a future release of the SDK? thank you.

https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/core/azure-core/src/main/java/com/azure/core/credential/SimpleTokenCache.java?ref=azureblue.io

github-actions[bot] commented 7 months ago

@KarishmaGhiya @maorleger

github-actions[bot] commented 7 months ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.

xirzec commented 7 months ago

This sounds very much like what the tokenCycler in core does.

It's not exposed directly, but is used by our bearerTokenAuthenticationPolicy: https://github.com/Azure/azure-sdk-for-js/blob/e0dc8c2c350b43b0be0f8dd53ee3ecb397094f80/sdk/core/core-rest-pipeline/src/policies/bearerTokenAuthenticationPolicy.ts#L153

ghost commented 7 months ago

Thanks, can that class be used for this? I am trying to use node js with azure user managed Identity with PostgreSQL. and I am trying to handle token refresh scenario's. thanks.

maorleger commented 6 months ago

Hey @rassiju06 - Appreciate your patience here and apologies for the delayed followup.

I think what @xirzec was alluding to is that you should have no need to manage token lifetimes and refreshing yourself. Instead, our core libraries handle this for you as an implementation detail. Are you seeing behavior that you do not expect with token refreshing or mostly looking to understand whether you need to implement token caching yourself?

If the former, could you share some information about your scenario and what you observe / expected to see?

If the latter, the answer is no as far as I am aware - @KarishmaGhiya thoughts?

github-actions[bot] commented 6 months ago

Hi @rassiju06. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

ghost commented 6 months ago

Hello, yes so we are using a user managed identity to connect to our postgres DB from an app service. We are using type orm from nest js, and after 24hrs the aad token refreshes and we start seeing token refresh errors unless the application is restarted. We except that the token will refresh in 23 hrs, but we were expecting the implementation to handle token refresh gracefully like the java implementation. Thank you.

Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Maor Leger @.> Sent: Wednesday, May 1, 2024 10:52:59 AM To: Azure/azure-sdk-for-js @.> Cc: Justin Rassi @.>; Mention @.> Subject: Re: [Azure/azure-sdk-for-js] SimpleTokenCache (java) for node js azure Identity (Issue #29387)

You don't often get email from @.*** Learn why this is importanthttps://aka.ms/LearnAboutSenderIdentification

Hey @rassiju06https://github.com/rassiju06 - Appreciate your patience here and apologies for the delayed followup.

I think what @xirzechttps://github.com/xirzec was alluding to is that you should have no need to manage token lifetimes and refreshing yourself. Instead, our core libraries handle this for you as an implementation detail. Are you seeing behavior that you do not expect with token refreshing or mostly looking to understand whether you need to implement token caching yourself?

If the former, could you share some information about your scenario and what you observe / expected to see?

If the latter, the answer is no as far as I am aware - @KarishmaGhiyahttps://github.com/KarishmaGhiya thoughts?

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-sdk-for-js/issues/29387#issuecomment-2088668872, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BIAE5KHXBJC2VHGXCOJKS2TZAEFVXAVCNFSM6AAAAABGTA4GKSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBYGY3DQOBXGI. You are receiving this because you were mentioned.Message ID: @.***>

maorleger commented 6 months ago

Hey @rassiju06 - thank you for that added information. I also expect the token refresh logic to handle token refresh gracefully and so I'm surprised to hear that in your case you're seeing token refresh errors. It's something that we will need to investigate on our end. I do expect the Azure SDK to handle this for you without requiring additional configuration or using a custom TokenCache.

What would help me (if at all possible) is seeing the logs - I know it's likely not possible but if you're able to turn on verbose logging and see if anything in the logs stands out as interesting or suspicious I'd love to see the logs! Specifically around the 24 hour mark when you start seeing errors.

In the meanwhile, I'll try to repro this on my end and check with the MSAL folks to see if they have any guidance. If it turns out that this is expected, we can consider exposing something similar to what Java has.

maorleger commented 6 months ago

hey @rassiju06 I wanted to get more information about your scenario - you say you're using user managed identity to connect to our postgres DB from an app service

Can you expand a bit on this?

Are you using an Azure client library for postgres? Are you using @azure/identity to get tokens? Are you using Azure postgres?

A small code sample would help clarify a lot of this if you can provide one - either putting something on github or just sharing the package.json and a small js script that mimics your setup so I can better understand your scenario.

I have a few suspicions but need the above information to confirm. thanks so much!

github-actions[bot] commented 6 months ago

Hi @rassiju06. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

ghost commented 6 months ago

hey @rassiju06 I wanted to get more information about your scenario - you say you're using user managed identity to connect to our postgres DB from an app service

Can you expand a bit on this?

Are you using an Azure client library for postgres? Are you using @azure/identity to get tokens? Are you using Azure postgres?

A small code sample would help clarify a lot of this if you can provide one - either putting something on github or just sharing the package.json and a small js script that mimics your setup so I can better understand your scenario.

I have a few suspicions but need the above information to confirm. thanks so much!

Hello so to answer you question we are using azure Postgres and we are using the PG npm library. we are using azure identity package as well, and following the examples from this document. I cannot share my full code, but here are the relevant parts the code works fine, and we are able to connect to the database, but once the app service reaches the 24 hour mar the connection is severed because the class cannot update the connection without a restart of the application (we are using TypeORM). perhaps a working example would help more? I have tried generic application examples from the sdk and they all seem to have the same token issue? thank you.

https://learn.microsoft.com/en-us/azure/app-service/tutorial-connect-msi-azure-database?tabs=sqldatabase-sc%2Cuserassigned-sc%2Cdotnet%2Cdotnet-mysql-mi%2Cdotnet-postgres-mi%2Cwindowsclient

` import { TypeOrmModuleOptions } from '@nestjs/typeorm'; import { ConfigService } from '@nestjs/config'; import { DefaultAzureCredential } from '@azure/identity';

export const datasourceConfig = async ( configService: ConfigService, ): Promise => { const credentials = await getCredentials(configService);

  const newConnectionOptions: ConnectionOptions = {
    name: connectionName,
    type: 'postgres',
    host: configService.get('APP_DB_HOST'),
    username: configService.get('APP_DB_USER'),
    password: accessToken,
    database: configService.get('APP_DB_DATABASE'),
    port: configService.get<number>('APP_DB_PORT'),

async function getCredentials(configService: ConfigService) { if (configService.get('APP_DB_MANAGED_IDENTITY', { infer: true })) { return await getManagedIdentityCredentials(configService); } else { return getBasicAuthCredentials(configService); } }

async function getManagedIdentityCredentials(configService: ConfigService) { const managedClientId = configService.get('APP_DB_MANAGED_IDENTITY', { infer: true, }); const credential = new DefaultAzureCredential({ managedIdentityClientId: managedClientId, });

const accessToken = await credential.getToken( 'https://ossrdbms-aad.database.windows.net/.default', );

return { username: configService.get('APP_DB_USER', { infer: true }), password: accessToken.token, }; } `

maorleger commented 6 months ago

That helps a lot, thanks so much! I'll try to repro this using the code you shared and a long-running process and see what I can find

maorleger commented 6 months ago

Just wanted to follow up here as I think I understand what may be the issue.

You are getting the token once and then passing that string to the PG connection options. That token, as you know, is valid for 24 hours and so feeding it the hardcoded string (as you already observed) will not work once the token expires.

But, you should be able to call getToken on the credential as many times as you'd like and the DefaultAzureCredential's implementation will intelligently return the same token or a refreshed token as needed.

It looks like node-postgres (the pg library) supports this exact scenario!

Take a look at these docs - there's a section about short-lived authentication tokens. The short of it is that this package supports passing a callback instead of a hardcoded string for the password to support short-lived credentials.

I believe if you do something like (this rough pseudocode):

const credential = new DefaultAzureCredential({managedIdentityClientId: managedClientId})
const getToken = async () => (await credential.getToken('https://ossrdbms-aad.database.windows.net/.default')).token;
const newConnectionOptions: ConnectionOptions = {
    name: connectionName,
    type: 'postgres',
    host: configService.get('APP_DB_HOST'),
    username: configService.get('APP_DB_USER'),
    password: getToken, // <-- pass the callback instead of the accesstoken directly
    database: configService.get('APP_DB_DATABASE'),
    port: configService.get<number>('APP_DB_PORT'),
}
// etc.

It should work as you'd expect with token refreshes occurring as needed. The docs I linked to should provide a more complete example, the above was not tested so excuse any typos 😄

Hope this helps! Let me know if that improves your scenario. If it does, I can work with the right folks on the documentation side to update the examples to showcase using short-lived credentials with a long-lived process

github-actions[bot] commented 6 months ago

Hi @rassiju06. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

ghost commented 6 months ago

ok thanks, Ill try this out and see

github-actions[bot] commented 6 months ago

Hi @rassiju06. 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.

github-actions[bot] commented 6 months ago

Hi @rassiju06, 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.