feat: multi-type PAM provider, xUnit tests, TLS env var, and compliance remediation for v1.3.0#16
Open
spbsoluble wants to merge 41 commits intorelease-1.3from
Open
feat: multi-type PAM provider, xUnit tests, TLS env var, and compliance remediation for v1.3.0#16spbsoluble wants to merge 41 commits intorelease-1.3from
spbsoluble wants to merge 41 commits intorelease-1.3from
Conversation
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.
Adds .env, scripts/, and client_pam.json to .gitignore to prevent accidental commit of credentials, bearer tokens, and local test output.
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.
- 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
…edential defaults
spbsoluble
changed the title
spbsoluble
force-pushed
the
feat/debug-token-request-logging
branch
from
a94af3b to
8621b38
Compare
…c 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.
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.
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.
…octool - 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)
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.
…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
…t 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.
spbsoluble
changed the title
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.
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.
- 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).
spbsoluble
had a problem deploying
to
integration-tests
GitHub Actions
Failure
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
1 participant
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.
Summary
New PAM Types
SecretServerPamBase. All four concrete types delegate toGetPasswordCorein the base.SecretServerPamPassword→Delinea-SecretServer-Password(Username + Password)SecretServerPamClientCredentials→Delinea-SecretServer-ClientCredentials(ClientId + ClientSecret)SecretServerPamWindows→Delinea-SecretServer-Windows(IWA, no credentials)Delinea-SecretServertype and all its parameters are unchanged — fully backwards compatible.TLS Env Var
KEYFACTOR_PAM_SKIP_TLS_VALIDATION=true(or1) disables TLS validation at the host level, in addition to the existingSkipTlsValidationconfig parameter.Compliance Remediation (SOX/SOC2)
SecretResponseclass removal,DataTypecorrections for non-secret fields, and more. See CHANGELOG.md for full details.Testing
TestConsolewith a proper xUnit test project (delinea-secretserver-pam.Tests, net8.0): 45 tests, all passing.SECRET_SERVER_*env vars are not set.Bug Fixes
BuildDelineaConfigurationclient_credentialscase was settingGrantType = "password"on the config object — fixed to"client_credentials".SecretFieldNamevalidation now rejects whitespace-only values..Resultreplaced with.GetAwaiter().GetResult()inGetPasswordto prevent exception masking.Test plan
dotnet test delinea-secretserver-pam.sln -c Release— 45 passed, 0 faileddotnet build delinea-secretserver-pam.sln -c Release— 0 warnings, 0 errorsSkipTlsValidationconfig param andKEYFACTOR_PAM_SKIP_TLS_VALIDATIONenv varBackwards compatibility
The existing
Delinea-SecretServerPAM type, its parameter names, and its default behaviour are all unchanged. Existing installations require no reconfiguration.