Support key access in all services #110

decentralgabe · 2022-09-24T04:03:10Z

A few things:

Types to support an abstraction over string and []byte request/responses for key access
Added key storage to all services
Swapped out key storage for the DID service to use the key store
Found a few issues with manifest, swapped out the impl to rely on the credential service instead of building credentials directly. Added a number of TODOs to go over with @nitro-neal
Refactored some tests to mock out test services/routers a bit more uniformly

next up: actually signing / verifying objects

codecov-commenter · 2022-09-24T04:04:48Z

Codecov Report

Attention: Patch coverage is 25.31646% with 59 lines in your changes missing coverage. Please review.

Project coverage is 28.20%. Comparing base (ce425c4) to head (26067d8).
Report is 369 commits behind head on main.

Files with missing lines	Patch %	Lines
config/config.go	0.00%	26 Missing ⚠️
pkg/service/keystore/keystore.go	0.00%	21 Missing ⚠️
internal/keyaccess/jwt.go	59.25%	6 Missing and 5 partials ⚠️
pkg/server/router/manifest.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #110      +/-   ##
==========================================
- Coverage   28.53%   28.20%   -0.34%     
==========================================
  Files          18       18              
  Lines        1146     1202      +56     
==========================================
+ Hits          327      339      +12     
- Misses        775      815      +40     
- Partials       44       48       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…-all-services-ose-122 * origin/main: SIP-4: DWN Routes (#113) Update README.md Add Sip 4 to table (#109) Adding SIP4 Doc (#108) # Conflicts: # config/compose.toml # config/config.go # config/config.toml # go.mod # go.sum # pkg/server/server_test.go # pkg/service/service.go

nitro-neal · 2022-09-28T00:39:00Z

internal/keyaccess/dataintegrity.go

-func (ka DataIntegrityKeyAccess) Sign(payload cryptosuite.Provable) ([]byte, error) {
+// DataIntegrityJSON represents a response from a DataIntegrityKeyAccess.Sign() call represented
+// as a serialized JSON object
+type DataIntegrityJSON struct {


nitro-neal · 2022-09-28T00:41:13Z

pkg/server/router/dwn_test.go

@@ -1,13 +1,15 @@
 package router

 import (
+	"os"


I need to make sure I'm on the same linter

nitro-neal · 2022-09-28T00:46:22Z

pkg/service/did/storage/bolt.go

@@ -31,16 +33,16 @@ func NewBoltDIDStorage(db *storage.BoltDB) (*BoltDIDStorage, error) {
 }

 func (b BoltDIDStorage) StoreDID(did StoredDID) error {
-	couldNotStoreDIDErr := fmt.Sprintf("could not store DID: %s", did.DID.ID)
-	namespace, err := getNamespaceForDID(did.DID.ID)
+	couldNotStoreDIDErr := fmt.Sprintf("could not store DID: %s", did.ID)


good catch, I think Schema does this too?

nitro-neal · 2022-09-28T00:48:01Z

pkg/service/did/storage/storage.go

@@ -10,9 +10,8 @@ import (
 )

 type StoredDID struct {
+	ID  string          `json:"id"`


oh you put a top level ID I see

for simpler indexing in the db

nitro-neal · 2022-09-28T00:52:04Z

pkg/service/did/key.go

+		return nil, errors.Wrap(err, "could not store did:key private key")
+	}
+
+	privKeyBase58, err := privateKeyToBase58(privKey)


Should this happen before you store the key?

Because there could be a case where you store a key but you can't convert it

yeah, good call out

nitro-neal · 2022-09-28T00:55:07Z

pkg/service/keystore/storage/bolt.go

-	return b.db.Write(namespace, id, keyBytes)
+
+	// encrypt key before storing
+	encryptedKey, err := util.XChaCha20Poly1305Encrypt(b.serviceKey, keyBytes)


Wild names for encryption these days

nitro-neal · 2022-09-28T00:58:41Z

pkg/service/manifest/manifest.go

 	}, nil
 }

 func (s Service) CreateManifest(request CreateManifestRequest) (*CreateManifestResponse, error) {
-	logrus.Debugf("creating manifest: %+v", request)
+	logrus.Debugf("creating m: %+v", request)


change log lines 'm' to manifest

nitro-neal · 2022-09-28T00:59:20Z

pkg/service/manifest/manifest.go

@@ -154,7 +147,7 @@ func isValidApplication(gotManifest *manifeststorage.StoredManifest, application
 	return nil
 }

-func (s Service) SubmitApplication(request SubmitApplicationRequest) (*SubmitApplicationResponse, error) {
+func (s Service) ProcessApplicationSubmission(request SubmitApplicationRequest) (*SubmitApplicationResponse, error) {


good name change

nitro-neal · 2022-09-28T18:29:42Z

pkg/service/manifest/manifest.go

-		credentialBuilder.SetIssuanceDate(time.Now().Format(time.RFC3339))
-
-		cred, err := credentialBuilder.Build()
+		createdResponse, err := s.credential.CreateCredential(credentialRequest)


This probably needs to be credentialResponse

decentralgabe

made changes

decentralgabe · 2022-09-28T21:51:56Z

pkg/service/did/storage/storage.go

@@ -10,9 +10,8 @@ import (
 )

 type StoredDID struct {
+	ID  string          `json:"id"`


for simpler indexing in the db

decentralgabe · 2022-09-28T21:52:35Z

pkg/service/manifest/manifest.go

 	}, nil
 }

 func (s Service) CreateManifest(request CreateManifestRequest) (*CreateManifestResponse, error) {
-	logrus.Debugf("creating manifest: %+v", request)
+	logrus.Debugf("creating m: %+v", request)


decentralgabe · 2022-09-28T21:54:19Z

pkg/service/manifest/manifest.go

-		credentialBuilder.SetIssuanceDate(time.Now().Format(time.RFC3339))
-
-		cred, err := credentialBuilder.Build()
+		createdResponse, err := s.credential.CreateCredential(credentialRequest)


gabe added 7 commits September 21, 2022 14:26

utility wrappers

2e91e7f

init

c775567

utilize key encryption

4b7cdda

config updatse

4117baa

temp

e887a72

did svc uses keystore pt 1

60aba8a

using the keystore pt 2

8d77c81

gabe added 5 commits September 27, 2022 13:50

temp

d082912

add storage, update manifest with some todos

b47cad6

merge

a9ab57b

fix dwn tests

75a9961

decentralgabe marked this pull request as ready for review September 28, 2022 00:31

decentralgabe requested a review from nitro-neal as a code owner September 28, 2022 00:31

bad rename

266bffd

nitro-neal reviewed Sep 28, 2022

View reviewed changes

nitro-neal approved these changes Sep 28, 2022

View reviewed changes

nitro-neal reviewed Sep 28, 2022

View reviewed changes

decentralgabe commented Sep 28, 2022

View reviewed changes

pr comments

26067d8

decentralgabe merged commit 23ad921 into main Sep 28, 2022

decentralgabe deleted the support-key-access-in-all-services-ose-122 branch September 28, 2022 23:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support key access in all services #110

Support key access in all services #110

decentralgabe commented Sep 24, 2022 •

edited

Loading

codecov-commenter commented Sep 24, 2022 •

edited

Loading

nitro-neal Sep 28, 2022

nitro-neal Sep 28, 2022

decentralgabe Sep 28, 2022

nitro-neal Sep 28, 2022

nitro-neal Sep 28, 2022

decentralgabe Sep 28, 2022

nitro-neal Sep 28, 2022

decentralgabe Sep 28, 2022

nitro-neal Sep 28, 2022

nitro-neal Sep 28, 2022

decentralgabe Sep 28, 2022

nitro-neal Sep 28, 2022

nitro-neal Sep 28, 2022

decentralgabe Sep 28, 2022

decentralgabe left a comment

decentralgabe Sep 28, 2022

decentralgabe Sep 28, 2022

decentralgabe Sep 28, 2022

Support key access in all services #110

Support key access in all services #110

Conversation

decentralgabe commented Sep 24, 2022 • edited Loading

codecov-commenter commented Sep 24, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

decentralgabe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

decentralgabe commented Sep 24, 2022 •

edited

Loading

codecov-commenter commented Sep 24, 2022 •

edited

Loading