Skip to content

Commit 5e92037

Browse files
woutslakhorstwoutslakhorst
committed
check OpenIDConfiguration signature with did keys and add test for htp client
test for OpenIDConfiguration server API disable useless test bug more coverage
1 parent 405bd1f commit 5e92037

File tree

5 files changed

+221
-35
lines changed

5 files changed

+221
-35
lines changed

‎auth/api/iam/api.go

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ func (r Wrapper) OpenIDConfiguration(ctx context.Context, request OpenIDConfigur
639639
}
640640
if !exists {
641641
return nil, oauth.OAuth2Error{
642-
Code: "not_found",
642+
Code: oauth.InvalidRequest,
643643
Description: "subject not found",
644644
}
645645
}
@@ -680,22 +680,8 @@ func (r Wrapper) OpenIDConfiguration(ctx context.Context, request OpenIDConfigur
680680
// we sign with a JWK, the receiving party can verify with the signature but not if the key corresponds to the DID since the DID method might not be supported.
681681
// this is a shortcoming of the openID federation vs OpenID4VP/DID worlds
682682
// issuer URL equals server baseURL + :/oauth2/:subject
683-
baseURL := r.auth.PublicURL()
684-
if baseURL == nil {
685-
return nil, oauth.OAuth2Error{
686-
Code: oauth.ServerError,
687-
Description: "misconfiguration: missing public URL",
688-
}
689-
}
690-
issuerURL, err := baseURL.Parse("/oauth2/" + request.Subject)
691-
if err != nil {
692-
return nil, oauth.OAuth2Error{
693-
Code: oauth.ServerError,
694-
Description: "internal server error",
695-
InternalError: err,
696-
}
697-
}
698-
configuration := openIDConfiguration(*issuerURL, set, r.vdr.SupportedMethods())
683+
issuerURL := r.subjectToBaseURL(request.Subject)
684+
configuration := openIDConfiguration(issuerURL, set, r.vdr.SupportedMethods())
699685
claims := make(map[string]interface{})
700686
asJson, _ := json.Marshal(configuration)
701687
_ = json.Unmarshal(asJson, &claims)

‎auth/api/iam/api_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@ import (
2626
"fmt"
2727
"github.com/nuts-foundation/nuts-node/core/to"
2828
"github.com/nuts-foundation/nuts-node/crypto/storage/spi"
29+
test2 "github.com/nuts-foundation/nuts-node/crypto/test"
2930
"github.com/nuts-foundation/nuts-node/http/user"
3031
"github.com/nuts-foundation/nuts-node/test"
3132
"github.com/nuts-foundation/nuts-node/vdr/didsubject"
33+
"io"
3234
"net/http"
3335
"net/http/httptest"
3436
"net/url"
@@ -121,6 +123,76 @@ func TestWrapper_GetOAuthClientMetadata(t *testing.T) {
121123
assert.IsType(t, OAuthClientMetadata200JSONResponse{}, res)
122124
})
123125
}
126+
127+
func TestWrapper_OpenIDConfiguration(t *testing.T) {
128+
testKey := test2.GenerateECKey()
129+
t.Run("ok", func(t *testing.T) {
130+
ctx := newTestClient(t)
131+
ctx.keyResolver.EXPECT().ResolveKey(verifierDID, nil, resolver.AssertionMethod).Return("kid", testKey.Public(), nil)
132+
ctx.jwtSigner.EXPECT().SignJWT(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(_ interface{}, claims interface{}, headers interface{}, kid interface{}) (string, error) {
133+
asMap := claims.(map[string]interface{})
134+
assert.Equal(t, "https://example.com/oauth2/verifier", asMap["iss"])
135+
assert.Len(t, asMap["jwks"], 1)
136+
return "token", nil
137+
})
138+
139+
res, err := ctx.client.OpenIDConfiguration(nil, OpenIDConfigurationRequestObject{Subject: verifierSubject})
140+
141+
require.NoError(t, err)
142+
assert.IsType(t, OpenIDConfiguration200ApplicationentityStatementJwtResponse{}, res)
143+
successResponse := res.(OpenIDConfiguration200ApplicationentityStatementJwtResponse)
144+
bodyBytes, err := io.ReadAll(successResponse.Body)
145+
require.NoError(t, err)
146+
assert.Equal(t, "token", string(bodyBytes))
147+
})
148+
t.Run("error - subject does not exist", func(t *testing.T) {
149+
ctx := newTestClient(t)
150+
151+
res, err := ctx.client.OpenIDConfiguration(nil, OpenIDConfigurationRequestObject{Subject: unknownSubjectID})
152+
153+
requireOAuthError(t, err, oauth.InvalidRequest, "subject not found")
154+
assert.Nil(t, res)
155+
})
156+
t.Run("error - subject exists returns error", func(t *testing.T) {
157+
ctx := newTestClient(t)
158+
ctx.subjectManager.EXPECT().Exists(gomock.Any(), "error").Return(false, assert.AnError)
159+
160+
res, err := ctx.client.OpenIDConfiguration(nil, OpenIDConfigurationRequestObject{Subject: "error"})
161+
162+
requireOAuthError(t, err, oauth.ServerError, "internal server error")
163+
assert.Nil(t, res)
164+
})
165+
t.Run("error - subject existslist DIDs returns error", func(t *testing.T) {
166+
ctx := newTestClient(t)
167+
ctx.subjectManager.EXPECT().Exists(gomock.Any(), "error").Return(true, nil)
168+
ctx.subjectManager.EXPECT().List(gomock.Any(), "error").Return(nil, assert.AnError)
169+
170+
res, err := ctx.client.OpenIDConfiguration(nil, OpenIDConfigurationRequestObject{Subject: "error"})
171+
172+
requireOAuthError(t, err, oauth.ServerError, "internal server error")
173+
assert.Nil(t, res)
174+
})
175+
t.Run("error - key resolution error", func(t *testing.T) {
176+
ctx := newTestClient(t)
177+
ctx.keyResolver.EXPECT().ResolveKey(verifierDID, nil, resolver.AssertionMethod).Return("", nil, assert.AnError)
178+
179+
res, err := ctx.client.OpenIDConfiguration(nil, OpenIDConfigurationRequestObject{Subject: verifierSubject})
180+
181+
requireOAuthError(t, err, oauth.ServerError, "internal server error")
182+
assert.Nil(t, res)
183+
})
184+
t.Run("error - signing error", func(t *testing.T) {
185+
ctx := newTestClient(t)
186+
ctx.keyResolver.EXPECT().ResolveKey(verifierDID, nil, resolver.AssertionMethod).Return("kid", testKey.Public(), nil)
187+
ctx.jwtSigner.EXPECT().SignJWT(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("", assert.AnError)
188+
189+
res, err := ctx.client.OpenIDConfiguration(nil, OpenIDConfigurationRequestObject{Subject: verifierSubject})
190+
191+
requireOAuthError(t, err, oauth.ServerError, "internal server error")
192+
assert.Nil(t, res)
193+
})
194+
}
195+
124196
func TestWrapper_PresentationDefinition(t *testing.T) {
125197
ctx := audit.TestContext()
126198
walletOwnerMapping := pe.WalletOwnerMapping{pe.WalletOwnerOrganization: pe.PresentationDefinition{Id: "test"}}

‎auth/client/iam/client.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,15 @@ import (
2323
"context"
2424
"encoding/json"
2525
"fmt"
26+
"github.com/lestrrat-go/jwx/v2/jws"
2627
"github.com/lestrrat-go/jwx/v2/jwt"
28+
"github.com/nuts-foundation/nuts-node/crypto"
29+
"github.com/nuts-foundation/nuts-node/vdr/resolver"
2730
"io"
2831
"net/http"
2932
"net/url"
3033
"strings"
34+
"time"
3135

3236
"github.com/nuts-foundation/go-did/vc"
3337
"github.com/nuts-foundation/nuts-node/auth/log"
@@ -38,8 +42,9 @@ import (
3842

3943
// HTTPClient holds the server address and other basic settings for the http client
4044
type HTTPClient struct {
41-
strictMode bool
42-
httpClient core.HTTPRequestDoer
45+
strictMode bool
46+
keyResolver resolver.KeyResolver
47+
httpClient core.HTTPRequestDoer
4348
}
4449

4550
// OAuthAuthorizationServerMetadata retrieves the OAuth authorization server metadata for the given oauth issuer.
@@ -232,10 +237,8 @@ func (hb HTTPClient) OpenIDConfiguration(ctx context.Context, issuerURL string)
232237
return nil, err
233238
}
234239
var configuration oauth.OpenIDConfiguration
235-
request, err := http.NewRequestWithContext(ctx, http.MethodGet, metadataURL.String(), nil)
236-
if err != nil {
237-
return nil, err
238-
}
240+
// url already checked
241+
request, _ := http.NewRequestWithContext(ctx, http.MethodGet, metadataURL.String(), nil)
239242
response, err := hb.httpClient.Do(request.WithContext(ctx))
240243
if err != nil {
241244
return nil, fmt.Errorf("failed to call endpoint: %w", err)
@@ -247,8 +250,8 @@ func (hb HTTPClient) OpenIDConfiguration(ctx context.Context, issuerURL string)
247250
if data, err = core.LimitedReadAll(response.Body); err != nil {
248251
return nil, fmt.Errorf("unable to read response: %w", err)
249252
}
250-
// todo check kid against something? get keys from somewhere? (issuerURL to keys)
251-
token, err := jwt.Parse(data, jwt.WithVerify(false))
253+
// kid is checked against did resolver
254+
token, err := jwt.Parse(data, jwt.WithKeyProvider(hb.KeyProvider()), jwt.WithAcceptableSkew(5*time.Second))
252255
if err != nil {
253256
return nil, fmt.Errorf("unable to parse response: %w", err)
254257
}
@@ -259,13 +262,28 @@ func (hb HTTPClient) OpenIDConfiguration(ctx context.Context, issuerURL string)
259262
// hack, broken iat
260263
claims["iat"] = token.IssuedAt().Unix()
261264
asJSON, _ := json.Marshal(claims)
262-
println("TOKEN ", string(asJSON))
263265
if err = json.Unmarshal(asJSON, &configuration); err != nil {
264266
return nil, fmt.Errorf("unable to unmarshal response: %w", err)
265267
}
266268
return &configuration, err
267269
}
268270

271+
func (hb HTTPClient) KeyProvider() jws.KeyProviderFunc {
272+
return func(context context.Context, keySink jws.KeySink, signature *jws.Signature, message *jws.Message) error {
273+
keyID := signature.ProtectedHeaders().KeyID()
274+
publicKey, err := hb.keyResolver.ResolveKeyByID(keyID, nil, resolver.AssertionMethod)
275+
if err != nil {
276+
return fmt.Errorf("failed to resolve key (kid=%s): %w", keyID, err)
277+
}
278+
alg, err := crypto.SignatureAlgorithm(publicKey)
279+
if err != nil {
280+
return fmt.Errorf("failed to resolve key (kid=%s): %w", keyID, err)
281+
}
282+
keySink.Key(alg, publicKey)
283+
return nil
284+
}
285+
}
286+
269287
// CredentialRequest represents ths request to fetch a credential, the JSON object holds the proof as
270288
// CredentialRequestProof.
271289
type CredentialRequest struct {

‎auth/client/iam/client_test.go

Lines changed: 116 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,21 @@ package iam
2020

2121
import (
2222
"context"
23+
"crypto"
24+
"crypto/ecdsa"
25+
"encoding/json"
26+
"github.com/google/uuid"
27+
"github.com/lestrrat-go/jwx/v2/jws"
28+
"github.com/nuts-foundation/go-did/did"
29+
"github.com/nuts-foundation/nuts-node/audit"
30+
nutsCrypto "github.com/nuts-foundation/nuts-node/crypto"
31+
test2 "github.com/nuts-foundation/nuts-node/crypto/test"
32+
"github.com/nuts-foundation/nuts-node/vdr/resolver"
2333
"net/http"
2434
"net/http/httptest"
2535
"net/url"
2636
"testing"
37+
"time"
2738

2839
ssi "github.com/nuts-foundation/go-did"
2940
"github.com/nuts-foundation/go-did/vc"
@@ -173,6 +184,83 @@ func TestHTTPClient_ClientMetadata(t *testing.T) {
173184
})
174185
}
175186

187+
func TestHTTPClient_OpenIDConfiguration(t *testing.T) {
188+
ctx := context.Background()
189+
configuration := oauth.OpenIDConfiguration{
190+
Issuer: "issuer",
191+
}
192+
193+
// create jwt
194+
createToken := func(t *testing.T, client *HTTPClient) string {
195+
testKey := client.keyResolver.(testKeyResolver).key
196+
claims := make(map[string]interface{})
197+
asJson, _ := json.Marshal(configuration)
198+
_ = json.Unmarshal(asJson, &claims)
199+
alg, _ := nutsCrypto.SignatureAlgorithm(testKey.Public())
200+
headers := map[string]interface{}{jws.AlgorithmKey: alg, jws.KeyIDKey: "test"}
201+
token, err := nutsCrypto.SignJWT(audit.TestContext(), testKey, alg, claims, headers)
202+
require.NoError(t, err)
203+
return token
204+
}
205+
206+
t.Run("ok", func(t *testing.T) {
207+
handler := http2.Handler{StatusCode: http.StatusOK}
208+
tlsServer, client := testServerAndClient(t, &handler)
209+
handler.ResponseData = createToken(t, client)
210+
211+
response, err := client.OpenIDConfiguration(ctx, tlsServer.URL)
212+
213+
require.NoError(t, err)
214+
require.NotNil(t, response)
215+
assert.Equal(t, configuration, *response)
216+
require.NotNil(t, handler.Request)
217+
})
218+
t.Run("error - invalid url", func(t *testing.T) {
219+
handler := http2.Handler{StatusCode: http.StatusOK}
220+
_, client := testServerAndClient(t, &handler)
221+
handler.ResponseData = createToken(t, client)
222+
223+
_, err := client.OpenIDConfiguration(ctx, ":")
224+
225+
require.Error(t, err)
226+
assert.EqualError(t, err, "parse \":\": missing protocol scheme")
227+
})
228+
t.Run("error - error return", func(t *testing.T) {
229+
handler := http2.Handler{StatusCode: http.StatusInternalServerError}
230+
tlsServer, client := testServerAndClient(t, &handler)
231+
232+
response, err := client.OpenIDConfiguration(ctx, tlsServer.URL)
233+
234+
require.Error(t, err)
235+
require.Nil(t, response)
236+
assert.EqualError(t, err, "server returned HTTP 500 (expected: 200)")
237+
})
238+
t.Run("error - not a signed jwt", func(t *testing.T) {
239+
handler := http2.Handler{StatusCode: http.StatusOK, ResponseData: ""}
240+
tlsServer, client := testServerAndClient(t, &handler)
241+
242+
response, err := client.OpenIDConfiguration(ctx, tlsServer.URL)
243+
244+
require.Error(t, err)
245+
require.Nil(t, response)
246+
assert.EqualError(t, err, "unable to parse response: failed to parse jws: invalid byte sequence")
247+
})
248+
t.Run("error - unknown key", func(t *testing.T) {
249+
otherClient := &HTTPClient{
250+
keyResolver: newTestKeyResolver(),
251+
}
252+
handler := http2.Handler{StatusCode: http.StatusOK}
253+
tlsServer, client := testServerAndClient(t, &handler)
254+
handler.ResponseData = createToken(t, otherClient)
255+
256+
response, err := client.OpenIDConfiguration(ctx, tlsServer.URL)
257+
258+
require.Error(t, err)
259+
require.Nil(t, response)
260+
assert.EqualError(t, err, "unable to parse response: could not verify message using any of the signatures or keys")
261+
})
262+
}
263+
176264
func TestHTTPClient_PostError(t *testing.T) {
177265
redirectReturn := oauth.Redirect{
178266
RedirectURI: "http://test.test",
@@ -299,13 +387,6 @@ func TestHTTPClient_RequestObjectPost(t *testing.T) {
299387
})
300388
}
301389

302-
func testServerAndClient(t *testing.T, handler http.Handler) (*httptest.Server, *HTTPClient) {
303-
tlsServer := http2.TestTLSServer(t, handler)
304-
return tlsServer, &HTTPClient{
305-
httpClient: tlsServer.Client(),
306-
}
307-
}
308-
309390
func TestHTTPClient_doGet(t *testing.T) {
310391
t.Run("error - non 200 return value", func(t *testing.T) {
311392
handler := http2.Handler{StatusCode: http.StatusBadRequest}
@@ -333,3 +414,31 @@ func TestHTTPClient_doGet(t *testing.T) {
333414
assert.Error(t, err)
334415
})
335416
}
417+
418+
func newTestKeyResolver() resolver.KeyResolver {
419+
return testKeyResolver{
420+
kid: uuid.NewString(),
421+
key: test2.GenerateECKey(),
422+
}
423+
}
424+
425+
type testKeyResolver struct {
426+
kid string
427+
key *ecdsa.PrivateKey
428+
}
429+
430+
func (t testKeyResolver) ResolveKeyByID(keyID string, validAt *time.Time, relationType resolver.RelationType) (crypto.PublicKey, error) {
431+
return t.key.Public(), nil
432+
}
433+
434+
func (t testKeyResolver) ResolveKey(id did.DID, validAt *time.Time, relationType resolver.RelationType) (string, crypto.PublicKey, error) {
435+
return t.kid, t.key.Public(), nil
436+
}
437+
438+
func testServerAndClient(t *testing.T, handler http.Handler) (*httptest.Server, *HTTPClient) {
439+
tlsServer := http2.TestTLSServer(t, handler)
440+
return tlsServer, &HTTPClient{
441+
httpClient: tlsServer.Client(),
442+
keyResolver: newTestKeyResolver(),
443+
}
444+
}

‎auth/client/iam/openid4vp.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,9 @@ func NewClient(wallet holder.Wallet, keyResolver resolver.KeyResolver, subjectMa
6262
ldDocumentLoader ld.DocumentLoader, strictMode bool, httpClientTimeout time.Duration) *OpenID4VPClient {
6363
return &OpenID4VPClient{
6464
httpClient: HTTPClient{
65-
strictMode: strictMode,
66-
httpClient: client.NewWithCache(httpClientTimeout),
65+
strictMode: strictMode,
66+
httpClient: client.NewWithCache(httpClientTimeout),
67+
keyResolver: keyResolver,
6768
},
6869
keyResolver: keyResolver,
6970
jwtSigner: jwtSigner,

0 commit comments

Comments
 (0)