Skip to content

Commit bf30562

Browse files
indroraspbsolubleKeyfactor
authored
feat: multi-type PAM provider, xUnit tests, TLS env var, and compliance remediation for v1.3.0 (#16) (#20)
* feat(logging): Log obfuscated token request body and raw error response Logs the token endpoint request body (with password/client_secret redacted) and the raw response body on non-2xx status codes to aid troubleshooting of authentication failures without exposing credentials. * chore(security): Exclude sensitive local files from version control Adds .env, scripts/, and client_pam.json to .gitignore to prevent accidental commit of credentials, bearer tokens, and local test output. * chore(deps): Bump TestConsole target framework and global SDK to .NET 10 * refactor(config): Remove dead IValidatableObject implementation Validate() was never invoked — validation is enforced imperatively in ValidateServerConfigurationParams() and ValidateInstanceParams(). Calling Validator.TryValidateObject() would also incorrectly fail for the client_credentials flow (which stores GrantType="password" internally to satisfy the Delinea API). Data annotation attributes are retained for documentation value. * feat(logging): Add structured audit logging for SOX/SOC2 compliance - Log caller identity (Environment.UserName, MachineName), SecretId, field name, target URL, and grant type on every GetPassword invocation - Record API call duration (Stopwatch) for both token and secret endpoints - Include SecretId, field, grant type, and URL in success and failure log events - Improve auth failure log with URL, grant type, and caller identity context - Truncate Secret Server error response bodies to 500 chars before logging - Remove raw token response body from deserialization failure log path - Switch .Result to .GetAwaiter().GetResult() to avoid exception masking * fix(test-console): Mask password value in test output * docs(changelog): Update v1.3.0 changelog with compliance logging improvements * fix(logging): Throw on missing secret field instead of returning empty string * fix(logging): Truncate token endpoint error body before logging * fix(logging): Add authentication attempt log event for Windows auth path * feat(logging): Add authentication success log event with caller identity * feat(logging): Thread correlation ID through all PAM operation log events * fix(logging): Capture HTTP call duration in exception paths * refactor: Remove duplicate SecretResponse class * fix(manifest): Set Username and ClientId to non-secret DataType * fix(test-console): Require environment variables, remove hardcoded credential defaults * docs(logging): Document Environment.UserName OS identity limitation * docs(changelog): Add compliance remediation items to v1.3.0 changelog * docs(changelog): Remove audit severity labels from compliance remediation entries * docs: Clean up changelog and document SkipTlsValidation in DelineaConfiguration * docs(readme): Set Username and ClientId to DataType 1 in manifest example * chore(docs): Update CHANGELOG.md * refactor(pam): extract SecretServerPamBase and add grant-type-specific PAM types Extract all shared HTTP, validation, and secret-retrieval logic into an abstract base class SecretServerPamBase. Add three concrete subclasses — SecretServerPamPassword, SecretServerPamClientCredentials, and SecretServerPamWindows — each implementing IPAMProvider with a hardcoded grant type. The existing SecretServerPam class is unchanged (backwards compatible). Also fixes a pre-existing bug where BuildDelineaConfiguration set GrantType = "password" on the DelineaConfiguration returned for the client_credentials case; it now correctly sets "client_credentials". SecretFieldName validation is tightened to reject whitespace-only values (previously only empty string was rejected). InternalsVisibleTo("delinea-secretserver-pam.Tests") added via AssemblyInfo.cs to allow the xUnit test project to reach the internal test constructors. * test: add xUnit test project covering all four PAM types and auth flows Adds delinea-secretserver-pam.Tests (net8.0) with 38 tests covering: - Happy path for password, client_credentials, and windows grant types - Missing required server and instance parameters - Non-success HTTP responses from token and secret endpoints - Empty/null token responses - Field-not-found in secret response - Token request body field name verification (Delinea API constraint: client_credentials still uses username/password key names) - Windows auth correctly targets winauthwebservices endpoint and never calls the token endpoint - All four PAM type Names are distinct TestHttpMessageHandler fake allows request interception without network access. * feat(manifest): register three new grant-type-specific PAM types integration-manifest.json: add Delinea-SecretServer-Password, Delinea-SecretServer-ClientCredentials, and Delinea-SecretServer-Windows PAM type blocks. Each exposes only the fields relevant to its auth flow, removing the Command UI requirement to fill in irrelevant credentials. manifest.json: add InitializationInfo example blocks for the three new types alongside the existing Delinea-SecretServer block. CHANGELOG.md and README.md updated to document all four types, including recommended usage guidance and kfutil commands for the new variants. * Update generated docs * docs: add docsource files for new PAM types and regenerate docs via doctool - Add docsource/overview.md documenting all four PAM types - Add per-type docsource files for Password, ClientCredentials, Windows variants - Update docsource/delinea-secretserver.md for backwards-compat type - Remove deprecated readme-src/ directory - Regenerate README.md and docs/ via doctool (adam_dotNetVpython_Fixes branch) * Update generated docs * feat: support KEYFACTOR_PAM_SKIP_TLS_VALIDATION environment variable Allows TLS certificate validation to be disabled via environment variable in addition to the existing SkipTlsValidation configuration parameter. Either setting is sufficient to disable validation; the env var does not need to be set if the config parameter is already true. * test: add tests for SkipTlsValidation config param and KEYFACTOR_PAM_SKIP_TLS_VALIDATION env var - SkipTlsValidation config param: verified succeeds when set to true - KEYFACTOR_PAM_SKIP_TLS_VALIDATION env var: verified true and 1 both enable skip - Env var set to false does not interfere when config param is also false - client_credentials token body: asserts grant_type=password (Delinea API constraint) - Integration-tested both TLS skip paths against live Secret Server instance * Update generated docs * test: add integration tests that skip gracefully when env vars are not set Adds IntegrationFactAttribute which sets Skip at attribute construction time if any of the required SECRET_SERVER_* env vars are absent, producing a clean skip rather than a failure in CI environments without live server access. * chore(changelog): merge v1.4.0 entries into v1.3.0 * feat: treat N/A as empty input for all config and instance parameters Adds NormalizeConfig which maps 'N/A' (case-insensitive, whitespace-trimmed) to empty string before validation runs, so users can enter N/A as a dummy value in the Keyfactor Command UI for fields irrelevant to their auth flow. * docs: document N/A dummy value support in overview and changelog * Update generated docs * test(integration): add live-server test for N/A dummy value passthrough Verifies that N/A dummy values in connection config fields irrelevant to the password auth flow are stripped by NormalizeConfig before validation and do not prevent secret retrieval from a live Secret Server instance. * ci: add dotnet-ci workflow and Terraform config for GitHub environment - Add .github/workflows/dotnet-ci.yml: unit-test job runs on every PR/push with no env vars (integration tests auto-skip); integration-test job is gated by vars.INTEGRATION_TESTS_ENABLED and uses the integration-tests GitHub environment provisioned by Terraform. - Add terraform/ config backfilling the existing repo, team access, and branch ruleset; creates the integration-tests environment with SECRET_SERVER_* secrets and INTEGRATION_TESTS_ENABLED repo variable. - State stored in Azure Blob (kfghtfstatesn84ro / tfstate container). * chore: remove terraform/ from version control (gitignore) * fix(ci): add Keyfactor GitHub Packages NuGet feed before build * ci: trigger workflow run for TLS validation test --------- Co-authored-by: spb <1661003+spbsoluble@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io>
1 parent ba3fb27 commit bf30562

29 files changed

Lines changed: 3018 additions & 836 deletions

‎.github/workflows/dotnet-ci.yml‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
name: Build and Test
2+
3+
on:
4+
pull_request:
5+
push:
6+
branches:
7+
- main
8+
- release-*
9+
10+
jobs:
11+
unit-test:
12+
name: Build and Unit Test
13+
runs-on: ubuntu-latest
14+
steps:
15+
- uses: actions/checkout@v4
16+
17+
- uses: actions/setup-dotnet@v4
18+
with:
19+
dotnet-version: '10.x'
20+
21+
- name: Add Keyfactor NuGet feed
22+
run: dotnet nuget add source https://nuget.pkg.github.com/Keyfactor/index.json -n github -u ${{ github.actor }} -p ${{ secrets.GITHUB_TOKEN }} --store-password-in-clear-text
23+
24+
- name: Build
25+
run: dotnet build delinea-secretserver-pam.sln -c Release
26+
27+
- name: Unit Test
28+
# No SECRET_SERVER_* env vars set — IntegrationFactAttribute auto-skips integration tests
29+
run: dotnet test delinea-secretserver-pam.Tests/delinea-secretserver-pam.Tests.csproj --no-build -c Release --logger "console;verbosity=normal"
30+
31+
integration-test:
32+
name: Integration Test
33+
runs-on: ubuntu-latest
34+
needs: unit-test
35+
# Skipped entirely when the environment hasn't been provisioned via Terraform
36+
if: vars.INTEGRATION_TESTS_ENABLED == 'true'
37+
environment: integration-tests
38+
steps:
39+
- uses: actions/checkout@v4
40+
41+
- uses: actions/setup-dotnet@v4
42+
with:
43+
dotnet-version: '10.x'
44+
45+
- name: Add Keyfactor NuGet feed
46+
run: dotnet nuget add source https://nuget.pkg.github.com/Keyfactor/index.json -n github -u ${{ github.actor }} -p ${{ secrets.GITHUB_TOKEN }} --store-password-in-clear-text
47+
48+
- name: Build
49+
run: dotnet build delinea-secretserver-pam.sln -c Release
50+
51+
- name: Integration Test
52+
run: dotnet test delinea-secretserver-pam.Tests/delinea-secretserver-pam.Tests.csproj --no-build -c Release --logger "console;verbosity=normal"
53+
env:
54+
SECRET_SERVER_URL: ${{ secrets.SECRET_SERVER_URL }}
55+
SECRET_SERVER_USERNAME: ${{ secrets.SECRET_SERVER_USERNAME }}
56+
SECRET_SERVER_PASSWORD: ${{ secrets.SECRET_SERVER_PASSWORD }}
57+
SECRET_SERVER_SECRET_ID: ${{ secrets.SECRET_SERVER_SECRET_ID }}
58+
SECRET_SERVER_SKIP_TLS_VALIDATION: ${{ vars.SECRET_SERVER_SKIP_TLS_VALIDATION }}
59+
KEYFACTOR_PAM_SKIP_TLS_VALIDATION: ${{ vars.SECRET_SERVER_SKIP_TLS_VALIDATION }}

‎.gitignore‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,7 @@ obj/
44
riderModule.iml
55
/_ReSharper.Caches/
66
.idea/*
7-
.vs/
7+
.vs/
8+
.env
9+
scripts/
10+
client_pam.jsonterraform/

‎CHANGELOG.md‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,55 @@
1+
# v1.3.0
2+
3+
## Features
4+
5+
- Added three grant-type-specific PAM type variants. Each type exposes only the fields relevant to its authentication flow, resolving the Keyfactor Command UI requirement that all declared fields be populated.
6+
- `Delinea-SecretServer-Password` — Username + Password authentication. Server parameters: `Host`, `Username`, `Password`, `SkipTlsValidation`.
7+
- `Delinea-SecretServer-ClientCredentials` — OAuth2 client credentials flow. Server parameters: `Host`, `ClientId`, `ClientSecret`, `SkipTlsValidation`.
8+
- `Delinea-SecretServer-Windows` — Integrated Windows Authentication (IWA). Server parameters: `Host`, `SkipTlsValidation`. NOTE: IWA is not supported on Secret Server Cloud.
9+
- All shared logic (HTTP, validation, secret retrieval, audit logging) is implemented once in the new `SecretServerPamBase` abstract class.
10+
- The existing `Delinea-SecretServer` type is unchanged and fully backwards compatible.
11+
- TLS certificate validation can now be disabled via the `KEYFACTOR_PAM_SKIP_TLS_VALIDATION` environment variable (`true` or `1`), in addition to the existing `SkipTlsValidation` configuration parameter. Either setting is sufficient to disable validation.
12+
- Parameter values of `N/A` (case-insensitive, whitespace-trimmed) are now treated as empty/absent. This allows users of the backwards-compatible `Delinea-SecretServer` type to enter `N/A` as a dummy value in the Keyfactor Command UI for fields that are not applicable to their authentication flow.
13+
14+
## Compliance Remediation (SOX/SOC2)
15+
16+
- `GetDelineaSecretAsync` now throws `InvalidSecretConfigurationException` when the requested field is not found in the secret, rather than silently returning an empty string. This prevents silent credential resolution failures from going undetected.
17+
- Token endpoint error response body is now truncated to 500 characters before logging to prevent secret metadata exposure in log sinks.
18+
- Added an explicit `LogInformation` audit event for the Windows authentication path recording OS identity, machine name, target URL, and SecretId before the HTTP call is made.
19+
- Added an authentication success `LogInformation` event in `GetAccessToken` recording the caller identity and target URL with a structured `AuthenticationResult=Success` field.
20+
- A `Guid`-based correlation ID is generated at the start of each `GetPassword` invocation and threaded as a trailing structured field through all `LogInformation` and `LogError` calls in `GetDelineaSecretAsync` and `GetAccessToken`, enabling log correlation across a full PAM operation.
21+
- `Stopwatch` instances for the token POST and secret GET HTTP calls are now declared outside their try blocks; catch blocks record elapsed duration and emit a structured `HTTP call failed` log event so network failure timing is preserved in exception paths.
22+
- Removed the duplicate `SecretResponse` class defined inline at the bottom of `SecretServerPam.cs`. The canonical definition in `Models/SecretResponse.cs` (which includes `Id`, `Name`, `SecretTemplateId`, `FolderId`, and `Active` in addition to `Items`) is now the sole definition, resolved via the existing `using Keyfactor.Extensions.Pam.Delinea.Models;` import.
23+
- `Username` and `ClientId` parameters in `integration-manifest.json` changed from `DataType: 2` (secret/masked) to `DataType: 1` (plain text). These are non-secret identifiers and should not be stored or displayed as secrets in the Keyfactor Command UI.
24+
- Added inline comments at each `Environment.UserName` usage site documenting that this value reflects the OS service account identity, not the Keyfactor Command caller identity, since `IPAMProvider` does not expose caller context.
25+
26+
## Improvements
27+
28+
- Enhanced debug logging for token endpoint requests: the obfuscated request body (credentials redacted) and raw response body are now logged on token request failures to aid troubleshooting.
29+
- Added structured audit log event on every `GetPassword` invocation recording caller identity, machine name, target URL, grant type, SecretId, and field name.
30+
- Added response duration logging (ms) for both the OAuth token endpoint and secret retrieval API calls.
31+
- Success and failure log events now include SecretId, field name, grant type, and URL for complete audit trail.
32+
- Auth failure log events now include the target URL, grant type, and caller identity.
33+
- Error responses from Secret Server are truncated to 500 characters before logging to prevent sensitive metadata exposure.
34+
- Removed raw token response body from deserialization failure log path to prevent accidental bearer token exposure.
35+
36+
## Bug Fixes
37+
38+
- Fixed `client_credentials` case in `BuildDelineaConfiguration` where `GrantType` was incorrectly set to `"password"` instead of `"client_credentials"` on the resulting `DelineaConfiguration` object.
39+
- Validation of `SecretFieldName` now rejects whitespace-only values (previously only empty string was rejected).
40+
- Replaced `.Result` with `.GetAwaiter().GetResult()` in `GetPassword` to prevent exception masking on async task failures.
41+
42+
## Testing
43+
44+
- Replaced the manual `TestConsole` project with a proper `xUnit` test project (`delinea-secretserver-pam.Tests`, targeting `net8.0`) covering all four PAM types, all auth flows, and error paths including missing parameters, token failures, field-not-found, and non-success HTTP responses.
45+
- Integration tests skip automatically when `SECRET_SERVER_*` environment variables are not set.
46+
47+
## Maintenance
48+
49+
- Removed dead `IValidatableObject` implementation from `DelineaConfiguration`; validation is enforced in `ValidateServerConfigurationParams`.
50+
- Masked password value in TestConsole output.
51+
- Bumped TestConsole target framework to net10.0 and global SDK pin to 10.0.0.
52+
153
# v1.2.0
254

355
## Features
@@ -9,4 +61,4 @@
961
- Added support for `client_credential` grant type for OAuth2 authentication.
1062

1163
# v1.0.0
12-
- Initial release of Delinea SecretServer PAM Provider
64+
- Initial release of Delinea SecretServer PAM Provider

0 commit comments

Comments
 (0)