From c76908efa1ced309694fb945e15e1d85254b9209 Mon Sep 17 00:00:00 2001 From: Wout Slakhorst Date: Tue, 12 Dec 2023 15:39:24 +0100 Subject: [PATCH] added authorize endpoint as specified by rfc6549 authorization code (#2626) wip added bunch of tests added missing API tests fix failing test add tests for failing directpost responses refactor ami client and reuse response parsing/code checking happy flow tests for holder service error tests for holder service comment touchup on some comments --- auth/api/iam/openid4vp.go | 63 ++++++++++++++----- auth/api/iam/openid4vp_test.go | 60 +++++++++++++----- auth/client/iam/client.go | 10 +-- auth/services/oauth/holder_test.go | 49 +++------------ auth/services/oauth/interface.go | 1 - .../oauth-flow/rfc021/docker-compose.yml | 4 ++ 6 files changed, 109 insertions(+), 78 deletions(-) diff --git a/auth/api/iam/openid4vp.go b/auth/api/iam/openid4vp.go index 1354e27438..24c033d778 100644 --- a/auth/api/iam/openid4vp.go +++ b/auth/api/iam/openid4vp.go @@ -190,35 +190,58 @@ func (r Wrapper) handleAuthorizeRequestFromVerifier(ctx context.Context, walletD } // check the response URL because later errors will redirect to this URL responseURI, responseOK := params[responseURIParam] + + // get the original authorization request of the client, if something fails we need the redirectURI from this request + // get the state parameter + state, ok := params[stateParam] + if !ok { + // post error to responseURI, if it fails, it'll render error page + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.InvalidRequest, "missing state parameter", nil), responseURI) + } + + // check client state + // if no state, post error + var session OAuthSession + err := r.oauthClientStateStore().Get(state, &session) + if err != nil { + if !responseOK { + return nil, oauthError(oauth.ServerError, "something went wrong", nil) + } + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.InvalidRequest, "state has expired", nil), responseURI) + } + clientRedirectURL := session.redirectURI() if !responseOK { - return nil, oauth.OAuth2Error{Code: oauth.InvalidRequest, Description: "missing response_uri parameter"} + if clientRedirectURL != nil { + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.InvalidRequest, "missing response_uri parameter", clientRedirectURL), clientRedirectURL.String()) + } + return nil, oauthError(oauth.ServerError, "something went wrong", nil) } clientIDScheme := params[clientIDSchemeParam] if clientIDScheme != didScheme { - return r.sendAndHandleDirectPostError(ctx, oauth.OAuth2Error{Code: oauth.InvalidRequest, Description: "invalid client_id_scheme parameter"}, responseURI) + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.InvalidRequest, "invalid client_id_scheme parameter", clientRedirectURL), responseURI) } verifierID := params[clientIDParam] // the verifier must be a did:web verifierDID, err := did.ParseDID(verifierID) if err != nil || verifierDID.Method != "web" { - return r.sendAndHandleDirectPostError(ctx, oauth.OAuth2Error{Code: oauth.InvalidRequest, Description: "invalid client_id parameter (only did:web is supported)"}, responseURI) + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.InvalidRequest, "invalid client_id parameter (only did:web is supported)", clientRedirectURL), responseURI) } nonce, ok := params[nonceParam] if !ok { - return r.sendAndHandleDirectPostError(ctx, oauth.OAuth2Error{Code: oauth.InvalidRequest, Description: "missing nonce parameter"}, responseURI) + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.InvalidRequest, "missing nonce parameter", clientRedirectURL), responseURI) } // get verifier metadata clientMetadataURI := params[clientMetadataURIParam] // we ignore any client_metadata, but officially an error must be returned when that param is present. metadata, err := r.auth.Holder().ClientMetadata(ctx, clientMetadataURI) if err != nil { - return r.sendAndHandleDirectPostError(ctx, oauth.OAuth2Error{Code: oauth.ServerError, Description: "failed to get client metadata (verifier)"}, responseURI) + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.ServerError, "failed to get client metadata (verifier)", clientRedirectURL), responseURI) } // get presentation_definition from presentation_definition_uri presentationDefinitionURI := params[presentationDefUriParam] presentationDefinition, err := r.auth.Holder().PresentationDefinition(ctx, presentationDefinitionURI) if err != nil { - return r.sendAndHandleDirectPostError(ctx, oauth.OAuth2Error{Code: oauth.InvalidPresentationDefinitionURI, Description: fmt.Sprintf("failed to retrieve presentation definition on %s", presentationDefinitionURI)}, responseURI) + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.InvalidPresentationDefinitionURI, fmt.Sprintf("failed to retrieve presentation definition on %s", presentationDefinitionURI), clientRedirectURL), responseURI) } // at this point in the flow it would be possible to ask the user to confirm the credentials to use @@ -227,28 +250,40 @@ func (r Wrapper) handleAuthorizeRequestFromVerifier(ctx context.Context, walletD vp, submission, err := r.auth.Holder().BuildPresentation(ctx, walletDID, *presentationDefinition, metadata.VPFormats, nonce) if err != nil { if errors.Is(err, oauthServices.ErrNoCredentials) { - return r.sendAndHandleDirectPostError(ctx, oauth.OAuth2Error{Code: oauth.InvalidRequest, Description: "no credentials available"}, responseURI) + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.InvalidRequest, "no credentials available", clientRedirectURL), responseURI) } - return r.sendAndHandleDirectPostError(ctx, oauth.OAuth2Error{Code: oauth.ServerError, Description: err.Error()}, responseURI) + return r.sendAndHandleDirectPostError(ctx, oauthError(oauth.ServerError, err.Error(), clientRedirectURL), responseURI) } // any error here is a server error, might need a fixup to prevent exposing to a user - return r.sendAndHandleDirectPost(ctx, *vp, *submission, responseURI, params[stateParam]) + return r.sendAndHandleDirectPost(ctx, *vp, *submission, responseURI, *clientRedirectURL, state), nil } // sendAndHandleDirectPost sends OpenID4VP direct_post to the verifier. The verifier responds with a redirect to the client (including error fields if needed). // If the direct post fails, the user-agent will be redirected back to the client with an error. (Original redirect_uri). -func (r Wrapper) sendAndHandleDirectPost(ctx context.Context, vp vc.VerifiablePresentation, presentationSubmission pe.PresentationSubmission, verifierResponseURI string, state string) (HandleAuthorizeRequestResponseObject, error) { +func (r Wrapper) sendAndHandleDirectPost(ctx context.Context, vp vc.VerifiablePresentation, presentationSubmission pe.PresentationSubmission, verifierResponseURI string, clientRedirectURL url.URL, state string) HandleAuthorizeRequestResponseObject { redirectURI, err := r.auth.Holder().PostAuthorizationResponse(ctx, vp, presentationSubmission, verifierResponseURI, state) - if err != nil { - return nil, err + if err == nil { + return HandleAuthorizeRequest302Response{ + HandleAuthorizeRequest302ResponseHeaders{ + Location: redirectURI, + }, + } } + msg := fmt.Sprintf("failed to post authorization response to verifier @ %s", verifierResponseURI) + log.Logger().WithError(err).Error(msg) + + // clientRedirectURI has been checked earlier in te process. + clientRedirectURL = httpNuts.AddQueryParams(clientRedirectURL, map[string]string{ + oauth.ErrorParam: string(oauth.ServerError), + oauth.ErrorDescriptionParam: msg, + }) return HandleAuthorizeRequest302Response{ HandleAuthorizeRequest302ResponseHeaders{ - Location: redirectURI, + Location: clientRedirectURL.String(), }, - }, nil + } } // sendAndHandleDirectPostError sends errors from handleAuthorizeRequestFromVerifier as direct_post to the verifier. The verifier responds with a redirect to the client (including error fields). diff --git a/auth/api/iam/openid4vp_test.go b/auth/api/iam/openid4vp_test.go index 40ef2227b7..55bed5d0f8 100644 --- a/auth/api/iam/openid4vp_test.go +++ b/auth/api/iam/openid4vp_test.go @@ -66,19 +66,6 @@ func TestWrapper_handleAuthorizeRequestFromHolder(t *testing.T) { requireOAuthError(t, err, oauth.InvalidRequest, "invalid client_id parameter (only did:web is supported)") }) - t.Run("missing did in supported_client_id_schemes", func(t *testing.T) { - ctx := newTestClient(t) - params := defaultParams() - ctx.verifierRole.EXPECT().AuthorizationServerMetadata(gomock.Any(), holderDID).Return(&oauth.AuthorizationServerMetadata{ - AuthorizationEndpoint: "http://example.com", - ClientIdSchemesSupported: []string{"not_did"}, - }, nil) - ctx.verifierRole.EXPECT().ClientMetadataURL(verifierDID).Return(test.MustParseURL("https://example.com/.well-known/authorization-server/iam/verifier"), nil) - - _, err := ctx.client.handleAuthorizeRequestFromHolder(context.Background(), verifierDID, params) - - requireOAuthError(t, err, oauth.InvalidRequest, "wallet metadata does not contain did in client_id_schemes_supported") - }) t.Run("error on authorization server metadata", func(t *testing.T) { ctx := newTestClient(t) ctx.verifierRole.EXPECT().AuthorizationServerMetadata(gomock.Any(), holderDID).Return(nil, assert.AnError) @@ -124,12 +111,14 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) { responseURIParam: responseURI, responseTypeParam: responseTypeVPToken, scopeParam: "test", + stateParam: "state", } } t.Run("invalid client_id", func(t *testing.T) { ctx := newTestClient(t) params := defaultParams() + putState(ctx, "state") params[clientIDParam] = "did:nuts:1" expectPostError(t, ctx, oauth.InvalidRequest, "invalid client_id parameter (only did:web is supported)", responseURI) @@ -140,6 +129,7 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) { t.Run("invalid client_id_scheme", func(t *testing.T) { ctx := newTestClient(t) params := defaultParams() + putState(ctx, "state") params[clientIDSchemeParam] = "other" expectPostError(t, ctx, oauth.InvalidRequest, "invalid client_id_scheme parameter", responseURI) @@ -150,6 +140,7 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) { t.Run("missing client_metadata_uri", func(t *testing.T) { ctx := newTestClient(t) params := defaultParams() + putState(ctx, "state") delete(params, clientMetadataURIParam) ctx.holderRole.EXPECT().ClientMetadata(gomock.Any(), "").Return(nil, assert.AnError) expectPostError(t, ctx, oauth.ServerError, "failed to get client metadata (verifier)", responseURI) @@ -161,6 +152,7 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) { t.Run("missing nonce", func(t *testing.T) { ctx := newTestClient(t) params := defaultParams() + putState(ctx, "state") delete(params, nonceParam) expectPostError(t, ctx, oauth.InvalidRequest, "missing nonce parameter", responseURI) @@ -171,6 +163,7 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) { t.Run("invalid presentation_definition_uri", func(t *testing.T) { ctx := newTestClient(t) params := defaultParams() + putState(ctx, "state") params[presentationDefUriParam] = "://example.com" ctx.holderRole.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(nil, assert.AnError) expectPostError(t, ctx, oauth.ServerError, "failed to get client metadata (verifier)", responseURI) @@ -191,11 +184,22 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) { t.Run("missing response_uri", func(t *testing.T) { ctx := newTestClient(t) params := defaultParams() + putState(ctx, "state") delete(params, responseURIParam) + expectPostError(t, ctx, oauth.InvalidRequest, "missing response_uri parameter", "https://example.com/iam/holder/cb") _, err := ctx.client.handleAuthorizeRequestFromVerifier(context.Background(), holderDID, params) - assert.EqualError(t, err, "invalid_request - missing response_uri parameter") + require.NoError(t, err) + }) + t.Run("missing or expired state", func(t *testing.T) { + ctx := newTestClient(t) + params := defaultParams() + expectPostError(t, ctx, oauth.InvalidRequest, "state has expired", responseURI) + + _, err := ctx.client.handleAuthorizeRequestFromVerifier(context.Background(), holderDID, params) + + require.NoError(t, err) }) t.Run("missing state and missing response_uri", func(t *testing.T) { ctx := newTestClient(t) @@ -206,9 +210,20 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) { require.Error(t, err) }) + t.Run("missing state param", func(t *testing.T) { + ctx := newTestClient(t) + params := defaultParams() + delete(params, stateParam) + expectPostError(t, ctx, oauth.InvalidRequest, "missing state parameter", responseURI) + + _, err := ctx.client.handleAuthorizeRequestFromVerifier(context.Background(), holderDID, params) + + require.NoError(t, err) + }) t.Run("invalid presentation_definition_uri", func(t *testing.T) { ctx := newTestClient(t) params := defaultParams() + putState(ctx, "state") ctx.holderRole.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(&clientMetadata, nil) ctx.holderRole.EXPECT().PresentationDefinition(gomock.Any(), "https://example.com/iam/verifier/presentation_definition?scope=test").Return(nil, assert.AnError) expectPostError(t, ctx, oauth.InvalidPresentationDefinitionURI, "failed to retrieve presentation definition on https://example.com/iam/verifier/presentation_definition?scope=test", responseURI) @@ -220,6 +235,7 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) { t.Run("failed to create verifiable presentation", func(t *testing.T) { ctx := newTestClient(t) params := defaultParams() + putState(ctx, "state") ctx.holderRole.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(&clientMetadata, nil) ctx.holderRole.EXPECT().PresentationDefinition(gomock.Any(), "https://example.com/iam/verifier/presentation_definition?scope=test").Return(&pe.PresentationDefinition{}, nil) ctx.holderRole.EXPECT().BuildPresentation(gomock.Any(), holderDID, pe.PresentationDefinition{}, clientMetadata.VPFormats, "nonce").Return(nil, nil, assert.AnError) @@ -232,6 +248,7 @@ func TestWrapper_handleAuthorizeRequestFromVerifier(t *testing.T) { t.Run("missing credentials in wallet", func(t *testing.T) { ctx := newTestClient(t) params := defaultParams() + putState(ctx, "state") ctx.holderRole.EXPECT().ClientMetadata(gomock.Any(), "https://example.com/.well-known/authorization-server/iam/verifier").Return(&clientMetadata, nil) ctx.holderRole.EXPECT().PresentationDefinition(gomock.Any(), "https://example.com/iam/verifier/presentation_definition?scope=test").Return(&pe.PresentationDefinition{}, nil) ctx.holderRole.EXPECT().BuildPresentation(gomock.Any(), holderDID, pe.PresentationDefinition{}, clientMetadata.VPFormats, "nonce").Return(nil, nil, oauth2.ErrNoCredentials) @@ -258,9 +275,16 @@ func TestWrapper_sendAndHandleDirectPost(t *testing.T) { t.Run("failed to post response", func(t *testing.T) { ctx := newTestClient(t) ctx.holderRole.EXPECT().PostAuthorizationResponse(gomock.Any(), gomock.Any(), gomock.Any(), "response", "").Return("", assert.AnError) - _, err := ctx.client.sendAndHandleDirectPost(context.Background(), vc.VerifiablePresentation{}, pe.PresentationSubmission{}, "response", "") + redirectURI := test.MustParseURL("https://example.com/redirect") + expected := HandleAuthorizeRequest302Response{ + Headers: HandleAuthorizeRequest302ResponseHeaders{ + Location: "https://example.com/redirect?error=server_error&error_description=failed+to+post+authorization+response+to+verifier+%40+response", + }, + } - require.Error(t, err) + redirect := ctx.client.sendAndHandleDirectPost(context.Background(), vc.VerifiablePresentation{}, pe.PresentationSubmission{}, "response", *redirectURI, "") + + assert.Equal(t, expected, redirect) }) } @@ -408,3 +432,7 @@ func (s *stubResponseWriter) Write(i []byte) (int, error) { func (s *stubResponseWriter) WriteHeader(statusCode int) { s.statusCode = statusCode } + +func putState(ctx *testCtx, state string) { + _ = ctx.client.oauthClientStateStore().Put(state, OAuthSession{OwnDID: holderDID, RedirectURI: "https://example.com/iam/holder/cb"}) +} diff --git a/auth/client/iam/client.go b/auth/client/iam/client.go index b47375a145..f0ca5c1e83 100644 --- a/auth/client/iam/client.go +++ b/auth/client/iam/client.go @@ -104,7 +104,7 @@ func (hb HTTPClient) ClientMetadata(ctx context.Context, endpoint string) (*oaut return nil, err } var metadata oauth.OAuthClientMetadata - return &metadata, hb.doRequest(request, &metadata) + return &metadata, hb.doRequest(ctx, request, &metadata) } // PresentationDefinition retrieves the presentation definition from the presentation definition endpoint (as specified by RFC021) for the given scope. @@ -115,7 +115,7 @@ func (hb HTTPClient) PresentationDefinition(ctx context.Context, presentationDef return nil, err } var presentationDefinition pe.PresentationDefinition - return &presentationDefinition, hb.doRequest(request, &presentationDefinition) + return &presentationDefinition, hb.doRequest(ctx, request, &presentationDefinition) } func (hb HTTPClient) AccessToken(ctx context.Context, tokenEndpoint string, vp vc.VerifiablePresentation, submission pe.PresentationSubmission, scopes string) (oauth.TokenResponse, error) { @@ -205,14 +205,14 @@ func (hb HTTPClient) postFormExpectRedirect(ctx context.Context, form url.Values request.Header.Add("Accept", "application/json") request.Header.Add("Content-Type", "application/x-www-form-urlencoded") var redirect oauth.Redirect - if err := hb.doRequest(request, &redirect); err != nil { + if err := hb.doRequest(ctx, request, &redirect); err != nil { return "", err } return redirect.RedirectURI, nil } -func (hb HTTPClient) doRequest(request *http.Request, target interface{}) error { - response, err := hb.httpClient.Do(request) +func (hb HTTPClient) doRequest(ctx context.Context, request *http.Request, target interface{}) error { + response, err := hb.httpClient.Do(request.WithContext(ctx)) if err != nil { return fmt.Errorf("failed to call endpoint: %w", err) } diff --git a/auth/services/oauth/holder_test.go b/auth/services/oauth/holder_test.go index 0d7696046a..44138cce45 100644 --- a/auth/services/oauth/holder_test.go +++ b/auth/services/oauth/holder_test.go @@ -174,28 +174,6 @@ func TestHolderService_BuildPresentation(t *testing.T) { }) } -func TestHolderService_PresentationDefinition(t *testing.T) { - t.Run("ok", func(t *testing.T) { - ctx := createOAuthHolderContext(t) - endpoint := fmt.Sprintf("%s/presentation_definition", ctx.tlsServer.URL) - - pd, err := ctx.holder.PresentationDefinition(context.Background(), endpoint) - - assert.NoError(t, err) - assert.NotNil(t, pd) - }) - t.Run("error", func(t *testing.T) { - ctx := createOAuthHolderContext(t) - endpoint := fmt.Sprintf("%s/presentation_definition", ctx.tlsServer.URL) - ctx.presentationDefinition = nil - - pd, err := ctx.holder.PresentationDefinition(context.Background(), endpoint) - - assert.Error(t, err) - assert.Nil(t, pd) - }) -} - type holderTestContext struct { ctrl *gomock.Controller audit context.Context @@ -226,14 +204,13 @@ func createHolderContext(t *testing.T, tlsConfig *tls.Config) *holderTestContext type holderOAuthTestContext struct { *holderTestContext - authzServerMetadata *oauth.AuthorizationServerMetadata - handler http.HandlerFunc - tlsServer *httptest.Server - verifierDID did.DID - metadata func(writer http.ResponseWriter) - errorResponse func(writer http.ResponseWriter) - response func(writer http.ResponseWriter) - presentationDefinition func(writer http.ResponseWriter) + authzServerMetadata *oauth.AuthorizationServerMetadata + handler http.HandlerFunc + tlsServer *httptest.Server + verifierDID did.DID + metadata func(writer http.ResponseWriter) + errorResponse func(writer http.ResponseWriter) + response func(writer http.ResponseWriter) } func createOAuthHolderContext(t *testing.T) *holderOAuthTestContext { @@ -256,13 +233,6 @@ func createOAuthHolderContext(t *testing.T) *holderOAuthTestContext { _, _ = writer.Write(bytes) return }, - presentationDefinition: func(writer http.ResponseWriter) { - writer.Header().Add("Content-Type", "application/json") - writer.WriteHeader(http.StatusOK) - bytes, _ := json.Marshal(pe.PresentationDefinition{}) - _, _ = writer.Write(bytes) - return - }, response: func(writer http.ResponseWriter) { writer.Header().Add("Content-Type", "application/json") writer.WriteHeader(http.StatusOK) @@ -287,11 +257,6 @@ func createOAuthHolderContext(t *testing.T) *holderOAuthTestContext { ctx.errorResponse(writer) return } - case "/presentation_definition": - if ctx.presentationDefinition != nil { - ctx.presentationDefinition(writer) - return - } case "/response": if ctx.response != nil { assert.NotEmpty(t, request.FormValue(oauth.VpTokenParam)) diff --git a/auth/services/oauth/interface.go b/auth/services/oauth/interface.go index e7de3fa5e0..8d46be654b 100644 --- a/auth/services/oauth/interface.go +++ b/auth/services/oauth/interface.go @@ -34,7 +34,6 @@ type RelyingParty interface { CreateJwtGrant(ctx context.Context, request services.CreateJwtGrantRequest) (*services.JwtBearerTokenResult, error) // CreateAuthorizationRequest creates an OAuth2.0 authorizationRequest redirect URL that redirects to the authorization server. CreateAuthorizationRequest(ctx context.Context, requestHolder did.DID, verifier did.DID, scopes string, clientState string) (*url.URL, error) - // RequestRFC003AccessToken is called by the local EHR node to request an access token from a remote Nuts node using Nuts RFC003. RequestRFC003AccessToken(ctx context.Context, jwtGrantToken string, authServerEndpoint url.URL) (*oauth.TokenResponse, error) // RequestRFC021AccessToken is called by the local EHR node to request an access token from a remote Nuts node using Nuts RFC021. diff --git a/e2e-tests/oauth-flow/rfc021/docker-compose.yml b/e2e-tests/oauth-flow/rfc021/docker-compose.yml index ff22a27e89..c498092832 100644 --- a/e2e-tests/oauth-flow/rfc021/docker-compose.yml +++ b/e2e-tests/oauth-flow/rfc021/docker-compose.yml @@ -15,7 +15,11 @@ services: # So we need to mount that file to the OS CA bundle location, otherwise did:web resolving will fail due to untrusted certs. - "../../tls-certs/truststore.pem:/etc/ssl/certs/Nuts_RootCA.pem:ro" - "../../tls-certs/truststore.pem:/etc/ssl/certs/truststore.pem:ro" +<<<<<<< HEAD - "./node-A/presentationexchangemapping.json:/opt/nuts/policies/presentationexchangemapping.json:ro" +======= + - "./node-A/presentationexchangemapping.json:/opt/nuts/presentationexchangemapping.json:ro" +>>>>>>> cb23775a (added authorize endpoint as specified by rfc6549 authorization code (#2626)) healthcheck: interval: 1s # Make test run quicker by checking health status more often nodeA: