-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[BUG]: Error authenticating API requests from Octokit JS client when using private key in Azure key vault #2623
Comments
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with |
PS, this seems related to: |
So, after digging into this, it looks like this might a documentation opportunity rather than a bug, as it's more to do with a limitation of the Azure Key Vault and the guideines in the GitHub documentation. Going back to the drawing board and trying to store the private key as a secret - not a key - in AKV seems to work. But there's a catch: the RSA private key you download for the GitHub app needs to be encoded as a Here's the TL;DR of what you need to do to get it to work:
const vaultName = process.env.KEY_VAULT_NAME;
const keyName = process.env.KEY_NAME;
const appId = process.env.APP_ID;
const vaultURL = `https://${vaultName}.vault.azure.net`;
const credential = new DefaultAzureCredential();
const client = new SecretClient(vaultURL, credential);
const secretBundle = await client.getSecret(keyName);
const privateKeyString = Buffer.from(secretBundle.value, 'base64').toString('ascii');
const octokit = new Octokit({
authStrategy: createAppAuth,
auth: { appId, privateKey: privateKeyString, installationId },
}); BUT, that being said, it would be interesting to find out if there's any opportunity to add a new feature to Octokit that would allow users to pass the key object from AKV to the new It would be good to get the 2cents of the Octokit team on this. Specifically to find what the appetite for this would be with respect to i) whether or not enough users have a desire for this feature, and ii) whether the API/Octokit product and engineering teams see this as something that's worth the time/effort. |
We wouldn't want to add a feature for only one platform. If it uses a standardized process that is supported on many platforms then it would be fine |
@danielhardej Not sure why it didn't work the way I did it for my other projects, but thank you a lot! I tried to fix it with 3 other solutions over 1-2 hours. That is the only approach that worked in production too. I will start to encode all of my privateKeys in the future, because I guess that is the most robust way. |
@wolfy1339 , while I fully understand and support not implementing a feature for a specific platform, it is a bit frustrating to see the official documentation recommending storing the key in a key vault as "sign-only" without any obvious way to use it that way in the official SDK. Would it be a viable approach to "inject" a method for generating a JWT rather than passing the private key for auth directly? This way, octokit could handle any refresh issues etc. without adding platform-specific code. Or am I missing something here, and there is already a similar approach for achieving this? :) |
What happened?
Note: this could be more to do with a gap in documentation, or possibly even a limitation of using Microsoft's Azure Key Vault with a GitHub app, but I feel it's still pertinent given that it is related to advice given in GitHub's documentation (See: Secure your app's credentials under Best practices for creating a GitHub App)
The problem
When running Octokit.JS in an Azure function that is part of a GitHub app, the error
Error: secretOrPrivateKey must be an asymmetric key when using RS256
gets thrown when making an API request such as:This happens when storing the app private key used to authenticate Octokit in Azure key vault as a key or a secret, as an environment variable, or just as a text string.
This happens with the following set up:
Storing as a key in Azure Key Vault
The first scenario was storing the
.pem
file (the one downloaded from thePrivate keys
section of the app's settings) as a key in Azure key vault, in line with the guidance in the documentation on Private keys. It can be accessed by the Azure function app from the key vault without errors.The private key is obtained in the following way, in line with the guidance in Microsoft's documentation: Azure Key Vault Key client library for JavaScript
Storing as a secret in Azure Key Vault
I then also tried storing the private key (the contents of the
.pem
file, rather than the file itself) as a secret in AKV, rather than a key, and then accessing it with:This ended up with the same problem:
Error: secretOrPrivateKey must be an asymmetric key when using RS256
.Additionally, attempting to print the key to the console revealed that AKV provides a buffer object, rather than the key itself. This doesn't seem to be very useful, as the guidelines on Authentication in the Octokit README suggest that the private key needs to be passed as a string.
Using environment variables/plain text
Putting Azure key vault aside, I also tried:
Both gave the
Error: secretOrPrivateKey must be an asymmetric key when using RS256
again.Another interesting thing
One curious thing is when the errors are thrown: no errors are thrown when a new instance of
Octokit
is created and authenticated as a GitHub app installation (whether that's using Azure key vault, env variables, or plain text.)Instead, errors are only thrown at the point when an API request is make with Octokit.
The Octokit instance is created in the following way in accordance with the guidelines in the README (note the installation ID is obtained from the webhook payload):
Versions
@octokit/rest version: 20.0.2
Node version v18.12.1
Relevant log output
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: