Skip to content

Code cleanup #3083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
run:
tests: false
issues:
exclude-files:
- ".*/generated.go"
- ".*/.*_test.go"
- ".*/test.go"
- "docs/.*"
42 changes: 17 additions & 25 deletions auth/api/iam/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,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
Expand All @@ -89,7 +90,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
Expand Down Expand Up @@ -143,7 +143,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{
Expand Down Expand Up @@ -280,7 +280,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
Expand All @@ -298,7 +298,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)
Expand Down Expand Up @@ -783,38 +783,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())
}

Expand All @@ -830,19 +823,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.
Expand Down
2 changes: 1 addition & 1 deletion auth/api/iam/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1542,7 +1542,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
Expand Down
34 changes: 15 additions & 19 deletions auth/api/iam/openid4vp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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...),
}
}
10 changes: 0 additions & 10 deletions auth/api/iam/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions auth/api/iam/s2s_vptoken.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 0 additions & 13 deletions auth/api/iam/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"
5 changes: 2 additions & 3 deletions auth/client/iam/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 2 additions & 3 deletions auth/services/dummy/dummy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion auth/services/irma/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion auth/services/selfsigned/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion core/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
3 changes: 2 additions & 1 deletion crypto/crypto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Loading
Loading