From eaadd82a839b89ddcc9e0455132fc86ff6f827d6 Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Tue, 4 May 2021 18:59:03 -0400 Subject: [PATCH] support for optional mixed-mode encryption --- CHANGELOG.md | 6 ++ cmd/drone-server/config/config.go | 4 ++ cmd/drone-server/inject_store.go | 54 +++++++++++--- cmd/drone-server/wire_gen.go | 10 +-- store/batch/batch_test.go | 4 +- store/batch2/batch_test.go | 4 +- store/perm/perm_test.go | 8 ++- store/secret/secret_test.go | 54 ++++++++++++++ store/shared/encrypt/aesgcm.go | 29 ++++++-- store/shared/encrypt/aesgcm_test.go | 32 +++++++++ store/shared/encrypt/encrypt.go | 2 +- store/shared/migrate/mysql/ddl_gen.go | 4 +- .../mysql/files/001_create_table_user.sql | 4 +- store/shared/migrate/postgres/ddl_gen.go | 4 +- .../postgres/files/001_create_table_user.sql | 4 +- store/user/scan.go | 42 ++++++++--- store/user/user.go | 41 +++++++---- store/user/user_test.go | 72 +++++++++++++++++-- 18 files changed, 315 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 383629be..ed5170af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +### Added +- feature flags for mixed-mode database encryption. + +### Changed +- user-interface re-design + ### Breaking - removed deprecated kubernetes integration in favor of official kubernetes runner. - removed deprecated nomad integration in favor of official nomad runner. diff --git a/cmd/drone-server/config/config.go b/cmd/drone-server/config/config.go index 1d77b0e3..9fd870b9 100644 --- a/cmd/drone-server/config/config.go +++ b/cmd/drone-server/config/config.go @@ -117,6 +117,10 @@ type ( // Feature flag LegacyBatch bool `envconfig:"DRONE_DATABASE_LEGACY_BATCH"` + + // Feature flag + EncryptUserTable bool `envconfig:"DRONE_DATABASE_ENCRYPT_USER_TABLE"` + EncryptMixedContent bool `envconfig:"DRONE_DATABASE_ENCRYPT_MIXED_MODE"` } // Docker provides docker configuration diff --git a/cmd/drone-server/inject_store.go b/cmd/drone-server/inject_store.go index 40af303d..161e213a 100644 --- a/cmd/drone-server/inject_store.go +++ b/cmd/drone-server/inject_store.go @@ -34,6 +34,7 @@ import ( "github.com/drone/drone/store/user" "github.com/google/wire" + "github.com/sirupsen/logrus" ) // wire set for loading the stores. @@ -66,7 +67,20 @@ func provideDatabase(config config.Config) (*db.DB, error) { // provideEncrypter is a Wire provider function that provides a // database encrypter, configured from the environment. func provideEncrypter(config config.Config) (encrypt.Encrypter, error) { - return encrypt.New(config.Database.Secret) + enc, err := encrypt.New(config.Database.Secret) + // mixed-content mode should be set to true if the database + // originally had encryption disabled and therefore has + // plaintext entries. This prevents Drone from returning an + // error if decryption fails; on failure, the ciphertext is + // returned as-is and the error is ignored. + if aesgcm, ok := enc.(*encrypt.Aesgcm); ok { + logrus.Debugln("main: database encryption enabled") + if config.Database.EncryptMixedContent { + logrus.Debugln("main: database encryption mixed-mode enabled") + aesgcm.Compat = true + } + } + return enc, err } // provideBuildStore is a Wire provider function that provides a @@ -123,15 +137,6 @@ func provideRepoStore(db *db.DB) core.RepositoryStore { return repos } -// provideUserStore is a Wire provider function that provides a -// user datastore, configured from the environment, with metrics -// enabled. -func provideUserStore(db *db.DB) core.UserStore { - users := user.New(db) - metric.UserCount(users) - return users -} - // provideBatchStore is a Wire provider function that provides a // batcher. If the experimental batcher is enabled it is returned. func provideBatchStore(db *db.DB, config config.Config) core.Batcher { @@ -140,3 +145,32 @@ func provideBatchStore(db *db.DB, config config.Config) core.Batcher { } return batch2.New(db) } + +// provideUserStore is a Wire provider function that provides a +// user datastore, configured from the environment, with metrics +// enabled. +func provideUserStore(db *db.DB, enc encrypt.Encrypter, config config.Config) core.UserStore { + // create the user store with encryption iff the user + // encryption feature flag is enabled. + // + // why not enable by default? because the user table is + // accessed on every http request and we are unsure what, + // if any performance implications user table encryption + // may have on the system. + // + // it is very possible there are zero material performance + // implications, however, if there is a performance regression + // we could look at implementing in-memory lru caching, which + // we already employ in other areas of the software. + if config.Database.EncryptUserTable { + logrus.Debugln("main: database encryption enabled for user table") + users := user.New(db, enc) + metric.UserCount(users) + return users + } + + noenc, _ := encrypt.New("") + users := user.New(db, noenc) + metric.UserCount(users) + return users +} diff --git a/cmd/drone-server/wire_gen.go b/cmd/drone-server/wire_gen.go index 353c4248..34f63f6b 100644 --- a/cmd/drone-server/wire_gen.go +++ b/cmd/drone-server/wire_gen.go @@ -44,7 +44,11 @@ func InitializeApplication(config2 config.Config) (application, error) { if err != nil { return application{}, err } - userStore := provideUserStore(db) + encrypter, err := provideEncrypter(config2) + if err != nil { + return application{}, err + } + userStore := provideUserStore(db, encrypter, config2) renewer := token.Renewer(refresher, userStore) commitService := commit.New(client, renewer) cronStore := cron.New(db) @@ -70,10 +74,6 @@ func InitializeApplication(config2 config.Config) (application, error) { logStore := provideLogStore(db, config2) logStream := livelog.New() netrcService := provideNetrcService(client, renewer, config2) - encrypter, err := provideEncrypter(config2) - if err != nil { - return application{}, err - } secretStore := secret.New(db, encrypter) globalSecretStore := global.New(db, encrypter) buildManager := manager.New(buildStore, configService, convertService, corePubsub, logStore, logStream, netrcService, repositoryStore, scheduler, secretStore, globalSecretStore, statusService, stageStore, stepStore, system, userStore, webhookSender) diff --git a/store/batch/batch_test.go b/store/batch/batch_test.go index 026ae6db..45d731f1 100644 --- a/store/batch/batch_test.go +++ b/store/batch/batch_test.go @@ -14,6 +14,7 @@ import ( "github.com/drone/drone/store/repos" "github.com/drone/drone/store/shared/db" "github.com/drone/drone/store/shared/db/dbtest" + "github.com/drone/drone/store/shared/encrypt" "github.com/drone/drone/store/user" ) @@ -330,7 +331,8 @@ func testBatchDuplicateRename( } func seedUser(db *db.DB) (*core.User, error) { + enc, _ := encrypt.New("") out := &core.User{Login: "octocat"} - err := user.New(db).Create(noContext, out) + err := user.New(db, enc).Create(noContext, out) return out, err } diff --git a/store/batch2/batch_test.go b/store/batch2/batch_test.go index 666c1747..72a8bc3b 100644 --- a/store/batch2/batch_test.go +++ b/store/batch2/batch_test.go @@ -14,6 +14,7 @@ import ( "github.com/drone/drone/store/repos" "github.com/drone/drone/store/shared/db" "github.com/drone/drone/store/shared/db/dbtest" + "github.com/drone/drone/store/shared/encrypt" "github.com/drone/drone/store/user" ) @@ -389,7 +390,8 @@ func testBatchDuplicateRename( } func seedUser(db *db.DB) (*core.User, error) { + enc, _ := encrypt.New("") out := &core.User{Login: "octocat"} - err := user.New(db).Create(noContext, out) + err := user.New(db, enc).Create(noContext, out) return out, err } diff --git a/store/perm/perm_test.go b/store/perm/perm_test.go index 5b785d77..b8f2d5c4 100644 --- a/store/perm/perm_test.go +++ b/store/perm/perm_test.go @@ -9,9 +9,10 @@ import ( "database/sql" "testing" - "github.com/drone/drone/store/shared/db/dbtest" "github.com/drone/drone/core" "github.com/drone/drone/store/repos" + "github.com/drone/drone/store/shared/db/dbtest" + "github.com/drone/drone/store/shared/encrypt" "github.com/drone/drone/store/user" ) @@ -28,9 +29,12 @@ func TestPerms(t *testing.T) { dbtest.Disconnect(conn) }() + // no-op encrypter + enc, _ := encrypt.New("") + // seeds the database with a dummy user account. auser := &core.User{Login: "spaceghost"} - users := user.New(conn) + users := user.New(conn, enc) err = users.Create(noContext, auser) if err != nil { t.Error(err) diff --git a/store/secret/secret_test.go b/store/secret/secret_test.go index 9a983f95..08fcaaf2 100644 --- a/store/secret/secret_test.go +++ b/store/secret/secret_test.go @@ -182,3 +182,57 @@ func testSecret(item *core.Secret) func(t *testing.T) { } } } + +// The purpose of this unit test is to ensure that plaintext +// data can still be read from the database if encryption is +// added at a later time. +func TestSecretCryptoChange(t *testing.T) { + conn, err := dbtest.Connect() + if err != nil { + t.Error(err) + return + } + defer func() { + dbtest.Reset(conn) + dbtest.Disconnect(conn) + }() + + // seeds the database with a dummy repository. + repo := &core.Repository{UID: "1", Slug: "octocat/hello-world"} + repos := repos.New(conn) + if err := repos.Create(noContext, repo); err != nil { + t.Error(err) + } + + store := New(conn, nil).(*secretStore) + store.enc, _ = encrypt.New("") + + item := &core.Secret{ + RepoID: repo.ID, + Name: "password", + Data: "correct-horse-battery-staple", + } + + // create the secret with the secret value stored as plaintext + err = store.Create(noContext, item) + if err != nil { + t.Error(err) + return + } + if item.ID == 0 { + t.Errorf("Want secret ID assigned, got %d", item.ID) + return + } + + // update the store to use encryption + store.enc, _ = encrypt.New("fb4b4d6267c8a5ce8231f8b186dbca92") + store.enc.(*encrypt.Aesgcm).Compat = true + + // fetch the secret from the database + got, err := store.Find(noContext, item.ID) + if err != nil { + t.Error(err) + } else { + t.Run("Fields", testSecret(got)) + } +} diff --git a/store/shared/encrypt/aesgcm.go b/store/shared/encrypt/aesgcm.go index a038b333..a7098e85 100644 --- a/store/shared/encrypt/aesgcm.go +++ b/store/shared/encrypt/aesgcm.go @@ -21,11 +21,15 @@ import ( "io" ) -type aesgcm struct { - block cipher.Block +// Aesgcm provides an encryper that uses the aesgcm encryption +// alogirthm. +type Aesgcm struct { + block cipher.Block + Compat bool } -func (e *aesgcm) Encrypt(plaintext string) ([]byte, error) { +// Encrypt encrypts the plaintext using aesgcm. +func (e *Aesgcm) Encrypt(plaintext string) ([]byte, error) { gcm, err := cipher.NewGCM(e.block) if err != nil { return nil, err @@ -40,13 +44,22 @@ func (e *aesgcm) Encrypt(plaintext string) ([]byte, error) { return gcm.Seal(nonce, nonce, []byte(plaintext), nil), nil } -func (e *aesgcm) Decrypt(ciphertext []byte) (string, error) { +// Decrypt decrypts the ciphertext using aesgcm. +func (e *Aesgcm) Decrypt(ciphertext []byte) (string, error) { gcm, err := cipher.NewGCM(e.block) if err != nil { return "", err } if len(ciphertext) < gcm.NonceSize() { + // if the decryption utility is running in compatibility + // mode, it will return the ciphertext as plain text if + // decryption fails. This should be used when running the + // database in mixed-mode, where there is a mix of encrypted + // and unecrypted content. + if e.Compat { + return string(ciphertext), nil + } return "", errors.New("malformed ciphertext") } @@ -55,5 +68,13 @@ func (e *aesgcm) Decrypt(ciphertext []byte) (string, error) { ciphertext[gcm.NonceSize():], nil, ) + // if the decryption utility is running in compatibility + // mode, it will return the ciphertext as plain text if + // decryption fails. This should be used when running the + // database in mixed-mode, where there is a mix of encrypted + // and unecrypted content. + if err != nil && e.Compat { + return string(ciphertext), nil + } return string(plaintext), err } diff --git a/store/shared/encrypt/aesgcm_test.go b/store/shared/encrypt/aesgcm_test.go index 055d8c5e..7e917c64 100644 --- a/store/shared/encrypt/aesgcm_test.go +++ b/store/shared/encrypt/aesgcm_test.go @@ -21,3 +21,35 @@ func TestAesgcm(t *testing.T) { t.Errorf("Want plaintext %q, got %q", want, got) } } + +func TestAesgcmFail(t *testing.T) { + s := "correct-horse-batter-staple" + n, _ := New("ea1c5a9145c8a5ce8231f8b186dbcabc") + ciphertext, err := n.Encrypt(s) + if err != nil { + t.Error(err) + } + n, _ = New("fb4b4d6267c8a5ce8231f8b186dbca92") + _, err = n.Decrypt(ciphertext) + if err == nil { + t.Error("Expect error when encryption and decryption keys mismatch") + } +} + +func TestAesgcmCompat(t *testing.T) { + s := "correct-horse-batter-staple" + n, _ := New("") + ciphertext, err := n.Encrypt(s) + if err != nil { + t.Error(err) + } + n, _ = New("ea1c5a9145c8a5ce8231f8b186dbcabc") + n.(*Aesgcm).Compat = true + plaintext, err := n.Decrypt(ciphertext) + if err != nil { + t.Error(err) + } + if want, got := plaintext, s; got != want { + t.Errorf("Want plaintext %q, got %q", want, got) + } +} diff --git a/store/shared/encrypt/encrypt.go b/store/shared/encrypt/encrypt.go index 0a4f36ed..c552bc69 100644 --- a/store/shared/encrypt/encrypt.go +++ b/store/shared/encrypt/encrypt.go @@ -43,5 +43,5 @@ func New(key string) (Encrypter, error) { if err != nil { return nil, err } - return &aesgcm{block: block}, nil + return &Aesgcm{block: block}, nil } diff --git a/store/shared/migrate/mysql/ddl_gen.go b/store/shared/migrate/mysql/ddl_gen.go index 4adf8fa7..5e872fb5 100644 --- a/store/shared/migrate/mysql/ddl_gen.go +++ b/store/shared/migrate/mysql/ddl_gen.go @@ -249,8 +249,8 @@ CREATE TABLE IF NOT EXISTS users ( ,user_created INTEGER ,user_updated INTEGER ,user_last_login INTEGER -,user_oauth_token VARCHAR(500) -,user_oauth_refresh VARCHAR(500) +,user_oauth_token BLOB +,user_oauth_refresh BLOB ,user_oauth_expiry INTEGER ,user_hash VARCHAR(500) ,UNIQUE(user_login) diff --git a/store/shared/migrate/mysql/files/001_create_table_user.sql b/store/shared/migrate/mysql/files/001_create_table_user.sql index 68c42641..b90f6695 100644 --- a/store/shared/migrate/mysql/files/001_create_table_user.sql +++ b/store/shared/migrate/mysql/files/001_create_table_user.sql @@ -13,8 +13,8 @@ CREATE TABLE IF NOT EXISTS users ( ,user_created INTEGER ,user_updated INTEGER ,user_last_login INTEGER -,user_oauth_token VARCHAR(500) -,user_oauth_refresh VARCHAR(500) +,user_oauth_token BLOB +,user_oauth_refresh BLOB ,user_oauth_expiry INTEGER ,user_hash VARCHAR(500) ,UNIQUE(user_login) diff --git a/store/shared/migrate/postgres/ddl_gen.go b/store/shared/migrate/postgres/ddl_gen.go index 7c1cbf56..ee5fe4e8 100644 --- a/store/shared/migrate/postgres/ddl_gen.go +++ b/store/shared/migrate/postgres/ddl_gen.go @@ -245,8 +245,8 @@ CREATE TABLE IF NOT EXISTS users ( ,user_created INTEGER ,user_updated INTEGER ,user_last_login INTEGER -,user_oauth_token VARCHAR(500) -,user_oauth_refresh VARCHAR(500) +,user_oauth_token BYTEA +,user_oauth_refresh BYTEA ,user_oauth_expiry INTEGER ,user_hash VARCHAR(500) ,UNIQUE(user_login) diff --git a/store/shared/migrate/postgres/files/001_create_table_user.sql b/store/shared/migrate/postgres/files/001_create_table_user.sql index f66a801d..47432c8d 100644 --- a/store/shared/migrate/postgres/files/001_create_table_user.sql +++ b/store/shared/migrate/postgres/files/001_create_table_user.sql @@ -13,8 +13,8 @@ CREATE TABLE IF NOT EXISTS users ( ,user_created INTEGER ,user_updated INTEGER ,user_last_login INTEGER -,user_oauth_token VARCHAR(500) -,user_oauth_refresh VARCHAR(500) +,user_oauth_token BYTEA +,user_oauth_refresh BYTEA ,user_oauth_expiry INTEGER ,user_hash VARCHAR(500) ,UNIQUE(user_login) diff --git a/store/user/scan.go b/store/user/scan.go index 54a1ce29..015b1e3b 100644 --- a/store/user/scan.go +++ b/store/user/scan.go @@ -19,11 +19,20 @@ import ( "github.com/drone/drone/core" "github.com/drone/drone/store/shared/db" + "github.com/drone/drone/store/shared/encrypt" ) // helper function converts the User structure to a set // of named query parameters. -func toParams(u *core.User) map[string]interface{} { +func toParams(encrypt encrypt.Encrypter, u *core.User) (map[string]interface{}, error) { + token, err := encrypt.Encrypt(u.Token) + if err != nil { + return nil, err + } + refresh, err := encrypt.Encrypt(u.Refresh) + if err != nil { + return nil, err + } return map[string]interface{}{ "user_id": u.ID, "user_login": u.Login, @@ -37,17 +46,18 @@ func toParams(u *core.User) map[string]interface{} { "user_created": u.Created, "user_updated": u.Updated, "user_last_login": u.LastLogin, - "user_oauth_token": u.Token, - "user_oauth_refresh": u.Refresh, + "user_oauth_token": token, + "user_oauth_refresh": refresh, "user_oauth_expiry": u.Expiry, "user_hash": u.Hash, - } + }, nil } // helper function scans the sql.Row and copies the column // values to the destination object. -func scanRow(scanner db.Scanner, dest *core.User) error { - return scanner.Scan( +func scanRow(encrypt encrypt.Encrypter, scanner db.Scanner, dest *core.User) error { + var token, refresh []byte + err := scanner.Scan( &dest.ID, &dest.Login, &dest.Email, @@ -60,22 +70,34 @@ func scanRow(scanner db.Scanner, dest *core.User) error { &dest.Created, &dest.Updated, &dest.LastLogin, - &dest.Token, - &dest.Refresh, + &token, + &refresh, &dest.Expiry, &dest.Hash, ) + if err != nil { + return err + } + dest.Token, err = encrypt.Decrypt(token) + if err != nil { + return err + } + dest.Refresh, err = encrypt.Decrypt(refresh) + if err != nil { + return err + } + return nil } // helper function scans the sql.Row and copies the column // values to the destination object. -func scanRows(rows *sql.Rows) ([]*core.User, error) { +func scanRows(encrypt encrypt.Encrypter, rows *sql.Rows) ([]*core.User, error) { defer rows.Close() users := []*core.User{} for rows.Next() { user := new(core.User) - err := scanRow(rows, user) + err := scanRow(encrypt, rows, user) if err != nil { return nil, err } diff --git a/store/user/user.go b/store/user/user.go index ebe4f311..741f5f5e 100644 --- a/store/user/user.go +++ b/store/user/user.go @@ -19,28 +19,30 @@ import ( "github.com/drone/drone/core" "github.com/drone/drone/store/shared/db" + "github.com/drone/drone/store/shared/encrypt" ) // New returns a new UserStore. -func New(db *db.DB) core.UserStore { - return &userStore{db} +func New(db *db.DB, enc encrypt.Encrypter) core.UserStore { + return &userStore{db, enc} } type userStore struct { - db *db.DB + db *db.DB + enc encrypt.Encrypter } // Find returns a user from the datastore. func (s *userStore) Find(ctx context.Context, id int64) (*core.User, error) { out := &core.User{ID: id} err := s.db.View(func(queryer db.Queryer, binder db.Binder) error { - params := toParams(out) + params := map[string]interface{}{"user_id": id} query, args, err := binder.BindNamed(queryKey, params) if err != nil { return err } row := queryer.QueryRow(query, args...) - return scanRow(row, out) + return scanRow(s.enc, row, out) }) return out, err } @@ -49,13 +51,13 @@ func (s *userStore) Find(ctx context.Context, id int64) (*core.User, error) { func (s *userStore) FindLogin(ctx context.Context, login string) (*core.User, error) { out := &core.User{Login: login} err := s.db.View(func(queryer db.Queryer, binder db.Binder) error { - params := toParams(out) + params := map[string]interface{}{"user_login": login} query, args, err := binder.BindNamed(queryLogin, params) if err != nil { return err } row := queryer.QueryRow(query, args...) - return scanRow(row, out) + return scanRow(s.enc, row, out) }) return out, err } @@ -64,13 +66,13 @@ func (s *userStore) FindLogin(ctx context.Context, login string) (*core.User, er func (s *userStore) FindToken(ctx context.Context, token string) (*core.User, error) { out := &core.User{Hash: token} err := s.db.View(func(queryer db.Queryer, binder db.Binder) error { - params := toParams(out) + params := map[string]interface{}{"user_hash": token} query, args, err := binder.BindNamed(queryToken, params) if err != nil { return err } row := queryer.QueryRow(query, args...) - return scanRow(row, out) + return scanRow(s.enc, row, out) }) return out, err } @@ -83,7 +85,7 @@ func (s *userStore) List(ctx context.Context) ([]*core.User, error) { if err != nil { return err } - out, err = scanRows(rows) + out, err = scanRows(s.enc, rows) return err }) return out, err @@ -99,7 +101,10 @@ func (s *userStore) Create(ctx context.Context, user *core.User) error { func (s *userStore) create(ctx context.Context, user *core.User) error { return s.db.Lock(func(execer db.Execer, binder db.Binder) error { - params := toParams(user) + params, err := toParams(s.enc, user) + if err != nil { + return err + } stmt, args, err := binder.BindNamed(stmtInsert, params) if err != nil { return err @@ -115,7 +120,10 @@ func (s *userStore) create(ctx context.Context, user *core.User) error { func (s *userStore) createPostgres(ctx context.Context, user *core.User) error { return s.db.Lock(func(execer db.Execer, binder db.Binder) error { - params := toParams(user) + params, err := toParams(s.enc, user) + if err != nil { + return err + } stmt, args, err := binder.BindNamed(stmtInsertPg, params) if err != nil { return err @@ -127,7 +135,10 @@ func (s *userStore) createPostgres(ctx context.Context, user *core.User) error { // Update persists an updated user to the datastore. func (s *userStore) Update(ctx context.Context, user *core.User) error { return s.db.Lock(func(execer db.Execer, binder db.Binder) error { - params := toParams(user) + params, err := toParams(s.enc, user) + if err != nil { + return err + } stmt, args, err := binder.BindNamed(stmtUpdate, params) if err != nil { return err @@ -140,7 +151,7 @@ func (s *userStore) Update(ctx context.Context, user *core.User) error { // Delete deletes a user from the datastore. func (s *userStore) Delete(ctx context.Context, user *core.User) error { return s.db.Lock(func(execer db.Execer, binder db.Binder) error { - params := toParams(user) + params := map[string]interface{}{"user_id": user.ID} stmt, args, err := binder.BindNamed(stmtDelete, params) if err != nil { return err @@ -163,7 +174,7 @@ func (s *userStore) Count(ctx context.Context) (int64, error) { func (s *userStore) CountHuman(ctx context.Context) (int64, error) { var out int64 err := s.db.View(func(queryer db.Queryer, binder db.Binder) error { - params := toParams(&core.User{Machine: false}) + params := map[string]interface{}{"user_machine": false} stmt, args, err := binder.BindNamed(queryCountHuman, params) if err != nil { return err diff --git a/store/user/user_test.go b/store/user/user_test.go index 898720a8..8c6b0c89 100644 --- a/store/user/user_test.go +++ b/store/user/user_test.go @@ -12,6 +12,7 @@ import ( "github.com/drone/drone/core" "github.com/drone/drone/store/shared/db/dbtest" + "github.com/drone/drone/store/shared/encrypt" ) var noContext = context.TODO() @@ -27,17 +28,20 @@ func TestUser(t *testing.T) { dbtest.Disconnect(conn) }() - store := New(conn).(*userStore) + store := New(conn, nil).(*userStore) + store.enc, _ = encrypt.New("fb4b4d6267c8a5ce8231f8b186dbca92") t.Run("Create", testUserCreate(store)) } func testUserCreate(store *userStore) func(t *testing.T) { return func(t *testing.T) { user := &core.User{ - Login: "octocat", - Email: "octocat@github.com", - Avatar: "https://avatars3.githubusercontent.com/u/583231?v=4", - Hash: "MjAxOC0wOC0xMVQxNTo1ODowN1o", + Login: "octocat", + Email: "octocat@github.com", + Avatar: "https://avatars3.githubusercontent.com/u/583231?v=4", + Hash: "MjAxOC0wOC0xMVQxNTo1ODowN1o", + Token: "9595fe015ca9b98c41ebf4e7d4e004ee", + Refresh: "268ef49df64ea8ff79ef11e995d41aed", } err := store.Create(noContext, user) if err != nil { @@ -72,7 +76,7 @@ func testUserCount(users *userStore) func(t *testing.T) { t.Error(err) } if got, want := count, int64(1); got != want { - t.Errorf("Want user table count %d, got %d", want, got) + t.Errorf("Want user table count %d for humans, got %d", want, got) } } } @@ -181,5 +185,61 @@ func testUser(user *core.User) func(t *testing.T) { if got, want := user.Avatar, "https://avatars3.githubusercontent.com/u/583231?v=4"; got != want { t.Errorf("Want user Avatar %q, got %q", want, got) } + if got, want := user.Token, "9595fe015ca9b98c41ebf4e7d4e004ee"; got != want { + t.Errorf("Want user Access Token %q, got %q", want, got) + } + if got, want := user.Refresh, "268ef49df64ea8ff79ef11e995d41aed"; got != want { + t.Errorf("Want user Refresh Token %q, got %q", want, got) + } + } +} + +// The purpose of this unit test is to ensure that plaintext +// data can still be read from the database if encryption is +// added at a later time. +func TestUserCryptoCompat(t *testing.T) { + conn, err := dbtest.Connect() + if err != nil { + t.Error(err) + return + } + defer func() { + dbtest.Reset(conn) + dbtest.Disconnect(conn) + }() + + store := New(conn, nil).(*userStore) + store.enc, _ = encrypt.New("") + + item := &core.User{ + Login: "octocat", + Email: "octocat@github.com", + Avatar: "https://avatars3.githubusercontent.com/u/583231?v=4", + Hash: "MjAxOC0wOC0xMVQxNTo1ODowN1o", + Token: "9595fe015ca9b98c41ebf4e7d4e004ee", + Refresh: "268ef49df64ea8ff79ef11e995d41aed", + } + + // create the secret with the secret value stored as plaintext + err = store.Create(noContext, item) + if err != nil { + t.Error(err) + return + } + if item.ID == 0 { + t.Errorf("Want secret ID assigned, got %d", item.ID) + return + } + + // update the store to use encryption + store.enc, _ = encrypt.New("fb4b4d6267c8a5ce8231f8b186dbca92") + store.enc.(*encrypt.Aesgcm).Compat = true + + // fetch the secret from the database + got, err := store.Find(noContext, item.ID) + if err != nil { + t.Errorf("cannot retrieve user from database: %s", err) + } else { + t.Run("Fields", testUser(got)) } }