Update PE/CM and simplify err logging (#163)

* add err fmting

* simplify err logging

* Update pkg/server/router/manifest.go

Co-authored-by: Andres Uribe <[email protected]>

* Apply suggestions from code review

Co-authored-by: Andres Uribe <[email protected]>

go.mod

@@ -4,7 +4,7 @@ go 1.19
 
 require (
 	github.com/BurntSushi/toml v1.2.1
-	github.com/TBD54566975/ssi-sdk v0.0.2-alpha.0.20221101200844-4aaa87cf0001
+	github.com/TBD54566975/ssi-sdk v0.0.2-alpha.0.20221104161850-6dad71a33c46
 	github.com/ardanlabs/conf v1.5.0
 	github.com/dimfeld/httptreemux/v5 v5.5.0
 	github.com/go-playground/locales v0.14.0

go.sum

@@ -4,6 +4,8 @@ github.com/TBD54566975/ssi-sdk v0.0.2-alpha.0.20221027011419-39fefd7f233c h1:tMb
 github.com/TBD54566975/ssi-sdk v0.0.2-alpha.0.20221027011419-39fefd7f233c/go.mod h1:Uvgv4WK66gwiV76SpWe6EUnUSwyH7r9EFqP0tTSDP18=
 github.com/TBD54566975/ssi-sdk v0.0.2-alpha.0.20221101200844-4aaa87cf0001 h1:FxNNYtGZSwoAz3NhphlANaETYiBq07j0e2tB8LhMXBs=
 github.com/TBD54566975/ssi-sdk v0.0.2-alpha.0.20221101200844-4aaa87cf0001/go.mod h1:Uvgv4WK66gwiV76SpWe6EUnUSwyH7r9EFqP0tTSDP18=
+github.com/TBD54566975/ssi-sdk v0.0.2-alpha.0.20221104161850-6dad71a33c46 h1:7S6fWVagOnkfBHoqj9g54aueyrUlpsYq+8wTTBm6WCk=
+github.com/TBD54566975/ssi-sdk v0.0.2-alpha.0.20221104161850-6dad71a33c46/go.mod h1:Uvgv4WK66gwiV76SpWe6EUnUSwyH7r9EFqP0tTSDP18=
 github.com/ardanlabs/conf v1.5.0 h1:5TwP6Wu9Xi07eLFEpiCUF3oQXh9UzHMDVnD3u/I5d5c=
 github.com/ardanlabs/conf v1.5.0/go.mod h1:ILsMo9dMqYzCxDjDXTiwMI0IgxOJd0MOiucbQY2wlJw=
 github.com/buger/jsonparser v1.1.1 h1:2PnMjfWD7wBILjqQbt530v576A/cAbQvEW9gGIpYMUs=

internal/util/util.go

@@ -30,12 +30,22 @@ func LoggingNewError(msg string) error {
 	return err
 }
 
+// LoggingNewErrorf is a utility to create an error from a formatted message, log it, and return it as an error
+func LoggingNewErrorf(msg string, args ...interface{}) error {
+	return LoggingNewError(fmt.Sprintf(msg, args...))
+}
+
 // LoggingErrorMsg is a utility to combine logging an error, and returning and error with a message
 func LoggingErrorMsg(err error, msg string) error {
 	logrus.WithError(err).Error(SanitizeLog(msg))
 	return errors.Wrap(err, msg)
 }
 
+// LoggingErrorMsgf is a utility to combine logging an error, and returning and error with a formatted message
+func LoggingErrorMsgf(err error, msg string, args ...interface{}) error {
+	return LoggingErrorMsg(err, fmt.Sprintf(msg, args...))
+}
+
 // SanitizeLog prevents certain classes of injection attacks before logging
 // https://codeql.github.com/codeql-query-help/go/go-log-injection/
 func SanitizeLog(log string) string {

pkg/server/router/manifest.go

@@ -41,6 +41,8 @@ func NewManifestRouter(s svcframework.Service) (*ManifestRouter, error) {
 // CreateManifestRequest is the request body for creating a manifest, which populates all remaining fields
 // and builds a well-formed manifest object.
 type CreateManifestRequest struct {
+	Name           *string                          `json:"name,omitempty"`
+	Description    *string                          `json:"description,omitempty"`
 	IssuerDID              string                           `json:"issuerDid" validate:"required"`
 	IssuerName             *string                          `json:"issuerName,omitempty"`
 	ClaimFormat            *exchange.ClaimFormat            `json:"format" validate:"required,dive"`
@@ -50,6 +52,8 @@ type CreateManifestRequest struct {
 
 func (c CreateManifestRequest) ToServiceRequest() manifest.CreateManifestRequest {
 	return manifest.CreateManifestRequest{
+		Name:           c.Name,
+		Description:    c.Description,
 		IssuerDID:              c.IssuerDID,
 		IssuerName:             c.IssuerName,
 		OutputDescriptors:      c.OutputDescriptors,

pkg/service/credential/service.go

@@ -66,8 +66,7 @@ func (s Service) Config() config.CredentialServiceConfig {
 func NewCredentialService(config config.CredentialServiceConfig, s storage.ServiceStorage, keyStore *keystore.Service, didResolver *didsdk.Resolver, schema *schema.Service) (*Service, error) {
 	credentialStorage, err := credstorage.NewCredentialStorage(s)
 	if err != nil {
-		errMsg := "could not instantiate storage for the credential service"
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsg(err, "could not instantiate storage for the credential service")
 	}
 	verifier, err := credint.NewCredentialVerifier(didResolver, schema)
 	if err != nil {
@@ -93,30 +92,26 @@ func (s Service) CreateCredential(request CreateCredentialRequest) (*CreateCrede
 	builder := credential.NewVerifiableCredentialBuilder()
 
 	if err := builder.SetIssuer(request.Issuer); err != nil {
-		errMsg := fmt.Sprintf("could not build credential when setting issuer: %s", request.Issuer)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not build credential when setting issuer: %s", request.Issuer)
 	}
 
 	// check if there's a conflict with subject ID
 	if id, ok := request.Data[credential.VerifiableCredentialIDProperty]; ok && id != request.Subject {
-		errMsg := fmt.Sprintf("cannot set subject<%s>, data already contains a different ID value: %s", request.Subject, id)
-		return nil, util.LoggingNewError(errMsg)
+		return nil, util.LoggingNewErrorf("cannot set subject<%s>, data already contains a different ID value: %s", request.Subject, id)
 	}
 
 	// set subject value
 	subject := credential.CredentialSubject(request.Data)
 	subject[credential.VerifiableCredentialIDProperty] = request.Subject
 
 	if err := builder.SetCredentialSubject(subject); err != nil {
-		errMsg := fmt.Sprintf("could not set subject: %+v", subject)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not set subject: %+v", subject)
 	}
 
 	// if a context value exists, set it
 	if request.Context != "" {
 		if err := builder.AddContext(request.Context); err != nil {
-			errMsg := fmt.Sprintf("could not add context to credential: %s", request.Context)
-			return nil, util.LoggingErrorMsg(err, errMsg)
+			return nil, util.LoggingErrorMsgf(err, "could not add context to credential: %s", request.Context)
 		}
 	}
 
@@ -126,8 +121,7 @@ func (s Service) CreateCredential(request CreateCredentialRequest) (*CreateCrede
 		// resolve schema and save it for validation later
 		gotSchema, err := s.schema.GetSchema(schema.GetSchemaRequest{ID: request.JSONSchema})
 		if err != nil {
-			errMsg := fmt.Sprintf("failed to create credential; could not get schema: %s", request.JSONSchema)
-			return nil, util.LoggingErrorMsg(err, errMsg)
+			return nil, util.LoggingErrorMsgf(err, "failed to create credential; could not get schema: %s", request.JSONSchema)
 		}
 		knownSchema = &gotSchema.Schema
 
@@ -136,16 +130,14 @@ func (s Service) CreateCredential(request CreateCredentialRequest) (*CreateCrede
 			Type: SchemaLDType,
 		}
 		if err = builder.SetCredentialSchema(credSchema); err != nil {
-			errMsg := fmt.Sprintf("could not set JSON Schema for credential: %s", request.JSONSchema)
-			return nil, util.LoggingErrorMsg(err, errMsg)
+			return nil, util.LoggingErrorMsgf(err, "could not set JSON Schema for credential: %s", request.JSONSchema)
 		}
 	}
 
 	// if an expiry value exists, set it
 	if request.Expiry != "" {
 		if err := builder.SetExpirationDate(request.Expiry); err != nil {
-			errMsg := fmt.Sprintf("could not set expirty for credential: %s", request.Expiry)
-			return nil, util.LoggingErrorMsg(err, errMsg)
+			return nil, util.LoggingErrorMsgf(err, "could not set expiry for credential: %s", request.Expiry)
 		}
 	}
 
@@ -156,15 +148,13 @@ func (s Service) CreateCredential(request CreateCredentialRequest) (*CreateCrede
 
 	cred, err := builder.Build()
 	if err != nil {
-		errMsg := "could not build credential"
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsg(err, "could not build credential")
 	}
 
 	// verify the built schema complies with the schema we've set
 	if knownSchema != nil {
 		if err = schemalib.IsCredentialValidForVCJSONSchema(*cred, *knownSchema); err != nil {
-			errMsg := fmt.Sprintf("credential data does not comply with the provided schema: %s", request.JSONSchema)
-			return nil, util.LoggingErrorMsg(err, errMsg)
+			return nil, util.LoggingErrorMsgf(err, "credential data does not comply with the provided schema: %s", request.JSONSchema)
 		}
 	}
 
@@ -185,8 +175,7 @@ func (s Service) CreateCredential(request CreateCredentialRequest) (*CreateCrede
 		Container: container,
 	}
 	if err = s.storage.StoreCredential(storageRequest); err != nil {
-		errMsg := "could not store credential"
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsg(err, "could not store credential")
 	}
 
 	// return the result
@@ -198,18 +187,15 @@ func (s Service) CreateCredential(request CreateCredentialRequest) (*CreateCrede
 func (s Service) signCredentialJWT(issuer string, cred credential.VerifiableCredential) (*keyaccess.JWT, error) {
 	gotKey, err := s.keyStore.GetKey(keystore.GetKeyRequest{ID: issuer})
 	if err != nil {
-		errMsg := fmt.Sprintf("could not get key for signing credential with key<%s>", issuer)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not get key for signing credential with key<%s>", issuer)
 	}
 	keyAccess, err := keyaccess.NewJWKKeyAccess(gotKey.ID, gotKey.Key)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not create key access for signing credential with key<%s>", gotKey.ID)
-		return nil, errors.Wrap(err, errMsg)
+		return nil, errors.Wrapf(err, "could not create key access for signing credential with key<%s>", gotKey.ID)
 	}
 	credToken, err := keyAccess.SignVerifiableCredential(cred)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not sign credential with key<%s>", gotKey.ID)
-		return nil, errors.Wrap(err, errMsg)
+		return nil, errors.Wrapf(err, "could not sign credential with key<%s>", gotKey.ID)
 	}
 	return credToken, nil
 }
@@ -270,12 +256,10 @@ func (s Service) GetCredential(request GetCredentialRequest) (*GetCredentialResp
 
 	gotCred, err := s.storage.GetCredential(request.ID)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not get credential: %s", request.ID)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not get credential: %s", request.ID)
 	}
 	if !gotCred.IsValid() {
-		errMsg := fmt.Sprintf("credential returned is not valid: %s", request.ID)
-		return nil, util.LoggingNewError(errMsg)
+		return nil, util.LoggingNewErrorf("credential returned is not valid: %s", request.ID)
 	}
 	response := GetCredentialResponse{
 		credint.Container{
@@ -293,8 +277,7 @@ func (s Service) GetCredentialsByIssuer(request GetCredentialByIssuerRequest) (*
 
 	gotCreds, err := s.storage.GetCredentialsByIssuer(request.Issuer)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not get credential(s) for issuer: %s", request.Issuer)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not get credential(s) for issuer: %s", request.Issuer)
 	}
 
 	creds := make([]credint.Container, 0, len(gotCreds))
@@ -317,8 +300,7 @@ func (s Service) GetCredentialsBySubject(request GetCredentialBySubjectRequest)
 
 	gotCreds, err := s.storage.GetCredentialsBySubject(request.Subject)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not get credential(s) for subject: %s", request.Subject)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not get credential(s) for subject: %s", request.Subject)
 	}
 
 	creds := make([]credint.Container, 0, len(gotCreds))
@@ -340,8 +322,7 @@ func (s Service) GetCredentialsBySchema(request GetCredentialBySchemaRequest) (*
 
 	gotCreds, err := s.storage.GetCredentialsBySchema(request.Schema)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not get credential(s) for schema: %s", request.Schema)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not get credential(s) for schema: %s", request.Schema)
 	}
 
 	creds := make([]credint.Container, 0, len(gotCreds))
@@ -362,8 +343,7 @@ func (s Service) DeleteCredential(request DeleteCredentialRequest) error {
 	logrus.Debugf("deleting credential: %s", request.ID)
 
 	if err := s.storage.DeleteCredential(request.ID); err != nil {
-		errMsg := fmt.Sprintf("could not delete credential with id: %s", request.ID)
-		return util.LoggingErrorMsg(err, errMsg)
+		return util.LoggingErrorMsgf(err, "could not delete credential with id: %s", request.ID)
 	}
 
 	return nil

pkg/service/credential/storage/bolt.go

@@ -42,8 +42,7 @@ func (b BoltCredentialStorage) StoreCredential(request StoreCredentialRequest) e
 
 	storedCredBytes, err := json.Marshal(storedCredential)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not store request: %s", storedCredential.CredentialID)
-		return util.LoggingErrorMsg(err, errMsg)
+		return util.LoggingErrorMsgf(err, "could not store request: %s", storedCredential.CredentialID)
 	}
 	// TODO(gabe) conflict checking?
 	return b.db.Write(namespace, storedCredential.ID, storedCredBytes)
@@ -88,12 +87,10 @@ func buildStoredCredential(request StoreCredentialRequest) (*StoredCredential, e
 func (b BoltCredentialStorage) GetCredential(id string) (*StoredCredential, error) {
 	prefixValues, err := b.db.ReadPrefix(namespace, id)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not get credential from storage: %s", id)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not get credential from storage: %s", id)
 	}
 	if len(prefixValues) > 1 {
-		err := fmt.Errorf("multiple prefix values matched credential id: %s", id)
-		return nil, util.LoggingErrorMsg(err, "could not get credential from storage")
+		return nil, util.LoggingNewErrorf("could not get credential from storage; multiple prefix values matched credential id: %s", id)
 	}
 
 	// since we know the map now only has a single value, we break after the first element
@@ -103,14 +100,12 @@ func (b BoltCredentialStorage) GetCredential(id string) (*StoredCredential, erro
 		break
 	}
 	if len(credBytes) == 0 {
-		err := fmt.Errorf("%s with id: %s", credentialNotFoundErrMsg, id)
-		return nil, util.LoggingErrorMsg(err, "could not get credential from storage")
+		return nil, util.LoggingNewErrorf("could not get credential from storage %s with id: %s", credentialNotFoundErrMsg, id)
 	}
 
 	var stored StoredCredential
-	if err := json.Unmarshal(credBytes, &stored); err != nil {
-		errMsg := fmt.Sprintf("could not unmarshal stored credential: %s", id)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+	if err = json.Unmarshal(credBytes, &stored); err != nil {
+		return nil, util.LoggingErrorMsgf(err, "could not unmarshal stored credential: %s", id)
 	}
 	return &stored, nil
 }
@@ -125,8 +120,7 @@ func (b BoltCredentialStorage) GetCredential(id string) (*StoredCredential, erro
 func (b BoltCredentialStorage) GetCredentialsByIssuer(issuer string) ([]StoredCredential, error) {
 	keys, err := b.db.ReadAllKeys(namespace)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not read credential storage while searching for creds for issuer: %s", issuer)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not read credential storage while searching for creds for issuer: %s", issuer)
 	}
 	// see if the prefix keys contains the issuer value
 	var issuerKeys []string
@@ -148,7 +142,7 @@ func (b BoltCredentialStorage) GetCredentialsByIssuer(issuer string) ([]StoredCr
 			logrus.WithError(err).Errorf("could not read credential with key: %s", key)
 		} else {
 			var cred StoredCredential
-			if err := json.Unmarshal(credBytes, &cred); err != nil {
+			if err = json.Unmarshal(credBytes, &cred); err != nil {
 				logrus.WithError(err).Errorf("could not unmarshal credential with key: %s", key)
 			}
 			storedCreds = append(storedCreds, cred)
@@ -168,8 +162,7 @@ func (b BoltCredentialStorage) GetCredentialsByIssuer(issuer string) ([]StoredCr
 func (b BoltCredentialStorage) GetCredentialsBySubject(subject string) ([]StoredCredential, error) {
 	keys, err := b.db.ReadAllKeys(namespace)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not read credential storage while searching for creds for subject: %s", subject)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not read credential storage while searching for creds for subject: %s", subject)
 	}
 
 	// see if the prefix keys contains the subject value
@@ -212,8 +205,7 @@ func (b BoltCredentialStorage) GetCredentialsBySubject(subject string) ([]Stored
 func (b BoltCredentialStorage) GetCredentialsBySchema(schema string) ([]StoredCredential, error) {
 	keys, err := b.db.ReadAllKeys(namespace)
 	if err != nil {
-		errMsg := fmt.Sprintf("could not read credential storage while searching for creds for schema: %s", schema)
-		return nil, util.LoggingErrorMsg(err, errMsg)
+		return nil, util.LoggingErrorMsgf(err, "could not read credential storage while searching for creds for schema: %s", schema)
 	}
 
 	// see if the prefix keys contains the schema value
@@ -263,8 +255,7 @@ func (b BoltCredentialStorage) DeleteCredential(id string) error {
 			return nil
 		}
 
-		errMsg := fmt.Sprintf("could not get credential<%s> before deletion", id)
-		return util.LoggingErrorMsg(err, errMsg)
+		return util.LoggingErrorMsgf(err, "could not get credential<%s> before deletion", id)
 	}
 
 	// no error on deletion for a non-existent credential
@@ -275,9 +266,8 @@ func (b BoltCredentialStorage) DeleteCredential(id string) error {
 
 	// re-create the prefix key to delete
 	prefix := createPrefixKey(id, gotCred.Issuer, gotCred.Subject, gotCred.Schema)
-	if err := b.db.Delete(namespace, prefix); err != nil {
-		errMsg := fmt.Sprintf("could not delete credential: %s", id)
-		return util.LoggingErrorMsg(err, errMsg)
+	if err = b.db.Delete(namespace, prefix); err != nil {
+		return util.LoggingErrorMsgf(err, "could not delete credential: %s", id)
 	}
 	return nil
 }

pkg/service/credential/storage/storage.go

@@ -1,8 +1,6 @@
 package storage
 
 import (
-	"fmt"
-
 	"github.com/TBD54566975/ssi-sdk/credential"
 
 	cred "github.com/tbd54566975/ssi-service/internal/credential"
@@ -57,16 +55,14 @@ func NewCredentialStorage(s storage.ServiceStorage) (Storage, error) {
 	case storage.Bolt:
 		gotBolt, ok := s.(*storage.BoltDB)
 		if !ok {
-			errMsg := fmt.Sprintf("trouble instantiating : %s", s.Type())
-			return nil, util.LoggingNewError(errMsg)
+			return nil, util.LoggingNewErrorf("trouble instantiating : %s", s.Type())
 		}
 		boltStorage, err := NewBoltCredentialStorage(gotBolt)
 		if err != nil {
 			return nil, util.LoggingErrorMsg(err, "could not instantiate credential bolt storage")
 		}
 		return boltStorage, err
 	default:
-		errMsg := fmt.Errorf("unsupported storage type: %s", s.Type())
-		return nil, util.LoggingError(errMsg)
+		return nil, util.LoggingNewErrorf("unsupported storage type: %s", s.Type())
 	}
 }