fix: Validation error of user request returns HTTP 500

Fixed resolution and resolution with initial state to return bad request in case that user supplies invalid input

Operation processing has been fixed with issue #201 (operation validation on server side)

Closes #149

Signed-off-by: Sandra Vrtikapa <[email protected]>

pkg/dochandler/didvalidator/validator.go

@@ -18,9 +18,12 @@ import (
 )
 
 const (
-	didSuffix            = "did_suffix"
-	didContext           = "https://www.w3.org/ns/did/v1"
-	sidetreeContext      = "https://identity.foundation/sidetree/context-v1.jsonld"
+	didSuffix = "did_suffix"
+
+	didContext = "https://www.w3.org/ns/did/v1"
+	//sidetreeContext = "https://identity.foundation/sidetree/context-v1.jsonld"
+	trustblocContext = "https://trustbloc.github.io/context/did/trustbloc-v1.jsonld"
+
 	didResolutionContext = "https://www.w3.org/ns/did-resolution/v1"
 )
 
@@ -105,7 +108,8 @@ func (v *Validator) TransformDocument(doc document.Document) (*document.Resoluti
 	external := document.DidDocumentFromJSONLDObject(make(document.DIDDocument))
 
 	// add context and id
-	external[document.ContextProperty] = []interface{}{didContext, sidetreeContext}
+	// TODO: Add sidetree context once it gets fixed
+	external[document.ContextProperty] = []interface{}{didContext, trustblocContext}
 	external[document.IDProperty] = internal.ID()
 
 	result := &document.ResolutionResult{

pkg/dochandler/handler.go

@@ -21,6 +21,7 @@ package dochandler
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -38,6 +39,8 @@ import (
 
 const (
 	keyID = "id"
+
+	badRequest = "bad request"
 )
 
 // DocumentHandler implements document handler
@@ -145,12 +148,12 @@ func (r *DocumentHandler) ResolveDocument(idOrInitialDoc string) (*document.Reso
 	// extract did and optional initial document value
 	id, initial, err := request.GetParts(r.namespace, idOrInitialDoc)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%s: %s", badRequest, err.Error())
 	}
 
 	uniquePortion, err := getSuffix(r.namespace, id)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%s: %s", badRequest, err.Error())
 	}
 
 	// resolve document from the blockchain
@@ -188,26 +191,26 @@ func (r *DocumentHandler) resolveRequestWithID(uniquePortion string) (*document.
 func (r *DocumentHandler) resolveRequestWithDocument(id string, initial *model.CreateRequest) (*document.ResolutionResult, error) {
 	// verify size of each delta does not exceed the maximum allowed limit
 	if len(initial.Delta) > int(r.protocol.Current().MaxDeltaByteSize) {
-		return nil, errors.New("delta byte size exceeds protocol max delta byte size")
+		return nil, fmt.Errorf("%s: delta byte size exceeds protocol max delta byte size", badRequest)
 	}
 
 	initialBytes, err := json.Marshal(initial)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%s: marshal initial state: %s", badRequest, err.Error())
 	}
 
 	op, err := operation.ParseCreateOperation(initialBytes, r.protocol.Current())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%s: %s", badRequest, err.Error())
 	}
 
 	op.ID = r.namespace + docutil.NamespaceDelimiter + op.UniqueSuffix
 	if id != op.ID {
-		return nil, errors.New("provided did doesn't match did created from create request")
+		return nil, fmt.Errorf("%s: provided did doesn't match did created from initial state", badRequest)
 	}
 
 	if err := r.validateInitialDocument(op.Delta.Patches); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%s: validate initial document: %s", badRequest, err.Error())
 	}
 
 	return r.getCreateResponse(op)
@@ -271,12 +274,12 @@ func getSuffix(namespace, idOrDocument string) (string, error) {
 	ns := namespace + docutil.NamespaceDelimiter
 	pos := strings.Index(idOrDocument, ns)
 	if pos == -1 {
-		return "", errors.New("ID must start with configured namespace")
+		return "", errors.New("did must start with configured namespace")
 	}
 
 	adjustedPos := pos + len(ns)
 	if adjustedPos >= len(idOrDocument) {
-		return "", errors.New("unique portion is empty")
+		return "", errors.New("did suffix is empty")
 	}
 
 	return idOrDocument[adjustedPos:], nil

pkg/dochandler/handler_test.go

@@ -128,7 +128,7 @@ func TestDocumentHandler_ResolveDocument_DID(t *testing.T) {
 	result, err = dochandler.ResolveDocument(namespace + docutil.NamespaceDelimiter)
 	require.NotNil(t, err)
 	require.Nil(t, result)
-	require.Contains(t, err.Error(), "unique portion is empty")
+	require.Contains(t, err.Error(), "did suffix is empty")
 }
 
 func TestDocumentHandler_ResolveDocument_InitialValue(t *testing.T) {
@@ -150,7 +150,7 @@ func TestDocumentHandler_ResolveDocument_InitialValue(t *testing.T) {
 	result, err = dochandler.ResolveDocument(docID + initialStateParam)
 	require.NotNil(t, err)
 	require.Nil(t, result)
-	require.Contains(t, err.Error(), "initial values is present but empty")
+	require.Contains(t, err.Error(), "initial state is present but empty")
 
 	// create request not encoded
 	result, err = dochandler.ResolveDocument(docID + initialStateParam + "payload")
@@ -162,7 +162,7 @@ func TestDocumentHandler_ResolveDocument_InitialValue(t *testing.T) {
 	result, err = dochandler.ResolveDocument(dochandler.namespace + ":someID" + initialStateParam + initialState)
 	require.NotNil(t, err)
 	require.Nil(t, result)
-	require.Contains(t, err.Error(), "provided did doesn't match did created from create request")
+	require.Contains(t, err.Error(), "provided did doesn't match did created from initial state")
 
 	// delta and suffix data not encoded (parse create operation fails)
 	result, err = dochandler.ResolveDocument(docID + initialStateParam + "abc.123")
@@ -188,6 +188,26 @@ func TestDocumentHandler_ResolveDocument_InitialValue_MaxDeltaSizeError(t *testi
 	require.Contains(t, err.Error(), "delta byte size exceeds protocol max delta byte size")
 }
 
+func TestDocumentHandler_ResolveDocument_InitialDocumentNotValid(t *testing.T) {
+	dochandler := getDocumentHandler(mocks.NewMockOperationStore(nil))
+	require.NotNil(t, dochandler)
+
+	createReq, err := getCreateRequestWithDoc(invalidDocNoUsage)
+	require.NoError(t, err)
+
+	createOp, err := getCreateOperationWithInitialState(createReq.SuffixData, createReq.Delta)
+	require.NoError(t, err)
+
+	docID := createOp.ID
+
+	initialState := createReq.Delta + "." + createReq.SuffixData
+
+	result, err := dochandler.ResolveDocument(docID + initialStateParam + initialState)
+	require.Error(t, err)
+	require.Nil(t, result)
+	require.Contains(t, err.Error(), "missing usage")
+}
+
 func TestTransformToExternalDocument(t *testing.T) {
 	dochandler := getDocumentHandler(nil)
 
@@ -209,12 +229,12 @@ func TestGetUniquePortion(t *testing.T) {
 	// id doesn't contain namespace
 	uniquePortion, err := getSuffix(namespace, "invalid")
 	require.NotNil(t, err)
-	require.Contains(t, err.Error(), "ID must start with configured namespace")
+	require.Contains(t, err.Error(), "did must start with configured namespace")
 
 	// id equals namespace; unique portion is empty
 	uniquePortion, err = getSuffix(namespace, namespace+docutil.NamespaceDelimiter)
 	require.NotNil(t, err)
-	require.Contains(t, err.Error(), "unique portion is empty")
+	require.Contains(t, err.Error(), "did suffix is empty")
 
 	// valid unique portion
 	const unique = "exKwW0HjS5y4zBtJ7vYDwglYhtckdO15JDt1j5F5Q0A"
@@ -298,37 +318,63 @@ func getCreateOperation() *batchapi.Operation {
 		panic(err)
 	}
 
-	payload, err := json.Marshal(request)
+	op, err := getCreateOperationWithInitialState(request.SuffixData, request.Delta)
 	if err != nil {
 		panic(err)
 	}
 
-	uniqueSuffix, err := docutil.CalculateUniqueSuffix(request.SuffixData, sha2_256)
+	return op
+}
+
+func getCreateOperationWithInitialState(suffixData, delta string) (*batchapi.Operation, error) {
+	request := &model.CreateRequest{
+		Operation:  model.OperationTypeCreate,
+		SuffixData: suffixData,
+		Delta:      delta,
+	}
+
+	payload, err := json.Marshal(request)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
 
-	deltaBytes, err := docutil.DecodeString(request.Delta)
+	uniqueSuffix, err := docutil.CalculateUniqueSuffix(suffixData, sha2_256)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
 
-	delta := &model.DeltaModel{}
-	err = json.Unmarshal(deltaBytes, delta)
+	deltaBytes, err := docutil.DecodeString(delta)
 	if err != nil {
 		panic(err)
 	}
 
+	deltaModel := &model.DeltaModel{}
+	err = json.Unmarshal(deltaBytes, deltaModel)
+	if err != nil {
+		return nil, err
+	}
+
+	suffixDataBytes, err := docutil.DecodeString(suffixData)
+	if err != nil {
+		return nil, err
+	}
+
+	suffixDataModel := &model.SuffixDataModel{}
+	err = json.Unmarshal(suffixDataBytes, suffixDataModel)
+	if err != nil {
+		return nil, err
+	}
+
 	return &batchapi.Operation{
 		OperationBuffer:              payload,
-		Delta:                        delta,
-		EncodedDelta:                 request.Delta,
+		Delta:                        deltaModel,
+		EncodedDelta:                 delta,
 		Type:                         batchapi.OperationTypeCreate,
 		HashAlgorithmInMultiHashCode: sha2_256,
 		UniqueSuffix:                 uniqueSuffix,
 		ID:                           namespace + docutil.NamespaceDelimiter + uniqueSuffix,
-		SuffixData:                   getSuffixData(),
-	}
+		SuffixData:                   suffixDataModel,
+	}, nil
 }
 
 const validDoc = `{
@@ -345,54 +391,90 @@ const validDoc = `{
 	}]
 }`
 
+const invalidDocNoUsage = `{
+	"publicKey": [{
+		  "id": "key1",
+		  "type": "JwsVerificationKey2020",
+		  "usage": [],		
+		  "jwk": {
+			"kty": "EC",
+			"crv": "P-256K",
+			"x": "PUymIqdtF_qxaAqPABSw-C-owT1KYYQbsMKFM-L9fJA",
+			"y": "nM84jDHCMOTGTh_ZdHq4dBBdo4Z5PkEOW9jA8z8IsGc"
+		  }
+	}]
+}`
+
 func getCreateRequest() (*model.CreateRequest, error) {
-	delta, err := getDelta()
+	return getCreateRequestWithDoc(validDoc)
+}
+
+func getCreateRequestWithDoc(doc string) (*model.CreateRequest, error) {
+	delta, err := getDeltaWithDoc(doc)
 	if err != nil {
 		return nil, err
 	}
 
-	deltaBytes, err := json.Marshal(delta)
+	deltaBytes, err := canonicalizer.MarshalCanonical(delta)
 	if err != nil {
 		return nil, err
 	}
 
-	suffixDataBytes, err := canonicalizer.MarshalCanonical(getSuffixData())
+	encodedDelta := docutil.EncodeToString(deltaBytes)
+
+	suffixDataBytes, err := canonicalizer.MarshalCanonical(getSuffixData(encodedDelta))
 	if err != nil {
 		return nil, err
 	}
 
+	encodedSuffixData := docutil.EncodeToString(suffixDataBytes)
+
 	return &model.CreateRequest{
 		Operation:  model.OperationTypeCreate,
-		Delta:      docutil.EncodeToString(deltaBytes),
-		SuffixData: docutil.EncodeToString(suffixDataBytes),
+		Delta:      encodedDelta,
+		SuffixData: encodedSuffixData,
 	}, nil
 }
 
-func getDelta() (*model.DeltaModel, error) {
-	replacePatch, err := patch.NewReplacePatch(validDoc)
+func getDeltaWithDoc(doc string) (*model.DeltaModel, error) {
+	replacePatch, err := newReplacePatch(doc)
 	if err != nil {
 		return nil, err
 	}
 
 	return &model.DeltaModel{
 		Patches:          []patch.Patch{replacePatch},
-		UpdateCommitment: computeMultihash("updateReveal"),
+		UpdateCommitment: encodedMultihash("updateReveal"),
 	}, nil
 }
 
-func getSuffixData() *model.SuffixDataModel {
+// newReplacePatch creates new replace patch without validation
+func newReplacePatch(doc string) (patch.Patch, error) {
+	parsed, err := document.FromBytes([]byte(doc))
+	if err != nil {
+		return nil, err
+	}
+
+	p := make(patch.Patch)
+	p[patch.ActionKey] = patch.Replace
+	p[patch.DocumentKey] = parsed.JSONLdObject()
+
+	return p, nil
+}
+
+func getSuffixData(encodedDelta string) *model.SuffixDataModel {
 	return &model.SuffixDataModel{
-		DeltaHash: computeMultihash(validDoc),
+		DeltaHash: encodedMultihash(encodedDelta),
 		RecoveryKey: &jws.JWK{
 			Kty: "kty",
 			Crv: "crv",
 			X:   "x",
 		},
-		RecoveryCommitment: computeMultihash("recoveryReveal"),
+		RecoveryCommitment: encodedMultihash("recoveryReveal"),
 	}
 }
 
-func computeMultihash(data string) string {
+func encodedMultihash(data string) string {
 	mh, err := docutil.ComputeMultihash(sha2_256, []byte(data))
 	if err != nil {
 		panic(err)
@@ -415,7 +497,7 @@ func getUpdateRequest() (*model.UpdateRequest, error) {
 
 func getUpdateDelta() *model.DeltaModel {
 	return &model.DeltaModel{
-		UpdateCommitment: computeMultihash("updateReveal"),
+		UpdateCommitment: encodedMultihash("updateReveal"),
 	}
 }
 

pkg/internal/request/method.go

@@ -46,7 +46,7 @@ func GetParts(namespace, params string) (string, *model.CreateRequest, error) {
 
 	adjustedPos := pos + len(initialMatch)
 	if adjustedPos >= len(params) {
-		return "", nil, errors.New("initial values is present but empty")
+		return "", nil, errors.New("initial state is present but empty")
 	}
 
 	did := params[0:pos]

pkg/internal/request/method_test.go

@@ -47,7 +47,7 @@ func TestGetParts(t *testing.T) {
 	require.Error(t, err)
 	require.Empty(t, did)
 	require.Nil(t, initial)
-	require.Contains(t, err.Error(), "initial values is present but empty")
+	require.Contains(t, err.Error(), "initial state is present but empty")
 
 	did, initial, err = GetParts(namespace, testDID+initialStateParam+"xyz")
 	require.Error(t, err)

pkg/mocks/dochandler.go

@@ -9,6 +9,7 @@ package mocks
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"strings"
 
 	"github.com/trustbloc/sidetree-core-go/pkg/api/batch"
@@ -121,9 +122,10 @@ func applyID(doc document.Document, id string) document.Document {
 }
 
 func (m *MockDocumentHandler) resolveWithInitialState(idOrDocument string) (*document.ResolutionResult, error) {
+	const badRequest = "bad request"
 	id, initialState, err := request.GetParts(m.namespace, idOrDocument)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%s: %s", badRequest, err.Error())
 	}
 
 	decodedDelta, err := docutil.DecodeString(initialState.Delta)