nuts-foundation / nuts-foundation/nuts-node
PresentationDefinition fetches the remote PD endpoint twice per request
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 28
- Forks
- 23
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 76
Description
Summary
HTTPClient.PresentationDefinition issues the same GET request twice. Every presentation definition resolution therefore makes two identical calls to the remote authorization server's PD endpoint.
Where
auth/client/iam/client.go:100-118:
func (hb HTTPClient) PresentationDefinition(ctx context.Context, presentationDefinitionURL url.URL) (*pe.PresentationDefinition, error) {
// create a GET request with scope query param
request, err := http.NewRequestWithContext(ctx, http.MethodGet, presentationDefinitionURL.String(), nil)
if err != nil {
return nil, err
}
var presentationDefinition pe.PresentationDefinition
err = hb.doRequest(ctx, request, &presentationDefinition)
if err != nil {
// any OAuth error should be passed
// any other error should result in a 502 Bad Gateway
if errors.As(err, new(oauth.OAuth2Error)) {
return nil, err
}
return nil, errors.Join(ErrBadGateway, err)
}
return &presentationDefinition, hb.doRequest(ctx, request, &presentationDefinition)
}
The final line calls doRequest a second time, using it as the returned error expression. The first call has already fetched and unmarshalled the document, and its error has already been handled above.
Because the request is a GET with a nil body it is safely reusable, so both calls succeed and the duplication is invisible in normal operation.
Impact
Two consequences, neither security-relevant.
Outbound load on every peer's presentation definition endpoint is doubled. The call path is PresentationDefinitionResolver.resolveRemote (auth/client/iam/pd_resolver.go:64-79), reached from requestVPTokenAccessToken (auth/client/iam/openid4vp.go:302), so it runs on every RFC021 service access token request that resolves its PD remotely.
The returned error is the second call's error rather than the first's. A transient failure on the redundant request makes the function return a non-nil error alongside a valid *pe.PresentationDefinition. Callers check the error first, so a presentation definition that was fetched successfully gets discarded and the token request fails for no real reason.
Fix
Return the already-populated value:
return &presentationDefinition, nil
A test asserting that a single call reaches the endpoint once would prevent a recurrence. The existing tests pass with the duplicate because both requests succeed.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in auth/client/iam/client.go at HTTPClient.PresentationDefinition and inspect the existing tests for the IAM client. Add a focused test that counts requests to the remote presentation-definition endpoint, then run the relevant Go tests. Done means one GET per resolution, the populated presentation definition is returned, and successful requests produce no error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100