Skip to content

Commit

Permalink
fix: rename method GenerateKeySet to GenerateAndPersistKeySet
Browse files Browse the repository at this point in the history
  • Loading branch information
@aarmam
aarmam committed Dec 8, 2021
1 parent 5a6d5d3 commit 5f6f1e6
Show file tree
Hide file tree
Showing 13 changed files with 35 additions and 35 deletions.
2 changes: 1 addition & 1 deletion hsm/manager_hsm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func NewKeyManager(hsm Context) *KeyManager {
}
}

func (m *KeyManager) GenerateKeySet(_ context.Context, set, kid, alg, use string) (*jose.JSONWebKeySet, error) {
func (m *KeyManager) GenerateAndPersistKeySet(_ context.Context, set, kid, alg, use string) (*jose.JSONWebKeySet, error) {
m.Lock()
defer m.Unlock()

Expand Down
6 changes: 3 additions & 3 deletions hsm/manager_hsm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestDefaultKeyManager_HsmEnabled(t *testing.T) {
assert.IsType(t, &sql.Persister{}, reg.SoftwareKeyManager())
}

func TestKeyManager_GenerateKeySet(t *testing.T) {
func TestKeyManager_GenerateAndPersistKeySet(t *testing.T) {
ctrl := gomock.NewController(t)
hsmContext := NewMockContext(ctrl)
defer ctrl.Finish()
Expand Down Expand Up @@ -218,7 +218,7 @@ func TestKeyManager_GenerateKeySet(t *testing.T) {
m := &hsm.KeyManager{
Context: hsmContext,
}
got, err := m.GenerateKeySet(tt.args.ctx, tt.args.set, tt.args.kid, tt.args.alg, tt.args.use)
got, err := m.GenerateAndPersistKeySet(tt.args.ctx, tt.args.set, tt.args.kid, tt.args.alg, tt.args.use)
if tt.wantErr != nil {
require.Nil(t, got)
require.IsType(t, tt.wantErr, err)
Expand All @@ -228,7 +228,7 @@ func TestKeyManager_GenerateKeySet(t *testing.T) {
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("GenerateKeySet() got = %v, want %v", got, tt.want)
t.Errorf("GenerateAndPersistKeySet() got = %v, want %v", got, tt.want)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion hsm/manager_nohsm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func NewKeyManager(hsm Context) *KeyManager {
return nil
}

func (m *KeyManager) GenerateKeySet(_ context.Context, set, kid, alg, use string) (*jose.JSONWebKeySet, error) {
func (m *KeyManager) GenerateAndPersistKeySet(_ context.Context, set, kid, alg, use string) (*jose.JSONWebKeySet, error) {
return nil, errors.WithStack(ErrOpSysNotSupported)
}

Expand Down
2 changes: 1 addition & 1 deletion jwk/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (h *Handler) Create(w http.ResponseWriter, r *http.Request, ps httprouter.P
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
}

if keys, err := h.r.KeyManager().GenerateKeySet(r.Context(), set, keyRequest.KeyID, keyRequest.Algorithm, keyRequest.Use); err == nil {
if keys, err := h.r.KeyManager().GenerateAndPersistKeySet(r.Context(), set, keyRequest.KeyID, keyRequest.Algorithm, keyRequest.Use); err == nil {
keys = ExcludeOpaquePrivateKeys(keys)
h.r.Writer().WriteCreated(w, r, fmt.Sprintf("%s://%s/keys/%s", r.URL.Scheme, r.URL.Host, set), keys)
} else {
Expand Down
2 changes: 1 addition & 1 deletion jwk/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestHandlerWellKnown(t *testing.T) {
var IDKS *jose.JSONWebKeySet

if conf.HsmEnabled() {
IDKS, _ = reg.KeyManager().GenerateKeySet(context.TODO(), x.OpenIDConnectKeyName, "test-id-2", "RS256", "sig")
IDKS, _ = reg.KeyManager().GenerateAndPersistKeySet(context.TODO(), x.OpenIDConnectKeyName, "test-id-2", "RS256", "sig")
} else {
IDKS, _ = testGenerator.Generate("test-id-2", "sig")
if strings.ContainsAny(IDKS.Keys[1].KeyID, "public") {
Expand Down
4 changes: 2 additions & 2 deletions jwk/helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set,
keys, err := m.GetKeySet(ctx, set)
if errors.Is(err, x.ErrNotFound) || keys != nil && len(keys.Keys) == 0 {
r.Logger().Warnf("JSON Web Key Set \"%s\" does not exist yet, generating new key pair...", set)
keys, err = m.GenerateKeySet(ctx, set, kid, alg, "sig")
keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig")
if err != nil {
return nil, nil, err
}
Expand All @@ -65,7 +65,7 @@ func GetOrGenerateKeys(ctx context.Context, r InternalRegistry, m Manager, set,
} else {
r.Logger().Warnf("Private JSON Web Key not found in JSON Web Key Set %s, generating new key pair...", set)
}
keys, err = m.GenerateKeySet(ctx, set, kid, alg, "sig")
keys, err = m.GenerateAndPersistKeySet(ctx, set, kid, alg, "sig")
if err != nil {
return nil, nil, err
}
Expand Down
20 changes: 10 additions & 10 deletions jwk/helper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,18 +206,18 @@ func TestGetOrGenerateKeys(t *testing.T) {
assert.EqualError(t, err, "GetKeySetError")
})

t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GenerateKeySetError", func(t *testing.T) {
t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GenerateAndPersistKeySetError", func(t *testing.T) {
keyManager.EXPECT().GetKeySet(gomock.Any(), gomock.Eq(setId)).Return(nil, errors.Wrap(x.ErrNotFound, ""))
keyManager.EXPECT().GenerateKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(nil, errors.New("GetKeySetError"))
keyManager.EXPECT().GenerateAndPersistKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(nil, errors.New("GetKeySetError"))
pubKey, privKey, err := jwk.GetOrGenerateKeys(context.TODO(), reg, keyManager, setId, keyId, "RS256")
assert.Nil(t, pubKey)
assert.Nil(t, privKey)
assert.EqualError(t, err, "GetKeySetError")
})

t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GenerateKeySetError", func(t *testing.T) {
t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GenerateAndPersistKeySetError", func(t *testing.T) {
keyManager.EXPECT().GetKeySet(gomock.Any(), gomock.Eq(setId)).Return(keySetWithoutPrivateKey, nil)
keyManager.EXPECT().GenerateKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(nil, errors.New("GetKeySetError"))
keyManager.EXPECT().GenerateAndPersistKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(nil, errors.New("GetKeySetError"))
pubKey, privKey, err := jwk.GetOrGenerateKeys(context.TODO(), reg, keyManager, setId, keyId, "RS256")
assert.Nil(t, pubKey)
assert.Nil(t, privKey)
Expand All @@ -226,7 +226,7 @@ func TestGetOrGenerateKeys(t *testing.T) {

t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GetKeySet_ContainsMissingPublicKey", func(t *testing.T) {
keyManager.EXPECT().GetKeySet(gomock.Any(), gomock.Eq(setId)).Return(keySetWithoutPublicKey, nil)
keyManager.EXPECT().GenerateKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(keySet, nil)
keyManager.EXPECT().GenerateAndPersistKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(keySet, nil)
pubKey, privKey, err := jwk.GetOrGenerateKeys(context.TODO(), reg, keyManager, setId, keyId, "RS256")
assert.NoError(t, err)
assert.Equal(t, privKey, &keySet.Keys[0])
Expand All @@ -235,25 +235,25 @@ func TestGetOrGenerateKeys(t *testing.T) {

t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GetKeySet_ContainsMissingPrivateKey", func(t *testing.T) {
keyManager.EXPECT().GetKeySet(gomock.Any(), gomock.Eq(setId)).Return(keySetWithoutPrivateKey, nil)
keyManager.EXPECT().GenerateKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(keySet, nil)
keyManager.EXPECT().GenerateAndPersistKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(keySet, nil)
pubKey, privKey, err := jwk.GetOrGenerateKeys(context.TODO(), reg, keyManager, setId, keyId, "RS256")
assert.NoError(t, err)
assert.Equal(t, privKey, &keySet.Keys[0])
assert.Equal(t, pubKey, &keySet.Keys[1])
})

t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GenerateKeySet_ContainsMissingPublicKey", func(t *testing.T) {
t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GenerateAndPersistKeySet_ContainsMissingPublicKey", func(t *testing.T) {
keyManager.EXPECT().GetKeySet(gomock.Any(), gomock.Eq(setId)).Return(keySetWithoutPrivateKey, nil)
keyManager.EXPECT().GenerateKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(keySetWithoutPublicKey, nil)
keyManager.EXPECT().GenerateAndPersistKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(keySetWithoutPublicKey, nil)
pubKey, privKey, err := jwk.GetOrGenerateKeys(context.TODO(), reg, keyManager, setId, keyId, "RS256")
assert.Nil(t, pubKey)
assert.Nil(t, privKey)
assert.EqualError(t, err, "key not found")
})

t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GenerateKeySet_ContainsMissingPrivateKey", func(t *testing.T) {
t.Run("Test_Helper/Run_GetOrGenerateKeys_With_GenerateAndPersistKeySet_ContainsMissingPrivateKey", func(t *testing.T) {
keyManager.EXPECT().GetKeySet(gomock.Any(), gomock.Eq(setId)).Return(keySetWithoutPublicKey, nil)
keyManager.EXPECT().GenerateKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(keySetWithoutPrivateKey, nil)
keyManager.EXPECT().GenerateAndPersistKeySet(gomock.Any(), gomock.Eq(setId), gomock.Eq(keyId), gomock.Eq("RS256"), gomock.Eq("sig")).Return(keySetWithoutPrivateKey, nil)
pubKey, privKey, err := jwk.GetOrGenerateKeys(context.TODO(), reg, keyManager, setId, keyId, "RS256")
assert.Nil(t, pubKey)
assert.Nil(t, privKey)
Expand Down
4 changes: 2 additions & 2 deletions jwk/jwt_strategy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestRS256JWTStrategy(t *testing.T) {
reg := internal.NewRegistryMemory(t, conf)
m := reg.KeyManager()

_, err := m.GenerateKeySet(context.TODO(), "foo-set", "foo", "RS256", "sig")
_, err := m.GenerateAndPersistKeySet(context.TODO(), "foo-set", "foo", "RS256", "sig")
require.NoError(t, err)

s, err := NewRS256JWTStrategy(*conf, reg, func() string {
Expand All @@ -67,7 +67,7 @@ func TestRS256JWTStrategy(t *testing.T) {
kidFoo, err := s.GetPublicKeyID(context.TODO())
assert.NoError(t, err)

_, err = m.GenerateKeySet(context.TODO(), "foo-set", "bar", "RS256", "sig")
_, err = m.GenerateAndPersistKeySet(context.TODO(), "foo-set", "bar", "RS256", "sig")
require.NoError(t, err)

a, b, err = s.Generate(context.TODO(), jwt2.MapClaims{"foo": "bar"}, &jwt.Headers{})
Expand Down
2 changes: 1 addition & 1 deletion jwk/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var ErrUnsupportedEllipticCurve = &fosite.RFC6749Error{

type (
Manager interface {
GenerateKeySet(ctx context.Context, set, kid, alg, use string) (*jose.JSONWebKeySet, error)
GenerateAndPersistKeySet(ctx context.Context, set, kid, alg, use string) (*jose.JSONWebKeySet, error)

AddKey(ctx context.Context, set string, key *jose.JSONWebKey) error

Expand Down
12 changes: 6 additions & 6 deletions jwk/manager_mock_test.go
View file

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

4 changes: 2 additions & 2 deletions jwk/manager_test_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ func TestHelperManagerKeySet(m Manager, algo string, keys *jose.JSONWebKeySet, s
}
}

func TestHelperManagerGenerateKeySet(m Manager, alg string) func(t *testing.T) {
func TestHelperManagerGenerateAndPersistKeySet(m Manager, alg string) func(t *testing.T) {
return func(t *testing.T) {
_, err := m.GetKeySet(context.TODO(), "foo")
require.Error(t, err)

keys, err := m.GenerateKeySet(context.TODO(), "foo", "bar", alg, "sig")
keys, err := m.GenerateAndPersistKeySet(context.TODO(), "foo", "bar", alg, "sig")
require.NoError(t, err)
genPub, err := FindPublicKey(keys)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion persistence/sql/persister_jwk.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

var _ jwk.Manager = &Persister{}

func (p *Persister) GenerateKeySet(ctx context.Context, set, kid, alg, use string) (*jose.JSONWebKeySet, error) {
func (p *Persister) GenerateAndPersistKeySet(ctx context.Context, set, kid, alg, use string) (*jose.JSONWebKeySet, error) {
generator, found := p.r.KeyGenerators()[alg]
if !found {
return nil, errorsx.WithStack(jwk.ErrUnsupportedKeyAlgorithm)
Expand Down
8 changes: 4 additions & 4 deletions persistence/sql/persister_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,20 @@ func TestManagers(t *testing.T) {
t.Skipf("Skipping test. Not applicable for alg: %s", tc.alg)
}
if m.Config().HsmEnabled() {
t.Run("TestManagerGenerateKeySet", jwk.TestHelperManagerGenerateKeySet(m.KeyManager(), tc.alg))
t.Run("TestManagerGenerateAndPersistKeySet", jwk.TestHelperManagerGenerateAndPersistKeySet(m.KeyManager(), tc.alg))
} else {
kid := uuid.New()
ks, err := tc.keyGenerator.Generate(kid, "sig")
require.NoError(t, err)
t.Run("TestManagerKey", jwk.TestHelperManagerKey(m.KeyManager(), tc.alg, ks, kid))
t.Run("TestManagerKeySet", jwk.TestHelperManagerKeySet(m.KeyManager(), tc.alg, ks, kid))
t.Run("TestManagerGenerateKeySet", jwk.TestHelperManagerGenerateKeySet(m.KeyManager(), tc.alg))
t.Run("TestManagerGenerateAndPersistKeySet", jwk.TestHelperManagerGenerateAndPersistKeySet(m.KeyManager(), tc.alg))
}
})
}

t.Run("TestManagerGenerateKeySetWithUnsupportedKeyGenerator", func(t *testing.T) {
_, err := m.KeyManager().GenerateKeySet(context.TODO(), "foo", "bar", "UNKNOWN", "sig")
t.Run("TestManagerGenerateAndPersistKeySetWithUnsupportedKeyGenerator", func(t *testing.T) {
_, err := m.KeyManager().GenerateAndPersistKeySet(context.TODO(), "foo", "bar", "UNKNOWN", "sig")
require.Error(t, err)
assert.IsType(t, errors.WithStack(jwk.ErrUnsupportedKeyAlgorithm), err)
})
Expand Down

0 comments on commit 5f6f1e6

Please sign in to comment.