Code cleanup (#3083)

.golangci.yml

@@ -0,0 +1,8 @@
+run:
+  tests: false
+issues:
+  exclude-files:
+    - ".*/generated.go"
+    - ".*/.*_test.go"
+    - ".*/test.go"
+    - "docs/.*"

auth/api/iam/api.go

@@ -60,7 +60,8 @@ var oauthRequestObjectKey = []string{"oauth", "requestobject"}
 
 const apiPath = "iam"
 const apiModuleName = auth.ModuleName + "/" + apiPath
-const httpRequestContextKey = "http-request"
+
+type httpRequestContextKey struct{}
 
 // accessTokenValidity defines how long access tokens are valid.
 // TODO: Might want to make this configurable at some point
@@ -87,7 +88,6 @@ type Wrapper struct {
 	auth          auth.AuthenticationServices
 	policyBackend policy.PDPBackend
 	storageEngine storage.Engine
-	keyStore      cryptoNuts.KeyStore
 	vcr           vcr.VCR
 	vdr           vdr.VDR
 	jwtSigner     cryptoNuts.JWTSigner
@@ -147,7 +147,7 @@ func middleware(ctx echo.Context, operationID string) {
 	ctx.Set(core.ModuleNameContextKey, apiModuleName)
 
 	// Add http.Request to context, to allow reading URL query parameters
-	requestCtx := context.WithValue(ctx.Request().Context(), httpRequestContextKey, ctx.Request())
+	requestCtx := context.WithValue(ctx.Request().Context(), httpRequestContextKey{}, ctx.Request())
 	ctx.SetRequest(ctx.Request().WithContext(requestCtx))
 	if strings.HasPrefix(ctx.Request().URL.Path, "/oauth2/") {
 		ctx.Set(core.ErrorWriterContextKey, &oauth.Oauth2ErrorWriter{
@@ -284,7 +284,7 @@ func (r Wrapper) IntrospectAccessToken(_ context.Context, request IntrospectAcce
 	if token.InputDescriptorConstraintIdMap != nil {
 		for _, reserved := range []string{"iss", "sub", "exp", "iat", "active", "client_id", "scope"} {
 			if _, exists := token.InputDescriptorConstraintIdMap[reserved]; exists {
-				return nil, errors.New(fmt.Sprintf("IntrospectAccessToken: InputDescriptorConstraintIdMap contains reserved claim name '%s'", reserved))
+				return nil, fmt.Errorf("IntrospectAccessToken: InputDescriptorConstraintIdMap contains reserved claim name '%s'", reserved)
 			}
 		}
 		response.AdditionalProperties = token.InputDescriptorConstraintIdMap
@@ -302,7 +302,7 @@ func (r Wrapper) HandleAuthorizeRequest(ctx context.Context, request HandleAutho
 
 	// Workaround: deepmap codegen doesn't support dynamic query parameters.
 	//             See https://github.com/deepmap/oapi-codegen/issues/1129
-	httpRequest := ctx.Value(httpRequestContextKey).(*http.Request)
+	httpRequest := ctx.Value(httpRequestContextKey{}).(*http.Request)
 	queryParams := httpRequest.URL.Query()
 
 	// parse and validate as JAR (RFC9101, JWT Authorization Request)
@@ -748,38 +748,31 @@ func (r Wrapper) CallbackOid4vciCredentialIssuance(ctx context.Context, request
 	tokenEndpoint := oid4vciSession.IssuerTokenEndpoint
 	credentialEndpoint := oid4vciSession.IssuerCredentialEndpoint
 	if err != nil {
-		log.Logger().WithError(err).Error("cannot fetch the right endpoints")
 		return nil, withCallbackURI(oauthError(oauth.ServerError, fmt.Sprintf("cannot fetch the right endpoints: %s", err.Error())), oid4vciSession.remoteRedirectUri())
 	}
 	response, err := r.auth.IAMClient().AccessToken(ctx, code, *issuerDid, oid4vciSession.RedirectUri, *holderDid, pkceParams.Verifier)
 	if err != nil {
-		log.Logger().WithError(err).Errorf("error while fetching the access_token from endpoint: %s", tokenEndpoint)
 		return nil, withCallbackURI(oauthError(oauth.AccessDenied, fmt.Sprintf("error while fetching the access_token from endpoint: %s, error: %s", tokenEndpoint, err.Error())), oid4vciSession.remoteRedirectUri())
 	}
 	cNonce := response.Get(oauth.CNonceParam)
 	proofJWT, err := r.proofJwt(ctx, *holderDid, *issuerDid, &cNonce)
 	if err != nil {
-		log.Logger().WithError(err).Error("error while building proof")
-		return nil, withCallbackURI(oauthError(oauth.ServerError, fmt.Sprintf("error while fetching the credential from endpoint %s, error: %s", credentialEndpoint, err.Error())), oid4vciSession.remoteRedirectUri())
+		return nil, withCallbackURI(oauthError(oauth.ServerError, fmt.Sprintf("error building proof to fetch the credential from endpoint %s, error: %s", credentialEndpoint, err.Error())), oid4vciSession.remoteRedirectUri())
 	}
 	credentials, err := r.auth.IAMClient().VerifiableCredentials(ctx, credentialEndpoint, response.AccessToken, proofJWT)
 	if err != nil {
-		log.Logger().WithError(err).Errorf("error while fetching the credential from endpoint: %s", credentialEndpoint)
 		return nil, withCallbackURI(oauthError(oauth.ServerError, fmt.Sprintf("error while fetching the credential from endpoint %s, error: %s", credentialEndpoint, err.Error())), oid4vciSession.remoteRedirectUri())
 	}
 	credential, err := vc.ParseVerifiableCredential(credentials.Credential)
 	if err != nil {
-		log.Logger().WithError(err).Errorf("error while parsing the credential: %s", credentials.Credential)
 		return nil, withCallbackURI(oauthError(oauth.ServerError, fmt.Sprintf("error while parsing the credential: %s, error: %s", credentials.Credential, err.Error())), oid4vciSession.remoteRedirectUri())
 	}
 	err = r.vcr.Verifier().Verify(*credential, true, true, nil)
 	if err != nil {
-		log.Logger().WithError(err).Errorf("error while verifying the credential from issuer: %s", credential.Issuer.String())
 		return nil, withCallbackURI(oauthError(oauth.ServerError, fmt.Sprintf("error while verifying the credential from issuer: %s, error: %s", credential.Issuer.String(), err.Error())), oid4vciSession.remoteRedirectUri())
 	}
 	err = r.vcr.Wallet().Put(ctx, *credential)
 	if err != nil {
-		log.Logger().WithError(err).Errorf("error while storing credential with id: %s", credential.ID)
 		return nil, withCallbackURI(oauthError(oauth.ServerError, fmt.Sprintf("error while storing credential with id: %s, error: %s", credential.ID, err.Error())), oid4vciSession.remoteRedirectUri())
 	}
 
@@ -795,19 +788,18 @@ func (r Wrapper) openidIssuerEndpoints(ctx context.Context, issuerDid did.DID) (
 	if err != nil {
 		return "", "", "", err
 	}
-	for i := range metadata.AuthorizationServers {
-		serverURL := metadata.AuthorizationServers[i]
-		openIdConfiguration, err := r.auth.IAMClient().OpenIdConfiguration(ctx, serverURL)
-		if err != nil {
-			return "", "", "", err
-		}
-		authorizationEndpoint := openIdConfiguration.AuthorizationEndpoint
-		tokenEndpoint := openIdConfiguration.TokenEndpoint
-		credentialEndpoint := metadata.CredentialEndpoint
-		return authorizationEndpoint, tokenEndpoint, credentialEndpoint, nil
+	if len(metadata.AuthorizationServers) == 0 {
+		return "", "", "", fmt.Errorf("cannot locate any authorization endpoint in %s", issuerDid.String())
+	}
+	serverURL := metadata.AuthorizationServers[0]
+	openIdConfiguration, err := r.auth.IAMClient().OpenIdConfiguration(ctx, serverURL)
+	if err != nil {
+		return "", "", "", err
 	}
-	err = errors.New(fmt.Sprintf("cannot locate any authorization endpoint in %s", issuerDid.String()))
-	return "", "", "", err
+	authorizationEndpoint := openIdConfiguration.AuthorizationEndpoint
+	tokenEndpoint := openIdConfiguration.TokenEndpoint
+	credentialEndpoint := metadata.CredentialEndpoint
+	return authorizationEndpoint, tokenEndpoint, credentialEndpoint, nil
 }
 
 // CreateAuthorizationRequest creates an OAuth2.0 authorizationRequest redirect URL that redirects to the authorization server.

auth/api/iam/api_test.go

@@ -1412,7 +1412,7 @@ func requestContext(queryParams map[string]interface{}) context.Context {
 			RawQuery: vals.Encode(),
 		},
 	}
-	return context.WithValue(audit.TestContext(), httpRequestContextKey, httpRequest)
+	return context.WithValue(audit.TestContext(), httpRequestContextKey{}, httpRequest)
 }
 
 // statusCodeFrom returns the statuscode for the given error

auth/api/iam/openid4vp.go

@@ -110,7 +110,7 @@ func (r Wrapper) handleAuthorizeRequestFromHolder(ctx context.Context, verifier
 	}
 	metadata, err := r.auth.IAMClient().AuthorizationServerMetadata(ctx, *walletDID)
 	if err != nil {
-		return nil, withCallbackURI(oauthError(oauth.ServerError, "failed to get metadata from wallet"), redirectURL)
+		return nil, withCallbackURI(oauthError(oauth.ServerError, "failed to get metadata from wallet", err), redirectURL)
 	}
 	// check metadata for supported client_id_schemes
 	if !slices.Contains(metadata.ClientIdSchemesSupported, didScheme) {
@@ -408,20 +408,20 @@ func (r Wrapper) handleAuthorizeResponseSubmission(ctx context.Context, request
 
 	pexEnvelope, err := pe.ParseEnvelope([]byte(*request.Body.VpToken))
 	if err != nil || len(pexEnvelope.Presentations) == 0 {
-		return nil, oauthError(oauth.InvalidRequest, "invalid vp_token")
+		return nil, oauthError(oauth.InvalidRequest, "invalid vp_token", err)
 	}
 
 	// note: instead of using the challenge to lookup the oauth session, we could also add a client state from the verifier.
 	// this would allow us to lookup the redirectURI without checking the VP first.
 
 	// extract the nonce from the vp(s)
 	nonce, err := extractChallenge(pexEnvelope.Presentations[0])
-	if nonce == "" {
-		return nil, oauthError(oauth.InvalidRequest, "failed to extract nonce from vp_token")
+	if nonce == "" || err != nil {
+		return nil, oauthError(oauth.InvalidRequest, "failed to extract nonce from vp_token", err)
 	}
 	var stateFromNonce string
 	if err = r.oauthNonceStore().Get(nonce, &stateFromNonce); err != nil {
-		return nil, oauthError(oauth.InvalidRequest, "invalid or expired nonce")
+		return nil, oauthError(oauth.InvalidRequest, "invalid or expired nonce", err)
 	}
 	// Retrieve session through state, since we need to update it given the state.
 	// Also asserts that nonce and state reference the same OAuthSession
@@ -431,7 +431,7 @@ func (r Wrapper) handleAuthorizeResponseSubmission(ctx context.Context, request
 		return nil, oauthError(oauth.InvalidRequest, "invalid nonce/state")
 	}
 	if err = r.oauthClientStateStore().Get(state, &session); err != nil {
-		return nil, oauthError(oauth.InvalidRequest, "invalid or expired session")
+		return nil, oauthError(oauth.InvalidRequest, "invalid or expired session", err)
 	}
 
 	// any future error can be sent to the client using the redirectURI from the oauthSession
@@ -553,6 +553,9 @@ func (r Wrapper) validatePresentationNonce(presentations []vc.VerifiablePresenta
 	var returnErr error
 	for _, presentation := range presentations {
 		nextNonce, err := extractChallenge(presentation)
+		if err != nil {
+			return err
+		}
 		if nextNonce == "" {
 			// fallback on nonce instead of challenge, todo: should be uniform, check vc data model specs for JWT/JSON-LD
 			nextNonce, err = extractNonce(presentation)
@@ -595,7 +598,7 @@ func (r Wrapper) handleAccessTokenRequest(ctx context.Context, request HandleTok
 	var oauthSession OAuthSession
 	err := r.oauthCodeStore().Get(*request.Code, &oauthSession)
 	if err != nil {
-		return nil, oauthError(oauth.InvalidGrant, "invalid authorization code")
+		return nil, oauthError(oauth.InvalidGrant, "invalid authorization code", err)
 	}
 	// check if the client_id matches the one from the authorization request
 	if oauthSession.ClientID != *request.ClientId {
@@ -607,14 +610,6 @@ func (r Wrapper) handleAccessTokenRequest(ctx context.Context, request HandleTok
 		return nil, oauthError(oauth.InvalidGrant, "invalid code_verifier")
 	}
 
-	var submissions []PresentationSubmission
-	for _, submission := range oauthSession.OpenID4VPVerifier.Submissions {
-		submissions = append(submissions, submission)
-	}
-	presentationDefinitions := make([]PresentationDefinition, 0)
-	for _, curr := range oauthSession.OpenID4VPVerifier.RequiredPresentationDefinitions {
-		presentationDefinitions = append(presentationDefinitions, curr)
-	}
 	walletDID, err := did.ParseDID(oauthSession.ClientID)
 	if err != nil {
 		return nil, err
@@ -664,7 +659,7 @@ func (r Wrapper) handleCallback(ctx context.Context, request CallbackRequestObje
 	}
 	// lookup client state
 	if err := r.oauthClientStateStore().Get(*request.Params.State, &oauthSession); err != nil {
-		return nil, oauthError(oauth.InvalidRequest, "invalid or expired state")
+		return nil, oauthError(oauth.InvalidRequest, "invalid or expired state", err)
 	}
 	// extract callback URI at calling app from OAuthSession
 	// this is the URI where the user-agent will be redirected to
@@ -702,9 +697,10 @@ func (r Wrapper) oauthNonceStore() storage.SessionStore {
 	return r.storageEngine.GetSessionDatabase().GetStore(oAuthFlowTimeout, oauthNonceKey...)
 }
 
-func oauthError(code oauth.ErrorCode, description string) oauth.OAuth2Error {
+func oauthError(code oauth.ErrorCode, description string, internalError ...error) oauth.OAuth2Error {
 	return oauth.OAuth2Error{
-		Code:        code,
-		Description: description,
+		Code:          code,
+		Description:   description,
+		InternalError: errors.Join(internalError...),
 	}
 }

auth/api/iam/params.go

@@ -18,23 +18,13 @@
 
 package iam
 
-import "net/url"
-
 // oauthParameters is a helper for oauth params.
 // oauth params can be derived from query params or JWT claims (RFC9101).
 // in theory all params could be string, arrays or numbers. Our handlers only want single string values for all params.
 // since empty values always lead to validation errors, the get method will return an empty value if the param is not present or has a wrong format.
 // array values with len == 1 will be treated as single string values.
 type oauthParameters map[string]interface{}
 
-func parseQueryParams(values url.Values) oauthParameters {
-	underlying := make(map[string]interface{})
-	for key, value := range values {
-		underlying[key] = value
-	}
-	return underlying
-}
-
 func parseJWTClaims(claims map[string]interface{}) oauthParameters { return claims }
 
 // get returns the string value if present and if an actual string

auth/api/iam/s2s_vptoken.go

@@ -131,9 +131,7 @@ func (r Wrapper) createAccessToken(issuer did.DID, walletDID did.DID, issueTime
 		InputDescriptorConstraintIdMap: fieldsMap,
 	}
 	for _, envelope := range pexState.SubmittedEnvelopes {
-		for _, presentation := range envelope.Presentations {
-			accessToken.VPToken = append(accessToken.VPToken, presentation)
-		}
+		accessToken.VPToken = append(accessToken.VPToken, envelope.Presentations...)
 	}
 	err = r.accessTokenServerStore().Put(accessToken.Token, accessToken)
 	if err != nil {

auth/api/iam/types.go

@@ -25,7 +25,6 @@ import (
 	"github.com/nuts-foundation/nuts-node/vcr/pe"
 	"github.com/nuts-foundation/nuts-node/vdr/resolver"
 	"net/http"
-	"time"
 )
 
 // DIDDocument is an alias
@@ -66,10 +65,6 @@ type WalletOwnerType = pe.WalletOwnerType
 // RequiredPresentationDefinitions is an alias
 type RequiredPresentationDefinitions = pe.WalletOwnerMapping
 
-const (
-	sessionExpiry = 5 * time.Minute
-)
-
 // CookieReader is an interface for reading cookies from an HTTP request.
 // It is implemented by echo.Context and http.Request.
 type CookieReader interface {
@@ -159,11 +154,3 @@ const presentationDefParam = "presentation_definition"
 // presentationDefUriParam is the name of the OpenID4VP presentation_definition_uri parameter.
 // Specified by https://openid.bitbucket.io/connect/openid-4-verifiable-presentations-1_0.html#name-presentation_definition_uri
 const presentationDefUriParam = "presentation_definition_uri"
-
-// presentationSubmissionParam is the name of the OpenID4VP presentation_submission parameter.
-// Specified by https://openid.bitbucket.io/connect/openid-4-verifiable-presentations-1_0.html#name-response-parameters
-const presentationSubmissionParam = "presentation_submission"
-
-// vpTokenParam is the name of the OpenID4VP vp_token parameter.
-// Specified by https://openid.bitbucket.io/connect/openid-4-verifiable-presentations-1_0.html#name-response-type-vp_token
-const vpTokenParam = "vp_token"

auth/client/iam/client.go

@@ -154,7 +154,7 @@ func (hb HTTPClient) AccessToken(ctx context.Context, tokenEndpoint string, data
 		if len(responseBodyString) > core.HttpResponseBodyLogClipAt {
 			responseBodyString = responseBodyString[:core.HttpResponseBodyLogClipAt] + "...(clipped)"
 		}
-		return token, fmt.Errorf("unable to unmarshal response: %w, %s", err, string(responseData))
+		return token, fmt.Errorf("unable to unmarshal response: %w, %s", err, responseBodyString)
 	}
 	return token, nil
 }
@@ -231,7 +231,6 @@ type CredentialResponse struct {
 }
 
 func (hb HTTPClient) VerifiableCredentials(ctx context.Context, credentialEndpoint string, accessToken string, proofJwt string) (*CredentialResponse, error) {
-
 	credentialEndpointURL, err := url.Parse(credentialEndpoint)
 	if err != nil {
 		return nil, err
@@ -243,7 +242,7 @@ func (hb HTTPClient) VerifiableCredentials(ctx context.Context, credentialEndpoi
 			Jwt:       proofJwt,
 		},
 	}
-	jsonBody, err := json.Marshal(credentialRequest)
+	jsonBody, _ := json.Marshal(credentialRequest)
 	request, err := http.NewRequestWithContext(ctx, http.MethodPost, credentialEndpointURL.String(), bytes.NewBuffer(jsonBody))
 	if err != nil {
 		return nil, err

auth/services/dummy/dummy.go

@@ -182,8 +182,7 @@ func (d signingSessionResult) VerifiablePresentation() (*vc.VerifiablePresentati
 	}, nil
 }
 
-func (d Dummy) Start(ctx context.Context) {
-	return
+func (d Dummy) Start(_ context.Context) {
 }
 
 // VerifyVP check a Dummy VerifiablePresentation. It Returns a verificationResult if all was fine, an error otherwise.
@@ -264,7 +263,7 @@ func (d Dummy) StartSigningSession(contract contract.Contract, params map[string
 		return nil, errNotEnabled
 	}
 	sessionBytes := make([]byte, 16)
-	rand.Reader.Read(sessionBytes)
+	_, _ = rand.Reader.Read(sessionBytes)
 
 	sessionID := hex.EncodeToString(sessionBytes)
 	d.Status[sessionID] = SessionCreated

auth/services/irma/signer.go

@@ -77,7 +77,6 @@ func (s SessionPtr) MarshalJSON() ([]byte, error) {
 const NutsIrmaSignedContract = "NutsIrmaSignedContract"
 
 func (v Signer) Start(ctx context.Context) {
-	return
 }
 
 // StartSigningSession accepts a rawContractText and creates an IRMA signing session.

auth/services/selfsigned/signer.go

@@ -146,7 +146,6 @@ func (v *signer) createVP(ctx context.Context, s types.Session, issuanceDate tim
 
 func (v *signer) Start(ctx context.Context) {
 	v.store.Start(ctx)
-	return
 }
 
 func (v *signer) StartSigningSession(userContract contract.Contract, params map[string]interface{}) (contract.SessionPointer, error) {

cmd/root.go

@@ -136,7 +136,9 @@ func startServer(ctx context.Context, system *core.System) error {
 			if err != nil {
 				return err
 			}
-			pprof.StartCPUProfile(f)
+			if err := pprof.StartCPUProfile(f); err != nil {
+				return err
+			}
 			defer pprof.StopCPUProfile()
 		} else {
 			logrus.Warn("Ignoring CPU profile option, strictmode is enabled")

core/engine.go

@@ -179,7 +179,7 @@ func (system *System) VisitEnginesE(visitor func(engine Engine) error) error {
 // FindEngineByName looks up given target engine by name, or nil if not found.
 func (system *System) FindEngineByName(target string) Engine {
 	for _, curr := range system.engines {
-		if strings.ToLower(engineName(curr)) == strings.ToLower(target) {
+		if strings.EqualFold(engineName(curr), strings.ToLower(target)) {
 			return curr
 		}
 	}

crypto/crypto_test.go

@@ -45,7 +45,8 @@ func TestCrypto_Exists(t *testing.T) {
 	client := createCrypto(t)
 
 	kid := "kid"
-	client.New(audit.TestContext(), StringNamingFunc(kid))
+	_, err := client.New(audit.TestContext(), StringNamingFunc(kid))
+	require.NoError(t, err)
 
 	t.Run("returns true for existing key", func(t *testing.T) {
 		assert.True(t, client.Exists(ctx, kid))