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

Keyvault authentication is throwing 401s with @azure/identity : "4.1.0" #31279

Closed cs-samvith closed 1 month ago

cs-samvith commented 1 month ago

Describe the bug Application insight is showing 401s with the key vault authentication all the time even though it succeeds with the subsequent calls . We are using user assigned managed identity for auth. Wondering why it is throwing 401s when we have proper identity mechanisms in place. Please help thank you in advance .

When using the previous version of the package , not seeing it logged in app insight. it is either not logged in app insight or not happening.

image


import { get, put } from 'memory-cache';
import { DefaultAzureCredential } from '@azure/identity';
import { SecretClient } from '@azure/keyvault-secrets';

const keyVaultUrl = process.env.KeyVault__Uri || process.env.KEY_VAULT_URL;
const credential = new DefaultAzureCredential();
const cacheExpirationInMs = 300000;

export const getSecret = async id => {
    let secret = get(id);
    if (!secret) {
        //What if this fails? We should use the previous value, but it's lost.
        **const client = new SecretClient(keyVaultUrl, credential);**
        const foundSecret = await client.getSecret(id);
        secret = foundSecret.value ?? null;
        put(id, secret, cacheExpirationInMs);
    }
    return secret;
};

To Reproduce Steps to reproduce the behavior:

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

github-actions[bot] commented 1 month ago

@KarishmaGhiya @maorleger

github-actions[bot] commented 1 month ago

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

maorleger commented 1 month ago

Hey @cs-samvith - thanks for reaching out about this. While I admittedly don't have a good answer for why you were not seeing this 401 logged in applicationinsights in prior versions, I do believe that calls to KeyVault are expected to start with a 401 and succeed on a subsequent call as part of KeyVault's continuous access evaluation support - please see this document that describes the flow.

Looking at the code, I see you are initializing a new SecretClient every time getSecret is called. Consider restructuring your code to reuse the same credential and SecretClient to avoid unnecessary overhead like initializing caches and the first 401 response. Something like this:

import { get, put } from 'memory-cache';
import { DefaultAzureCredential } from '@azure/identity';
import { SecretClient } from '@azure/keyvault-secrets';

const keyVaultUrl = process.env.KeyVault__Uri || process.env.KEY_VAULT_URL;
const credential = new DefaultAzureCredential();
const cacheExpirationInMs = 300000;
const client = new SecretClient(keyVaultUrl, credential);

export const getSecret = async id => {
    let secret = get(id);
    if (!secret) {
        const foundSecret = await client.getSecret(id);
        secret = foundSecret.value ?? null;
        put(id, secret, cacheExpirationInMs);
    }
    return secret;
};

As for why you're seeing this now and not in previous versions, I wonder if enabling verbose logging and inspecting the calls would be helpful - you could do that by setting the AZURE_LOG_LEVEL environment variable to "verbose" - you could then inspect the logs to see what requests / responses are being made. Unfortunately I just don't know applicationinsights well enough to provide a better answer but if the logs show different calls being made we'll happily investigate!

cs-samvith commented 1 month ago

@maorleger - Thank you for the response and I tried your suggestion . and the results are

Before the change I was seeing 401s at 100% (ie for each 200 response there was a 401 response too )

After the change The rate has reduced to around 50% . (ie 200s - 1640 and 401s - 751 )

So 401s are still happening . Any idea why ?

image

401 with a call status true??

maorleger commented 1 month ago

Glad the rate was reduced! I believe this is expected per https://learn.microsoft.com/en-us/azure/key-vault/general/authentication-requests-and-responses#authentication

Specifically:

Key Vault SDK clients for secrets, certificates, and keys in the first call to Key Vault do not provide an access token to retrieve tenant information. It’s expected to receive an HTTP 401 using Key Vault SDK client where the Key Vault shows to the application the WWW-Authenticate header containing the resource and the tenant where it needs to go and ask for the token. If everything is configured correctly, the second call from the application to Key Vault will contain a valid token and will succeed.

So it sounds like as long as the end result is a 200, the intermediary 401s are expected

cs-samvith commented 1 month ago

Ok . Thank you @maorleger for clarifying . Am closing this one!