Repository: TBD54566975/ssi-service (go)

Issue:
Decouple ServiceKey namespace from KeyStoreAPI namespace
We are storing the ServiceKey in the same namespace as the one we use for the `KeyStoreAPI`. What this means is that a caller may override the ServiceKey that was stored, if they pass in `id == "ssi-service-key"` in the [Store Key Request](https://developer.tbd.website/docs/apis/ssi-service#tag/KeyStoreAPI/paths/~1v1~1keys/put)

Pull request description:
Separates service internal namespace from externally added keys.
# Overview
Fixes #367 by changing the namespace that the service key uses.

# Description
na

# How Has This Been Tested?
All tests pass.


Patch:
diff --git a/pkg/service/credential/storage.go b/pkg/service/credential/storage.go
index 1aa5482..47f194f 100644
--- a/pkg/service/credential/storage.go
+++ b/pkg/service/credential/storage.go
@@ -13,7 +13,6 @@ import (
 	"github.com/goccy/go-json"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-
 	credint "github.com/tbd54566975/ssi-service/internal/credential"
 	"github.com/tbd54566975/ssi-service/internal/keyaccess"
 	"github.com/tbd54566975/ssi-service/internal/util"
@@ -477,7 +476,7 @@ func (cs *Storage) GetCredentialsBySchema(ctx context.Context, schema string) ([
 	}
 
 	// see if the prefix keys contains the schema value
-	query := "sc:" + schema
+	query := storage.Join("sc", schema)
 	var schemaKeys []string
 	for _, k := range keys {
 		if strings.HasSuffix(k, query) {
@@ -524,7 +523,7 @@ func (cs *Storage) GetStatusListCredentialsByIssuerSchemaPurpose(ctx context.Con
 		return nil, sdkutil.LoggingErrorMsgf(err, "could not read credential storage while searching for creds for issuer: %s", issuer)
 	}
 
-	query := "sc:" + schema + "-sp:" + string(statusPurpose)
+	query := storage.Join("sc", schema, "sp", string(statusPurpose))
 	var issuerSchemaKeys []string
 	for _, k := range keys {
 		if strings.Contains(k, issuer) && strings.HasSuffix(k, query) {
@@ -567,7 +566,7 @@ func (cs *Storage) getCredentialsByIssuerAndSchema(ctx context.Context, issuer s
 		return nil, sdkutil.LoggingErrorMsgf(err, "could not read credential storage while searching for creds for issuer: %s", issuer)
 	}
 
-	query := "sc:" + schema
+	query := storage.Join("sc", schema)
 	var issuerSchemaKeys []string
 	for _, k := range keys {
 		if strings.Contains(k, issuer) && strings.HasSuffix(k, query) {
@@ -672,12 +671,12 @@ func (cs *Storage) GetStatusListCredentialKeyData(ctx context.Context, issuer st
 }
 
 func getStatusListKey(issuer, schema, statusPurpose string) string {
-	return strings.Join([]string{"is:" + issuer, "sc:" + schema, "sp:" + statusPurpose}, "-")
+	return storage.Join("is", issuer, "sc", schema, "sp", statusPurpose)
 }
 
 // unique key for a credential
 func createPrefixKey(id, issuer, subject, schema string) string {
-	return strings.Join([]string{id, "is:" + issuer, "su:" + subject, "sc:" + schema}, "-")
+	return storage.Join(id, "is", issuer, "su", subject, "sc", schema)
 }
 
 func randomUniqueNum(count int) []int {
diff --git a/pkg/service/keystore/storage.go b/pkg/service/keystore/storage.go
index f3b595d..ad25ba1 100644
--- a/pkg/service/keystore/storage.go
+++ b/pkg/service/keystore/storage.go
@@ -55,11 +55,17 @@ type ServiceKey struct {
 
 const (
 	namespace             = "keystore"
-	publicNamespaceSuffix = ":public-keys"
+	serviceInternalSuffix = "service-internal"
+	publicNamespaceSuffix = "public-keys"
 	skKey                 = "ssi-service-key"
 	keyNotFoundErrMsg     = "key not found"
 )
 
+var (
+	serviceNamespace   = storage.Join(namespace, serviceInternalSuffix)
+	publicKeyNamespace = storage.Join(namespace, publicNamespaceSuffix)
+)
+
 type Storage struct {
 	db        storage.ServiceStorage
 	tx        storage.Tx
@@ -192,14 +198,14 @@ func storeServiceKey(ctx context.Context, tx storage.Tx, key ServiceKey) error {
 	if err != nil {
 		return sdkutil.LoggingErrorMsg(err, "could not marshal service key")
 	}
-	if err = tx.Write(ctx, namespace, skKey, keyBytes); err != nil {
+	if err = tx.Write(ctx, serviceNamespace, skKey, keyBytes); err != nil {
 		return sdkutil.LoggingErrorMsg(err, "could store marshal service key")
 	}
 	return nil
 }
 
 func getServiceKey(ctx context.Context, db storage.ServiceStorage) ([]byte, error) {
-	storedKeyBytes, err := db.Read(ctx, namespace, skKey)
+	storedKeyBytes, err := db.Read(ctx, serviceNamespace, skKey)
 	if err != nil {
 		return nil, sdkutil.LoggingErrorMsg(err, "could not get service key")
 	}
@@ -302,7 +308,7 @@ func (kss *Storage) StoreKey(ctx context.Context, key StoredKey) error {
 		return sdkutil.LoggingErrorMsg(err, "marshalling JWK")
 	}
 
-	if err := kss.tx.Write(ctx, namespace+publicNamespaceSuffix, id, publicBytes); err != nil {
+	if err := kss.tx.Write(ctx, publicKeyNamespace, id, publicBytes); err != nil {
 		return sdkutil.LoggingErrorMsgf(err, "writing public key")
 	}
 
@@ -358,7 +364,7 @@ func (kss *Storage) GetKeyDetails(ctx context.Context, id string) (*KeyDetails,
 		return nil, sdkutil.LoggingErrorMsgf(err, "reading details for private key %q", id)
 	}
 
-	storedPublicKeyBytes, err := kss.db.Read(ctx, namespace+publicNamespaceSuffix, id)
+	storedPublicKeyBytes, err := kss.db.Read(ctx, publicKeyNamespace, id)
 	if err != nil {
 		return nil, sdkutil.LoggingErrorMsgf(err, "reading details for public key %q", id)
 	}
diff --git a/pkg/service/webhook/storage.go b/pkg/service/webhook/storage.go
index 6bd7a41..e391652 100644
--- a/pkg/service/webhook/storage.go
+++ b/pkg/service/webhook/storage.go
@@ -73,5 +73,5 @@ func (whs *Storage) DeleteWebhook(ctx context.Context, noun, verb string) error
 }
 
 func getWebhookKey(noun, verb string) string {
-	return noun + ":" + verb
+	return storage.Join(noun, verb)
 }
diff --git a/pkg/storage/bolt.go b/pkg/storage/bolt.go
index dd09649..b7705a4 100644
--- a/pkg/storage/bolt.go
+++ b/pkg/storage/bolt.go
@@ -5,7 +5,6 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
-	"strings"
 	"time"
 
 	sdkutil "github.com/TBD54566975/ssi-sdk/util"
@@ -408,8 +407,3 @@ func updateTx(tx *bolt.Tx, namespace string, key string, updater Updater) ([]byt
 	}
 	return data, nil
 }
-
-// MakeNamespace takes a set of possible namespace values and combines them as a convention
-func MakeNamespace(ns ...string) string {
-	return strings.Join(ns, "-")
-}
diff --git a/pkg/storage/redis.go b/pkg/storage/redis.go
index bfabd18..9834a13 100644
--- a/pkg/storage/redis.go
+++ b/pkg/storage/redis.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/cenkalti/backoff/v4"
@@ -21,11 +20,10 @@ func init() {
 }
 
 const (
-	NamespaceKeySeparator           = ":"
-	Pong                            = "PONG"
-	RedisScanBatchSize              = 1000
-	MaxElapsedTime                  = 6 * time.Second
-	RedisAddressOption    OptionKey = "redis-address-option"
+	Pong                         = "PONG"
+	RedisScanBatchSize           = 1000
+	MaxElapsedTime               = 6 * time.Second
+	RedisAddressOption OptionKey = "redis-address-option"
 )
 
 type RedisDB struct {
@@ -46,7 +44,7 @@ func (b *RedisDB) ReadPage(ctx context.Context, namespace string, pageToken stri
 	if err != nil {
 		return nil, "", err
 	}
-	results, err := readAll(ctx, keys, b)
+	results, err := readAll(ctx, namespace, keys, b)
 	if err != nil {
 		return nil, "", err
 	}
@@ -241,7 +239,7 @@ func (b *RedisDB) ReadPrefix(ctx context.Context, namespace, prefix string) (map
 		return nil, errors.Wrap(err, "read all keys")
 	}
 
-	return readAll(ctx, keys, b)
+	return readAll(ctx, namespace, keys, b)
 }
 
 func (b *RedisDB) ReadAll(ctx context.Context, namespace string) (map[string][]byte, error) {
@@ -250,11 +248,11 @@ func (b *RedisDB) ReadAll(ctx context.Context, namespace string) (map[string][]b
 		return nil, errors.Wrap(err, "read all keys")
 	}
 
-	return readAll(ctx, keys, b)
+	return readAll(ctx, namespace, keys, b)
 }
 
 // TODO: This potentially could dangerous as it might run out of memory as we populate result
-func readAll(ctx context.Context, keys []string, b *RedisDB) (map[string][]byte, error) {
+func readAll(ctx context.Context, namespace string, keys []string, b *RedisDB) (map[string][]byte, error) {
 	result := make(map[string][]byte, len(keys))
 
 	if len(keys) == 0 {
@@ -271,10 +269,10 @@ func readAll(ctx context.Context, keys []string, b *RedisDB) (map[string][]byte,
 	}
 
 	// result needs to take the namespace out of the key
-	namespaceDashIndex := strings.Index(keys[0], NamespaceKeySeparator)
+	keyStart := len(namespace) + 1
 	for i, val := range values {
 		byteValue := []byte(fmt.Sprintf("%v", val))
-		key := keys[i][namespaceDashIndex+1:]
+		key := keys[i][keyStart:]
 		result[key] = byteValue
 	}
 
@@ -291,9 +289,9 @@ func (b *RedisDB) ReadAllKeys(ctx context.Context, namespace string) ([]string,
 		return make([]string, 0), nil
 	}
 
-	namespaceDashIndex := strings.Index(keys[0], NamespaceKeySeparator)
+	keyStart := len(namespace) + 1
 	for i, key := range keys {
-		keyWithoutNamespace := key[namespaceDashIndex+1:]
+		keyWithoutNamespace := key[keyStart:]
 		keys[i] = keyWithoutNamespace
 	}
 
@@ -433,7 +431,7 @@ func txWithUpdater(ctx context.Context, namespace, key string, updater Updater,
 }
 
 func getRedisKey(namespace, key string) string {
-	return namespace + NamespaceKeySeparator + key
+	return Join(namespace, key)
 }
 
 func namespaceExists(ctx context.Context, namespace string, b *RedisDB) bool {
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
index 02783dd..b29e373 100644
--- a/pkg/storage/storage.go
+++ b/pkg/storage/storage.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"strings"
 
 	"github.com/sirupsen/logrus"
 )
@@ -124,3 +125,14 @@ func GetStorage(storageType Type) ServiceStorage {
 	// Not pointer:
 	return reflect.New(reflect.TypeOf(tmp)).Elem().Interface().(ServiceStorage)
 }
+
+// Join combines all parts using `:` as the separator.
+func Join(parts ...string) string {
+	const separator = ":"
+	return strings.Join(parts, separator)
+}
+
+// MakeNamespace takes a set of possible namespace values and combines them as a convention
+func MakeNamespace(ns ...string) string {
+	return strings.Join(ns, "-")
+}


Test patch:
diff --git a/pkg/server/router/credential_test.go b/pkg/server/router/credential_test.go
index b364723..b8bdef1 100644
--- a/pkg/server/router/credential_test.go
+++ b/pkg/server/router/credential_test.go
@@ -771,7 +771,7 @@ func TestCredentialRouter(t *testing.T) {
 				assert.NoError(tt, err)
 				assert.NotEmpty(tt, createdCredSuspendable)
 
-				revocationKey := strings.Join([]string{"is:" + issuerDID.DID.ID, "sc:" + createdSchema.ID, "sp:" + string(status.StatusRevocation)}, "-")
+				revocationKey := storage.Join("is", issuerDID.DID.ID, "sc", createdSchema.ID, "sp", string(status.StatusRevocation))
 
 				slcExists, err := s.Exists(context.Background(), "status-list-credential", revocationKey)
 				assert.NoError(tt, err)
@@ -785,7 +785,7 @@ func TestCredentialRouter(t *testing.T) {
 				assert.NoError(tt, err)
 				assert.True(tt, currentIndexExists)
 
-				suspensionKey := strings.Join([]string{"is:" + issuerDID.DID.ID, "sc:" + createdSchema.ID, "sp:" + string(status.StatusSuspension)}, "-")
+				suspensionKey := storage.Join("is", issuerDID.DID.ID, "sc", createdSchema.ID, "sp", string(status.StatusSuspension))
 
 				slcExists, err = s.Exists(context.Background(), "status-list-credential", suspensionKey)
 				assert.NoError(tt, err)
diff --git a/pkg/server/server_credential_test.go b/pkg/server/server_credential_test.go
index e38c998..8875ccf 100644
--- a/pkg/server/server_credential_test.go
+++ b/pkg/server/server_credential_test.go
@@ -214,8 +214,6 @@ func TestCredentialAPI(t *testing.T) {
 				assert.NoError(ttt, err)
 
 				assert.NotEmpty(ttt, resp.CredentialJWT)
-				println("HEREHERE")
-				println(*resp.CredentialJWT)
 				assert.NoError(ttt, err)
 				assert.Equal(ttt, resp.Credential.Issuer, issuerDID.DID.ID)
 				after, found := strings.CutPrefix(resp.Credential.ID, "https://ssi-service.com/v1/credentials/")