-
Notifications
You must be signed in to change notification settings - Fork 46
Update Lab API Calling Pattern, Fixes AB#3424041 #2809
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
Conversation
|
❌ Work item link check failed. Description does not contain AB#{ID}. Click here to Learn more. |
|
✅ Work item link check complete. Description contains link AB#3424041 to an Azure Boards work item. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the Lab API integration to migrate from a direct Lab API secret retrieval mechanism to Azure Key Vault-based secret management for Azure Function authentication codes. The changes introduce a new KeyVaultSecretsApi to directly retrieve secrets from Azure Key Vault and modify all API classes (ResetApi, EnablePolicyApi, DisablePolicyApi, DeleteDeviceApi, CreateTempUserApi) to require Azure Function codes passed as constructor parameters instead of using no-arg constructors.
Key changes:
- Introduces
KeyVaultSecretsApifor direct Key Vault access with separate authentication - Refactors API classes to require Azure Function codes as constructor parameters
- Updates helper classes to fetch function codes from Key Vault before instantiating APIs
- Adds new model classes
SecretBundleandSecretAttributesfor Key Vault response parsing
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
KeyVaultSecretsApi.java |
New API class for retrieving secrets directly from Azure Key Vault |
SecretBundle.java |
New model class representing Azure Key Vault secret response |
SecretAttributes.java |
New model class representing Key Vault secret metadata |
ResetApi.java, EnablePolicyApi.java, DisablePolicyApi.java, DeleteDeviceApi.java, CreateTempUserApi.java |
Modified to require Azure Function codes as constructor parameters and add them as query parameters |
Configuration.java |
Adds separate API client instances for different base paths (Lab API, user fetch, Key Vault) |
ApiClient.java |
Updates base path constants to support multiple endpoints |
ConfigApi.java |
Updated to use correct API client for user fetch operations |
LabClient.java |
Updated to use Key Vault API for secret retrieval with separate authentication client |
ILabClient.java |
Renamed method from getSecret to getKeyVaultSecret |
PolicyHelper.java, LabUserHelper.java, LabResetHelper.java, LabDeviceHelper.java |
Updated to fetch function codes before API instantiation |
LabAuthenticationHelper.java |
Fixed return type from ConfidentialClientHelper to LabAuthenticationHelper |
LabClientTest.java |
Ignored tests that are now broken due to API changes |
Comments suppressed due to low confidence (1)
testutils/src/main/java/com/microsoft/identity/internal/testutils/labutils/LabUserHelper.java:1
- [nitpick] Three consecutive blank lines (267-269) should be reduced to one or two blank lines maximum for consistent code formatting. Remove the extra blank lines.
// Copyright (c) Microsoft Corporation.
labapi/src/main/java/com/microsoft/identity/internal/test/labapi/api/KeyVaultSecretsApi.java
Outdated
Show resolved
Hide resolved
LabApiUtilities/src/main/com/microsoft/identity/labapi/utilities/client/LabClient.java
Outdated
Show resolved
Hide resolved
testutils/src/main/java/com/microsoft/identity/internal/testutils/labutils/LabDeviceHelper.java
Show resolved
Hide resolved
testutils/src/main/java/com/microsoft/identity/internal/testutils/labutils/LabDeviceHelper.java
Outdated
Show resolved
Hide resolved
testutils/src/main/java/com/microsoft/identity/internal/testutils/labutils/PolicyHelper.java
Outdated
Show resolved
Hide resolved
testutils/src/main/java/com/microsoft/identity/internal/testutils/labutils/LabUserHelper.java
Outdated
Show resolved
Hide resolved
testutils/src/main/java/com/microsoft/identity/internal/testutils/labutils/LabResetHelper.java
Outdated
Show resolved
Hide resolved
testutils/src/main/java/com/microsoft/identity/internal/testutils/labutils/LabDeviceHelper.java
Outdated
Show resolved
Hide resolved
LabApiUtilities/src/main/com/microsoft/identity/labapi/utilities/client/LabClient.java
Show resolved
Hide resolved
LabApiUtilities/src/main/com/microsoft/identity/labapi/utilities/client/LabClient.java
Outdated
Show resolved
Hide resolved
testutils/src/main/java/com/microsoft/identity/internal/testutils/labutils/PolicyHelper.java
Outdated
Show resolved
Hide resolved
p3dr0rv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, it looks good. However, I believe that getKeyVaultSecret should not be part of LabClient, as this causes changes to propagate through multiple layers of the code and results in repetitive patterns. It might be better to add it to the APIClient or another appropriate class.
It might be worth checking with the Copilot agent to see if it has any recommendations on how to avoid this issue.
I think this is fair. reason i have it in LabClient is because we are using it for the purposes of calling these lab apis. If we wanted to call it outside of lab client we can still do that, but we don't really implement ApiClient for anything else and we would have to add that implementation if we wanted to call the function outside of LabClient. Right now we just don't have a reason to |
…4041 (#2398) There are some functions that still don't work from lab api. This PR ignores 4 tests that use these apis. Common PR to fix lab api calls: AzureAD/microsoft-authentication-library-common-for-android#2809 [AB#3424041](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3424041)
The intermediate layer between our client code and the back end Lab api functions is no longer working, working theory is an azure security policy that was enforced. This PR updates our calling pattern to make it so we interface directly with the lab api's backend. The PR also introduces a Key Vault api in our code to access key vault secrets by using the automation certificate.
Validation runs:
https://identitydivision.visualstudio.com/Engineering/_build/results?buildId=1555631&view=results
https://identitydivision.visualstudio.com/Engineering/_build/results?buildId=1555744&view=results
AB#3424041