Add an Azure Resources API authentication layer #1284
Conversation
| 1. The client extension should then use the Azure Resources credential to retrieve the Azure Resources APIs by calling `getAzureResourcesApis`. | ||
|
|
||
| ### Diagram | ||
|  |
There was a problem hiding this comment.
Note: Some of these new links will look broken until the PR gets merged
There was a problem hiding this comment.
I assume you won't need to pay much attention to the changes from this file since it's auto-generated based on all the other types in the repo**
| clientCredential = await clientCredentialManager.createCredential(context.clientExtensionId); | ||
| } catch (err) { | ||
| if (err instanceof Error) { | ||
| await context.onApiRequestError?.({ code: AzureResourcesApiRequestErrors.CLIENT_FAILED_CREATE_CREDENTIAL.code, message: clientCredentialManager.maskCredentials(err.message) }) |
There was a problem hiding this comment.
For these onX you should void them or consider maybe VSCode's EventEmitter. If the awaited callback throws, then this would also throw.
There was a problem hiding this comment.
Good call, I changed to use void. That said, I actually realized while testing that because I'm already voiding here at the top level:
Any errors thrown won't actually bubble up to the user because it's already in another process (at least for the first part of the handshake).
I added this note as well so other clients understand the implications of this behavior:
I'll probably want to update the README with this as well.
Here's how I might show a user the error instead of throwing it (from updated ACA branch):
Let me know if you have any additional thoughts / concerns on doing it this way
There was a problem hiding this comment.
Looks good. I think we could get away with just output logging, sans telemetry...once we have it tied up right there's really no reason it should ever go wrong.
| /** | ||
| * List of errors that could occur during the authentication handshake between client extension and Azure Resources host extension. | ||
| */ | ||
| export const AzureResourcesApiRequestErrors = { |
There was a problem hiding this comment.
I think there should be a type here to enforce the shape on the errors.
There was a problem hiding this comment.
Changed this to an enum instead, and the type is defined as part of AzureResourcesApiRequestError below
|
Putting back into draft temporarily, I ended up reworking this logic a little bit after finishing up the test writing. Will take out of draft once I've updated with said changes. |
Overview
This PR adds a secure auth layer that clients should interface with to obtain the Azure Resources core APIs (used for registering branch data resources). For full details on the new auth handshake, please see the auth readme included with this PR. We will continue to publicly provide the core APIs while we let clients onboard. Afterwards we will remove these APIs from the public exports and only expose them through the dedicated auth layer.
Setup
Here are the steps if you would like to try out the changes:
npm install&npm run compilein Azure Resources rootnpm install&npm packin Azure Resources API rootnpm installand add the Azure Resources API package built from step 3onDidReceiveAzureResourcesApiscallback to verify we are getting matching Azure Resources APIsClient Extension Samples
Container Apps example (V2): microsoft/vscode-azurecontainerapps#992
Functions example (internal + V2): microsoft/vscode-azurefunctions#4777
Todos
To add in follow-up PRs.