Skip to content

Commit

Permalink
PR Tweaks
Browse files Browse the repository at this point in the history 
- Added logging
- Fixed broken test
- Fixed cleanup JWKS function
  • Loading branch information
@kian99
kian99 committed Jul 26, 2023
1 parent a1e669b commit 8844dff
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 19 deletions.
1 change: 1 addition & 0 deletions docker-compose.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ services:
VAULT_PATH: "/jimm-kv/"
VAULT_SECRET_FILE: "/vault/approle.json"
VAULT_AUTH_PATH: "/auth/approle/login"
# Note: By default we should use Vault as that is the primary means of secret storage.
# INSECURE_SECRET_STORAGE: "enabled"
# JIMM_DASHBOARD_LOCATION: ""
JIMM_DNS_NAME: "jimm.localhost"
Expand Down
38 changes: 36 additions & 2 deletions internal/db/secrets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
"github.com/canonical/jimm/internal/dbmodel"
"github.com/canonical/jimm/internal/errors"
"github.com/juju/names/v4"
"github.com/juju/zaputil/zapctx"
"github.com/lestrrat-go/jwx/v2/jwk"
"go.uber.org/zap"
"gorm.io/gorm/clause"
)

Expand Down Expand Up @@ -46,7 +48,7 @@ func (d *Database) UpsertSecret(ctx context.Context, secret *dbmodel.Secret) err

// GetSecret gets the secret with the specified type and tag.
func (d *Database) GetSecret(ctx context.Context, secret *dbmodel.Secret) error {
const op = errors.Op("db.AddSecret")
const op = errors.Op("db.GetSecret")
if err := d.ready(); err != nil {
return errors.E(op, err)
}
Expand Down Expand Up @@ -90,11 +92,13 @@ func (d *Database) Get(ctx context.Context, tag names.CloudCredentialTag) (map[s
secret := dbmodel.NewSecret(tag.Kind(), tag.String(), nil)
err := d.GetSecret(ctx, &secret)
if err != nil {
zapctx.Error(ctx, "failed to get secret data", zap.Error(err))
return nil, errors.E(op, err)
}
var attr map[string]string
err = json.Unmarshal(secret.Data, &attr)
if err != nil {
zapctx.Error(ctx, "failed to unmarshal secret data", zap.Error(err))
return nil, errors.E(op, err)
}
return attr, nil
Expand All @@ -108,6 +112,7 @@ func (d *Database) Put(ctx context.Context, tag names.CloudCredentialTag, attr m
}
dataJson, err := json.Marshal(attr)
if err != nil {
zapctx.Error(ctx, "failed to marshal secret data", zap.Error(err))
return errors.E(op, err, "failed to marshal secret data")
}
secret := dbmodel.NewSecret(tag.Kind(), tag.String(), dataJson)
Expand All @@ -126,11 +131,13 @@ func (d *Database) GetControllerCredentials(ctx context.Context, controllerName
secret := dbmodel.NewSecret(names.ControllerTagKind, controllerName, nil)
err := d.GetSecret(ctx, &secret)
if err != nil {
zapctx.Error(ctx, "failed to get secret data", zap.Error(err))
return "", "", errors.E(op, err)
}
var secretData map[string]string
err = json.Unmarshal(secret.Data, &secretData)
if err != nil {
zapctx.Error(ctx, "failed to unmarshal secret data", zap.Error(err))
return "", "", errors.E(op, err)
}
username, ok := secretData[usernameKey]
Expand All @@ -152,6 +159,7 @@ func (d *Database) PutControllerCredentials(ctx context.Context, controllerName
secretData[passwordKey] = password
dataJson, err := json.Marshal(secretData)
if err != nil {
zapctx.Error(ctx, "failed to unmarshal secret data", zap.Error(err))
return errors.E(op, err, "failed to marshal secret data")
}
secret := dbmodel.NewSecret(names.ControllerTagKind, controllerName, dataJson)
Expand All @@ -162,7 +170,24 @@ func (d *Database) PutControllerCredentials(ctx context.Context, controllerName
func (d *Database) CleanupJWKS(ctx context.Context) error {
const op = errors.Op("database.CleanupJWKS")
secret := dbmodel.NewSecret(jwksKind, jwksPublicKeyTag, nil)
return d.DeleteSecret(ctx, &secret)
err := d.DeleteSecret(ctx, &secret)
if err != nil {
zapctx.Error(ctx, "failed to cleanup jwks public key", zap.Error(err))
return errors.E(op, err, "failed to cleanup jwks public key")
}
secret = dbmodel.NewSecret(jwksKind, jwksPrivateKeyTag, nil)
err = d.DeleteSecret(ctx, &secret)
if err != nil {
zapctx.Error(ctx, "failed to cleanup jwks private key", zap.Error(err))
return errors.E(op, err, "failed to cleanup jwks private key")
}
secret = dbmodel.NewSecret(jwksKind, jwksExpiryTag, nil)
err = d.DeleteSecret(ctx, &secret)
if err != nil {
zapctx.Error(ctx, "failed to cleanup jwks expiry info", zap.Error(err))
return errors.E(op, err, "failed to cleanup jwks expiry info")
}
return nil
}

// GetJWKS returns the current key set stored within the DB.
Expand All @@ -171,10 +196,12 @@ func (d *Database) GetJWKS(ctx context.Context) (jwk.Set, error) {
secret := dbmodel.NewSecret(jwksKind, jwksPublicKeyTag, nil)
err := d.GetSecret(ctx, &secret)
if err != nil {
zapctx.Error(ctx, "failed to get jwks data", zap.Error(err))
return nil, errors.E(op, err)
}
ks, err := jwk.ParseString(string(secret.Data))
if err != nil {
zapctx.Error(ctx, "failed to parse jwk data", zap.Error(err))
return nil, errors.E(op, err)
}
return ks, nil
Expand All @@ -186,11 +213,13 @@ func (d *Database) GetJWKSPrivateKey(ctx context.Context) ([]byte, error) {
secret := dbmodel.NewSecret(jwksKind, jwksPrivateKeyTag, nil)
err := d.GetSecret(ctx, &secret)
if err != nil {
zapctx.Error(ctx, "failed to get jwks jwks private key", zap.Error(err))
return nil, errors.E(op, err)
}
var pem []byte
err = json.Unmarshal(secret.Data, &pem)
if err != nil {
zapctx.Error(ctx, "failed to unmarshal pem data data", zap.Error(err))
return nil, errors.E(op, err)
}
return pem, nil
Expand All @@ -202,11 +231,13 @@ func (d *Database) GetJWKSExpiry(ctx context.Context) (time.Time, error) {
secret := dbmodel.NewSecret(jwksKind, jwksExpiryTag, nil)
err := d.GetSecret(ctx, &secret)
if err != nil {
zapctx.Error(ctx, "failed to get jwks expiry", zap.Error(err))
return time.Time{}, errors.E(op, err)
}
var expiryTime time.Time
err = json.Unmarshal(secret.Data, &expiryTime)
if err != nil {
zapctx.Error(ctx, "failed to unmarshal jwks expiry data", zap.Error(err))
return time.Time{}, errors.E(op, err)
}
return expiryTime, nil
Expand All @@ -217,6 +248,7 @@ func (d *Database) PutJWKS(ctx context.Context, jwks jwk.Set) error {
const op = errors.Op("database.PutJWKS")
jwksJson, err := json.Marshal(jwks)
if err != nil {
zapctx.Error(ctx, "failed to marshal jwks", zap.Error(err))
return errors.E(op, err, "failed to marshal jwks data")
}
secret := dbmodel.NewSecret(jwksKind, jwksPublicKeyTag, jwksJson)
Expand All @@ -229,6 +261,7 @@ func (d *Database) PutJWKSPrivateKey(ctx context.Context, pem []byte) error {
const op = errors.Op("database.PutJWKSPrivateKey")
privateKeyJson, err := json.Marshal(pem)
if err != nil {
zapctx.Error(ctx, "failed to marshal pem data", zap.Error(err))
return errors.E(op, err, "failed to marshal jwks private key")
}
secret := dbmodel.NewSecret(jwksKind, jwksPrivateKeyTag, privateKeyJson)
Expand All @@ -240,6 +273,7 @@ func (d *Database) PutJWKSExpiry(ctx context.Context, expiry time.Time) error {
const op = errors.Op("database.PutJWKSExpiry")
expiryJson, err := json.Marshal(expiry)
if err != nil {
zapctx.Error(ctx, "failed to marshal jwks expiry data", zap.Error(err))
return errors.E(op, err, "failed to marshal jwks data")
}
secret := dbmodel.NewSecret(jwksKind, jwksExpiryTag, expiryJson)
Expand Down
53 changes: 45 additions & 8 deletions internal/db/secrets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ func (s *dbSuite) TestInsertSecret(c *qt.C) {
Data: nil,
}
c.Assert(s.Database.DB.Create(&u).Error, qt.IsNil)
secret := dbmodel.Secret{}
tx := s.Database.DB.First(&secret)
c.Assert(tx.Error, qt.IsNil)
c.Assert(secret.Time, qt.Equals, testTime)
c.Assert(secret.Type, qt.Equals, "generic")
c.Assert(secret.Tag, qt.Equals, "123")
c.Assert(secret.Data, qt.IsNil)
}

func (s *dbSuite) TestUpsertSecret(c *qt.C) {
Expand Down Expand Up @@ -162,6 +169,14 @@ func getJWKS(c *qt.C) jwk.Set {
"n": "yeNlzlub94YgerT030codqEztjfU_S6X4DbDA_iVKkjAWtYfPHDzz_sPCT1Axz6isZdf3lHpq_gYX4Sz-cbe4rjmigxUxr-FgKHQy3HeCdK6hNq9ASQvMK9LBOpXDNn7mei6RZWom4wo3CMvvsY1w8tjtfLb-yQwJPltHxShZq5-ihC9irpLI9xEBTgG12q5lGIFPhTl_7inA1PFK97LuSLnTJzW0bj096v_TMDg7pOWm_zHtF53qbVsI0e3v5nmdKXdFf9BjIARRfVrbxVxiZHjU6zL6jY5QJdh1QCmENoejj_ytspMmGW7yMRxzUqgxcAqOBpVm0b-_mW3HoBdjQ",
"e": "AQAB",
"kid": "32d2b213-d3fe-436c-9d4c-67a673890620"
},
{
"alg": "RS256",
"kty": "RSA",
"use": "sig",
"n": "jbglzlub94YgerT030codqEztjfU_S6X4DbDA_iVKkjAWtYfPHDzz_sPCT1Axz6isZdf3lHpq_gYX4Sz-cbe4rjmigxUxr-FgKHQy3HeCdK6hNq9ASQvMK9LBOpXDNn7mei6RZWom4wo3CMvvsY1w8tjtfLb-yQwJPltHxShZq5-ihC9irpLI9xEBTgG12q5lGIFPhTl_7inA1PFK97LuSLnTJzW0bj096v_TMDg7pOWm_zHtF53qbVsI0e3v5nmdKXdFf9BjIARRfVrbxVxiZHjU6zL6jY5QJdh1QCmENoejj_ytspMmGW7yMRxzUqgxcAqOBpVm0b-_mW3HoBdjQ",
"e": "FEGB",
"kid": "32d2b213-d3fe-436c-9d4c-67a673890621"
}
]
}
Expand All @@ -186,14 +201,16 @@ func (s *dbSuite) TestPutAndGetJWKS(c *qt.C) {
gotJwks, err := s.Database.GetJWKS(ctx)
c.Assert(err, qt.IsNil)
ki := gotJwks.Keys(ctx)
ki.Next(ctx)
key := ki.Pair().Value.(jwk.Key)

_, err = uuid.Parse(key.KeyID())
c.Assert(err, qt.IsNil)

c.Assert(key.KeyUsage(), qt.Equals, "sig")
c.Assert(key.Algorithm(), qt.Equals, jwa.RS256)
gotOneKey := false
for ki.Next(ctx) {
gotOneKey = true
key := ki.Pair().Value.(jwk.Key)
_, err = uuid.Parse(key.KeyID())
c.Assert(err, qt.IsNil)
c.Assert(key.KeyUsage(), qt.Equals, "sig")
c.Assert(key.Algorithm(), qt.Equals, jwa.RS256)
}
c.Assert(gotOneKey, qt.IsTrue)
}

func (s *dbSuite) TestPutAndGetJwksExpiry(c *qt.C) {
Expand Down Expand Up @@ -231,3 +248,23 @@ func (s *dbSuite) TestPutAndGetJwksPrivateKey(c *qt.C) {
c.Assert(err, qt.IsNil)
c.Assert(gotPem, qt.DeepEquals, pem)
}

func (s *dbSuite) TestCleanupJWKS(c *qt.C) {
err := s.Database.Migrate(context.Background(), true)
c.Assert(err, qt.Equals, nil)
ctx := context.Background()
pem := []byte("123")
c.Assert(s.Database.PutJWKSPrivateKey(ctx, pem), qt.IsNil)
expiryTime := time.Date(2023, 7, 26, 0, 0, 0, 0, time.UTC)
c.Assert(s.Database.PutJWKSExpiry(ctx, expiryTime), qt.IsNil)
jwks := getJWKS(c)
c.Assert(s.Database.PutJWKS(ctx, jwks), qt.IsNil)
// Verify 3 secrets exist
var count int64
c.Assert(s.Database.DB.Model(&dbmodel.Secret{}).Count(&count).Error, qt.IsNil)
c.Assert(count, qt.Equals, int64(3))
// Verify all JWKS secrets are removed
c.Assert(s.Database.CleanupJWKS(ctx), qt.IsNil)
c.Assert(s.Database.DB.Model(&dbmodel.Secret{}).Count(&count).Error, qt.IsNil)
c.Assert(count, qt.Equals, int64(0))
}
14 changes: 5 additions & 9 deletions internal/dbmodel/version_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"testing"

qt "github.com/frankban/quicktest"
"gorm.io/gorm"

"github.com/canonical/jimm/internal/dbmodel"
)
Expand All @@ -17,21 +16,18 @@ func TestVersion(t *testing.T) {

var v0 dbmodel.Version
result := db.First(&v0, "component = ?", dbmodel.Component)
c.Check(result.Error, qt.Equals, gorm.ErrRecordNotFound)
c.Check(result.Error, qt.IsNil)
c.Check(v0.Major, qt.DeepEquals, dbmodel.Major)
c.Check(v0.Minor, qt.Equals, dbmodel.Minor)

v1 := dbmodel.Version{
Component: dbmodel.Component,
Major: dbmodel.Major,
Minor: dbmodel.Minor,
}
result = db.Create(&v1)
result = db.First(&v1, "component = ?", dbmodel.Component)
c.Assert(result.Error, qt.IsNil)
c.Check(result.RowsAffected, qt.Equals, int64(1))

var v2 dbmodel.Version
result = db.First(&v2, "component = ?", dbmodel.Component)
c.Assert(result.Error, qt.IsNil)
c.Check(v2, qt.DeepEquals, v1)
c.Check(v1, qt.DeepEquals, v1)

v3 := dbmodel.Version{
Component: dbmodel.Component,
Expand Down
1 change: 1 addition & 0 deletions service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ func NewService(ctx context.Context, p Params) (*Service, error) {
} else {
// Only enable Postgres storage for secrets if explictly enabled.
if _, ok := os.LookupEnv("INSECURE_SECRET_STORAGE"); ok {
zapctx.Warn(ctx, "using plaintext postgres for secret storage")
s.jimm.CredentialStore = &s.jimm.Database
}
}
Expand Down

0 comments on commit 8844dff

Please sign in to comment.