Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
reinkrul committed Dec 8, 2023
1 parent d1d3ed7 commit a8d456c
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 22 deletions.
16 changes: 5 additions & 11 deletions auth/api/iam/s2s_vptoken.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,17 +239,11 @@ func (r *Wrapper) validateS2SPresentationNonce(presentation vc.VerifiablePresent
switch presentation.Format() {
case vc.JWTPresentationProofFormat:
nonceRaw, hasNonce := presentation.JWT().Get("nonce")
nonce, hasNonce = nonceRaw.(string)
if !hasNonce {
return oauth.OAuth2Error{
Code: oauth.InvalidRequest,
Description: "presentation is missing nonce",
}
}
nonce, _ = nonceRaw.(string)
if nonce == "" {
return oauth.OAuth2Error{
Code: oauth.InvalidRequest,
Description: "presentation has an invalid nonce",
Description: "presentation has invalid/missing nonce",
}
}
case vc.JSONLDPresentationProofFormat:
Expand All @@ -269,15 +263,15 @@ func (r *Wrapper) validateS2SPresentationNonce(presentation vc.VerifiablePresent
if nonceError != nil && errors.Is(nonceError, storage.ErrNotFound) {
// this is OK, nonce has not been used before
nonceError = nil
} else if nonceError != nil {
// other error occurred. Keep error to report after storing nonce
} else {
} else if nonceError == nil {
// no store error: value was retrieved from store, meaning the nonce has been used before
nonceError = oauth.OAuth2Error{
Code: oauth.InvalidRequest,
Description: "presentation nonce has already been used",
}
}
// Other error occurred. Keep error to report after storing nonce.

// Regardless the result of the nonce checking, the nonce of the VP must not be used again.
// So always store the nonce.
if err := nonceStore.Put(nonce, true); err != nil {
Expand Down
48 changes: 37 additions & 11 deletions auth/api/iam/s2s_vptoken_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,24 +213,50 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) {
assert.Equal(t, int(accessTokenValidity.Seconds()), *tokenResponse.ExpiresIn)
assert.NotEmpty(t, tokenResponse.AccessToken)
})
t.Run("replay attack (nonce is reused)", func(t *testing.T) {
ctx := newTestClient(t)
ctx.verifier.EXPECT().VerifyVP(presentation, true, true, gomock.Any()).Return(presentation.VerifiableCredential, nil)

_, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, requestedScope, submissionJSON, presentation.Raw())
require.NoError(t, err)

resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, requestedScope, submissionJSON, presentation.Raw())
assert.EqualError(t, err, "invalid_request - presentation nonce has already been used")
assert.Nil(t, resp)
})
t.Run("VP is not valid JSON", func(t *testing.T) {
ctx := newTestClient(t)
resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, requestedScope, submissionJSON, "[true, false]")

assert.EqualError(t, err, "invalid_request - assertion parameter is invalid: unable to parse PEX envelope as verifiable presentation: invalid JWT")
assert.Nil(t, resp)
})
t.Run("nonce", func(t *testing.T) {
t.Run("replay attack (nonce is reused)", func(t *testing.T) {
ctx := newTestClient(t)
ctx.verifier.EXPECT().VerifyVP(presentation, true, true, gomock.Any()).Return(presentation.VerifiableCredential, nil)

_, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, requestedScope, submissionJSON, presentation.Raw())
require.NoError(t, err)

resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, requestedScope, submissionJSON, presentation.Raw())
assert.EqualError(t, err, "invalid_request - presentation nonce has already been used")
assert.Nil(t, resp)
})
t.Run("JSON-LD VP is missing nonce", func(t *testing.T) {
ctx := newTestClient(t)

proofVisitor := test.LDProofVisitor(func(proof *proof.LDProof) {
proof.Domain = &issuerDIDStr
proof.Nonce = nil
})
presentation := test.CreateJSONLDPresentation(t, *subjectDID, proofVisitor, verifiableCredential)

resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, requestedScope, submissionJSON, presentation.Raw())
assert.EqualError(t, err, "invalid_request - presentation has invalid proof or nonce")
assert.Nil(t, resp)
})
t.Run("JWT VP is missing nonce", func(t *testing.T) {
ctx := newTestClient(t)
presentation := test.CreateJWTPresentation(t, *subjectDID, func(token jwt.Token) {
_ = token.Set(jwt.AudienceKey, issuerDID.String())
_ = token.Remove("nonce")
}, verifiableCredential)

_, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, requestedScope, submissionJSON, presentation.Raw())

require.EqualError(t, err, "invalid_request - presentation has invalid/missing nonce")
})
})
t.Run("audience", func(t *testing.T) {
t.Run("missing", func(t *testing.T) {
ctx := newTestClient(t)
Expand Down

0 comments on commit a8d456c

Please sign in to comment.