Merge 3.2.3 to main#80
Merged
Merged
Conversation
…should not be exportable from KeyVault Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io>
Added entry parameter to indicate the private key should not be exportable from KeyVault Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> --------- Co-authored-by: Joe VanWanzeele <76071503+joevanwanzeeleKF@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> * cleaned up docs, split RBAC permissions into seperate file for brevity * Update generated docs * Updated changelog, nuget package references * Explicit update of Newtonsoft.Json.Bson from 1.0.2 (used by Microsoft.AspNet.WebApi.Client) to 1.0.3 to address vulnerability --------- Co-authored-by: Morgan Gangwere <470584+indrora@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io>
* Merge 3.2.0 to main (#74) * chore: create 3.2 branch * feat: release 3.2, Added entry parameter to indicate the private key should not be exportable from KeyVault Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> --------- Co-authored-by: Joe VanWanzeele <76071503+joevanwanzeeleKF@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> * cleaned up docs, split RBAC permissions into seperate file for brevity * Update generated docs * Updated changelog, nuget package references * Explicit update of Newtonsoft.Json.Bson from 1.0.2 (used by Microsoft.AspNet.WebApi.Client) to 1.0.3 to address vulnerability * now returning the serialized certificate tags, as well as the exportable flag. Updated README image * Update generated docs --------- Co-authored-by: Morgan Gangwere <470584+indrora@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io>
* Merge 3.2.0 to main (#74) * chore: create 3.2 branch * feat: release 3.2, Added entry parameter to indicate the private key should not be exportable from KeyVault Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> --------- Co-authored-by: Joe VanWanzeele <76071503+joevanwanzeeleKF@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> * cleaned up docs, split RBAC permissions into seperate file for brevity * Update generated docs * Updated changelog, nuget package references * Explicit update of Newtonsoft.Json.Bson from 1.0.2 (used by Microsoft.AspNet.WebApi.Client) to 1.0.3 to address vulnerability * now returning the serialized certificate tags, as well as the exportable flag. Updated README image * added check for vault name parameter coming through as empty string. async safety. naming cleanup. * Update generated docs * replaced count() with count * Added helper method to determine key sized and curve in order to pass the value to AKV (the fix) * cleanup * Added unit tests * update changelog --------- Co-authored-by: Morgan Gangwere <470584+indrora@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io>
There was a problem hiding this comment.
Pull request overview
Automated merge of release-3.2 into main, primarily updating the Azure Key Vault orchestrator to enrich certificate import behavior (key metadata + exportability), extend store type entry parameters, and refresh accompanying documentation/scripts.
Changes:
- Added
NonExportableentry parameter end-to-end (manifest, scripts, docs) and surfaced entry parameters in inventory results. - Updated certificate import to derive key type/size from PFX and apply an explicit
CertificatePolicywhen importing into Azure Key Vault. - Restructured documentation (RBAC guidance split to
rbac.md, README updates) and added a dedicated unit test project.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/store_types/powershell/restmethod_create_store_types.ps1 | New PowerShell REST script for creating the AKV store type. |
| scripts/store_types/powershell/kfutil_create_store_types.ps1 | New kfutil-based PowerShell helper for creating the AKV store type. |
| scripts/store_types/bash/kfutil_create_store_types.sh | New kfutil-based bash helper for creating the AKV store type. |
| scripts/store_types/bash/curl_create_store_types.sh | New curl-based bash helper for creating the AKV store type (OAuth/basic auth). |
| README.md | Major documentation edits (RBAC guidance, discovery instructions, entry parameter docs, install matrix). |
| rbac.md | New standalone RBAC guidance and sample role definitions. |
| integration-manifest.json | Adds NonExportable entry parameter definition. |
| docsource/content.md | Mirrors README updates for generated docs. |
| CHANGELOG.md | Adds 3.2.x entries and notes key-size import fix. |
| AzureKeyVault/Jobs/Management.cs | Adds NonExportable handling and updates logging + sync-over-async calls. |
| AzureKeyVault/Jobs/Inventory.cs | Updates logging + sync-over-async call style; wording fixes. |
| AzureKeyVault/Jobs/Discovery.cs | Updates logging + count usage and truncation condition. |
| AzureKeyVault/Jobs/AzureKeyVaultJob.cs | Makes InitializeStore virtual; logger rename; adds InternalsVisibleTo for tests. |
| AzureKeyVault/Helpers.cs | Returns PKCS#12 bytes plus derived key type/size for imports. |
| AzureKeyVault/Constants.cs | Adds NON_EXPORTABLE constant. |
| AzureKeyVault/AzureKeyVault.csproj | Dependency updates and additions. |
| AzureKeyVault/AzureClient.cs | Applies explicit import policy (exportable/content type/key type/key size); returns entry params in inventory. |
| AzureKeyVault/AkvProperties.cs | Adjusts accessibility for internal properties. |
| AzureKeyVault.Tests/* | New unit test project validating management, discovery, inventory, and PKCS#12 conversion behavior. |
| AzureKeyVault.Tests/AzureKeyVault.Tests.csproj | New test project configuration and dependencies. |
| AzureKeyVault.sln | Adds rbac.md to solution items and includes the new test project. |
Comments suppressed due to low confidence (9)
README.md:88
- The Markdown link target
learn.microsoft.com/...is missing thehttps://scheme, which makes it a relative link on GitHub. Usehttps://learn.microsoft.com/...so the reference resolves correctly.
management operations. For more information and a comparison of the two access control strategies, refer
to [this article](learn.microsoft.com/en-us/azure/key-vault/general/rbac-access-policy).
README.md:596
- The compatibility matrix table row for
11.0.0–11.5.1/net8.0is malformed: two rows appear to have been concatenated with||on a single line, which will break the Markdown table rendering. Split this back into two separate table rows.
| --------- | ----------- | ----------- | ----------- |
| Older than `11.0.0` | | | `net6.0` |
| Between `11.0.0` and `11.5.1` (inclusive) | `net6.0` | | `net6.0` |
| Between `11.0.0` and `11.5.1` (inclusive) | `net8.0` | `Disable` | `net6.0` || Between `11.0.0` and `11.5.1` (inclusive) | `net8.0` | `LatestMajor` | `net8.0` |
| `11.6` _and_ newer | `net8.0` | | `net8.0` |
README.md:563
- The example JSON for tags uses single quotes (e.g.,
{'tag-name': 'tag-content'}), which is not valid JSON. Update the example to use double quotes so users can copy/paste a valid JSON object.
##### Certificate Tags
If desired, tags can be applied to the KeyVault entries. Provide them as a JSON string of key-value pairs ie: '{'tag-name': 'tag-content', 'other-tag-name': 'other-tag-content'}'
docsource/content.md:42
- Typo: “inheretable” should be “inheritable”.
Based Access Control (RBAC) and the classic Access Policy method. RBAC is the preferred method, and currently the default used by Azure.
It allows the assignment of granular level, inheretable access control on both the contents of the KeyVaults, as well as higher-level
management operations. For more information and a comparison of the two access control strategies, refer
docsource/content.md:44
- The Markdown link target
learn.microsoft.com/...is missing thehttps://scheme, so it will render as a relative link. Usehttps://learn.microsoft.com/...to ensure it resolves correctly.
management operations. For more information and a comparison of the two access control strategies, refer
to [this article](learn.microsoft.com/en-us/azure/key-vault/general/rbac-access-policy).
rbac.md:105
- The built-in role links use
github.com/...without a URL scheme, which makes them relative links in Markdown. Prefix these withhttps://(or switch to the corresponding learn.microsoft.com built-in roles pages) so the links work.
- built-in
role: ["Key Vault Reader"](github.com/MicrosoftDocs/azure-docs/blob/main/articles/role-based-access-control/built-in-roles/security.md#key-vault-reader)
- lowest level scope - a resource group
rbac.md:150
- This Markdown link is missing the
https://scheme (github.com/...), so it will render as a relative link. Prefix withhttps://(or use a learn.microsoft.com URL) so it works.
- built-in
role: ["Key Vault Certificates Officer"](github.com/MicrosoftDocs/azure-docs/blob/main/articles/role-based-access-control/built-in-roles/security.md#key-vault-certificates-officer)
- lowest level scope - an individual keyvault
rbac.md:205
- Several links in this section are missing the
https://scheme (bothgithub.qkg1.top/...andlearn.microsoft.com/...), which will make them relative and break navigation. Prefix these URLs withhttps://so they resolve correctly.
- minimally sufficient built-in roles (all are required):
- ["Key Vault Certificates Officer"](github.com/MicrosoftDocs/azure-docs/blob/main/articles/role-based-access-control/built-in-roles/security.md#key-vault-certificates-officer)
- ["Key Vault Contributor"](learn.microsoft.com/en-us/azure/role-based-access-control/built-in-roles/security#key-vault-contributor)
- ["Key Vault Access Administrator"](learn.microsoft.com/en-us/azure/role-based-access-control/built-in-roles/)
scripts/store_types/bash/curl_create_store_types.sh:84
- The
CertificateTagsdescription shows a JSON example using single quotes ({'tag-name': 'tag-content'}), which is not valid JSON. Update the description string to show a valid example with double quotes to avoid confusing users.
"Name": "CertificateTags",
"DisplayName": "Certificate Tags",
"Description": "If desired, tags can be applied to the KeyVault entries. Provide them as a JSON string of key-value pairs ie: '{'tag-name': 'tag-content', 'other-tag-name': 'other-tag-content'}'",
"Type": "string",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| tagsJSON = config.JobProperties[EntryParameters.TAGS] as string ?? string.Empty; | ||
| preserveTags = config.JobProperties[EntryParameters.PRESERVE_TAGS] as bool? ?? false; | ||
| preserveTags = config.JobProperties[EntryParameters.PRESERVE_TAGS] as bool? ?? true; |
| 1) [Configure the Azure Keyvault for client access](#configure-the-azure-keyvault-for-client-access) | ||
|
|
||
| 1) [Create the Store Type in Keyfactor](#create-the-akv-certificate-store-type) | ||
| 1) [Create the Store Type in Keyfactor](#AKV-Certificate-Store-Type) |
| Based Access Control (RBAC) and the classic Access Policy method. RBAC is the preferred method, as it allows the | ||
| assignment of granular level, inheretable access control on both the contents of the KeyVaults, as well as higher-level | ||
| Based Access Control (RBAC) and the classic Access Policy method. RBAC is the preferred method, and currently the default used by Azure. | ||
| It allows the assignment of granular level, inheretable access control on both the contents of the KeyVaults, as well as higher-level |
| 1) [Configure the Azure Keyvault for client access](#configure-the-azure-keyvault-for-client-access) | ||
|
|
||
| 1) [Create the Store Type in Keyfactor](#create-the-akv-certificate-store-type) | ||
| 1) [Create the Store Type in Keyfactor](#AKV-Certificate-Store-Type) |
| ### Configure Role Based Access Control (RBAC) | ||
|
|
||
| In order to illustrate the minimum permissions that the authenticating entity (service principal or managed identity) | ||
| requires we have created 3 seperate custom role definitions that you can use as a reference when creating an RBAC role definition |
Comment on lines
+255
to
+256
| > :warning: You still may decide to split the capabilities into seperate roles in order to apply each of them to the | ||
| > lowest level scope |
Comment on lines
+28
to
+37
| elif [ -n "${KEYFACTOR_AUTH_CLIENT_ID}" ] && [ -n "${KEYFACTOR_AUTH_CLIENT_SECRET}" ] && [ -n "${KEYFACTOR_AUTH_TOKEN_URL}" ]; then | ||
| echo "Fetching OAuth token..." | ||
| BEARER_TOKEN=$(curl -s -X POST "${KEYFACTOR_AUTH_TOKEN_URL}" \ | ||
| -H "Content-Type: application/x-www-form-urlencoded" \ | ||
| --data-urlencode "grant_type=client_credentials" \ | ||
| --data-urlencode "client_id=${KEYFACTOR_AUTH_CLIENT_ID}" \ | ||
| --data-urlencode "client_secret=${KEYFACTOR_AUTH_CLIENT_SECRET}" | jq -r '.access_token') | ||
| if [ -z "${BEARER_TOKEN}" ] || [ "${BEARER_TOKEN}" = "null" ]; then | ||
| echo "ERROR: Failed to fetch OAuth token from ${KEYFACTOR_AUTH_TOKEN_URL}" | ||
| exit 1 |
| { | ||
| "Name": "CertificateTags", | ||
| "DisplayName": "Certificate Tags", | ||
| "Description": "If desired, tags can be applied to the KeyVault entries. Provide them as a JSON string of key-value pairs ie: '{'tag-name': 'tag-content', 'other-tag-name': 'other-tag-content'}'", |
|
|
||
| - 3.2.0 | ||
| - Added an optional entry parameter to indicate whether the private key of the cert should be not exportable when stored in KeyVault | ||
| - Now specifying the pkcs12 format when wirting certs to Azure KeyVault. This should prevent the error when a PEM cert was added outside of Command and then we attempt to update without specifying the format (Azure assumes PEM and throws an error if not). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge release-3.2 to main - Automated PR