Skip to content

Commit

Permalink
revert refactor of KIDNamingFunc
Browse files Browse the repository at this point in the history
  • Loading branch information
reinkrul committed May 21, 2024
1 parent 3c974ff commit 9a9df48
Show file tree
Hide file tree
Showing 24 changed files with 58 additions and 77 deletions.
3 changes: 1 addition & 2 deletions crypto/crypto.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"errors"
"fmt"
"github.com/lestrrat-go/jwx/v2/jwk"
"github.com/nuts-foundation/nuts-node/crypto/storage"
"path"
"time"

Expand Down Expand Up @@ -150,7 +149,7 @@ func (client *Crypto) Configure(config core.ServerConfig) error {
// New generates a new key pair.
// Stores the private key, returns the public basicKey.
// It returns an error when a key with the resulting ID already exists.
func (client *Crypto) New(ctx context.Context, namingFunc storage.KIDNamingFunc) (Key, error) {
func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (Key, error) {
publicKey, kid, err := client.storage.NewPrivateKey(ctx, namingFunc)
if err != nil {
return nil, err
Expand Down
9 changes: 4 additions & 5 deletions crypto/crypto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/nuts-foundation/nuts-node/audit"
"github.com/nuts-foundation/nuts-node/crypto/storage/fs"
"github.com/nuts-foundation/nuts-node/crypto/storage/spi"
"github.com/nuts-foundation/nuts-node/crypto/test"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"net/http"
Expand All @@ -44,7 +43,7 @@ func TestCrypto_Exists(t *testing.T) {
client := createCrypto(t)

kid := "kid"
_, err := client.New(audit.TestContext(), test.StringNamingFunc(kid))
_, err := client.New(audit.TestContext(), StringNamingFunc(kid))
require.NoError(t, err)

t.Run("returns true for existing key", func(t *testing.T) {
Expand All @@ -69,7 +68,7 @@ func TestCrypto_New(t *testing.T) {
kid := "kid"
auditLogs := audit.CaptureLogs(t)

key, err := client.New(ctx, test.StringNamingFunc(kid))
key, err := client.New(ctx, StringNamingFunc(kid))

assert.NoError(t, err)
assert.NotNil(t, key.Public())
Expand All @@ -80,7 +79,7 @@ func TestCrypto_New(t *testing.T) {
t.Run("error - invalid KID", func(t *testing.T) {
kid := "../certificate"

key, err := client.New(ctx, test.StringNamingFunc(kid))
key, err := client.New(ctx, StringNamingFunc(kid))

assert.ErrorContains(t, err, "invalid key ID")
assert.Nil(t, key)
Expand Down Expand Up @@ -110,7 +109,7 @@ func TestCrypto_Resolve(t *testing.T) {
ctx := context.Background()
client := createCrypto(t)
kid := "kid"
key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid))
key, _ := client.New(audit.TestContext(), StringNamingFunc(kid))

t.Run("ok", func(t *testing.T) {
resolvedKey, err := client.Resolve(ctx, "kid")
Expand Down
3 changes: 1 addition & 2 deletions crypto/decrypter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"context"
"crypto/ecdsa"
"github.com/nuts-foundation/nuts-node/audit"
"github.com/nuts-foundation/nuts-node/crypto/test"
"github.com/stretchr/testify/assert"
"testing"
)
Expand All @@ -32,7 +31,7 @@ func TestCrypto_Decrypt(t *testing.T) {
t.Run("ok", func(t *testing.T) {
client := createCrypto(t)
kid := "kid"
key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid))
key, _ := client.New(audit.TestContext(), StringNamingFunc(kid))
pubKey := key.Public().(*ecdsa.PublicKey)

cipherText, err := EciesEncrypt(pubKey, []byte("hello!"))
Expand Down
3 changes: 1 addition & 2 deletions crypto/dpop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package crypto

import (
"encoding/base64"
"github.com/nuts-foundation/nuts-node/crypto/test"
"net/http"
"testing"

Expand All @@ -35,7 +34,7 @@ import (

func TestDPOP(t *testing.T) {
client := createCrypto(t)
privateKey, _ := client.New(audit.TestContext(), test.StringNamingFunc("kid"))
privateKey, _ := client.New(audit.TestContext(), StringNamingFunc("kid"))
keyAsJWK, _ := jwk.FromRaw(privateKey.Public())
_ = keyAsJWK.Set(jwk.AlgorithmKey, jwa.ES256)
request, _ := http.NewRequest("POST", "https://server.example.com/token", nil)
Expand Down
3 changes: 1 addition & 2 deletions crypto/ephemeral.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@
package crypto

import (
"github.com/nuts-foundation/nuts-node/crypto/storage"
"github.com/nuts-foundation/nuts-node/crypto/storage/spi"
)

// NewEphemeralKey returns a Key for single use.
func NewEphemeralKey(namingFunc storage.KIDNamingFunc) (Key, error) {
func NewEphemeralKey(namingFunc KIDNamingFunc) (Key, error) {
keyPair, kid, err := spi.GenerateKeyPairAndKID(namingFunc)
if err != nil {
return nil, err
Expand Down
5 changes: 2 additions & 3 deletions crypto/ephemeral_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ package crypto

import (
"errors"
"github.com/nuts-foundation/nuts-node/crypto/test"
"github.com/stretchr/testify/require"
"testing"

Expand All @@ -30,7 +29,7 @@ import (

func TestNewEphemeralKey(t *testing.T) {
t.Run("ok", func(t *testing.T) {
key, err := NewEphemeralKey(test.StringNamingFunc("kid"))
key, err := NewEphemeralKey(StringNamingFunc("kid"))

require.NoError(t, err)

Expand All @@ -40,7 +39,7 @@ func TestNewEphemeralKey(t *testing.T) {
})

t.Run("error", func(t *testing.T) {
_, err := NewEphemeralKey(test.ErrorNamingFunc(errors.New("b00m!")))
_, err := NewEphemeralKey(ErrorNamingFunc(errors.New("b00m!")))

assert.EqualError(t, err, "b00m!")
})
Expand Down
6 changes: 4 additions & 2 deletions crypto/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,19 @@ import (
"crypto"
"errors"
"github.com/nuts-foundation/nuts-node/crypto/dpop"
"github.com/nuts-foundation/nuts-node/crypto/storage"
)

// ErrPrivateKeyNotFound is returned when the private key doesn't exist
var ErrPrivateKeyNotFound = errors.New("private key not found")

// KIDNamingFunc is a function passed to New() which generates the kid for the pub/priv key
type KIDNamingFunc func(key crypto.PublicKey) (string, error)

// KeyCreator is the interface for creating key pairs.
type KeyCreator interface {
// New generates a keypair and returns a Key. The context is used to pass audit information.
// The KIDNamingFunc will provide the kid.
New(ctx context.Context, namingFunc storage.KIDNamingFunc) (Key, error)
New(ctx context.Context, namingFunc KIDNamingFunc) (Key, error)
}

// KeyResolver is the interface for resolving keys.
Expand Down
8 changes: 4 additions & 4 deletions crypto/jwx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func TestCrypto_SignJWT(t *testing.T) {
client := createCrypto(t)

kid := "kid"
key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid))
key, _ := client.New(audit.TestContext(), StringNamingFunc(kid))

t.Run("creates valid JWT", func(t *testing.T) {
tokenString, err := client.SignJWT(audit.TestContext(), map[string]interface{}{"iss": "nuts", "sub": "subject"}, nil, key)
Expand Down Expand Up @@ -198,7 +198,7 @@ func TestCrypto_SignJWS(t *testing.T) {
client := createCrypto(t)

kid := "kid"
key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid))
key, _ := client.New(audit.TestContext(), StringNamingFunc(kid))

t.Run("creates valid JWS", func(t *testing.T) {
payload, _ := json.Marshal(map[string]interface{}{"iss": "nuts"})
Expand Down Expand Up @@ -245,7 +245,7 @@ func TestCrypto_SignJWS(t *testing.T) {
func TestCrypto_EncryptJWE(t *testing.T) {
client := createCrypto(t)

key, _ := client.New(audit.TestContext(), test.StringNamingFunc("did:nuts:1234#key-1"))
key, _ := client.New(audit.TestContext(), StringNamingFunc("did:nuts:1234#key-1"))
public := key.Public()

headers := map[string]interface{}{"typ": "JWT", "kid": key.KID()}
Expand Down Expand Up @@ -345,7 +345,7 @@ func TestCrypto_DecryptJWE(t *testing.T) {
client := createCrypto(t)

kid := "did:nuts:1234#key-1"
key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid))
key, _ := client.New(audit.TestContext(), StringNamingFunc(kid))

t.Run("decrypts valid JWE", func(t *testing.T) {
payload, _ := json.Marshal(map[string]interface{}{"iss": "nuts"})
Expand Down
5 changes: 2 additions & 3 deletions crypto/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions crypto/storage/external/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"crypto"
"errors"
"fmt"
"github.com/nuts-foundation/nuts-node/crypto/storage"
"net/http"
"net/url"
"time"
Expand All @@ -43,7 +42,7 @@ type APIClient struct {
httpClient *ClientWithResponses
}

func (c APIClient) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) {
func (c APIClient) NewPrivateKey(ctx context.Context, namingFunc func(crypto.PublicKey) (string, error)) (crypto.PublicKey, string, error) {
return spi.GenerateAndStore(ctx, c, namingFunc)
}

Expand Down
3 changes: 1 addition & 2 deletions crypto/storage/fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"errors"
"fmt"
"github.com/nuts-foundation/nuts-node/crypto/log"
"github.com/nuts-foundation/nuts-node/crypto/storage"
"io/fs"
"os"
"path/filepath"
Expand Down Expand Up @@ -93,7 +92,7 @@ func NewFileSystemBackend(fspath string) (spi.Storage, error) {
return fsc, nil
}

func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) {
func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, namingFunc func(crypto.PublicKey) (string, error)) (crypto.PublicKey, string, error) {
return spi.GenerateAndStore(ctx, fsc, namingFunc)
}

Expand Down
5 changes: 0 additions & 5 deletions crypto/storage/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,3 @@
// Will be removed in a future release, in favor of the `external` storage backend.
// - `external` (External): a secret storage backend that stores secrets externally (e.g. Vault).
package storage

import "crypto"

// KIDNamingFunc is a function passed to New() which generates the kid for the pub/priv key
type KIDNamingFunc func(key crypto.PublicKey) (string, error)
7 changes: 3 additions & 4 deletions crypto/storage/spi/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/nuts-foundation/nuts-node/crypto/storage"
"regexp"

"github.com/lestrrat-go/jwx/v2/jwk"
Expand All @@ -49,7 +48,7 @@ type Storage interface {
// NewPrivateKey creates a new private key and returns its handler as an implementation of crypto.Signer.
// It should be preferred over generating a key in the application and saving it to the storage,
// as it allows for unexportable (safer) keys. If the resulting kid already exists, it returns an error
NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error)
NewPrivateKey(ctx context.Context, namingFunc func(crypto.PublicKey) (string, error)) (crypto.PublicKey, string, error)
// GetPrivateKey from the storage backend and return its handler as an implementation of crypto.Signer.
GetPrivateKey(ctx context.Context, kid string) (crypto.Signer, error)
// PrivateKeyExists checks if the private key indicated with the kid is stored in the storage backend.
Expand Down Expand Up @@ -105,7 +104,7 @@ func (pke PublicKeyEntry) JWK() jwk.Key {
return pke.parsedJWK
}

func GenerateAndStore(ctx context.Context, store Storage, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) {
func GenerateAndStore(ctx context.Context, store Storage, namingFunc func(crypto.PublicKey) (string, error)) (crypto.PublicKey, string, error) {
keyPair, kid, err := GenerateKeyPairAndKID(namingFunc)
if err != nil {
return nil, "", err
Expand All @@ -127,7 +126,7 @@ func GenerateKeyPair() (*ecdsa.PrivateKey, error) {

// GenerateKeyPairAndKID generates a new key pair and a KID using the provided naming function.
// It's intended to be used by crypto backends that don't create unexportable keys.
func GenerateKeyPairAndKID(namingFunc storage.KIDNamingFunc) (*ecdsa.PrivateKey, string, error) {
func GenerateKeyPairAndKID(namingFunc func(crypto.PublicKey) (string, error)) (*ecdsa.PrivateKey, string, error) {
keyPair, err := GenerateKeyPair()
if err != nil {
return nil, "", err
Expand Down
13 changes: 9 additions & 4 deletions crypto/storage/spi/interface_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"crypto/rand"
"errors"
"github.com/lestrrat-go/jwx/v2/jwk"
"github.com/nuts-foundation/nuts-node/crypto/test"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"testing"
Expand Down Expand Up @@ -75,7 +74,9 @@ func TestGenerateAndStore(t *testing.T) {
store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(nil)
kid := "123"

key, kid, err := GenerateAndStore(ctx, store, test.StringNamingFunc(kid))
key, kid, err := GenerateAndStore(ctx, store, func(_ crypto.PublicKey) (string, error) {
return kid, nil
})

assert.NoError(t, err)
assert.NotNil(t, key)
Expand All @@ -99,7 +100,9 @@ func TestGenerateAndStore(t *testing.T) {
store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(errors.New("foo"))
kid := "123"

_, _, err := GenerateAndStore(ctx, store, test.StringNamingFunc(kid))
_, _, err := GenerateAndStore(ctx, store, func(_ crypto.PublicKey) (string, error) {
return kid, nil
})

assert.ErrorContains(t, err, "could not create new keypair: could not save private key: foo")
})
Expand All @@ -110,7 +113,9 @@ func TestGenerateAndStore(t *testing.T) {
store.EXPECT().PrivateKeyExists(ctx, "123").Return(true)
kid := "123"

_, _, err := GenerateAndStore(ctx, store, test.StringNamingFunc(kid))
_, _, err := GenerateAndStore(ctx, store, func(_ crypto.PublicKey) (string, error) {
return kid, nil
})

assert.ErrorContains(t, err, "key with the given ID already exists")
})
Expand Down
3 changes: 1 addition & 2 deletions crypto/storage/spi/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions crypto/storage/spi/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"crypto"
"fmt"
"github.com/nuts-foundation/nuts-node/core"
"github.com/nuts-foundation/nuts-node/crypto/storage"
"regexp"
)

Expand Down Expand Up @@ -90,7 +89,7 @@ func (w wrapper) ListPrivateKeys(ctx context.Context) []string {
return w.wrappedBackend.ListPrivateKeys(ctx)
}

func (w wrapper) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) {
func (w wrapper) NewPrivateKey(ctx context.Context, namingFunc func(crypto.PublicKey) (string, error)) (crypto.PublicKey, string, error) {
publicKey, kid, err := w.wrappedBackend.NewPrivateKey(ctx, namingFunc)
if err != nil {
return nil, "", err
Expand Down
3 changes: 1 addition & 2 deletions crypto/storage/vault/vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
vault "github.com/hashicorp/vault/api"
"github.com/nuts-foundation/nuts-node/core"
"github.com/nuts-foundation/nuts-node/crypto/log"
"github.com/nuts-foundation/nuts-node/crypto/storage"
"github.com/nuts-foundation/nuts-node/crypto/storage/spi"
"github.com/nuts-foundation/nuts-node/crypto/util"
"path/filepath"
Expand Down Expand Up @@ -103,7 +102,7 @@ func NewVaultKVStorage(config Config) (spi.Storage, error) {
return vaultStorage, nil
}

func (v vaultKVStorage) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) {
func (v vaultKVStorage) NewPrivateKey(ctx context.Context, namingFunc func(crypto.PublicKey) (string, error)) (crypto.PublicKey, string, error) {
return spi.GenerateAndStore(ctx, v, namingFunc)
}

Expand Down
Loading

0 comments on commit 9a9df48

Please sign in to comment.