nuts-foundation / nuts-foundation/nuts-node

discovery/api/server is the only strict-handler wrapper without audit.StrictMiddleware

Open Beginner friendly
#4,436 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug discuss
Dominant language
Go
Stars
28
Forks
23
Avg merge
1d 10h
Merged PRs (30d)
76

Description

Summary

discovery/api/server is the only generated strict-handler API wrapper that does not install audit.StrictMiddleware. Nothing is broken today, but the failure mode if an audit-emitting operation is ever added to that path is a panic rather than a missing log line, so the inconsistency is worth closing.

Where

discovery/api/server/api.go:55-67 registers a single StrictMiddlewareFunc, which sets the context keys and stops:

func (w *Wrapper) Routes(router core.EchoRouter) {
	RegisterHandlers(router, NewStrictHandler(w, []StrictMiddlewareFunc{
		func(f StrictHandlerFunc, operationID string) StrictHandlerFunc {
			return func(ctx echo.Context, request interface{}) (response interface{}, err error) {
				ctx.Set(core.OperationIDContextKey, operationID)
				ctx.Set(core.ModuleNameContextKey, discovery.ModuleName)
				ctx.Set(core.StatusCodeResolverContextKey, w)
				return f(ctx, request)
			}
		},
	}))
}

Compare vcr/api/vcr/v2/api.go:63-77, which registers a second function calling audit.StrictMiddleware(f, vcr.ModuleName, operationID). The same pattern is present in crypto/api/v1, didman/api/v1, auth/api/auth/v1, discovery/api/v1, vcr/api/openid4vci/v0 and vdr/api/v2.

(auth/api/means/v1/api.go:32-34 also has no audit middleware, but that wrapper only delegates via w.Auth.ContractNotary().Routes(router) and registers no handlers of its own, so it is not comparable.)

Why this is currently harmless

audit.StrictMiddleware (audit/http.go:28-34) does not log anything. It calls SetOnEchoContext, which puts actor, moduleName and operationID into the request context so that later explicit audit calls can use them.

Audit entries come from nine audit.Log(...) call sites: JWE encrypt and decrypt, JWT and JWS signing, key create and delete (crypto/), credential removed (vcr/holder/sql_wallet.go:151), credential retrieved (vcr/issuer/openid.go:271) and access granted or denied (http/tokenV2/middleware.go:444).

None of those is reachable from the public discovery endpoints. GET and POST /discovery/:serviceID verify and store presentations; they sign nothing and do not touch the wallet or the key store. So no audit entry is currently being lost, and requests are still recorded by requestLoggerMiddleware, which applies globally and skips only /metrics, /status and /health.

Why it is still worth fixing

audit.Log panics when the context has no audit info (audit/audit.go:180-182):

info := InfoFromContext(ctx)
if info == nil {
	panic("audit: no audit info in context")
}

So the moment anyone adds an operation to a discovery server handler that emits an audit event, the request panics instead of logging.

That panic is not converted into a 500. Echo does not recover: Echo.ServeHTTP (echo.go:667-692 in labstack/echo v4.15.4) contains no recover(), and there is no middleware.Recover() anywhere in this repository. The panic therefore reaches Go's net/http per-connection recover in conn.serve, which logs http: panic serving ... with a stack trace and closes the connection. The caller sees a dropped connection rather than an HTTP error response.

This is a defensible fail-loud design, but it means a forgotten middleware surfaces as an unexplained connection reset plus a stack trace, which is a poor thing to debug under pressure.

Fix

Add the second StrictMiddlewareFunc to discovery/api/server/api.go, matching the other wrappers:

		func(f StrictHandlerFunc, operationID string) StrictHandlerFunc {
			return audit.StrictMiddleware(f, discovery.ModuleName, operationID)
		},

Worth considering separately: whether registering a presentation on a discovery service should emit an audit event at all. There is no event constant for it today (audit/audit.go:34-57), and it is a write to node state performed by a third party, so it is a reasonable candidate for the audit trail.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with discovery/api/server/api.go:55-67 and compare its StrictMiddlewareFunc registrations with vcr/api/vcr/v2/api.go:63-77. Read audit/http.go:28-34 to confirm the middleware contract; done means the discovery wrapper also installs audit.StrictMiddleware with discovery.ModuleName and the operation ID.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.