Skip to content

Commit

Permalink
fix(protocol): trailing credential data skipped
Browse files Browse the repository at this point in the history
This fixes an issue where the trailing data in the credential parsing function would be ignored.
  • Loading branch information
james-d-elliott committed Dec 1, 2023
1 parent 5719110 commit e6c1318
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 6 deletions.
13 changes: 11 additions & 2 deletions protocol/credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"crypto/sha256"
"encoding/base64"
"encoding/json"
"errors"
"io"
"net/http"
)
Expand Down Expand Up @@ -48,7 +49,7 @@ type CredentialCreationResponse struct {
PublicKeyCredential
AttestationResponse AuthenticatorAttestationResponse `json:"response"`

// Deprecated: Transports is deprecated due to upstream changes to the API.
// Deprecated: Transports is deprecated due to upstream changes to the API.
// Use the Transports field of AuthenticatorAttestationResponse
// instead. Transports is kept for backward compatibility, and should not
// be used by new clients.
Expand All @@ -72,10 +73,18 @@ func ParseCredentialCreationResponse(response *http.Request) (*ParsedCredentialC
func ParseCredentialCreationResponseBody(body io.Reader) (pcc *ParsedCredentialCreationData, err error) {
var ccr CredentialCreationResponse

if err = json.NewDecoder(body).Decode(&ccr); err != nil {
decoder := json.NewDecoder(body)

if err = decoder.Decode(&ccr); err != nil {
return nil, ErrBadRequest.WithDetails("Parse error for Registration").WithInfo(err.Error())
}

_, err = decoder.Token()

if !errors.Is(err, io.EOF) {
return nil, ErrBadRequest.WithDetails("Parse error for Registration").WithInfo("The body contains trailing data")
}

return ccr.Parse()
}

Expand Down
42 changes: 38 additions & 4 deletions protocol/credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ func TestParseCredentialCreationResponse(t *testing.T) {
byteClientDataJSON, _ := base64.RawURLEncoding.DecodeString("eyJjaGFsbGVuZ2UiOiJXOEd6RlU4cEdqaG9SYldyTERsYW1BZnFfeTRTMUNaRzFWdW9lUkxBUnJFIiwib3JpZ2luIjoiaHR0cHM6Ly93ZWJhdXRobi5pbyIsInR5cGUiOiJ3ZWJhdXRobi5jcmVhdGUifQ")

testCases := []struct {
name string
args args
expected *ParsedCredentialCreationData
errString string
name string
args args
expected *ParsedCredentialCreationData
errString string
errType string
errDetails string
errInfo string
}{
{
name: "ShouldParseCredentialRequest",
Expand Down Expand Up @@ -215,6 +218,17 @@ func TestParseCredentialCreationResponse(t *testing.T) {
},
errString: "",
},
{
name: "ShouldHandleTrailingData",
args: args{
responseName: "trailingData",
},
expected: nil,
errString: "Parse error for Registration",
errType: "invalid_request",
errDetails: "Parse error for Registration",
errInfo: "The body contains trailing data",
},
}

for _, tc := range testCases {
Expand All @@ -226,6 +240,8 @@ func TestParseCredentialCreationResponse(t *testing.T) {
if tc.errString != "" {
assert.EqualError(t, err, tc.errString)

AssertIsProtocolError(t, err, tc.errType, tc.errDetails, tc.errInfo)

return
}

Expand Down Expand Up @@ -371,6 +387,24 @@ var testCredentialRequestResponses = map[string]string{
"transports":["usb","nfc","fake"]
}
}
`,
`trailingData`: `
{
"id":"6xrtBhJQW6QU4tOaB4rrHaS2Ks0yDDL_q8jDC16DEjZ-VLVf4kCRkvl2xp2D71sTPYns-exsHQHTy3G-zJRK8g",
"rawId":"6xrtBhJQW6QU4tOaB4rrHaS2Ks0yDDL_q8jDC16DEjZ-VLVf4kCRkvl2xp2D71sTPYns-exsHQHTy3G-zJRK8g",
"type":"public-key",
"authenticatorAttachment":"platform",
"clientExtensionResults":{
"appid":true
},
"response":{
"attestationObject":"o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YVjEdKbqkhPJnC90siSSsyDPQCYqlMGpUKA5fyklC2CEHvBBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQOsa7QYSUFukFOLTmgeK6x2ktirNMgwy_6vIwwtegxI2flS1X-JAkZL5dsadg-9bEz2J7PnsbB0B08txvsyUSvKlAQIDJiABIVggLKF5xS0_BntttUIrm2Z2tgZ4uQDwllbdIfrrBMABCNciWCDHwin8Zdkr56iSIh0MrB5qZiEzYLQpEOREhMUkY6q4Vw",
"clientDataJSON":"eyJjaGFsbGVuZ2UiOiJXOEd6RlU4cEdqaG9SYldyTERsYW1BZnFfeTRTMUNaRzFWdW9lUkxBUnJFIiwib3JpZ2luIjoiaHR0cHM6Ly93ZWJhdXRobi5pbyIsInR5cGUiOiJ3ZWJhdXRobi5jcmVhdGUifQ",
"transports":["usb","nfc","fake"]
}
}
abc
`,
`successDeprecatedTransports`: `
{
Expand Down
19 changes: 19 additions & 0 deletions protocol/func_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package protocol

import (
"errors"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func AssertIsProtocolError(t *testing.T, err error, errType, errDetails, errInfo string) {
var e *Error

require.True(t, errors.As(err, &e))

assert.Equal(t, errType, e.Type)
assert.Equal(t, errDetails, e.Details)
assert.Equal(t, errInfo, e.DevInfo)
}

0 comments on commit e6c1318

Please sign in to comment.