nuts-foundation / nuts-foundation/nuts-node

PresentationDefinition fetches the remote PD endpoint twice per request

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

Nobody has claimed this yet.

auth bug
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

  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 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.