Add endpoint to list (about-to-)expired wallet credentials#4224
Add endpoint to list (about-to-)expired wallet credentials#4224reinkrul wants to merge 9 commits into
Conversation
Adds GET /internal/vcr/v2/holder/expiring which aggregates credentials across all wallets on the node and returns a JSON object grouping expiring credentials by subject ID. Operators can poll a single URL to monitor and refresh credentials before they expire (closes #4217). The response is a focused monitoring DTO (id, holder, issuer, type, expirationDate) rather than the raw VC, so the shape stays uniform regardless of whether the underlying credential is JSON-LD or JWT-encoded. Assisted by AI
reinkrul
requested review from
gerardsn,
stevenvegt and
woutslakhorst
as code owners
5 new issues
|
|
TODO / open question — filtering by credential type: Some credential types are expected to expire and shouldn't trigger refresh alerts, but operators still want to keep them in the wallet for audit / paper-trail purposes (e.g. We may want to add type-based filtering to this endpoint, e.g. Assisted by AI |
|
Coverage Impact ⬇️ Merging this pull request will decrease total coverage on Modified Files with Diff Coverage (5)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
stevenvegt
left a comment
stevenvegt
left a comment
There was a problem hiding this comment.
Didn't look into to the implementation yet, first lets discuss the API and how people might want to use this feature.
| summary: List credentials across all wallets on this node that are expired or about to expire. | ||
| description: | | ||
| Returns all credentials held by any subject on this node whose `expirationDate` is at or before | ||
| `now + within`. This includes credentials that are already expired. Credentials without an |
There was a problem hiding this comment.
Since eOverdracht (and others) uses NutsAuthVCs, this list will quickly become longer and longer. I think you might want to add some additional filtering here to include or exclude certain types. Also a param to ignore already expired VCs might be a good idea since you probably want to signal for upcoming expiration?
There was a problem hiding this comment.
Since eOverdracht (and others) uses NutsAuthVCs, this list will quickly become longer and longer. I think you might want to add some additional filtering here to include or exclude certain types.
That's right, we need something like that. I'm leaning towards an exclude model, because an include model quickly becomes out of date if a new credential is introduced. So you'd have something like:
/expiring?within=30d&exclude=NutsAuthorizationCredential
The downside is that you have to explicitly (in many cases, always) exclude certain types, but at least it'll be visible for operators if the configuration is off.
Also a param to ignore already expired VCs might be a good idea since you probably want to signal for upcoming expiration?
Maybe... You don't want to keep being bothered if you don't clean up expired VCs, on the other hand you could've missed/ignored (and forgot) about renewing it. We could make it more flexible (at the cost of a more complex API), by adding a parameter which specifies for how long we'll keep returning it, after it expired. E.g., return VCs that expired less than a week ago.
Now I think of it, you also don't want to send e-mails (if your monitoring system does that) every day for the same VC, 30 days straight (if you're checking for VCs that expire within 30 days, every day). Not sure if we should solve that here, but it complicates things.
Proposal: keep this feature simple at first;
- Add
excludeTypesparameter - Let the monitoring system deal with not sending too many notifications for the same VC every hour/day (we're not building a monitoring system here, just feeding it with data)
There was a problem hiding this comment.
Thinking about it, lets include expired credentials, since those should be visible so vendors can clean them up.
|
We also need to add this to the official monitoring documentation. |
|
We'll be adding |
Allows operators to suppress credentials that are expected to expire and are kept for audit purposes (e.g. NutsAuthorizationCredential). A credential is excluded if any of its types matches any supplied value. Assisted by AI
TODO: push filtering down to the SQL storeNeither For larger wallets this means a full load + JSON parse of every credential of every subject on the node on every call. Both filters should be propagated all the way down to the SQL store before this merges. What that needs:
Assisted by AI |
|
Given the previous comment, I think this PR needs some more work? I've converted it to state "draft". |
Replaces Wallet.SearchCredential with Wallet.Search(opts...) that filters on holder_did, type, and a new expiration_date column directly in SQL. GetExpiringCredentialsInWallet now issues one cross-wallet query instead of looping per holder DID and filtering in Go. - New credential.expiration_date column (epoch seconds) + index. - CredentialRecord.ExpirationDate populated by Store(); idempotent Go-side backfill on startup for rows stored before the column existed (TODO: remove in v7). - Wallet.Search options: HolderDID, ExcludeCredentialTypes, ExpiresAt. - Handler groups results back into subjects via an inverse holderDID → subjectID map; credentials whose holder no longer maps to a subject (post-deactivation) are silently skipped. Assisted by AI
- Spell out accepted duration units on `within` and use a non-default example. - Add monitoring documentation for the expiring wallet credentials endpoint. Assisted by AI
The substring pre-filter (raw LIKE '%expirationDate%') skipped JWT VCs, whose expiry lives in a base64 `exp` claim that never appears as literal text. Parse every NULL-expiration row instead, paginating by id as a keyset cursor so never-expiring rows (which stay NULL) don't stall or short-circuit the walk. Assisted by AI
|
@stevenvegt — design question on the
Background: we landed on this after dropping an earlier Given the general boot-time issues we've had lately, do you think we should reconsider? Two options, each with a real cost:
Or we keep it as-is and accept the recurring scan, betting credential tables stay small enough. Happy to implement whichever you prefer. Assisted by AI |
`ALTER TABLE ... ADD COLUMN` is rejected by MSSQL/Azure SQL ("Incorrect
syntax near the keyword 'column'"), failing node startup on those
backends. Drop the COLUMN keyword to match migrations 009/010, which is
valid across SQLite, Postgres, MySQL and SQL Server.
Assisted by AI
|
Or use a "9999-12-31" sentinel to indicate the migration has been run, but the credential is valid until the end of times. Delaying the problem to future generations. |
…rationDate Store() and the backfill now write a far-future sentinel (9999-12-31) for credentials without an expirationDate, so NULL means only "not yet backfilled". The backfill therefore converges to a one-shot operation instead of re-scanning every never-expiring credential on each startup, which avoids the recurring boot-time cost. The expiring query drops its IS NOT NULL guard since <= excludes both the sentinel and legacy NULLs. Assisted by AI
|
@stevenvegt good call — went with your idea. Implemented in 1121a93. What it does:
On your two flavours: I went with the readable "Delaying it to future generations" — yep, that's the trade, and the v7 cleanup is flagged. Let me know if you'd rather use Assisted by AI |
2 participants
Closes #4217.
Summary
Adds
GET /internal/vcr/v2/holder/expiring— lists credentials across all wallets on the node that are expired or about to expire, so a monitoring system can alert operators to renew them in time. Returns a JSON object keyed by subject ID with the list of expiring credentials per subject; subjects with none are omitted.Query parameters
within— time window (relative to now) for a credential'sexpirationDateto count as expiring. Go duration string (largest unit is the hour, e.g.720h,24h,30m). Defaults to720h(30 days). Non-negative;0sreturns only already-expired credentials. Invalid/negative values give400.excludeTypes— credential type(s) to exclude (repeatable). Suppresses credentials that are expected to expire but are kept for audit purposes (e.g.NutsAuthorizationCredential).Already-expired credentials are included by default so they stay visible until cleaned up.
Response uses a focused monitoring DTO (
id,holder,issuer,type,expirationDate) rather than the raw VC, so the shape is uniform regardless of JSON-LD vs JWT encoding. The full VC can be fetched by id via the existing wallet endpoints.{ "90BC1AE9-752B-432F-ADC3-DD9F9C61843C": [ { "id": "did:web:issuer.example.com#c4199b74-0c0a-4e09-a463-6927553e65f5", "holder": "did:web:example.com:iam:123", "issuer": "did:web:issuer.example.com", "type": ["NutsOrganizationCredential"], "expirationDate": "2026-05-15T12:00:00Z" } ] }Filtering is pushed down to SQL
Both
withinandexcludeTypesare applied in the SQL store rather than in Go, so a call no longer loads and JSON-parses every credential of every subject on the node.expiration_datecolumn oncredential(seconds since epoch, NULL = never expires) + index. Migration011_credential_expiration.sql.Store()populates it for new credentials.BackfillExpirationDatesruns on node start to populate the column for credentials stored before it existed. It parses every row whoseexpiration_dateis still NULL (a substring pre-filter can't work: JWT-encoded credentials keep their expiry in a base64expclaim, so it'd silently skip them). Paginates byidas a keyset cursor, batched, logged-and-continued on error. Tagged for removal in v7. Trade-off: never-expiring credentials are re-scanned on every boot — see the open discussion below.SearchCredential(holderDID)→Search(opts ...SearchOption)with functional optionsHolderDID,ExpiresAt,ExcludeCredentialTypes(combined with AND); the expiring endpoint queries cross-wallet with noHolderDID.Docs
ExpiringCredentialschema.docs/pages/deployment/monitoring.rst).Test plan
within/excludeTypespass-through, orphaned holder skipped, empty map, invalid/negativewithin, subject manager errorStore()populates/leaves NULLexpiration_date;BackfillExpirationDatesbackfills, leaves no-expiry rows untouched, idempotentSearch: no options,HolderDID,ExpiresAt,ExcludeCredentialTypes, combined optionsgo build ./...go test ./vcr/...Assisted by AI