fix(protocol): trailing credential data skipped

This fixes an issue where the trailing data in the credential parsing function would be ignored. This also adds some clarity documentation and cleanup to the ParseCredentialCreationResponse function.

protocol/credential.go

@@ -4,6 +4,7 @@ import (
 	"crypto/sha256"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"io"
 	"net/http"
 )
@@ -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.
@@ -61,21 +62,36 @@ type ParsedCredentialCreationData struct {
 	Raw      CredentialCreationResponse
 }
 
+// ParseCredentialCreationResponse is a non-agnostic function for parsing a registration response from the http library
+// from stdlib. It handles some standard cleanup operations.
 func ParseCredentialCreationResponse(response *http.Request) (*ParsedCredentialCreationData, error) {
 	if response == nil || response.Body == nil {
 		return nil, ErrBadRequest.WithDetails("No response given")
 	}
 
+	defer response.Body.Close()
+	defer io.Copy(io.Discard, response.Body)
+
 	return ParseCredentialCreationResponseBody(response.Body)
 }
 
+// ParseCredentialCreationResponseBody is an agnostic version of ParseCredentialCreationResponse. Implementers are
+// therefore responsible for managing cleanup.
 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()
 }
 

protocol/credential_test.go

@@ -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",
@@ -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 {
@@ -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
 			}
 
@@ -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`: `
 {

protocol/func_test.go

@@ -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)
+}