Add the external key custodian sample - #45
Merged
Conversation
…ey custodian A slim OIDC provider whose signing and encryption keys live in a Vault / OpenBao Transit engine or Azure Key Vault, never in process memory. The client_credentials grant issues service tokens signed inside the custodian; the public-only keys back the JWKS and local verification, and the custodian is selected by configuration. Ships a docker-compose that provisions OpenBao Transit, the Vault and Azure adapters behind IExternalSigner / IExternalKeyEncryptor, and a README with the run and verification steps including the encryption path via introspection. Held until 2.4: references the 2.4 dev package that first carries the external-key seam.
The Azure custodian builds a ClientSecretCredential when TenantId, ClientId and ClientSecret are set in the Azure config section, and falls back to DefaultAzureCredential (managed identity, or the AZURE_* environment variables) otherwise. appsettings carries blank placeholders so the secret stays out of source control. Verified end to end against a live Azure Key Vault: RS256 signing and RSA-OAEP-256 unwrap both work, JWKS is public-only, and the signature checks out against the published key.
A ready-to-run HTTP request collection (token, JWKS, introspection) for the VS Code REST Client, JetBrains IDEs, Postman, Bruno, or Hoppscotch. The token request is named so the introspection call chains its access_token.
Replace the REST Client response-chaining syntax, which JetBrains IDEs do not share, with a plain @accesstoken variable the reader pastes the token into. The file now runs the same way in Visual Studio 2022, Rider / IntelliJ, and VS Code.
Replace the sample's hand-written Vault and Azure custodian code with the Abblix.Oidc.Server.Vault and .Azure packages: one AddVaultExternalKeys or AddAzureExternalKeys call each, selected by configuration, with no custodian code in the sample. The project references the in-development packages locally until 2.4 publishes, when these become package references.
…ySample README Match the consolidated IKeyCustodian: the backend does sign and unwrap by version plus version enumeration by key name, not a single public-key fetch.
The external-key packages split their registration into two: which custodian holds the keys, and how the library uses them. The sample now shows that split, which suits it well: the KeyCustodian setting picks the custodian, and the tier call after it is the same line for either one. Each enum member's name doubles as its configuration section, so the section name is no longer repeated at the call site. The key names move from the Vault and Azure sections to Provider, where they belong: they are not part of a connection, and the same names work whichever custodian holds them. The encryption key is now named only when the sample issues encrypted tokens, since an unnamed one publishes nothing rather than being guessed. The README loses a claim that was wrong: a key name is not the published kid. Transit publishes oidc-sign:1 and Key Vault oidc-sign/<version>, which is what lets a rotation overlap.
The comment read "only needed while EncryptAccessToken is on", which is true here and wrong in general: an encryption key also decrypts inbound JWE a client sent, and that has nothing to do with encrypting our own tokens. The condition itself is right for this sample, since client_credentials sends no request object or client assertion, so the comment now says that rather than stating a rule that does not hold.
Extends ExternalKeySample to demonstrate both key-handling tiers: a UseKeysIn setting picks between keys held in the custodian and keys minted in-process and sealed to it, wired for both Vault (KV ring) and Azure (Blob ring). The compose file provisions the key-encryption key, the README shows the opposite kill-switch behaviour, and the encryption algorithm now comes from configuration. Renames the tier calls to UseKeysInCustodian / UseKeysInProcess, dispatches the tier with an exhaustive switch, and adds the project to GettingStarted.slnx.
Two breakages, one of them long-standing. The project references still pointed at the pre-restructure layout, so the sample stopped building when the library moved its projects under src/. That went unnoticed because nothing builds this sample in CI. The key ring has since moved from Abblix.Oidc.Server into Abblix.Jwt, where it belongs: it mints, seals and rotates JsonWebKeys and knows nothing about being an OpenID Provider. CustodianHeldKeys and MintedKeys came with it, so the sample needs the new namespace alongside the one it already had - the tier calls themselves are unchanged, and so is every line of the sample's own code. Builds clean.
The backend packages are Abblix.Jwt.Vault and Abblix.Jwt.Azure now: they hold keys and know nothing about being an OpenID Provider, so they no longer carry the server's name or reference it. The choice they open is where a key lives rather than a tier - the word implied a ranking these two options do not have. Renamed in the sample's own code and prose to match, so a reader moving between the sample and the library meets one vocabulary. Nothing about what the sample does changed: the same two calls, in the same order, with the same settings. Builds clean.
The sample called AddOidcMinimalApi with an options delegate, and that overload is gone: AddOidcMinimalApi is now the transport alone, while AddOidcServices is core plus transport and takes the options. The MVC package spells the latter identically, which is the point of the name - swapping adapters changes the package reference and the endpoint mapping, not this line. The sample had not built since that change. Verified both ways this time: against the referenced tree, and against a copy repointed at the branch that moves the key placement calls into Abblix.Jwt, since the sample is what says whether that move reaches a consumer. It does, without an edit: the placement calls resolve from Abblix.Jwt.ExternalKeys, which the sample already imports for CustodianHeldKeys and MintedKeys. Also dropped the word tier, which the library no longer uses for the key placement.
The step downloaded a release archive and unpacked it straight into a binary it then ran with the job's permissions, so whatever the network answered was executed. The sum comes from the release's own actionlint_<version>_checksums.txt, and --strict fails the job on a malformed line rather than passing a check that verified nothing. The sum lives beside the version it belongs to, so raising one without the other stops the job instead of silently trusting the download.
…y-sample # Conflicts: # .github/workflows/lint-workflows.yaml
The sample was held against the sibling library checkout and a private feed because the packages carrying the external-key seam had not shipped. They have, so it now references Abblix.Oidc.Server.MinimalApi, Abblix.Jwt.Vault and Abblix.Jwt.Azure at 2.4.0, and the NuGet.config that reached for the private feed is gone. A reader cloning this repository alone can now build it, which the project references made impossible. Restored as a reader would have it, from nuget.org into an empty package folder: eight Abblix packages, all 2.4.0, and the build carries no warnings. Two README claims did not survive being run. The signing header carries "oidc-sign:1" and the encryption header "oidc-enc:1", not the bare key names - the same passage further down already said a bare key name is never a kid, so the two examples contradicted it. Both now print what the server prints, and the encryption example names its "enc" as well.
Four statements in the sample described what it was meant to guarantee rather than what it did, and the fifth left a security-relevant default to chance. The Azure section carried empty TenantId, ClientId and ClientSecret fields while the README said credentials never come from configuration. The package really reads all three, so the file offered a labelled place to put a service-principal secret in a public repository. The fields are gone and the README now says the package supports both paths while the sample configures only the credential chain. The client secret moves to the same shape both sibling providers use: the configuration carries its SHA-512 hash, so no provider here keeps a recoverable secret in a file. Driven both ways - the demo secret still issues a token, a wrong one gets 401 invalid_client. The posture settings are read as nullable and refused when absent. Bound straight to the enum, a missing one took the first member, so deleting the setting picked a security posture by declaration order while the comment beside it promised a startup failure. It fails now, which is what the comment always said. The settings table claimed the Vault token is taken from an environment variable. That spelling is just the section path; the row now says so, and points at the AppRole and Kubernetes sections a deployment uses instead. A launch profile joins the one every other sample here ships, pinning the URL the README uses and naming the environment, because the environment decides what the sealed-custodian failure returns: a bare 500, or the exception and its stack trace naming the custodian key path. The README now says which is which. The Vault token is deliberately not in that file.
No member of the provider's options carries a default any more. A default in code for the client secret's hash is credential material in code, which is the thing this sample exists to argue against, and the rest were a second copy of values appsettings.json already holds - two sources for one value, drifting silently. `required` alone does not achieve that: the configuration binder ignores it and leaves an absent setting null, and the null then surfaces deep inside whatever first reads it, naming a framework parameter. Driven with the hash removed, the failure was `ArgumentNullException ... (Parameter 's')` from the base64 decoder. So the settings are checked explicitly after binding, and a mistyped hash is decoded through a named path. Both refusals were driven: an absent hash and one holding the secret itself each name `Provider:ClientSecretSha512Hash` and say what belongs there. The README gains the command that computes the hash, which was run and reproduces the value in the file. Also from the round: the sentence introducing the hash promised the recoverable secret is in no file the provider ships, and printed it in the next clause; both it and the README row now claim only what is checkable, that the configuration the server loads carries the hash rather than the secret. The compose comment naming all three provisioned keys had landed while the README command beside it still named two, and the Azure paragraph offered a credential path without ever spelling it - the three settings are named now, with no slot for them in any committed file, and the paragraph says the signed-in identity needs Key Vault Crypto User, which stopped being optional once configuration credentials were no longer an alternative.
The list checked up front demanded three names the posture in use never reads: the encryption key only matters with encryption on, the key-encryption key only where the server mints its own keys, and the signing key only where it does not. So the shipped configuration could be refused over a value nothing would have touched, while three sentences elsewhere told the reader those settings are used only in those cases. Each value now passes through one Required call at the point it is consumed, so a setting that stops being read cannot leave a check behind demanding it. Five configurations were driven: with the shipped posture both conditional names may be absent and the server starts; in the minting posture and with encryption on each is demanded by name; the signing key name is demanded where it is read. The comment above it claimed every member is required and none carries a default. Two are optional and say so by not being marked: the encryption toggle, and the algorithm that decides whether a minted encryption key exists at all. Both the comment and the type now name them. The hash command in the README used a base64 flag that exists only in the GNU tool, so it fails on macOS. It goes through openssl instead, which is already in the pipe; driven, and the output is the same single line.
The type summary said the members without `required` are the optional ones, and the comment above the binding said the same. Both are wrong in the same way: the keyword constrains object initializers and the binder ignores it, so it says nothing about which settings a configuration must carry. Two marked members may be absent in the posture the sample ships with, which the driven configurations already showed, and the comment contradicted itself four lines further down. Both texts now state the rule and leave the specifics to each member, where a setting read in one posture only says so - true now of every such member, the signing key name included, which had said nothing about the posture that reads it. The counts are gone with them. A sentence that says how many members are optional ages silently the moment the set changes, and nothing in a build reads it.
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.
A provider whose signing and decryption keys live in an external custodian - a Vault / OpenBao Transit engine or Azure Key Vault - so the private half never enters the application's memory. The server publishes only public JWKs; every private operation is a call out.
The branch was written while the packages carrying that seam were still unreleased, so it pointed at a sibling library checkout and a private feed. Both are gone: it now takes
Abblix.Oidc.Server.MinimalApi,Abblix.Jwt.VaultandAbblix.Jwt.Azureat 2.4.0 from nuget.org, which is the first state in which somebody who clones this repository alone can build it.What it shows
The sample carries no custodian code - the packages do. What it demonstrates is the placement choice, both ways round.
UseKeysIn=Custodiansends every private operation to the custodian and publishes only the public halves.UseKeysIn=Processmints the signing key locally, seals it to a key-encryption key and keeps the sealed ring in the custodian's own store. The kill switch behaves oppositely in each, which is what makes the trade visible rather than described.What was run
Restored the way a reader would have it, with only nuget.org configured and an empty package folder: the Abblix packages resolve at 2.4.0 and the build carries no warnings.
Then the provider was started against the compose file's OpenBao and driven end to end.
client_credentialsreturns an RS256 JWSalgRS256nandepresent,dandpabsentRSA-OAEP-256, introspection returnsactive: truekidis the minted key's thumbprintinvalid_clientTwo README claims did not survive that: the headers carry
oidc-sign:1andoidc-enc:1, not the bare key names. The same page already said further down that a bare key name is never akid, so the two examples contradicted their own explanation. Both now print what the server prints.Configuration carries the values, and the code carries none
No value the sample cannot invent has a default in code. That started with the client secret - a default for its hash is credential material in code, in the one sample whose subject is keeping private material out of the application - and extends to the rest, because a default beside
appsettings.jsonis a second copy that drifts.requireddoes not achieve this on its own: the configuration binder ignores it, and an absent setting then surfaces deep inside whatever reads it first, naming a framework parameter. So each value is checked where it is read, by a call that names the section and the member. Checking them in a list up front was tried and removed: several are read in one posture only, and the list refused configurations that were complete for the posture in use.The
Azuresection carries no credential fields. The package accepts a service principal there, and the README says so and names the settings, but a sample is not the place to ship a labelled slot for a secret.The sealed-custodian failure
The token endpoint answers 500. RFC 6749 section 5.2 lists the error codes that endpoint may return and an unreachable key store is none of them;
server_errorbelongs to the authorization endpoint, which needs it because a 500 cannot travel through a redirect. What the body carries depends on the environment, and the README now says so: the launch profile selects Development, where the exception and its stack trace are returned, while Production answers a bare 500. Neither is a shaped error, and that is the library's behaviour rather than the sample's.Also here
masterwas merged in. Both sides had independently added a checksum verification for the actionlint download;master's is on the newer version, so its side was kept whole.