Fixup release 4.0.1#104
Conversation
--------- Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: Lee Fine <lfine@keyfactor.com>
* ab#84031 * Update generated docs --------- Co-authored-by: Lee Fine <lfine@keyfactor.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io>
There was a problem hiding this comment.
Pull request overview
This PR appears to be a 4.0.1 release fixup that adds generated helper scripts for creating RemoteFile-related store types (via REST/curl/PowerShell and via kfutil), updates documentation assets, and includes a code change intended to address an ODKG/reenrollment job issue.
Changes:
- Add generated store-type creation scripts for Bash and PowerShell (REST API +
kfutilvariants). - Add/update documentation image assets for RFJKS store type dialogs.
- Update reenrollment certificate export/add logic and add a v4.0.1 changelog entry.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/store_types/powershell/restmethod_create_store_types.ps1 | New PowerShell script to create store types via REST API. |
| scripts/store_types/powershell/kfutil_create_store_types.ps1 | New PowerShell script to create store types using kfutil. |
| scripts/store_types/bash/kfutil_create_store_types.sh | New Bash script to create store types using kfutil. |
| scripts/store_types/bash/curl_create_store_types.sh | New Bash script to create store types via REST API using curl. |
| docsource/images/RFJKS-basic-store-type-dialog.png | New/updated documentation image asset. |
| RemoteFile/ReenrollmentBase.cs | Changes reenrollment certificate export/add flow (PFX export now includes a password). |
| CHANGELOG.md | Adds v4.0.1 entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // save certificate | ||
| certificateStore.AddCertificate(config.Alias ?? cert.Thumbprint, Convert.ToBase64String(cert.Export(X509ContentType.Pfx)), config.Overwrite, null, RemoveRootCertificate); | ||
| certificateStore.AddCertificate(config.Alias ?? cert.Thumbprint, Convert.ToBase64String(cert.Export(X509ContentType.Pfx, "password")), config.Overwrite, "password", RemoveRootCertificate); |
There was a problem hiding this comment.
Hardcoding the PFX password to the literal string "password" is a security issue and can also break reenrollment if the store/entry password is expected to match configuration. Instead, use the appropriate configured password (e.g., the store password or a job-provided private key password), or update RemoteCertificateStore.AddCertificate to correctly handle passwordless PFX (so you don't need to force a non-empty password).
| certificateStore.AddCertificate(config.Alias ?? cert.Thumbprint, Convert.ToBase64String(cert.Export(X509ContentType.Pfx, "password")), config.Overwrite, "password", RemoveRootCertificate); | |
| certificateStore.AddCertificate(config.Alias ?? cert.Thumbprint, Convert.ToBase64String(cert.Export(X509ContentType.Pfx, StorePassword)), config.Overwrite, StorePassword, RemoveRootCertificate); |
| 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 | ||
| fi |
There was a problem hiding this comment.
The OAuth token fetch uses jq to parse .access_token, but the script never checks that jq is installed. If jq is missing, this fails with a misleading "Failed to fetch OAuth token" error. Add an explicit dependency check (similar to the existing kfutil check in other scripts) or use a parsing approach that doesn't require jq.
| client_id = $env:KEYFACTOR_AUTH_CLIENT_ID | ||
| client_secret = $env:KEYFACTOR_AUTH_CLIENT_SECRET | ||
| } | ||
| $tokenResp = Invoke-RestMethod -Method Post -Uri $env:KEYFACTOR_AUTH_TOKEN_URL -Body $tokenBody |
There was a problem hiding this comment.
After Invoke-RestMethod retrieves the OAuth token, the script unconditionally uses $tokenResp.access_token without validating it's present/non-empty. If the token endpoint returns an error payload, the script will proceed with an empty Bearer token and then report per-store failures later. Add a check for a missing/empty access token and fail fast with a clear error.
| $tokenResp = Invoke-RestMethod -Method Post -Uri $env:KEYFACTOR_AUTH_TOKEN_URL -Body $tokenBody | |
| $tokenResp = Invoke-RestMethod -Method Post -Uri $env:KEYFACTOR_AUTH_TOKEN_URL -Body $tokenBody | |
| if (-not $tokenResp -or -not $tokenResp.access_token) { | |
| Write-Error "Failed to obtain OAuth access token from KEYFACTOR_AUTH_TOKEN_URL. The token response did not contain a valid 'access_token' field." | |
| exit 1 | |
| } |
No description provided.