Skip to content

Commit

Permalink
fixes for rfc021 client status codes (#3433)
Browse files Browse the repository at this point in the history
  • Loading branch information
woutslakhorst authored Oct 2, 2024
1 parent 5aa37ef commit ee55a89
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 19 deletions.
14 changes: 9 additions & 5 deletions auth/api/iam/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,33 +27,34 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/labstack/echo/v4"
"github.com/lestrrat-go/jwx/v2/jwk"
"github.com/nuts-foundation/nuts-node/http/cache"
"github.com/nuts-foundation/nuts-node/http/user"
"github.com/nuts-foundation/nuts-node/vdr/didsubject"
"html/template"
"net/http"
"net/url"
"strings"
"time"

"github.com/labstack/echo/v4"
"github.com/lestrrat-go/jwx/v2/jwk"
"github.com/lestrrat-go/jwx/v2/jwt"
"github.com/nuts-foundation/go-did/did"
"github.com/nuts-foundation/nuts-node/audit"
"github.com/nuts-foundation/nuts-node/auth"
"github.com/nuts-foundation/nuts-node/auth/api/iam/assets"
iamclient "github.com/nuts-foundation/nuts-node/auth/client/iam"
"github.com/nuts-foundation/nuts-node/auth/log"
"github.com/nuts-foundation/nuts-node/auth/oauth"
"github.com/nuts-foundation/nuts-node/core"
nutsCrypto "github.com/nuts-foundation/nuts-node/crypto"
nutsHttp "github.com/nuts-foundation/nuts-node/http"
"github.com/nuts-foundation/nuts-node/http/cache"
"github.com/nuts-foundation/nuts-node/http/user"
"github.com/nuts-foundation/nuts-node/jsonld"
"github.com/nuts-foundation/nuts-node/policy"
"github.com/nuts-foundation/nuts-node/storage"
"github.com/nuts-foundation/nuts-node/vcr"
"github.com/nuts-foundation/nuts-node/vcr/pe"
"github.com/nuts-foundation/nuts-node/vdr"
"github.com/nuts-foundation/nuts-node/vdr/didsubject"
"github.com/nuts-foundation/nuts-node/vdr/resolver"
)

Expand Down Expand Up @@ -202,6 +203,9 @@ func (r Wrapper) ResolveStatusCode(err error) int {
resolver.ErrDIDNotManagedByThisNode: http.StatusBadRequest,
pe.ErrNoCredentials: http.StatusPreconditionFailed,
didsubject.ErrSubjectNotFound: http.StatusNotFound,
iamclient.ErrInvalidClientCall: http.StatusBadRequest,
iamclient.ErrBadGateway: http.StatusBadGateway,
iamclient.ErrPreconditionFailed: http.StatusPreconditionFailed,
})
}

Expand Down
26 changes: 25 additions & 1 deletion auth/client/iam/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"github.com/lestrrat-go/jwx/v2/jws"
"github.com/lestrrat-go/jwx/v2/jwt"
Expand All @@ -40,6 +41,12 @@ import (
"github.com/nuts-foundation/nuts-node/vcr/pe"
)

// ErrInvalidClientCall is returned when the node makes a http call as client based on wrong information passed by the client.
var ErrInvalidClientCall = errors.New("invalid client call")

// ErrBadGateway is returned when the node makes a http call as client and the upstream returns an unexpected result.
var ErrBadGateway = errors.New("upstream returned unexpected result")

// HTTPClient holds the server address and other basic settings for the http client
type HTTPClient struct {
strictMode bool
Expand All @@ -63,7 +70,14 @@ func (hb HTTPClient) OAuthAuthorizationServerMetadata(ctx context.Context, oauth
}
var metadata oauth.AuthorizationServerMetadata
if err = hb.doGet(ctx, metadataURL.String(), &metadata); err != nil {
return nil, err
// if this is a core.HttpError and the status code >= 500 then we want the caller to receive a 502 Bad Gateway
// we do this by changing the status code of the error
// any other error should result in a 400 Bad Request
if httpErr, ok := err.(core.HttpError); ok && httpErr.StatusCode >= 500 {
httpErr.StatusCode = http.StatusBadGateway
return nil, httpErr
}
return nil, errors.Join(ErrInvalidClientCall, err)
}
return &metadata, err
}
Expand Down Expand Up @@ -93,6 +107,16 @@ func (hb HTTPClient) PresentationDefinition(ctx context.Context, presentationDef
return nil, err
}
var presentationDefinition pe.PresentationDefinition
err = hb.doRequest(ctx, request, &presentationDefinition)
if err != nil {
// a 404 (defined by scope) should result in a 400 for the client
// any other error should result in a 502 Bad Gateway
if httpErr, ok := err.(core.HttpError); ok && httpErr.StatusCode == 404 {
return nil, errors.Join(ErrInvalidClientCall, err)
}
return nil, errors.Join(ErrBadGateway, err)
}

return &presentationDefinition, hb.doRequest(ctx, request, &presentationDefinition)
}

Expand Down
11 changes: 11 additions & 0 deletions auth/client/iam/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) {
assert.Equal(t, "GET", handler.Request.Method)
assert.Equal(t, "/.well-known/oauth-authorization-server/iam/123", handler.Request.URL.Path)
})
t.Run("error - server error changes status code to 502", func(t *testing.T) {
handler := http2.Handler{StatusCode: http.StatusInternalServerError}
tlsServer, client := testServerAndClient(t, &handler)

_, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL)

require.Error(t, err)
httpErr, ok := err.(core.HttpError)
require.True(t, ok)
assert.Equal(t, http.StatusBadGateway, httpErr.StatusCode)
})
}

func TestHTTPClient_PresentationDefinition(t *testing.T) {
Expand Down
8 changes: 6 additions & 2 deletions auth/client/iam/openid4vp.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package iam
import (
"context"
"encoding/json"
"errors"
"fmt"
"github.com/nuts-foundation/nuts-node/http/client"
"github.com/nuts-foundation/nuts-node/vcr/credential"
Expand All @@ -46,6 +47,9 @@ import (
"github.com/nuts-foundation/nuts-node/vdr/resolver"
)

// ErrPreconditionFailed is returned when a precondition is not met.
var ErrPreconditionFailed = errors.New("precondition failed")

var _ Client = (*OpenID4VPClient)(nil)

type OpenID4VPClient struct {
Expand Down Expand Up @@ -282,7 +286,7 @@ func (c *OpenID4VPClient) RequestRFC021AccessToken(ctx context.Context, clientID
for key := range maps.Keys(allMethods) {
availableMethods = append(availableMethods, key)
}
return nil, fmt.Errorf("did method mismatch, requested: %v, available: %v", metadata.DIDMethodsSupported, availableMethods)
return nil, errors.Join(ErrPreconditionFailed, fmt.Errorf("did method mismatch, requested: %v, available: %v", metadata.DIDMethodsSupported, availableMethods))
}

// each additional credential can be used by each DID
Expand Down Expand Up @@ -320,7 +324,7 @@ func (c *OpenID4VPClient) RequestRFC021AccessToken(ctx context.Context, clientID
}
dpopHeader, dpopKid, err = c.dpop(ctx, *subjectDID, *request)
if err != nil {
return nil, fmt.Errorf("failed tocreate DPoP header: %w", err)
return nil, fmt.Errorf("failed to create DPoP header: %w", err)
}
}

Expand Down
22 changes: 14 additions & 8 deletions auth/client/iam/openid4vp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ func TestIAMClient_AuthorizationServerMetadata(t *testing.T) {
_, err := ctx.client.AuthorizationServerMetadata(context.Background(), ctx.tlsServer.URL)

require.Error(t, err)
assert.EqualError(t, err, "failed to retrieve remote OAuth Authorization Server metadata: server returned HTTP 404 (expected: 200)")
assert.ErrorIs(t, err, ErrInvalidClientCall)
assert.ErrorContains(t, err, "server returned HTTP 404 (expected: 200)")
})
}

Expand Down Expand Up @@ -275,7 +276,9 @@ func TestRelyingParty_RequestRFC021AccessToken(t *testing.T) {

response, err := ctx.client.RequestRFC021AccessToken(context.Background(), subjectClientID, subjectID, ctx.verifierURL.String(), scopes, false, nil)

assert.EqualError(t, err, "did method mismatch, requested: [other], available: [test]")
require.Error(t, err)
assert.ErrorIs(t, err, ErrPreconditionFailed)
assert.ErrorContains(t, err, "did method mismatch, requested: [other], available: [test]")
assert.Nil(t, response)
})
t.Run("with additional credentials", func(t *testing.T) {
Expand Down Expand Up @@ -357,17 +360,19 @@ func TestRelyingParty_RequestRFC021AccessToken(t *testing.T) {

_, err := ctx.client.RequestRFC021AccessToken(context.Background(), subjectClientID, subjectID, ctx.verifierURL.String(), scopes, false, nil)

assert.Error(t, err)
assert.EqualError(t, err, "failed to retrieve presentation definition: server returned HTTP 404 (expected: 200)")
require.Error(t, err)
assert.ErrorIs(t, err, ErrInvalidClientCall)
assert.ErrorContains(t, err, "server returned HTTP 404 (expected: 200)")
})
t.Run("error - failed to get authorization server metadata", func(t *testing.T) {
ctx := createClientServerTestContext(t)
ctx.metadata = nil

_, err := ctx.client.RequestRFC021AccessToken(context.Background(), subjectClientID, subjectID, ctx.verifierURL.String(), scopes, false, nil)

assert.Error(t, err)
assert.EqualError(t, err, "failed to retrieve remote OAuth Authorization Server metadata: server returned HTTP 404 (expected: 200)")
require.Error(t, err)
assert.ErrorIs(t, err, ErrInvalidClientCall)
assert.ErrorContains(t, err, "server returned HTTP 404 (expected: 200)")
})
t.Run("error - faulty presentation definition", func(t *testing.T) {
ctx := createClientServerTestContext(t)
Expand All @@ -379,8 +384,9 @@ func TestRelyingParty_RequestRFC021AccessToken(t *testing.T) {

_, err := ctx.client.RequestRFC021AccessToken(context.Background(), subjectClientID, subjectID, ctx.verifierURL.String(), scopes, false, nil)

assert.Error(t, err)
assert.EqualError(t, err, "failed to retrieve presentation definition: unable to unmarshal response: unexpected end of JSON input")
require.Error(t, err)
assert.ErrorIs(t, err, ErrBadGateway)
assert.ErrorContains(t, err, "unable to unmarshal response: unexpected end of JSON input")
})
t.Run("error - failed to build vp", func(t *testing.T) {
ctx := createClientServerTestContext(t)
Expand Down
7 changes: 4 additions & 3 deletions docs/_static/auth/v2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ paths:
It'll initiate a s2s (RFC021) flow.
error returns:
* 400 - one of the parameters has the wrong format or an OAuth error occurred
* 412 - the organization wallet does not contain the correct credentials
* 503 - the authorizer could not be reached or returned an error
* 400 - one of the parameters has the wrong format, an OAuth error occurred, or the http client calling the authorizer returned an error due to incorrect input
* 412 - the organization wallet does not contain the correct credentials or doesn't support the right DID methods
* 502 - the authorizer returned an error
* 503 - the authorizer could not be reached
tags:
- auth
parameters:
Expand Down
37 changes: 37 additions & 0 deletions vdr/didsubject/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit ee55a89

Please sign in to comment.