From 0c95590ae67a59ea452290d8721c5eafd042fce6 Mon Sep 17 00:00:00 2001 From: MacRae Linton <55759+macrael@users.noreply.github.com> Date: Thu, 5 Sep 2019 20:27:10 -0700 Subject: [PATCH 1/8] add account support to simplestore --- api/account.go | 11 - ...0905111111_remove_dead_account_columns.sql | 22 ++ api/simplestore/accounts.go | 87 +++++++ api/simplestore/accounts_test.go | 243 ++++++++++++++++++ api/simplestore/attachments.go | 2 +- api/simplestore/error_db.go | 36 +++ api/simplestore/helpers.go | 38 ++- api/simplestore/simple_store.go | 13 +- api/store.go | 14 + 9 files changed, 446 insertions(+), 20 deletions(-) create mode 100644 api/migrations/20190905111111_remove_dead_account_columns.sql create mode 100644 api/simplestore/accounts.go create mode 100644 api/simplestore/accounts_test.go create mode 100644 api/simplestore/error_db.go diff --git a/api/account.go b/api/account.go index 37d14a011..28346a1d5 100644 --- a/api/account.go +++ b/api/account.go @@ -11,9 +11,6 @@ var ( // ErrPasswordDoesNotMatch is an error when a user inputs an invalid password ErrPasswordDoesNotMatch = errors.New("Password does not match") - // ErrAccountDoesNotExist is an error when a users account does not exist - ErrAccountDoesNotExist = errors.New("Account does not exist") - // ErrDatastoreConnection is an error when a database connection cannot be made ErrDatastoreConnection = errors.New("Unable to connect to datastore") ) @@ -47,10 +44,6 @@ const ( type Account struct { ID int Username string - Firstname string - Lastname string - Token string - TokenUsed bool Email sql.NullString Status string FormType string `db:"form_type"` @@ -233,10 +226,6 @@ func (entity *Account) BasicAuthentication(context DatabaseService, password str if basicMembership.Account != nil { entity.ID = basicMembership.Account.ID entity.Username = basicMembership.Account.Username - entity.Firstname = basicMembership.Account.Firstname - entity.Lastname = basicMembership.Account.Lastname - entity.Token = basicMembership.Account.Token - entity.TokenUsed = basicMembership.Account.TokenUsed entity.Email = basicMembership.Account.Email entity.Status = basicMembership.Account.Status entity.FormType = basicMembership.Account.FormType diff --git a/api/migrations/20190905111111_remove_dead_account_columns.sql b/api/migrations/20190905111111_remove_dead_account_columns.sql new file mode 100644 index 000000000..6dd151856 --- /dev/null +++ b/api/migrations/20190905111111_remove_dead_account_columns.sql @@ -0,0 +1,22 @@ + +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied +-- +goose StatementBegin +ALTER TABLE accounts DROP COLUMN firstname; +ALTER TABLE accounts DROP COLUMN lastname; +ALTER TABLE accounts DROP COLUMN token; +ALTER TABLE accounts DROP COLUMN token_used; + +-- +goose StatementEnd + + +-- +goose Down +-- SQL section 'Down' is executed when this migration is rolled back +-- +goose StatementBegin +ALTER TABLE accounts ADD COLUMN ( + firstname character varying(200) + lastname character varying(200) + token character varying(16) + token_used boolean +); +-- +goose StatementEnd diff --git a/api/simplestore/accounts.go b/api/simplestore/accounts.go new file mode 100644 index 000000000..5bcd72ecf --- /dev/null +++ b/api/simplestore/accounts.go @@ -0,0 +1,87 @@ +package simplestore + +import ( + "database/sql" + + "github.com/18F/e-QIP-prototype/api" + "github.com/pkg/errors" +) + +// CreateAccount creates a new account +func (s SimpleStore) CreateAccount(account *api.Account) error { + + createQuery := `INSERT INTO accounts (username, email, status, form_type, form_version, external_id) + VALUES ($1, $2, $3, $4, $5, $6) + RETURNING id` + + var newID int + + createErr := s.db.Get(&newID, createQuery, account.Username, account.Email, account.Status, account.FormType, account.FormVersion, account.ExternalID) + if createErr != nil { + return errors.Wrap(createErr, "Failed to create Account") + } + + account.ID = newID + + return nil +} + +var selectProlouge = `SELECT id, username, email, status, form_type, form_version, external_id FROM accounts ` + +// FetchAccountByUsername returns an account with the given username +func (s SimpleStore) FetchAccountByUsername(username string) (api.Account, error) { + + selectQuery := selectProlouge + `WHERE username = $1` + + var account api.Account + + selectErr := s.db.Get(&account, selectQuery, username) + if selectErr != nil { + if selectErr == sql.ErrNoRows { + return api.Account{}, api.ErrAccountDoesNotExist + } + return api.Account{}, errors.Wrap(selectErr, "Couldn't find Account") + } + + return account, nil +} + +//FetchAccountByExternalID fetches an account with the given external id +func (s SimpleStore) FetchAccountByExternalID(externalID string) (api.Account, error) { + + selectQuery := selectProlouge + `WHERE external_id = $1` + + var account api.Account + + selectErr := s.db.Get(&account, selectQuery, externalID) + if selectErr != nil { + if selectErr == sql.ErrNoRows { + return api.Account{}, api.ErrAccountDoesNotExist + } + return api.Account{}, errors.Wrap(selectErr, "Couldn't find Account") + } + + return account, nil +} + +// UpdateAccountStatus updates the status of an account. That should be the only thing we update post creation. +func (s SimpleStore) UpdateAccountStatus(account *api.Account) error { + + updateQuery := `UPDATE accounts SET status = $1 WHERE id = $2` + + result, updateErr := s.db.Exec(updateQuery, account.Status, account.ID) + if updateErr != nil { + return errors.Wrap(updateErr, "Couldn't update Account") + } + + rows, affectedErr := result.RowsAffected() + if affectedErr != nil { + return errors.Wrap(affectedErr, "Bizzarely unable to read affected rows") + } + + if rows != 1 { + return api.ErrAccountDoesNotExist + } + + return nil +} diff --git a/api/simplestore/accounts_test.go b/api/simplestore/accounts_test.go new file mode 100644 index 000000000..278249c02 --- /dev/null +++ b/api/simplestore/accounts_test.go @@ -0,0 +1,243 @@ +package simplestore + +import ( + "database/sql" + "testing" + + "github.com/18F/e-QIP-prototype/api" + "github.com/pkg/errors" +) + +func TestFetchAccountEmail(t *testing.T) { + + store := getSimpleStore() + defer store.Close() + + newAccount := CreateTestAccount(t, store) + + if newAccount.ID <= 0 { + t.Fatal("didn't get a valid ID") + } + + // Can we fetch the same newAccount? + fetchedAccount, fetchErr := store.FetchAccountByUsername(newAccount.Username) + if fetchErr != nil { + t.Fatal(fetchErr) + } + + if newAccount.Username != fetchedAccount.Username { + t.Log("Should have got back the same Username") + t.Fail() + } + + if newAccount.ID != fetchedAccount.ID { + t.Log("Should have got back the same ID") + t.Fail() + } + + if newAccount.Email != fetchedAccount.Email { + t.Log("Should have got back the same Email") + t.Fail() + } + + if newAccount.ExternalID != fetchedAccount.ExternalID { + t.Log("Should have got back the same ExternalID") + t.Fail() + } + + if newAccount.FormVersion != fetchedAccount.FormVersion { + t.Log("Should have got back the same FormVersion") + t.Fail() + } + + if newAccount.FormType != fetchedAccount.FormType { + t.Log("Should have got back the same FormType") + t.Fail() + } + + if newAccount.Status != fetchedAccount.Status { + t.Log("Should have got back the same Status") + t.Fail() + } + +} + +func TestFetchAccountExternalID(t *testing.T) { + + store := getSimpleStore() + defer store.Close() + + newAccount := CreateTestAccount(t, store) + + if newAccount.ID <= 0 { + t.Fatal("didn't get a valid ID") + } + + // Can we fetch the same newAccount? + fetchedAccount, fetchErr := store.FetchAccountByExternalID(newAccount.ExternalID) + if fetchErr != nil { + t.Fatal(fetchErr) + } + + if newAccount.Username != fetchedAccount.Username { + t.Log("Should have got back the same Username") + t.Fail() + } + + if newAccount.ID != fetchedAccount.ID { + t.Log("Should have got back the same ID") + t.Fail() + } + + if newAccount.Email != fetchedAccount.Email { + t.Log("Should have got back the same Email") + t.Fail() + } + + if newAccount.ExternalID != fetchedAccount.ExternalID { + t.Log("Should have got back the same ExternalID") + t.Fail() + } + + if newAccount.FormVersion != fetchedAccount.FormVersion { + t.Log("Should have got back the same FormVersion") + t.Fail() + } + + if newAccount.FormType != fetchedAccount.FormType { + t.Log("Should have got back the same FormType") + t.Fail() + } + + if newAccount.Status != fetchedAccount.Status { + t.Log("Should have got back the same Status") + t.Fail() + } + +} + +func TestUpdateAccountStatus(t *testing.T) { + + store := getSimpleStore() + defer store.Close() + + newAccount := CreateTestAccount(t, store) + + success := newAccount.Submit() + if !success { + t.Fatal("Should have been able to submit from starting.") + } + + updateErr := store.UpdateAccountStatus(&newAccount) + if updateErr != nil { + t.Fatal(updateErr) + } + + // Can we fetch the same newAccount? + fetchedAccount, fetchErr := store.FetchAccountByUsername(newAccount.Username) + if fetchErr != nil { + t.Fatal(fetchErr) + } + + if fetchedAccount.Status != api.StatusSubmitted { + t.Log("Should have got back the new Status") + t.Fail() + } + +} + +func TestCreateErr(t *testing.T) { + store := getSimpleStore() + defer store.Close() + + newAccount := api.Account{ + Username: "joe", + } + + // There is a unique constraint on username, so we can trigger an error there. + createErr := store.CreateAccount(&newAccount) + if createErr == nil { + // try again in case this is the first time this has been run on this db + createErr = store.CreateAccount(&newAccount) + if createErr == nil { + t.Fatal("Should have errored creating an incomplete account.") + } + } + +} + +func TestFetchUnknownError(t *testing.T) { + store := getErrorStore() + + _, fetchErr := store.FetchAccountByUsername("foo") + if fetchErr == nil && fetchErr != api.ErrAccountDoesNotExist { + t.Fatal("Should have gotten an unknown error") + } + + _, fetchErr = store.FetchAccountByExternalID("foo") + if fetchErr == nil && fetchErr != api.ErrAccountDoesNotExist { + t.Fatal("Should have gotten an unknown error") + } +} + +func TestDoesntExist(t *testing.T) { + store := getSimpleStore() + defer store.Close() + + // Can we fetch the same newAccount? + _, fetchErr := store.FetchAccountByUsername("NONSENSE") + if fetchErr != api.ErrAccountDoesNotExist { + t.Fatal(fetchErr) + } + + _, fetchErr = store.FetchAccountByExternalID("NONSENSE") + if fetchErr != api.ErrAccountDoesNotExist { + t.Fatal(fetchErr) + } + + account := api.Account{ + ID: -9000, + Username: "NONSENSE", + } + + updateErr := store.UpdateAccountStatus(&account) + if updateErr != api.ErrAccountDoesNotExist { + t.Fatal(updateErr) + } +} + +type badResult struct { +} + +func (r badResult) LastInsertId() (int64, error) { + return 0, errors.New("BAD RESULT") +} + +func (r badResult) RowsAffected() (int64, error) { + return 0, errors.New("BAD RESULT") +} + +type badResultDB struct { + errorDB +} + +func (db *badResultDB) Exec(query string, args ...interface{}) (sql.Result, error) { + return badResult{}, nil +} + +func TestUpdateErrors(t *testing.T) { + store := getErrorStore() + + updateErr := store.UpdateAccountStatus(&api.Account{}) + if updateErr == nil { + t.Fatal("Should have gotten an error") + } + + store.db = &badResultDB{} + + badResultErr := store.UpdateAccountStatus(&api.Account{}) + if badResultErr == nil || badResultErr == updateErr { + t.Fatal("Should have gotten a different error. ") + } + +} diff --git a/api/simplestore/attachments.go b/api/simplestore/attachments.go index 0990fe673..e68501054 100644 --- a/api/simplestore/attachments.go +++ b/api/simplestore/attachments.go @@ -21,7 +21,7 @@ func (s SimpleStore) CreateAttachment(attachment *api.Attachment) error { createErr := s.db.Get(&newID, createQuery, attachment.AccountID, metadata, body) if createErr != nil { - return errors.Wrap(createErr, "Failed to create Application") + return errors.Wrap(createErr, "Failed to create Attachment") } attachment.ID = newID diff --git a/api/simplestore/error_db.go b/api/simplestore/error_db.go new file mode 100644 index 000000000..e2c81fcde --- /dev/null +++ b/api/simplestore/error_db.go @@ -0,0 +1,36 @@ +package simplestore + +import ( + "database/sql" + + "github.com/jmoiron/sqlx" + "github.com/pkg/errors" +) + +// errrorDB throws errors on every call to a +type errorDB struct { +} + +func (db *errorDB) Exec(query string, args ...interface{}) (sql.Result, error) { + return nil, errors.New("MOCK ERR") +} + +func (db *errorDB) Get(dest interface{}, query string, args ...interface{}) error { + return errors.New("MOCK ERR") +} + +func (db *errorDB) MustExec(query string, args ...interface{}) sql.Result { + return nil +} + +func (db *errorDB) Beginx() (*sqlx.Tx, error) { + return nil, errors.New("MOCK ERR") +} + +func (db *errorDB) Queryx(query string, args ...interface{}) (*sqlx.Rows, error) { + return nil, errors.New("MOCK ERR") +} + +func (db *errorDB) Close() error { + return errors.New("MOCK ERR") +} diff --git a/api/simplestore/helpers.go b/api/simplestore/helpers.go index e289557e9..037e27f09 100644 --- a/api/simplestore/helpers.go +++ b/api/simplestore/helpers.go @@ -44,6 +44,26 @@ func getSimpleStore() SimpleStore { return store } +func getErrorStore() SimpleStore { + env := &env.Native{} + os.Setenv(api.LogLevel, "info") + env.Configure() + + logger := &log.Service{Log: log.NewLogger()} + + serializer := JSONSerializer{} + + db := &errorDB{} + + store := SimpleStore{ + db, + serializer, + logger, + } + + return store +} + // randomEmail an example.com email address with 10 random characters func randomEmail() string { @@ -89,19 +109,23 @@ func areEqualJSON(t *testing.T, s1, s2 []byte) bool { func CreateTestAccount(t *testing.T, store SimpleStore) api.Account { t.Helper() - createQuery := `INSERT INTO accounts (username, email, status, form_type, form_version, external_id) VALUES ($1, $1, $2, $3, $4, $5) RETURNING id, username, email, status, form_type, form_version, external_id` - email := randomEmail() - - result := api.Account{} - externalID := uuid.New().String() - createErr := store.db.Get(&result, createQuery, email, api.StatusIncomplete, "SF86", "2017-07", externalID) + account := api.Account{ + Username: email, + Email: NonNullString(email), + Status: api.StatusIncomplete, + FormType: "SF86", + FormVersion: "2017-07", + ExternalID: externalID, + } + + createErr := store.CreateAccount(&account) if createErr != nil { t.Log("Failed to create Account", createErr) t.Fatal() } - return result + return account } diff --git a/api/simplestore/simple_store.go b/api/simplestore/simple_store.go index ba3b371c7..870d71305 100644 --- a/api/simplestore/simple_store.go +++ b/api/simplestore/simple_store.go @@ -18,9 +18,20 @@ type simpleConnection interface { Get(dest interface{}, query string, args ...interface{}) error } +// In order to be able to mock the db for coverage reasons, we here declare an interface of +// everything we actually use in sqlx. +type sqlxConnection interface { + simpleConnection + + MustExec(query string, args ...interface{}) sql.Result + Beginx() (*sqlx.Tx, error) + Queryx(query string, args ...interface{}) (*sqlx.Rows, error) + Close() error +} + // SimpleStore saves JSON in the db for applications type SimpleStore struct { - db *sqlx.DB + db sqlxConnection serializer api.Serializer logger api.LogService } diff --git a/api/store.go b/api/store.go index 693f10e28..dfd5ed1df 100644 --- a/api/store.go +++ b/api/store.go @@ -14,6 +14,9 @@ var ( // ErrApplicationAlreadyExists is an error when a given application does not exist ErrApplicationAlreadyExists = errors.New("Application already exists") + // ErrAccountDoesNotExist is an error when a given account cannot be + ErrAccountDoesNotExist = errors.New("Account does not exist") + // ErrAttachmentDoesNotExist is returned when the requested attchment does not exist. // Note: this could mean that you requested a valid ID but for a different user. ErrAttachmentDoesNotExist = errors.New("Attachment does not exist") @@ -52,6 +55,17 @@ type StorageService interface { // ExtendAndFetchSessionAccount fetches an account and session data from the db ExtendAndFetchSessionAccount(sessionKey string, expirationDuration time.Duration) (Account, Session, error) + // Can fetch sessions, that's good. + // CreateAccount creates a new account + CreateAccount(account *Account) error + // FetchAccountByUsername updates the given account + FetchAccountByUsername(username string) (Account, error) + // FetchAccountByExternalID updates the given account + FetchAccountByExternalID(externalID string) (Account, error) + + // UpdateAccountStatus updates the given account + UpdateAccountStatus(account *Account) error + // Close closes the db connection Close() error } From 31b8fa8f76e099a912620bf3f8edcae2d664ef0d Mon Sep 17 00:00:00 2001 From: MacRae Linton <55759+macrael@users.noreply.github.com> Date: Thu, 5 Sep 2019 21:24:31 -0700 Subject: [PATCH 2/8] add fetch with password hash --- api/account.go | 16 ++++++----- api/simplestore/accounts.go | 39 ++++++++++++++++++++++++-- api/simplestore/accounts_test.go | 48 ++++++++++++++++++++++++++++++-- api/store.go | 10 +++++-- 4 files changed, 98 insertions(+), 15 deletions(-) diff --git a/api/account.go b/api/account.go index 28346a1d5..95ee83190 100644 --- a/api/account.go +++ b/api/account.go @@ -42,13 +42,15 @@ const ( // Account represents a user account type Account struct { - ID int - Username string - Email sql.NullString - Status string - FormType string `db:"form_type"` - FormVersion string `db:"form_version"` - ExternalID string `db:"external_id"` // ExternalID is an identifier used by external systems to id applications + ID int + Username string + Email sql.NullString + Status string + FormType string `db:"form_type"` + FormVersion string `db:"form_version"` + ExternalID string `db:"external_id"` // ExternalID is an identifier used by external systems to id applications + PasswordHash sql.NullString `db:"password_hash"` // PasswordHash is only used for basic auth, never in production. + // The only way for PasswordHash to be filled out is to call simplestore.FetchAccountWithPasswordHash } // validate returns nil if the account is valid or an error describing what's wrong otherwise. diff --git a/api/simplestore/accounts.go b/api/simplestore/accounts.go index 5bcd72ecf..b1c3167e5 100644 --- a/api/simplestore/accounts.go +++ b/api/simplestore/accounts.go @@ -2,6 +2,8 @@ package simplestore import ( "database/sql" + "fmt" + "strings" "github.com/18F/e-QIP-prototype/api" "github.com/pkg/errors" @@ -26,12 +28,12 @@ func (s SimpleStore) CreateAccount(account *api.Account) error { return nil } -var selectProlouge = `SELECT id, username, email, status, form_type, form_version, external_id FROM accounts ` +var accountFields = []string{"id", "username", "email", "status", "form_type", "form_version", "external_id"} // FetchAccountByUsername returns an account with the given username func (s SimpleStore) FetchAccountByUsername(username string) (api.Account, error) { - selectQuery := selectProlouge + `WHERE username = $1` + selectQuery := fmt.Sprintf(`SELECT %s FROM accounts WHERE username = $1`, strings.Join(accountFields, ",")) var account api.Account @@ -49,7 +51,7 @@ func (s SimpleStore) FetchAccountByUsername(username string) (api.Account, error //FetchAccountByExternalID fetches an account with the given external id func (s SimpleStore) FetchAccountByExternalID(externalID string) (api.Account, error) { - selectQuery := selectProlouge + `WHERE external_id = $1` + selectQuery := fmt.Sprintf(`SELECT %s FROM accounts WHERE external_id = $1`, strings.Join(accountFields, ",")) var account api.Account @@ -64,6 +66,37 @@ func (s SimpleStore) FetchAccountByExternalID(externalID string) (api.Account, e return account, nil } +// FetchAccountWithPasswordHash returns an account with the PasswordHash field filled out +// it raises an error if the given account has no password +func (s SimpleStore) FetchAccountWithPasswordHash(username string) (api.Account, error) { + + var prefixedFields []string + for _, field := range accountFields { + prefixedFields = append(prefixedFields, "accounts."+field) + } + + selectQuery := fmt.Sprintf(`SELECT %s, basic_auth_memberships.password_hash + FROM accounts LEFT JOIN basic_auth_memberships + ON accounts.id = basic_auth_memberships.account_id + WHERE accounts.username = $1`, strings.Join(prefixedFields, ",")) + + var account api.Account + + selectErr := s.db.Get(&account, selectQuery, username) + if selectErr != nil { + if selectErr == sql.ErrNoRows { + return api.Account{}, api.ErrAccountDoesNotExist + } + return api.Account{}, errors.Wrap(selectErr, "Couldn't find Account") + } + + if !account.PasswordHash.Valid { + return api.Account{}, api.ErrAccountHasNoPassword + } + + return account, nil +} + // UpdateAccountStatus updates the status of an account. That should be the only thing we update post creation. func (s SimpleStore) UpdateAccountStatus(account *api.Account) error { diff --git a/api/simplestore/accounts_test.go b/api/simplestore/accounts_test.go index 278249c02..38cfbd582 100644 --- a/api/simplestore/accounts_test.go +++ b/api/simplestore/accounts_test.go @@ -2,6 +2,7 @@ package simplestore import ( "database/sql" + "fmt" "testing" "github.com/18F/e-QIP-prototype/api" @@ -9,7 +10,6 @@ import ( ) func TestFetchAccountEmail(t *testing.T) { - store := getSimpleStore() defer store.Close() @@ -62,8 +62,24 @@ func TestFetchAccountEmail(t *testing.T) { } -func TestFetchAccountExternalID(t *testing.T) { +func TestFetchAccountWithPassword(t *testing.T) { + store := getSimpleStore() + defer store.Close() + + // Can we fetch the same newAccount? + fetchedAccount, fetchErr := store.FetchAccountWithPasswordHash("test01") + if fetchErr != nil { + t.Fatal(fetchErr) + } + if !fetchedAccount.PasswordHash.Valid { + t.Fatal("Invalid password hash!") + } + + fmt.Println(fetchedAccount.PasswordHash.String) +} + +func TestFetchAccountExternalID(t *testing.T) { store := getSimpleStore() defer store.Close() @@ -116,8 +132,24 @@ func TestFetchAccountExternalID(t *testing.T) { } -func TestUpdateAccountStatus(t *testing.T) { +func TestFetchNoPassword(t *testing.T) { + store := getSimpleStore() + defer store.Close() + + newAccount := CreateTestAccount(t, store) + if newAccount.ID <= 0 { + t.Fatal("didn't get a valid ID") + } + + // Can we fetch the same newAccount? + _, fetchErr := store.FetchAccountWithPasswordHash(newAccount.Username) + if fetchErr != api.ErrAccountHasNoPassword { + t.Fatal(fetchErr) + } +} + +func TestUpdateAccountStatus(t *testing.T) { store := getSimpleStore() defer store.Close() @@ -178,6 +210,11 @@ func TestFetchUnknownError(t *testing.T) { if fetchErr == nil && fetchErr != api.ErrAccountDoesNotExist { t.Fatal("Should have gotten an unknown error") } + + _, fetchErr = store.FetchAccountWithPasswordHash("foo") + if fetchErr == nil && fetchErr != api.ErrAccountDoesNotExist { + t.Fatal("Should have gotten an unknown error") + } } func TestDoesntExist(t *testing.T) { @@ -195,6 +232,11 @@ func TestDoesntExist(t *testing.T) { t.Fatal(fetchErr) } + _, fetchErr = store.FetchAccountWithPasswordHash("NONSENSE") + if fetchErr != api.ErrAccountDoesNotExist { + t.Fatal(fetchErr) + } + account := api.Account{ ID: -9000, Username: "NONSENSE", diff --git a/api/store.go b/api/store.go index dfd5ed1df..2f3bf421c 100644 --- a/api/store.go +++ b/api/store.go @@ -11,15 +11,18 @@ var ( // ErrApplicationDoesNotExist is an error when a given application does not exist ErrApplicationDoesNotExist = errors.New("Application does not exist") - // ErrApplicationAlreadyExists is an error when a given application does not exist + // ErrApplicationAlreadyExists is an error when a given application already exists ErrApplicationAlreadyExists = errors.New("Application already exists") - // ErrAccountDoesNotExist is an error when a given account cannot be + // ErrAccountDoesNotExist is an error when a given account cannot be found ErrAccountDoesNotExist = errors.New("Account does not exist") // ErrAttachmentDoesNotExist is returned when the requested attchment does not exist. // Note: this could mean that you requested a valid ID but for a different user. ErrAttachmentDoesNotExist = errors.New("Attachment does not exist") + + // ErrAccountHasNoPassword is an error when a given account has no password + ErrAccountHasNoPassword = errors.New("Account has no password") ) // StorageService stores eapp related data @@ -62,6 +65,9 @@ type StorageService interface { FetchAccountByUsername(username string) (Account, error) // FetchAccountByExternalID updates the given account FetchAccountByExternalID(externalID string) (Account, error) + // FetchAccountWithPasswordHash returns an account with the PasswordHash field filled out + // it raises an error if the given account has no password + FetchAccountWithPasswordHash(username string) (Account, error) // UpdateAccountStatus updates the given account UpdateAccountStatus(account *Account) error From e88163c967481a24495861d440557442330180d7 Mon Sep 17 00:00:00 2001 From: MacRae Linton <55759+macrael@users.noreply.github.com> Date: Fri, 6 Sep 2019 00:48:01 -0700 Subject: [PATCH 3/8] Remove the postgresql package and replace almost all usage with simplestore --- api/account.go | 112 +++-------- api/account_test.go | 53 +----- api/admin/admin_test.go | 28 ++- api/admin/reject.go | 9 +- api/admin/submit.go | 15 +- api/basicauth_membership.go | 82 --------- api/cmd/cmd.go | 30 ++- api/cmd/dbmigrate/main.go | 6 +- api/cmd/dbreset/main.go | 6 +- api/cmd/flush/main.go | 2 +- api/cmd/kickback/main.go | 4 +- api/cmd/server/main.go | 37 ++-- api/cmd/sftype/main.go | 32 ++-- api/cmd/unlock/main.go | 7 +- api/database.go | 39 ---- api/form_metadata.go | 21 --- api/http/attachment.go | 28 ++- api/http/basic_auth.go | 26 +-- api/http/form.go | 7 +- api/http/form_test.go | 18 +- api/http/refresh.go | 5 +- api/http/refresh_test.go | 7 +- api/http/saml.go | 45 ++--- api/http/saml_test.go | 16 +- api/http/save.go | 7 +- api/http/save_test.go | 82 +++------ api/http/status.go | 7 +- api/http/submit.go | 8 +- api/integration/add_empty_value_test.go | 2 +- api/integration/application_status_test.go | 27 ++- api/integration/basic_auth_test.go | 131 +++++-------- api/integration/clear_yes_no_test.go | 42 ++--- api/integration/csrf_test.go | 48 +++-- api/integration/delete_attachment_test.go | 66 +++---- api/integration/form_version_test.go | 11 +- api/integration/helpers_test.go | 50 +++-- api/integration/http_session_test.go | 68 +++---- api/integration/list_attachment_test.go | 43 ++--- api/integration/reject_test.go | 28 ++- api/integration/save_attachment_test.go | 93 +++++----- api/integration/save_sections_test.go | 6 +- api/integration/session_test.go | 5 +- api/integration/submission_test.go | 24 +-- api/integration/validation_test.go | 8 +- api/log.go | 1 + api/mock/database.go | 75 -------- api/mock/store.go | 21 +++ api/postgresql/db.go | 205 --------------------- api/postgresql/db_test.go | 94 ---------- api/postgresql/testdata | 1 - api/session/session_test.go | 68 +++++-- api/simplestore/accounts.go | 61 ++++++ api/simplestore/accounts_test.go | 176 +++++++++++++++++- api/simplestore/applications_test.go | 4 +- api/simplestore/attachments_test.go | 4 +- api/simplestore/error_db.go | 4 + api/simplestore/helpers.go | 11 +- api/simplestore/sessions.go | 12 -- api/simplestore/sessions_test.go | 28 +-- api/simplestore/simple_store.go | 46 +++++ api/transmission.go | 36 ++-- 61 files changed, 878 insertions(+), 1360 deletions(-) delete mode 100644 api/basicauth_membership.go delete mode 100644 api/database.go delete mode 100644 api/form_metadata.go delete mode 100644 api/mock/database.go delete mode 100644 api/postgresql/db.go delete mode 100644 api/postgresql/db_test.go delete mode 120000 api/postgresql/testdata diff --git a/api/account.go b/api/account.go index 95ee83190..7f7008ab3 100644 --- a/api/account.go +++ b/api/account.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/pkg/errors" + "golang.org/x/crypto/bcrypt" ) var ( @@ -53,6 +54,18 @@ type Account struct { // The only way for PasswordHash to be filled out is to call simplestore.FetchAccountWithPasswordHash } +// Helper methods for sql.NullString + +// NonNullString returns a valid sql.NullString +func NonNullString(value string) sql.NullString { + return sql.NullString{Valid: true, String: value} +} + +// NullString returns an invalid sql.NullString +func NullString() sql.NullString { + return sql.NullString{} +} + // validate returns nil if the account is valid or an error describing what's wrong otherwise. // This is intended to ensure data integrity by not saving invalid data to the database func (entity Account) validate() error { @@ -79,73 +92,6 @@ func (entity Account) validate() error { } -// Save the Account entity. -func (entity *Account) Save(context DatabaseService, account int) (int, error) { - - if err := entity.validate(); err != nil { - return entity.ID, err - } - - if err := context.Save(entity); err != nil { - return entity.ID, err - } - - return entity.ID, nil -} - -// Delete the Account entity. -func (entity *Account) Delete(context DatabaseService, account int) (int, error) { - - if entity.ID != 0 { - if err := context.Delete(entity); err != nil { - return entity.ID, err - } - } - - return entity.ID, nil -} - -// Get the Account entity. -func (entity *Account) Get(context DatabaseService, account int) (int, error) { - - if err := entity.Find(context); err != nil { - return entity.ID, err - } - - return entity.ID, nil -} - -// GetID returns the entity identifier. -func (entity *Account) GetID() int { - return entity.ID -} - -// SetID sets the entity identifier. -func (entity *Account) SetID(id int) { - entity.ID = id -} - -// Find will search for account by `username` if no identifier is present or by ID if it is. -func (entity *Account) Find(context DatabaseService) error { - if entity.ExternalID != "" { - return entity.FindByExternalID(context) - } - - if entity.ID == 0 { - return context.Where(entity, "Account.username = ?", entity.Username) - } - return context.Select(entity) -} - -// FindByExternalID will search for account by `request id` -func (entity *Account) FindByExternalID(context DatabaseService) error { - if entity.ExternalID == "" { - return fmt.Errorf("No request id was given") - } - - return context.Where(entity, "Account.external_id = ?", entity.ExternalID) -} - // CanSubmit returns wether the account is in a valid state for submission func (entity *Account) CanSubmit() bool { if entity.Status == StatusSubmitted { @@ -209,29 +155,25 @@ func DefaultFormVersion(formType string) (string, error) { return versions[0], nil } -// BasicAuthentication checks if the username and password are valid and returns the users account -func (entity *Account) BasicAuthentication(context DatabaseService, password string) error { - var basicMembership BasicAuthMembership - - // Find if basic auth record exists for given account username - err := context.ColumnsWhere(&basicMembership, []string{"basic_auth_membership.*", "Account"}, "Account.username = ?", entity.Username) +// HashPassword converts a plaintext password and generates a hash +func (entity *Account) HashPassword(password string) error { + hashedPassword, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) if err != nil { - fmt.Printf("Basic Authentication Error: [%v]\n", err) - return ErrAccountDoesNotExist + return err } + entity.PasswordHash = NonNullString(string(hashedPassword)) + return nil +} - // Check if plaintext password matches hashed password - if matches := basicMembership.PasswordMatch(password); !matches { - return ErrPasswordDoesNotMatch +// CheckPassword checks if the username and password are valid and returns the users account +func (entity *Account) CheckPassword(password string) error { + if !entity.PasswordHash.Valid { + return ErrAccountHasNoPassword } - if basicMembership.Account != nil { - entity.ID = basicMembership.Account.ID - entity.Username = basicMembership.Account.Username - entity.Email = basicMembership.Account.Email - entity.Status = basicMembership.Account.Status - entity.FormType = basicMembership.Account.FormType - entity.FormVersion = basicMembership.Account.FormVersion + if bcrypt.CompareHashAndPassword([]byte(entity.PasswordHash.String), []byte(password)) != nil { + return ErrPasswordDoesNotMatch } + return nil } diff --git a/api/account_test.go b/api/account_test.go index 52afffb47..d2e2cc10b 100644 --- a/api/account_test.go +++ b/api/account_test.go @@ -13,25 +13,25 @@ func TestBasicAuthentication(t *testing.T) { account := &Account{ Username: username, - Firstname: "Admin", - Lastname: "Last", FormType: "SF86", FormVersion: "2017-07", } - membership := &BasicAuthMembership{ - AccountID: account.ID, - Account: account, + setErr := account.HashPassword(password) + if setErr != nil { + t.Fatal(setErr) } - membership.HashPassword(password) - if matched := membership.PasswordMatch(password); !matched { - t.Fatal("Expected password to match") + checkErr := account.CheckPassword(password) + if checkErr != nil { + t.Fatal(checkErr) } - if matched := membership.PasswordMatch("incorrect-password"); matched { - t.Fatal("Expected incorrect password") + badErr := account.CheckPassword("incorrect-password") + if badErr != ErrPasswordDoesNotMatch { + t.Fatal("Should have not matched!", badErr) } + } func TestValidFormType(t *testing.T) { @@ -104,39 +104,6 @@ func TestAccountFormTypeValidation(t *testing.T) { } } -func TestGetAccountID(t *testing.T) { - - accountID := 148958398 - account := &Account{ - Username: "glarbal@example.com", - Email: sql.NullString{}, - FormType: "Dogs", - FormVersion: "2017-07", - ID: accountID, - } - if account.GetID() != accountID { - t.Errorf("incorrect account ID") - t.Fail() - } -} - -func TestSetAccountID(t *testing.T) { - - accountID := 148958398 - account := &Account{ - Username: "glarbal@example.com", - Email: sql.NullString{}, - FormType: "Dogs", - FormVersion: "2017-07", - ID: 1835833, - } - account.SetID(accountID) - if account.ID != accountID { - t.Errorf("account ID set failure") - t.Fail() - } -} - func TestSubmitAccount(t *testing.T) { account := &Account{ diff --git a/api/admin/admin_test.go b/api/admin/admin_test.go index f4966585d..25c9fe602 100644 --- a/api/admin/admin_test.go +++ b/api/admin/admin_test.go @@ -6,7 +6,6 @@ import ( "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/mock" "github.com/18F/e-QIP-prototype/api/pdf" - "github.com/18F/e-QIP-prototype/api/simplestore" "github.com/18F/e-QIP-prototype/api/xml" "github.com/google/uuid" "github.com/pkg/errors" @@ -41,21 +40,20 @@ func (s *errorAdminListAttachmentsMetadataStore) ListAttachmentsMetadata(account // func TestRejectAccountCanKickbackFailure(t *testing.T) { - var mockDB mock.DatabaseService var mockStore mock.StorageService email := "dogs@iloveyou.com" // Make an account with Status that isn't StatusSubmitted account := api.Account{ Username: email, - Email: simplestore.NonNullString(email), + Email: api.NonNullString(email), FormType: "SF86", FormVersion: "2017-07", Status: api.StatusIncomplete, ExternalID: uuid.New().String(), } // Reject this submission - rejector := NewRejecter(&mockDB, &mockStore) + rejector := NewRejecter(&mockStore) err := rejector.Reject(&account) if err == nil { t.Log("Should have received an error from Kickback, instead got nil. ") @@ -64,20 +62,19 @@ func TestRejectAccountCanKickbackFailure(t *testing.T) { } func TestRejectAccountLoadApplicationFailure(t *testing.T) { - var mockDB mock.DatabaseService var mockStore errorAdminLoadStore email := "dogs@iloveyou.com" account := api.Account{ Username: email, - Email: simplestore.NonNullString(email), + Email: api.NonNullString(email), FormType: "SF86", FormVersion: "2017-07", Status: api.StatusSubmitted, ExternalID: uuid.New().String(), } // Reject this submission - rejector := NewRejecter(&mockDB, &mockStore) + rejector := NewRejecter(&mockStore) err := rejector.Reject(&account) if err == nil { t.Log("Should have received an error from LoadApplication, instead got nil. ") @@ -86,20 +83,19 @@ func TestRejectAccountLoadApplicationFailure(t *testing.T) { } func TestRejectAccountUpdateApplicationFailure(t *testing.T) { - var mockDB mock.DatabaseService var mockStore errorAdminUpdateStore email := "dogs@iloveyou.com" account := api.Account{ Username: email, - Email: simplestore.NonNullString(email), + Email: api.NonNullString(email), FormType: "SF86", FormVersion: "2017-07", Status: api.StatusSubmitted, ExternalID: uuid.New().String(), } // Reject this submission - rejector := NewRejecter(&mockDB, &mockStore) + rejector := NewRejecter(&mockStore) err := rejector.Reject(&account) if err == nil { t.Log("Should have received an error from UpdateApplication, instead got nil. ") @@ -108,20 +104,19 @@ func TestRejectAccountUpdateApplicationFailure(t *testing.T) { } func TestRejectAccountListAttachmentsMetadataFailure(t *testing.T) { - var mockDB mock.DatabaseService var mockStore errorAdminListAttachmentsMetadataStore email := "dogs@iloveyou.com" account := api.Account{ Username: email, - Email: simplestore.NonNullString(email), + Email: api.NonNullString(email), FormType: "SF86", FormVersion: "2017-07", Status: api.StatusSubmitted, ExternalID: uuid.New().String(), } // Reject this submission - rejector := NewRejecter(&mockDB, &mockStore) + rejector := NewRejecter(&mockStore) err := rejector.Reject(&account) if err == nil { t.Log("Should have received an error from ListAttachmentsMetadata, instead got nil. ") @@ -134,13 +129,12 @@ func TestRejectAccountListAttachmentsMetadataFailure(t *testing.T) { // func TestSubmitLoadApplicationFailure(t *testing.T) { - var mockDB mock.DatabaseService var mockStore errorAdminLoadStore email := "dogs@iloveyou.com" account := api.Account{ Username: email, - Email: simplestore.NonNullString(email), + Email: api.NonNullString(email), FormType: "SF86", FormVersion: "2017-07", Status: api.StatusSubmitted, @@ -150,8 +144,8 @@ func TestSubmitLoadApplicationFailure(t *testing.T) { pdfService := pdf.NewPDFService("../pdf/templates/") // Submit this submission - submitter := NewSubmitter(&mockDB, &mockStore, xmlService, pdfService) - _, _, err := submitter.FilesForSubmission(account.ID) + submitter := NewSubmitter(&mockStore, xmlService, pdfService) + _, _, err := submitter.FilesForSubmission(account) if err == nil { t.Log("Should have received an error from LoadApplication, instead got nil. ") t.Fail() diff --git a/api/admin/reject.go b/api/admin/reject.go index 4ce3c4a2a..fdbbb22cb 100644 --- a/api/admin/reject.go +++ b/api/admin/reject.go @@ -2,23 +2,22 @@ package admin import ( "fmt" - "github.com/pkg/errors" "strings" + "github.com/pkg/errors" + "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/pdf" ) // Rejecter is used to reject/kickback an application type Rejecter struct { - db api.DatabaseService store api.StorageService } // NewRejecter returns a configured Rejecter -func NewRejecter(db api.DatabaseService, store api.StorageService) Rejecter { +func NewRejecter(store api.StorageService) Rejecter { return Rejecter{ - db, store, } } @@ -86,7 +85,7 @@ func (r Rejecter) Reject(account *api.Account) error { return errors.New("The account got into a bad state during the rejection") } - _, saveAccErr := account.Save(r.db, account.ID) + saveAccErr := r.store.UpdateAccountStatus(account) if saveAccErr != nil { return errors.Wrap(saveAccErr, "couldn't save the account after changing the status") } diff --git a/api/admin/submit.go b/api/admin/submit.go index f435d1540..d67180432 100644 --- a/api/admin/submit.go +++ b/api/admin/submit.go @@ -8,16 +8,14 @@ import ( // Submitter is used to reject/kickback an application type Submitter struct { - db api.DatabaseService store api.StorageService xml api.XMLService pdf api.PdfService } // NewSubmitter returns a configured Submitter -func NewSubmitter(db api.DatabaseService, store api.StorageService, xml api.XMLService, pdf api.PdfService) Submitter { +func NewSubmitter(store api.StorageService, xml api.XMLService, pdf api.PdfService) Submitter { return Submitter{ - db, store, xml, pdf, @@ -25,16 +23,9 @@ func NewSubmitter(db api.DatabaseService, store api.StorageService, xml api.XMLS } // FilesForSubmission returns the XML and any Attachments for submission. -func (s Submitter) FilesForSubmission(accountID int) ([]byte, []api.Attachment, error) { +func (s Submitter) FilesForSubmission(account api.Account) ([]byte, []api.Attachment, error) { - // Get the account information from the data store - account := api.Account{ID: accountID} - _, accountErr := account.Get(s.db, accountID) - if accountErr != nil { - return []byte{}, []api.Attachment{}, accountErr - } - - application, appErr := s.store.LoadApplication(accountID) + application, appErr := s.store.LoadApplication(account.ID) if appErr != nil { return []byte{}, []api.Attachment{}, errors.Wrap(appErr, "Can't load application") } diff --git a/api/basicauth_membership.go b/api/basicauth_membership.go deleted file mode 100644 index c33ee8ec9..000000000 --- a/api/basicauth_membership.go +++ /dev/null @@ -1,82 +0,0 @@ -package api - -import ( - "time" - - "golang.org/x/crypto/bcrypt" -) - -// BasicAuthMembership stores basic authentication information for the account. -type BasicAuthMembership struct { - ID int - AccountID int - Account *Account - PasswordHash string - Created time.Time -} - -// Save the basic membership. -func (entity *BasicAuthMembership) Save(context DatabaseService, account int) (int, error) { - - if err := context.Save(entity); err != nil { - return entity.ID, err - } - - return entity.ID, nil -} - -// Delete the basic membership. -func (entity *BasicAuthMembership) Delete(context DatabaseService, account int) (int, error) { - - if entity.ID != 0 { - if err := context.Delete(entity); err != nil { - return entity.ID, err - } - } - - return entity.ID, nil -} - -// Get the basic membership. -func (entity *BasicAuthMembership) Get(context DatabaseService, account int) (int, error) { - - if err := entity.Find(context); err != nil { - return entity.ID, err - } - - return entity.ID, nil -} - -// GetID returns the entity identifier. -func (entity *BasicAuthMembership) GetID() int { - return entity.ID -} - -// SetID sets the entity identifier. -func (entity *BasicAuthMembership) SetID(id int) { - entity.ID = id -} - -// Find the basic membership. -func (entity *BasicAuthMembership) Find(context DatabaseService) error { - // if entity.ID == 0 { - // return context.Where(entity, "Account.username = ?", entity.Username) - // } - return context.Select(entity) -} - -// PasswordMatch determines if a plain text password matches its equivalent password hash. -func (entity *BasicAuthMembership) PasswordMatch(password string) bool { - return bcrypt.CompareHashAndPassword([]byte(entity.PasswordHash), []byte(password)) == nil -} - -// HashPassword converts a plaintext password and generates a hash and updates the -// `PasswordHash` value. -func (entity *BasicAuthMembership) HashPassword(password string) error { - hashedPassword, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) - if err != nil { - return err - } - entity.PasswordHash = string(hashedPassword) - return nil -} diff --git a/api/cmd/cmd.go b/api/cmd/cmd.go index ba0669d32..609f3db0c 100644 --- a/api/cmd/cmd.go +++ b/api/cmd/cmd.go @@ -7,7 +7,6 @@ import ( "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/cloudfoundry" "github.com/18F/e-QIP-prototype/api/env" - "github.com/18F/e-QIP-prototype/api/postgresql" "github.com/18F/e-QIP-prototype/api/simplestore" ) @@ -16,22 +15,22 @@ var ( ) // Command represents a basic utility command. -func Command(log api.LogService, action func(api.DatabaseService, api.StorageService, *api.Account)) { +func Command(log api.LogService, action func(api.StorageService, *api.Account)) { cloudfoundry.Configure() settings := &env.Native{} settings.Configure() - dbConf := postgresql.DBConfig{ + dbConf := simplestore.DBConfig{ User: settings.String(api.DatabaseUser), Password: settings.String(api.DatabasePassword), Address: settings.String(api.DatabaseHost), DBName: settings.String(api.DatabaseName), SSLMode: settings.String(api.DatabaseSSLMode), } - database := postgresql.NewPostgresService(dbConf, log) + // database := postgresql.NewPostgresService(dbConf, log) serializer := simplestore.NewJSONSerializer() - store, storeErr := simplestore.NewSimpleStore(postgresql.PostgresConnectURI(dbConf), log, serializer) + store, storeErr := simplestore.NewSimpleStore(simplestore.PostgresConnectURI(dbConf), log, serializer) if storeErr != nil { log.WarnError("Error configuring Simple Store", storeErr, api.LogFields{}) return @@ -42,34 +41,29 @@ func Command(log api.LogService, action func(api.DatabaseService, api.StorageSer if *flagAll { // Retrieve all accounts within the system from the database so we // may iterate through them. - var accounts []*api.Account - if err := database.FindAll(&accounts); err != nil { - log.WarnError(api.NoAccount, err, api.LogFields{}) + accounts, listErr := store.ListAccounts() + if listErr != nil { + log.WarnError(api.NoAccount, listErr, api.LogFields{}) return } // Iterate through the accounts with a given database context. for _, account := range accounts { - if _, err := account.Get(database, 0); err != nil { - log.WarnError(api.NoAccount, err, api.LogFields{"account": account.Username}) - continue - } - // Perform the provided actions on the given account. - action(database, store, account) + action(store, &account) } } else { // Assume all arguments are a username delimited by white space. for _, username := range os.Args[1:] { // Retrieve the account with the provided username and database context. - account := &api.Account{Username: username} - if _, err := account.Get(database, 0); err != nil { - log.WarnError(api.NoAccount, err, api.LogFields{"account": account.Username}) + account, fetchErr := store.FetchAccountByUsername(username) + if fetchErr != nil { + log.WarnError(api.NoAccount, fetchErr, api.LogFields{"account": account.Username}) continue } // Perform the provided actions on the given account. - action(database, store, account) + action(store, &account) } } } diff --git a/api/cmd/dbmigrate/main.go b/api/cmd/dbmigrate/main.go index 55fb268a6..f6ec69f08 100644 --- a/api/cmd/dbmigrate/main.go +++ b/api/cmd/dbmigrate/main.go @@ -10,7 +10,7 @@ import ( "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/env" - "github.com/18F/e-QIP-prototype/api/postgresql" + "github.com/18F/e-QIP-prototype/api/simplestore" ) func pathToPattern(path, pattern string) string { @@ -33,7 +33,7 @@ func runMigrations(dbName string, migrationsPath string) error { settings := env.Native{} settings.Configure() - dbConf := postgresql.DBConfig{ + dbConf := simplestore.DBConfig{ User: settings.String(api.DatabaseUser), Password: settings.String(api.DatabasePassword), Address: settings.String(api.DatabaseHost), @@ -41,7 +41,7 @@ func runMigrations(dbName string, migrationsPath string) error { SSLMode: settings.String(api.DatabaseSSLMode), } - connStr := postgresql.PostgresConnectURI(dbConf) + connStr := simplestore.PostgresConnectURI(dbConf) apiPath := filepath.Dir(filepath.Clean(migrationsPath)) diff --git a/api/cmd/dbreset/main.go b/api/cmd/dbreset/main.go index ce6202f1d..05769d59a 100644 --- a/api/cmd/dbreset/main.go +++ b/api/cmd/dbreset/main.go @@ -13,7 +13,7 @@ import ( "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/env" - "github.com/18F/e-QIP-prototype/api/postgresql" + "github.com/18F/e-QIP-prototype/api/simplestore" ) func checkDBNameIsAllowed(dbName string) bool { @@ -35,7 +35,7 @@ func resetDB(dbName string, force bool) error { settings := env.Native{} settings.Configure() - dbConf := postgresql.DBConfig{ + dbConf := simplestore.DBConfig{ User: settings.String(api.DatabaseUser), Password: settings.String(api.DatabasePassword), Address: settings.String(api.DatabaseHost), @@ -43,7 +43,7 @@ func resetDB(dbName string, force bool) error { SSLMode: settings.String(api.DatabaseSSLMode), } - connStr := postgresql.PostgresConnectURI(dbConf) + connStr := simplestore.PostgresConnectURI(dbConf) db, openErr := sql.Open("postgres", connStr) if openErr != nil { diff --git a/api/cmd/flush/main.go b/api/cmd/flush/main.go index bc8bd3497..4f79701f0 100644 --- a/api/cmd/flush/main.go +++ b/api/cmd/flush/main.go @@ -8,7 +8,7 @@ import ( func main() { logger := &log.Service{Log: log.NewLogger()} - cmd.Command(logger, func(context api.DatabaseService, store api.StorageService, account *api.Account) { + cmd.Command(logger, func(store api.StorageService, account *api.Account) { delErr := store.DeleteApplication(account.ID) if delErr != nil { logger.Warn("Failed to purge account information", api.LogFields{"account": account.Username}) diff --git a/api/cmd/kickback/main.go b/api/cmd/kickback/main.go index 18abb7868..88f1bc5af 100644 --- a/api/cmd/kickback/main.go +++ b/api/cmd/kickback/main.go @@ -9,8 +9,8 @@ import ( func main() { logger := &log.Service{Log: log.NewLogger()} - cmd.Command(logger, func(context api.DatabaseService, store api.StorageService, account *api.Account) { - rejector := admin.NewRejecter(context, store) + cmd.Command(logger, func(store api.StorageService, account *api.Account) { + rejector := admin.NewRejecter(store) rejectErr := rejector.Reject(account) if rejectErr != nil { diff --git a/api/cmd/server/main.go b/api/cmd/server/main.go index ac0be336b..1bbd44365 100644 --- a/api/cmd/server/main.go +++ b/api/cmd/server/main.go @@ -14,7 +14,6 @@ import ( "github.com/18F/e-QIP-prototype/api/http" "github.com/18F/e-QIP-prototype/api/log" "github.com/18F/e-QIP-prototype/api/pdf" - "github.com/18F/e-QIP-prototype/api/postgresql" "github.com/18F/e-QIP-prototype/api/saml" "github.com/18F/e-QIP-prototype/api/session" "github.com/18F/e-QIP-prototype/api/simplestore" @@ -33,17 +32,15 @@ func main() { settings := env.Native{} settings.Configure() - dbConf := postgresql.DBConfig{ + dbConf := simplestore.DBConfig{ User: settings.String(api.DatabaseUser), Password: settings.String(api.DatabasePassword), Address: settings.String(api.DatabaseHost), DBName: settings.String(api.DatabaseName), } - database := postgresql.NewPostgresService(dbConf, logger) - serializer := simplestore.NewJSONSerializer() - store, storeErr := simplestore.NewSimpleStore(postgresql.PostgresConnectURI(dbConf), logger, serializer) + store, storeErr := simplestore.NewSimpleStore(simplestore.PostgresConnectURI(dbConf), logger, serializer) if storeErr != nil { logger.WarnError("Error configuring Simple Store", storeErr, api.LogFields{}) return @@ -53,7 +50,7 @@ func main() { xmlsvc := xml.NewXMLService(xmlTemplatePath) pdfTemplatePath := "./pdf/templates/" pdfsvc := pdf.NewPDFService(pdfTemplatePath) - submitter := admin.NewSubmitter(database, store, xmlsvc, pdfsvc) + submitter := admin.NewSubmitter(store, xmlsvc, pdfsvc) sessionTimeout := time.Duration(settings.Int(api.SessionTimeout)) * time.Minute sessionService := session.NewSessionService(sessionTimeout, store, logger) @@ -68,7 +65,7 @@ func main() { if !*flagSkipMigration { ex, _ := os.Executable() migration := api.Migration{Env: settings} - connStr := postgresql.PostgresConnectURI(dbConf) + connStr := simplestore.PostgresConnectURI(dbConf) if err := migration.Up(connStr, filepath.Dir(ex), settings.String(api.GolangEnv), ""); err != nil { logger.WarnError(api.WarnFailedMigration, err, api.LogFields{}) } @@ -94,30 +91,30 @@ func main() { o := r.PathPrefix("/auth").Subrouter() if settings.True(api.BasicEnabled) { - o.HandleFunc("/basic", http.BasicAuthHandler{Env: settings, Log: logger, Database: database, Store: store, Cookie: cookieService, Session: sessionService}.ServeHTTP).Methods("POST") + o.HandleFunc("/basic", http.BasicAuthHandler{Env: settings, Log: logger, Store: store, Cookie: cookieService, Session: sessionService}.ServeHTTP).Methods("POST") } if settings.True(api.SamlEnabled) { - o.HandleFunc("/saml", http.SamlRequestHandler{Env: settings, Log: logger, Database: database, SAML: samlsvc}.ServeHTTP).Methods("GET") + o.HandleFunc("/saml", http.SamlRequestHandler{Env: settings, Log: logger, SAML: samlsvc}.ServeHTTP).Methods("GET") // SLO uses the logged in session - o.Handle("/saml_slo", sec(http.SamlSLORequestHandler{Env: settings, Log: logger, Database: database, SAML: samlsvc, Session: sessionService})).Methods("GET") - o.HandleFunc("/saml/callback", http.SamlResponseHandler{Env: settings, Log: logger, Database: database, SAML: samlsvc, Session: sessionService, Cookie: cookieService}.ServeHTTP).Methods("POST") + o.Handle("/saml_slo", sec(http.SamlSLORequestHandler{Env: settings, Log: logger, SAML: samlsvc, Session: sessionService})).Methods("GET") + o.HandleFunc("/saml/callback", http.SamlResponseHandler{Env: settings, Log: logger, SAML: samlsvc, Session: sessionService, Cookie: cookieService}.ServeHTTP).Methods("POST") } // Account specific actions - r.Handle("/refresh", sec(http.RefreshHandler{Env: settings, Log: logger, Database: database})).Methods("POST") + r.Handle("/refresh", sec(http.RefreshHandler{Env: settings, Log: logger})).Methods("POST") a := r.PathPrefix("/me").Subrouter() a.Handle("/logout", sec(http.LogoutHandler{Log: logger, Session: sessionService})).Methods("POST") - a.Handle("/save", sec(http.SaveHandler{Env: settings, Log: logger, Database: database, Store: store})).Methods("POST", "PUT") - a.Handle("/status", sec(http.StatusHandler{Env: settings, Log: logger, Database: database, Store: store})).Methods("GET") + a.Handle("/save", sec(http.SaveHandler{Env: settings, Log: logger, Store: store})).Methods("POST", "PUT") + a.Handle("/status", sec(http.StatusHandler{Env: settings, Log: logger, Store: store})).Methods("GET") a.Handle("/validate", sec(http.ValidateHandler{Log: logger})).Methods("POST") - a.Handle("/form", sec(http.FormHandler{Env: settings, Log: logger, Database: database, Store: store})).Methods("GET") - a.Handle("/form/submit", sec(http.SubmitHandler{Env: settings, Log: logger, Database: database, Store: store, Submitter: submitter})).Methods("POST") - a.Handle("/attachment", sec(http.AttachmentListHandler{Env: settings, Log: logger, Database: database, Store: store})).Methods("GET") - a.Handle("/attachment/{id}", sec(http.AttachmentGetHandler{Env: settings, Log: logger, Database: database, Store: store})).Methods("GET") + a.Handle("/form", sec(http.FormHandler{Env: settings, Log: logger, Store: store})).Methods("GET") + a.Handle("/form/submit", sec(http.SubmitHandler{Env: settings, Log: logger, Store: store, Submitter: submitter})).Methods("POST") + a.Handle("/attachment", sec(http.AttachmentListHandler{Env: settings, Log: logger, Store: store})).Methods("GET") + a.Handle("/attachment/{id}", sec(http.AttachmentGetHandler{Env: settings, Log: logger, Store: store})).Methods("GET") if settings.True(api.AttachmentsEnabled) { - a.Handle("/attachment", sec(http.AttachmentSaveHandler{Env: settings, Log: logger, Database: database, Store: store})).Methods("POST", "PUT") - a.Handle("/attachment/{id}/delete", sec(http.AttachmentDeleteHandler{Env: settings, Log: logger, Database: database, Store: store})).Methods("POST", "DELETE") + a.Handle("/attachment", sec(http.AttachmentSaveHandler{Env: settings, Log: logger, Store: store})).Methods("POST", "PUT") + a.Handle("/attachment/{id}/delete", sec(http.AttachmentDeleteHandler{Env: settings, Log: logger, Store: store})).Methods("POST", "DELETE") } // Inject middleware diff --git a/api/cmd/sftype/main.go b/api/cmd/sftype/main.go index 810a0dc5a..fa331e8dd 100644 --- a/api/cmd/sftype/main.go +++ b/api/cmd/sftype/main.go @@ -7,7 +7,7 @@ import ( "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/env" "github.com/18F/e-QIP-prototype/api/log" - "github.com/18F/e-QIP-prototype/api/postgresql" + "github.com/18F/e-QIP-prototype/api/simplestore" ) const usage string = `Usage: @@ -17,13 +17,13 @@ const usage string = `Usage: // sftype allows you to the SF type for an account. -func configureDB() api.DatabaseService { +func configureDB() simplestore.SimpleStore { os.Setenv(api.LogLevel, "info") logger := &log.Service{Log: log.NewLogger()} settings := &env.Native{} settings.Configure() - dbConf := postgresql.DBConfig{ + dbConf := simplestore.DBConfig{ User: os.Getenv(api.DatabaseUser), Password: os.Getenv(api.DatabasePassword), Address: os.Getenv(api.DatabaseHost), @@ -31,9 +31,14 @@ func configureDB() api.DatabaseService { SSLMode: os.Getenv(api.DatabaseSSLMode), } - db := postgresql.NewPostgresService(dbConf, logger) + serializer := simplestore.NewJSONSerializer() + store, storeErr := simplestore.NewSimpleStore(simplestore.PostgresConnectURI(dbConf), logger, serializer) + if storeErr != nil { + fmt.Println(storeErr) + os.Exit(2) + } - return db + return store } func main() { @@ -45,13 +50,11 @@ func main() { username := os.Args[1] - db := configureDB() + store := configureDB() - account := api.Account{} - account.Username = username - err := account.Find(db) - if err != nil { - fmt.Println("Error: ", err) + account, fetchErr := store.FetchAccountByUsername(username) + if fetchErr != nil { + fmt.Println("Error: ", fetchErr) os.Exit(1) } @@ -61,9 +64,10 @@ func main() { account.FormType = sfType account.FormVersion = sfVersion - _, err = account.Save(db, account.ID) - if err != nil { - fmt.Println("Error: ", err) + + updateErr := store.UpdateAccountInfo(&account) + if updateErr != nil { + fmt.Println("Error: ", updateErr) os.Exit(1) } } diff --git a/api/cmd/unlock/main.go b/api/cmd/unlock/main.go index 5b504460d..00f918d07 100644 --- a/api/cmd/unlock/main.go +++ b/api/cmd/unlock/main.go @@ -8,10 +8,11 @@ import ( func main() { logger := &log.Service{Log: log.NewLogger()} - cmd.Command(logger, func(context api.DatabaseService, store api.StorageService, account *api.Account) { + cmd.Command(logger, func(store api.StorageService, account *api.Account) { account.Unsubmit() - if _, err := account.Save(context, account.ID); err != nil { - logger.WarnError("Failed to save unlocked account", err, api.LogFields{"account": account.Username}) + updateErr := store.UpdateAccountStatus(account) + if updateErr != nil { + logger.WarnError("Failed to save unlocked account", updateErr, api.LogFields{"account": account.Username}) } else { logger.Warn("Account unlocked", api.LogFields{"account": account.Username}) } diff --git a/api/database.go b/api/database.go deleted file mode 100644 index bca235ba7..000000000 --- a/api/database.go +++ /dev/null @@ -1,39 +0,0 @@ -package api - -import ( - "github.com/pkg/errors" -) - -// DatabaseErrorNotFound is an error that indicates that the function failed because -// the seach returned zero results. It is expected to be returned by Select et al. -type DatabaseErrorNotFound string - -func (d DatabaseErrorNotFound) Error() string { - return string(d) -} - -// IsDatabaseErrorNotFound returns true if the error is a DatabaseErrorNotFound error -func IsDatabaseErrorNotFound(err error) bool { - if _, ok := errors.Cause(err).(DatabaseErrorNotFound); ok { - return true - } - return false -} - -// DatabaseService represents a persisted data storage. -type DatabaseService interface { - Raw(query interface{}, params ...interface{}) error - Find(query interface{}, callback func(query interface{})) - FindAll(model interface{}) error - Where(model interface{}, condition string, params ...interface{}) error - ColumnsWhere(model interface{}, columns []string, condition string, params ...interface{}) error - Insert(query ...interface{}) error - Update(query interface{}) error - Save(query ...interface{}) error - Delete(query interface{}) error - Select(query interface{}) error - Count(model interface{}, condition string, params ...interface{}) int - CountExpr(model interface{}, expr string, retval interface{}, condition string, params ...interface{}) - Array(model interface{}, expr string, retval interface{}, condition string, params ...interface{}) - Close() error -} diff --git a/api/form_metadata.go b/api/form_metadata.go deleted file mode 100644 index a8325e71c..000000000 --- a/api/form_metadata.go +++ /dev/null @@ -1,21 +0,0 @@ -package api - -// GetFormMetadata returns the metadata for the form -func GetFormMetadata(context DatabaseService, accountID int) (map[string]string, error) { - account := Account{ - ID: accountID, - } - - err := account.Find(context) - if err != nil { - return nil, err - } - - metadata := make(map[string]string) - metadata["type"] = "metadata" - - metadata["form_type"] = account.FormType - metadata["form_version"] = account.FormVersion - - return metadata, nil -} diff --git a/api/http/attachment.go b/api/http/attachment.go index decc9883f..1bbb0ee48 100644 --- a/api/http/attachment.go +++ b/api/http/attachment.go @@ -16,10 +16,9 @@ import ( // AttachmentListHandler is the handler for listing attachments. type AttachmentListHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - Store api.StorageService + Env api.Settings + Log api.LogService + Store api.StorageService } // ServeHTTP serves the HTTP response. @@ -49,10 +48,9 @@ func (service AttachmentListHandler) ServeHTTP(w http.ResponseWriter, r *http.Re // AttachmentSaveHandler is the handler for saving attachments. type AttachmentSaveHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - Store api.StorageService + Env api.Settings + Log api.LogService + Store api.StorageService } // ServeHTTP serves the HTTP response. @@ -143,10 +141,9 @@ func (service AttachmentSaveHandler) ServeHTTP(w http.ResponseWriter, r *http.Re // AttachmentGetHandler is the handler for getting attachments. type AttachmentGetHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - Store api.StorageService + Env api.Settings + Log api.LogService + Store api.StorageService } // ServeHTTP serves the HTTP response. @@ -187,10 +184,9 @@ func (service AttachmentGetHandler) ServeHTTP(w http.ResponseWriter, r *http.Req // AttachmentDeleteHandler is the handler for deleting attachments. type AttachmentDeleteHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - Store api.StorageService + Env api.Settings + Log api.LogService + Store api.StorageService } // ServeHTTP serves the HTTP response. diff --git a/api/http/basic_auth.go b/api/http/basic_auth.go index 8f3e7ffa2..a98664af0 100644 --- a/api/http/basic_auth.go +++ b/api/http/basic_auth.go @@ -4,17 +4,15 @@ import ( "net/http" "github.com/18F/e-QIP-prototype/api" - "github.com/18F/e-QIP-prototype/api/simplestore" ) // BasicAuthHandler is the handler for basic authentication. type BasicAuthHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - Store api.StorageService - Session api.SessionService - Cookie SessionCookieService + Env api.Settings + Log api.LogService + Store api.StorageService + Session api.SessionService + Cookie SessionCookieService } // ServeHTTP processes a users request to login with a Username and Password @@ -53,25 +51,21 @@ func (service BasicAuthHandler) ServeHTTP(w http.ResponseWriter, r *http.Request return } - account := &api.Account{ - Username: respBody.Username, - } - - // Associate with a database context. - if _, err := account.Get(service.Database, 0); err != nil { - service.Log.WarnError(api.AccountUpdateError, err, api.LogFields{"username": account.Username}) + account, fetchErr := service.Store.FetchAccountWithPasswordHash(respBody.Username) + if fetchErr != nil { + service.Log.WarnError(api.AccountUpdateError, fetchErr, api.LogFields{"username": account.Username}) RespondWithStructuredError(w, api.AccountUpdateError, http.StatusInternalServerError) return } // Validate the user name and password combination - if err := account.BasicAuthentication(service.Database, respBody.Password); err != nil { + if err := account.CheckPassword(respBody.Password); err != nil { service.Log.WarnError(api.BasicAuthInvalid, err, api.LogFields{"account": account.ID}) RespondWithStructuredError(w, api.BasicAuthInvalid, http.StatusUnauthorized) return } - sessionKey, authErr := service.Session.UserDidAuthenticate(account.ID, simplestore.NullString()) + sessionKey, authErr := service.Session.UserDidAuthenticate(account.ID, api.NullString()) if authErr != nil { service.Log.WarnError("bad session get", authErr, api.LogFields{"account": account.ID}) RespondWithStructuredError(w, "bad session get", http.StatusInternalServerError) diff --git a/api/http/form.go b/api/http/form.go index d6eb1652a..1679bfde3 100644 --- a/api/http/form.go +++ b/api/http/form.go @@ -11,10 +11,9 @@ import ( // FormHandler is the handler for the form. type FormHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - Store api.StorageService + Env api.Settings + Log api.LogService + Store api.StorageService } // ServeHTTP will return a JSON object of all currently saved application diff --git a/api/http/form_test.go b/api/http/form_test.go index 895400cdb..6d8a90059 100644 --- a/api/http/form_test.go +++ b/api/http/form_test.go @@ -14,17 +14,14 @@ import ( func TestFormHandlerLockedAccount(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore mock.StorageService var mockLog mock.LogService handler := FormHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } reqBody := strings.NewReader(validJSON) @@ -69,17 +66,14 @@ func (s errorFormStore) LoadApplication(accountID int) (api.Application, error) func TestFormHandlerUnexpectedStoreErr(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore errorFormStore var mockLog mock.LogService handler := FormHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } reqBody := strings.NewReader(validJSON) diff --git a/api/http/refresh.go b/api/http/refresh.go index de81329f4..20c13bf47 100644 --- a/api/http/refresh.go +++ b/api/http/refresh.go @@ -8,9 +8,8 @@ import ( // RefreshHandler is the handler for refreshing the session type RefreshHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService + Env api.Settings + Log api.LogService } // ServeHTTP refreshes a given token. diff --git a/api/http/refresh_test.go b/api/http/refresh_test.go index 803005f1a..b0837ee9d 100644 --- a/api/http/refresh_test.go +++ b/api/http/refresh_test.go @@ -12,14 +12,11 @@ import ( func TestRefreshHandler(t *testing.T) { - var mockDB mock.DatabaseService - var mockLog mock.LogService handler := RefreshHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, + Env: nil, + Log: &mockLog, } reqBody := strings.NewReader(validJSON) diff --git a/api/http/saml.go b/api/http/saml.go index a8c319c37..b0fc0a457 100644 --- a/api/http/saml.go +++ b/api/http/saml.go @@ -6,7 +6,6 @@ import ( "os" "github.com/18F/e-QIP-prototype/api" - "github.com/18F/e-QIP-prototype/api/simplestore" ) var ( @@ -15,10 +14,9 @@ var ( // SamlRequestHandler is the handler for creating a SAML request. type SamlRequestHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - SAML api.SamlService + Env api.Settings + Log api.LogService + SAML api.SamlService } // ServeHTTP is the initial entry point for authentication. @@ -47,11 +45,10 @@ func (service SamlRequestHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque // SamlSLORequestHandler is the handler for creating a SAML Logout request type SamlSLORequestHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - SAML api.SamlService - Session api.SessionService + Env api.Settings + Log api.LogService + SAML api.SamlService + Session api.SessionService } // ServeHTTP is the initial entry point for authentication. @@ -90,12 +87,12 @@ func (service SamlSLORequestHandler) ServeHTTP(w http.ResponseWriter, r *http.Re // SamlResponseHandler is the callback handler for both login and logout SAML Responses. type SamlResponseHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - SAML api.SamlService - Session api.SessionService - Cookie SessionCookieService + Env api.Settings + Log api.LogService + SAML api.SamlService + Session api.SessionService + Store api.StorageService + Cookie SessionCookieService } // ServeHTTP is the callback handler for both login and logout SAML Responses. @@ -140,20 +137,16 @@ func (service SamlResponseHandler) serveAuthnResponse(encodedResponse string, w return } - // Associate with a database context. - account := &api.Account{ - Username: username, - } - if _, err := account.Get(service.Database, account.ID); err != nil { - service.Log.WarnError(api.NoAccount, err, api.LogFields{"username": username}) - + account, fetchErr := service.Store.FetchAccountByUsername(username) + if fetchErr != nil { + service.Log.WarnError(api.NoAccount, fetchErr, api.LogFields{"username": username}) redirectAccessDenied(w, r) } - sessionKey, authErr := service.Session.UserDidAuthenticate(account.ID, simplestore.NonNullString(sessionIndex)) + sessionKey, authErr := service.Session.UserDidAuthenticate(account.ID, api.NonNullString(sessionIndex)) if authErr != nil { - service.Log.WarnError("bad session get", authErr, api.LogFields{"account": account.ID}) - RespondWithStructuredError(w, "bad session get", http.StatusInternalServerError) + service.Log.WarnError(api.SessionCreationFailed, authErr, api.LogFields{"account": account.ID}) + RespondWithStructuredError(w, api.SessionCreationFailed, http.StatusInternalServerError) return } diff --git a/api/http/saml_test.go b/api/http/saml_test.go index 47b3538ed..c6d9cca12 100644 --- a/api/http/saml_test.go +++ b/api/http/saml_test.go @@ -16,7 +16,6 @@ import ( func TestSamlRequestHandlerSamlNotEnabled(t *testing.T) { os.Setenv(api.SamlEnabled, "0") - var mockDB mock.DatabaseService var mockLog mock.LogService samlService := &saml.Service{ @@ -25,10 +24,9 @@ func TestSamlRequestHandlerSamlNotEnabled(t *testing.T) { } handler := SamlRequestHandler{ - Env: &env.Native{}, - Log: &mockLog, - Database: &mockDB, - SAML: samlService, + Env: &env.Native{}, + Log: &mockLog, + SAML: samlService, } reqBody := strings.NewReader(validJSON) @@ -64,7 +62,6 @@ func TestSamlRequestHandlerSamlNotEnabled(t *testing.T) { func TestSamlRequestHandlerSamlSLONotEnabled(t *testing.T) { os.Setenv(api.SamlSLONotEnabled, "0") - var mockDB mock.DatabaseService var mockLog mock.LogService samlService := &saml.Service{ @@ -73,10 +70,9 @@ func TestSamlRequestHandlerSamlSLONotEnabled(t *testing.T) { } handler := SamlSLORequestHandler{ - Env: &env.Native{}, - Log: &mockLog, - Database: &mockDB, - SAML: samlService, + Env: &env.Native{}, + Log: &mockLog, + SAML: samlService, } reqBody := strings.NewReader(validJSON) diff --git a/api/http/save.go b/api/http/save.go index e6d8629e7..fc7c179f5 100644 --- a/api/http/save.go +++ b/api/http/save.go @@ -9,10 +9,9 @@ import ( // SaveHandler is the handler for saving the application. type SaveHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - Store api.StorageService + Env api.Settings + Log api.LogService + Store api.StorageService } // ServeHTTP saves a payload of information for the provided account. diff --git a/api/http/save_test.go b/api/http/save_test.go index 62ad4b6b3..9e7a30b71 100644 --- a/api/http/save_test.go +++ b/api/http/save_test.go @@ -88,17 +88,14 @@ func (s *saveStoreOtherCreateError) CreateApplication(app api.Application) error func TestSaveHandler(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore saveStore var mockLog mock.LogService handler := SaveHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } reqBody := strings.NewReader(validJSON) @@ -136,17 +133,11 @@ func TestSaveHandler(t *testing.T) { func TestSaveHandlerBadEntity(t *testing.T) { - var mockDB mock.DatabaseService - mockDB.SelectFn = func(query interface{}) error { - return nil - } - var mockLog mock.LogService handler := SaveHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, + Env: nil, + Log: &mockLog, } requestJSON := ` @@ -185,17 +176,14 @@ func TestSaveHandlerBadEntity(t *testing.T) { func TestSaveHandlerLockedAccount(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore saveStore var mockLog mock.LogService handler := SaveHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } reqBody := strings.NewReader(validJSON) @@ -231,17 +219,14 @@ func TestSaveHandlerLockedAccount(t *testing.T) { func TestSaveHandlerBadPayloadDeserialization(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore saveStore var mockLog mock.LogService handler := SaveHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } // Pass in bad JSON--undeserializeable requestJSON := `{[}` @@ -277,17 +262,14 @@ func TestSaveHandlerBadPayloadDeserialization(t *testing.T) { func TestCreateApplicationExistsFailure(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore saveStoreApplicationExistsError var mockLog mock.LogService handler := SaveHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } reqBody := strings.NewReader(validJSON) @@ -326,17 +308,14 @@ func TestCreateApplicationExistsFailure(t *testing.T) { func TestCreateApplicationAppDoesNotExistFailure(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore saveStoreApplicationExistsError var mockLog mock.LogService handler := SaveHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } reqBody := strings.NewReader(validJSON) @@ -375,17 +354,14 @@ func TestCreateApplicationAppDoesNotExistFailure(t *testing.T) { func TestSaveApplicationErrorResolves(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore saveStoreErrorResolves var mockLog mock.LogService handler := SaveHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } reqBody := strings.NewReader(validJSON) @@ -422,17 +398,14 @@ func TestSaveApplicationErrorResolves(t *testing.T) { func TestOtherSaveAppFailure(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore saveStoreOtherSaveError var mockLog mock.LogService handler := SaveHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } reqBody := strings.NewReader(validJSON) @@ -471,17 +444,14 @@ func TestOtherSaveAppFailure(t *testing.T) { func TestOtherCreateApplicationFailure(t *testing.T) { - var mockDB mock.DatabaseService - var mockStore saveStoreOtherCreateError var mockLog mock.LogService handler := SaveHandler{ - Env: nil, - Log: &mockLog, - Database: &mockDB, - Store: &mockStore, + Env: nil, + Log: &mockLog, + Store: &mockStore, } reqBody := strings.NewReader(validJSON) diff --git a/api/http/status.go b/api/http/status.go index f40a2d5a3..7f4042d34 100644 --- a/api/http/status.go +++ b/api/http/status.go @@ -10,10 +10,9 @@ import ( // StatusHandler is the handler for the application status. type StatusHandler struct { - Env api.Settings - Log api.LogService - Database api.DatabaseService - Store api.StorageService + Env api.Settings + Log api.LogService + Store api.StorageService } // formStatusInfo represents extra information associated with the application diff --git a/api/http/submit.go b/api/http/submit.go index ab8e017dd..300cbaaf6 100644 --- a/api/http/submit.go +++ b/api/http/submit.go @@ -13,7 +13,6 @@ import ( type SubmitHandler struct { Env api.Settings Log api.LogService - Database api.DatabaseService Store api.StorageService Submitter admin.Submitter } @@ -31,7 +30,7 @@ func (service SubmitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - xml, _, submitErr := service.Submitter.FilesForSubmission(account.ID) + xml, _, submitErr := service.Submitter.FilesForSubmission(account) if submitErr != nil { service.Log.WarnError(api.PdfError, submitErr, api.LogFields{}) RespondWithStructuredError(w, api.PdfError, http.StatusInternalServerError) @@ -46,8 +45,9 @@ func (service SubmitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - if _, err := account.Save(service.Database, account.ID); err != nil { - service.Log.WarnError(api.AccountUpdateError, err, api.LogFields{}) + updateErr := service.Store.UpdateAccountStatus(&account) + if updateErr != nil { + service.Log.WarnError(api.AccountUpdateError, updateErr, api.LogFields{}) RespondWithStructuredError(w, api.AccountUpdateError, http.StatusInternalServerError) return } diff --git a/api/integration/add_empty_value_test.go b/api/integration/add_empty_value_test.go index d9428776f..15f6492aa 100644 --- a/api/integration/add_empty_value_test.go +++ b/api/integration/add_empty_value_test.go @@ -9,7 +9,7 @@ import ( func TestAddEmptyValue(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) section := readTestData(t, "../testdata/history/history-employment-no-value.json") diff --git a/api/integration/application_status_test.go b/api/integration/application_status_test.go index 597a6684e..ccf916cc7 100644 --- a/api/integration/application_status_test.go +++ b/api/integration/application_status_test.go @@ -18,7 +18,7 @@ func TestStatus(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) sections := []struct { path string @@ -45,10 +45,9 @@ func TestStatus(t *testing.T) { w, req := standardResponseAndRequest("GET", "/me/status", nil, account) statusHandler := http.StatusHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } statusHandler.ServeHTTP(w, req) @@ -139,15 +138,14 @@ func TestStatusFetchError(t *testing.T) { var mockStore errorStatusStore services := cleanTestServices(t) defer services.closeDB() - account := createLockedTestAccount(t, services.db) + account := createLockedTestAccount(t, services.store) w, req := standardResponseAndRequest("GET", "/me/status", nil, account) statusHandler := http.StatusHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: &mockStore, + Env: services.env, + Log: services.log, + Store: &mockStore, } statusHandler.ServeHTTP(w, req) @@ -175,15 +173,14 @@ func TestLockedStatus(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createLockedTestAccount(t, services.db) + account := createLockedTestAccount(t, services.store) w, req := standardResponseAndRequest("GET", "/me/status", nil, account) statusHandler := http.StatusHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } statusHandler.ServeHTTP(w, req) diff --git a/api/integration/basic_auth_test.go b/api/integration/basic_auth_test.go index 3964aba0a..7cff5e247 100644 --- a/api/integration/basic_auth_test.go +++ b/api/integration/basic_auth_test.go @@ -11,6 +11,8 @@ import ( "testing" "time" + "github.com/18F/e-QIP-prototype/api/simplestore" + "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/http" "github.com/18F/e-QIP-prototype/api/mock" @@ -25,6 +27,15 @@ func (s *errorDeleteApplicationStore) DeleteApplication(accountID int) error { return errors.New("delete application error") } +func setAccountPassword(t *testing.T, store simplestore.SimpleStore, account api.Account, password string) { + account.HashPassword(password) + + saveErr := store.SetAccountPasswordHash(account) + if saveErr != nil { + t.Fatal(saveErr) + } +} + func TestBasicAuthDisabled(t *testing.T) { // Basic Auth not enabled os.Setenv(api.BasicEnabled, "0") @@ -32,27 +43,17 @@ func TestBasicAuthDisabled(t *testing.T) { defer services.closeDB() sessionService := session.NewSessionService(5*time.Minute, services.store, services.log) - account := createTestAccount(t, services.db) - - authMembership := api.BasicAuthMembership{ - AccountID: account.ID, - } + account := createTestAccount(t, services.store) password := "so-basic" - authMembership.HashPassword(password) - - _, saveErr := authMembership.Save(services.db, 0) - if saveErr != nil { - t.Fatal(saveErr) - } + setAccountPassword(t, services.store, account, password) loginRequestHandler := http.BasicAuthHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, - Session: sessionService, - Cookie: http.NewSessionCookieService(true), + Env: services.env, + Log: services.log, + Store: services.store, + Session: sessionService, + Cookie: http.NewSessionCookieService(true), } responseWriter := httptest.NewRecorder() @@ -97,27 +98,17 @@ func TestBasicNoPassword(t *testing.T) { defer services.closeDB() sessionService := session.NewSessionService(5*time.Minute, services.store, services.log) - account := createTestAccount(t, services.db) - - authMembership := api.BasicAuthMembership{ - AccountID: account.ID, - } + account := createTestAccount(t, services.store) password := "" - authMembership.HashPassword(password) - - _, saveErr := authMembership.Save(services.db, 0) - if saveErr != nil { - t.Fatal(saveErr) - } + setAccountPassword(t, services.store, account, password) loginRequestHandler := http.BasicAuthHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, - Session: sessionService, - Cookie: http.NewSessionCookieService(true), + Env: services.env, + Log: services.log, + Store: services.store, + Session: sessionService, + Cookie: http.NewSessionCookieService(true), } responseWriter := httptest.NewRecorder() @@ -161,27 +152,17 @@ func TestBasicNoUsername(t *testing.T) { defer services.closeDB() sessionService := session.NewSessionService(5*time.Minute, services.store, services.log) - account := createTestAccount(t, services.db) - - authMembership := api.BasicAuthMembership{ - AccountID: account.ID, - } + account := createTestAccount(t, services.store) password := "sekrit" - authMembership.HashPassword(password) - - _, saveErr := authMembership.Save(services.db, 0) - if saveErr != nil { - t.Fatal(saveErr) - } + setAccountPassword(t, services.store, account, password) loginRequestHandler := http.BasicAuthHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, - Session: sessionService, - Cookie: http.NewSessionCookieService(true), + Env: services.env, + Log: services.log, + Store: services.store, + Session: sessionService, + Cookie: http.NewSessionCookieService(true), } responseWriter := httptest.NewRecorder() @@ -225,27 +206,17 @@ func TestBasicAccountError(t *testing.T) { defer services.closeDB() sessionService := session.NewSessionService(5*time.Minute, services.store, services.log) - account := createTestAccount(t, services.db) - - authMembership := api.BasicAuthMembership{ - AccountID: account.ID, - } + account := createTestAccount(t, services.store) password := "sekrit" - authMembership.HashPassword(password) - - _, saveErr := authMembership.Save(services.db, 0) - if saveErr != nil { - t.Fatal(saveErr) - } + setAccountPassword(t, services.store, account, password) loginRequestHandler := http.BasicAuthHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, - Session: sessionService, - Cookie: http.NewSessionCookieService(true), + Env: services.env, + Log: services.log, + Store: services.store, + Session: sessionService, + Cookie: http.NewSessionCookieService(true), } responseWriter := httptest.NewRecorder() @@ -291,27 +262,17 @@ func TestBasicWrongPassword(t *testing.T) { defer services.closeDB() sessionService := session.NewSessionService(5*time.Minute, services.store, services.log) - account := createTestAccount(t, services.db) - - authMembership := api.BasicAuthMembership{ - AccountID: account.ID, - } + account := createTestAccount(t, services.store) password := "sekrit" - authMembership.HashPassword(password) - - _, saveErr := authMembership.Save(services.db, 0) - if saveErr != nil { - t.Fatal(saveErr) - } + setAccountPassword(t, services.store, account, password) loginRequestHandler := http.BasicAuthHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, - Session: sessionService, - Cookie: http.NewSessionCookieService(true), + Env: services.env, + Log: services.log, + Store: services.store, + Session: sessionService, + Cookie: http.NewSessionCookieService(true), } responseWriter := httptest.NewRecorder() diff --git a/api/integration/clear_yes_no_test.go b/api/integration/clear_yes_no_test.go index e95882eee..9b8234ec3 100644 --- a/api/integration/clear_yes_no_test.go +++ b/api/integration/clear_yes_no_test.go @@ -20,17 +20,17 @@ func TestClearEmptyAccount(t *testing.T) { // get a setup environment. services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) // Hacky, but seems OK for these tests. Technically you shouldn't be able to submit // anything but a complete application, but I think it's ok to make these tests smaller. account.Submit() - _, saveErr := account.Save(services.db, account.ID) - if saveErr != nil { - t.Fatal(saveErr) + updateErr := services.store.UpdateAccountStatus(&account) + if updateErr != nil { + t.Fatal(updateErr) } - rejector := admin.NewRejecter(services.db, services.store) + rejector := admin.NewRejecter(services.store) err := rejector.Reject(&account) if err != nil { @@ -109,7 +109,7 @@ func getBasicBranches(t *testing.T, section api.Section, branchName string) (*ap } func rejectSection(t *testing.T, services serviceSet, json []byte, sectionName string) api.Section { - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) resp := saveJSON(services, json, account) if resp.StatusCode != 200 { @@ -119,23 +119,17 @@ func rejectSection(t *testing.T, services serviceSet, json []byte, sectionName s // Hacky, but seems OK for these tests. Technically you shouldn't be able to submit // anything but a complete application, but I think it's ok to make these tests smaller. account.Submit() - _, saveErr := account.Save(services.db, account.ID) - if saveErr != nil { - t.Fatal(saveErr) + updateErr := services.store.UpdateAccountStatus(&account) + if updateErr != nil { + t.Fatal(updateErr) } - rejector := admin.NewRejecter(services.db, services.store) + rejector := admin.NewRejecter(services.store) err := rejector.Reject(&account) if err != nil { t.Fatal("Failed to reject account: ", err) } - // reload the account now that it's been rejected - _, reloadErr := account.Get(services.db, account.ID) - if reloadErr != nil { - t.Fatal(reloadErr) - } - resetApp := getApplication(t, services, account) section := resetApp.Section(sectionName) @@ -1061,7 +1055,7 @@ func TestClearComplexSectionNos(t *testing.T) { t.Run(path.Base(clearTest.path), func(t *testing.T) { - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) sectionJSON := readTestData(t, clearTest.path) @@ -1073,23 +1067,17 @@ func TestClearComplexSectionNos(t *testing.T) { // Hacky, but seems OK for these tests. Technically you shouldn't be able to submit // anything but a complete application, but I think it's ok to make these tests smaller. account.Submit() - _, saveErr := account.Save(services.db, account.ID) - if saveErr != nil { - t.Fatal(saveErr) + updateErr := services.store.UpdateAccountStatus(&account) + if updateErr != nil { + t.Fatal(updateErr) } - rejector := admin.NewRejecter(services.db, services.store) + rejector := admin.NewRejecter(services.store) err := rejector.Reject(&account) if err != nil { t.Fatal("Failed to reject account: ", err) } - // reload the account now that it's been rejected - _, reloadErr := account.Get(services.db, account.ID) - if reloadErr != nil { - t.Fatal(reloadErr) - } - resetApp := getApplication(t, services, account) section := resetApp.Section(clearTest.name) diff --git a/api/integration/csrf_test.go b/api/integration/csrf_test.go index d98646df5..0a31e9108 100644 --- a/api/integration/csrf_test.go +++ b/api/integration/csrf_test.go @@ -24,15 +24,14 @@ func giveCookie(t *testing.T, response *gohttp.Response, request *gohttp.Request func TestCSRFToken(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) statusRespW, statusReq := standardResponseAndRequest("GET", "/me/status", nil, account) statusHandler := http.StatusHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } authKey := []byte("123456789012345678901234567890") @@ -69,10 +68,9 @@ func TestCSRFToken(t *testing.T) { giveCookie(t, statusResp, badReq, "_gorilla_csrf") saveHandler := http.SaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } wrappedSaveHandler := csrfMiddleware.Middleware(saveHandler) @@ -112,15 +110,14 @@ func TestCSRFToken(t *testing.T) { func TestGoodCSRFToken(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) statusRespW, statusReq := standardResponseAndRequest("GET", "/me/status", nil, account) statusHandler := http.StatusHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } authKey := []byte("123456789012345678901234567890") @@ -153,10 +150,9 @@ func TestGoodCSRFToken(t *testing.T) { idSection := readTestData(t, "../testdata/identification/identification-birthplace-full.json") saveHandler := http.SaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } wrappedSaveHandler := csrfMiddleware.Middleware(saveHandler) @@ -179,15 +175,14 @@ func TestGoodCSRFToken(t *testing.T) { func TestGarbageCSRFToken(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) statusRespW, statusReq := standardResponseAndRequest("GET", "/me/status", nil, account) statusHandler := http.StatusHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } authKey := []byte("123456789012345678901234567890") @@ -221,10 +216,9 @@ func TestGarbageCSRFToken(t *testing.T) { idSection := readTestData(t, "../testdata/identification/identification-birthplace-full.json") saveHandler := http.SaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } wrappedSaveHandler := csrfMiddleware.Middleware(saveHandler) diff --git a/api/integration/delete_attachment_test.go b/api/integration/delete_attachment_test.go index 2ceecf21f..3293210b2 100644 --- a/api/integration/delete_attachment_test.go +++ b/api/integration/delete_attachment_test.go @@ -27,7 +27,7 @@ func TestDeleteAttachment(t *testing.T) { os.Setenv(api.AttachmentsEnabled, "1") services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) certificationPath := "../testdata/attachments/signature-form.pdf" @@ -37,10 +37,9 @@ func TestDeleteAttachment(t *testing.T) { w := httptest.NewRecorder() createAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } createAttachmentHandler.ServeHTTP(w, req) @@ -69,10 +68,9 @@ func TestDeleteAttachment(t *testing.T) { }) delAttachmentHandler := http.AttachmentDeleteHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } delAttachmentHandler.ServeHTTP(delW, delReq) @@ -103,10 +101,9 @@ func TestDeleteAttachment(t *testing.T) { }) getAttachmentHandler := http.AttachmentGetHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } getAttachmentHandler.ServeHTTP(getW, getReq) @@ -124,7 +121,7 @@ func TestDeleteAttachmentDisabled(t *testing.T) { os.Setenv(api.AttachmentsEnabled, "1") services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) certificationPath := "../testdata/attachments/signature-form.pdf" @@ -134,10 +131,9 @@ func TestDeleteAttachmentDisabled(t *testing.T) { w := httptest.NewRecorder() createAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } createAttachmentHandler.ServeHTTP(w, req) @@ -168,10 +164,9 @@ func TestDeleteAttachmentDisabled(t *testing.T) { }) delAttachmentHandler := http.AttachmentDeleteHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } delAttachmentHandler.ServeHTTP(delW, delReq) @@ -197,15 +192,14 @@ func TestDeleteAttachmentDisabled(t *testing.T) { func TestDeleteAttachmentLockedAccount(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) // Lock account account.Status = api.StatusSubmitted delAttachmentHandler := http.AttachmentDeleteHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } w, indexReq := standardResponseAndRequest("GET", "/me/attachments/", nil, account) @@ -236,13 +230,12 @@ func TestDeleteAttachmentError(t *testing.T) { var mockStore errorDeleteAttachmentStore services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) delAttachmentHandler := http.AttachmentDeleteHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: &mockStore, + Env: services.env, + Log: services.log, + Store: &mockStore, } attachmentID := "121342" @@ -282,13 +275,12 @@ func TestDeleteAttachmentError(t *testing.T) { func TestDeleteAttachmentBadID(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) delAttachmentHandler := http.AttachmentDeleteHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } attachmentID := "NaN" diff --git a/api/integration/form_version_test.go b/api/integration/form_version_test.go index 9014ddc6c..c663e737e 100644 --- a/api/integration/form_version_test.go +++ b/api/integration/form_version_test.go @@ -13,15 +13,14 @@ func TestFormVersionReturned(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) w, r := standardResponseAndRequest("GET", "/me/form", nil, account) formHandler := http.FormHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } formHandler.ServeHTTP(w, r) @@ -67,7 +66,7 @@ func TestFormVersionSave(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) fmt.Println("Account ID", account.ID) diff --git a/api/integration/helpers_test.go b/api/integration/helpers_test.go index d7cfdf2f4..cd3307a58 100644 --- a/api/integration/helpers_test.go +++ b/api/integration/helpers_test.go @@ -23,7 +23,6 @@ import ( "github.com/18F/e-QIP-prototype/api/env" "github.com/18F/e-QIP-prototype/api/http" "github.com/18F/e-QIP-prototype/api/log" - "github.com/18F/e-QIP-prototype/api/postgresql" "github.com/18F/e-QIP-prototype/api/simplestore" ) @@ -36,16 +35,14 @@ var serializerInitializer = func() api.Serializer { type serviceSet struct { env api.Settings log api.LogService - db api.DatabaseService - store api.StorageService + store simplestore.SimpleStore } func (s serviceSet) closeDB() { - dbErr := s.db.Close() storeErr := s.store.Close() - if dbErr != nil || storeErr != nil { + if storeErr != nil { // Since this is always called in tests in defer, we just swallow the errors and log them. - fmt.Println("Got an error trying to close the db: ", dbErr, storeErr) + fmt.Println("Got an error trying to close the db: ", storeErr) } } @@ -58,7 +55,7 @@ func cleanTestServices(t *testing.T) serviceSet { log := &log.Service{Log: log.NewLogger()} - dbConf := postgresql.DBConfig{ + dbConf := simplestore.DBConfig{ User: env.String(api.DatabaseUser), Password: env.String(api.DatabasePassword), Address: env.String(api.DatabaseHost), @@ -66,11 +63,9 @@ func cleanTestServices(t *testing.T) serviceSet { SSLMode: env.String(api.DatabaseSSLMode), } - db := postgresql.NewPostgresService(dbConf, log) - serializer := serializerInitializer() - store, storeErr := simplestore.NewSimpleStore(postgresql.PostgresConnectURI(dbConf), log, serializer) + store, storeErr := simplestore.NewSimpleStore(simplestore.PostgresConnectURI(dbConf), log, serializer) if storeErr != nil { t.Fatal(storeErr) } @@ -78,7 +73,6 @@ func cleanTestServices(t *testing.T) serviceSet { return serviceSet{ env, log, - db, store, } } @@ -103,45 +97,45 @@ func randomEmail() string { } -func createLockedTestAccount(t *testing.T, db api.DatabaseService) api.Account { +func createLockedTestAccount(t *testing.T, store api.StorageService) api.Account { t.Helper() email := randomEmail() account := api.Account{ Username: email, - Email: simplestore.NonNullString(email), + Email: api.NonNullString(email), FormType: "SF86", FormVersion: "2017-07", Status: api.StatusSubmitted, ExternalID: uuid.New().String(), } - _, err := account.Save(db, -1) - if err != nil { - t.Fatal(err) + createErr := store.CreateAccount(&account) + if createErr != nil { + t.Fatal(createErr) } return account } -func createTestAccount(t *testing.T, db api.DatabaseService) api.Account { +func createTestAccount(t *testing.T, store api.StorageService) api.Account { t.Helper() email := randomEmail() account := api.Account{ Username: email, - Email: simplestore.NonNullString(email), + Email: api.NonNullString(email), FormType: "SF86", FormVersion: "2017-07", Status: api.StatusIncomplete, ExternalID: uuid.New().String(), } - _, err := account.Save(db, -1) - if err != nil { - t.Fatal(err) + createErr := store.CreateAccount(&account) + if createErr != nil { + t.Fatal(createErr) } return account @@ -175,10 +169,9 @@ func saveJSON(services serviceSet, json []byte, account api.Account) *gohttp.Res w, r := standardResponseAndRequest("POST", "/me/save", strings.NewReader(string(json)), account) saveHandler := http.SaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } saveHandler.ServeHTTP(w, r) @@ -218,10 +211,9 @@ func getForm(services serviceSet, account api.Account) *gohttp.Response { w, r := standardResponseAndRequest("GET", "/me/form", nil, account) formHandler := http.FormHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } formHandler.ServeHTTP(w, r) diff --git a/api/integration/http_session_test.go b/api/integration/http_session_test.go index 92c7112d0..7385f611a 100644 --- a/api/integration/http_session_test.go +++ b/api/integration/http_session_test.go @@ -24,10 +24,9 @@ func makeAuthenticatedFormRequest(services serviceSet, sessionService *session.S sessionMiddleware := http.NewSessionMiddleware(services.log, sessionService) formHandler := http.FormHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } wrappedHandler := sessionMiddleware.Middleware(formHandler) @@ -82,15 +81,14 @@ func TestFormIndent(t *testing.T) { os.Setenv(api.IndentJSON, "1") services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) w, r := standardResponseAndRequest("GET", "/me/form", nil, account) formHandler := http.FormHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } formHandler.ServeHTTP(w, r) @@ -125,11 +123,11 @@ func TestFullSessionHTTPFlow_SAMLAuthenticated(t *testing.T) { } loginRequestHandler := http.SamlResponseHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - SAML: samlService, - Session: sessionService, + Env: services.env, + Log: services.log, + SAML: samlService, + Store: services.store, + Session: sessionService, } conf := saml.TestResponseConfig{ @@ -205,11 +203,10 @@ func TestFullSessionHTTPFlow_SAMLAuthenticated(t *testing.T) { // now, get a logout request, make sure it has a session index logoutRequestHandler := http.SamlSLORequestHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - SAML: samlService, - Session: sessionService, + Env: services.env, + Log: services.log, + SAML: samlService, + Session: sessionService, } sessionMiddleware := http.NewSessionMiddleware(services.log, sessionService) @@ -265,11 +262,10 @@ func TestFullSessionHTTPFlow_SAMLFailure(t *testing.T) { } loginRequestHandler := http.SamlResponseHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - SAML: samlService, - Session: sessionService, + Env: services.env, + Log: services.log, + SAML: samlService, + Session: sessionService, } conf := saml.TestResponseConfig{ @@ -315,27 +311,17 @@ func TestFullSessionHTTPFlow_BasicAuthenticated(t *testing.T) { defer services.closeDB() sessionService := session.NewSessionService(5*time.Minute, services.store, services.log) - account := createTestAccount(t, services.db) - - authMembership := api.BasicAuthMembership{ - AccountID: account.ID, - } + account := createTestAccount(t, services.store) password := "so-basic" - authMembership.HashPassword(password) - - _, saveErr := authMembership.Save(services.db, 0) - if saveErr != nil { - t.Fatal(saveErr) - } + setAccountPassword(t, services.store, account, password) loginRequestHandler := http.BasicAuthHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, - Session: sessionService, - Cookie: http.NewSessionCookieService(true), + Env: services.env, + Log: services.log, + Store: services.store, + Session: sessionService, + Cookie: http.NewSessionCookieService(true), } responseWriter := httptest.NewRecorder() diff --git a/api/integration/list_attachment_test.go b/api/integration/list_attachment_test.go index 809f747c1..9330b2901 100644 --- a/api/integration/list_attachment_test.go +++ b/api/integration/list_attachment_test.go @@ -29,20 +29,18 @@ func TestListAttachments(t *testing.T) { os.Setenv(api.AttachmentsEnabled, "1") services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) listAttachmentHandler := http.AttachmentListHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } createAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } createdAttachments := map[int]api.Attachment{} @@ -141,13 +139,12 @@ func TestListAttachmentDisabled(t *testing.T) { os.Setenv(api.AttachmentsEnabled, "0") services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) listAttachmentHandler := http.AttachmentListHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } w, indexReq := standardResponseAndRequest("GET", "/me/attachments/", nil, account) @@ -176,13 +173,12 @@ func TestListAttachmentError(t *testing.T) { var mockStore errorListAttachmentStore services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) listAttachmentHandler := http.AttachmentListHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: &mockStore, + Env: services.env, + Log: services.log, + Store: &mockStore, } w, indexReq := standardResponseAndRequest("GET", "/me/attachments/", nil, account) @@ -214,13 +210,12 @@ func TestListNoAttachments(t *testing.T) { os.Setenv(api.AttachmentsEnabled, "1") services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) listAttachmentHandler := http.AttachmentListHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } w, indexReq := standardResponseAndRequest("GET", "/me/attachments/", nil, account) diff --git a/api/integration/reject_test.go b/api/integration/reject_test.go index ef15dd8de..ce67c39f6 100644 --- a/api/integration/reject_test.go +++ b/api/integration/reject_test.go @@ -17,16 +17,15 @@ func TestDeleteSignaturePDFs(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) xmlService := xml.NewXMLService("../templates/") pdfService := pdf.NewPDFService("../pdf/templates/") - submitter := admin.NewSubmitter(services.db, services.store, xmlService, pdfService) + submitter := admin.NewSubmitter(services.store, xmlService, pdfService) submitHandler := http.SubmitHandler{ Env: services.env, Log: services.log, - Database: services.db, Store: services.store, Submitter: submitter, } @@ -41,10 +40,9 @@ func TestDeleteSignaturePDFs(t *testing.T) { w := httptest.NewRecorder() createAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } createAttachmentHandler.ServeHTTP(w, req) @@ -79,14 +77,13 @@ func TestDeleteSignaturePDFs(t *testing.T) { t.Fatal("submit didn't succeed") } - // reload account now that it's been submitted. - loadErr := account.Find(services.db) - if loadErr != nil { - t.Fatal(loadErr) + account, fetchErr := services.store.FetchAccountByUsername(account.Username) + if fetchErr != nil { + t.Fatal(fetchErr) } // Reject this submission - rejector := admin.NewRejecter(services.db, services.store) + rejector := admin.NewRejecter(services.store) err := rejector.Reject(&account) if err != nil { t.Fatal("Failed to reject account: ", err) @@ -94,10 +91,9 @@ func TestDeleteSignaturePDFs(t *testing.T) { // Finally, check that only the orignally uploaded attachment remains listAttachmentHandler := http.AttachmentListHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } w, indexReq := standardResponseAndRequest("GET", "/me/attachments/", nil, account) diff --git a/api/integration/save_attachment_test.go b/api/integration/save_attachment_test.go index 1e1c18254..5caab134f 100644 --- a/api/integration/save_attachment_test.go +++ b/api/integration/save_attachment_test.go @@ -45,7 +45,7 @@ func TestSaveAttachment(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) certificationPath := "../testdata/attachments/signature-form.pdf" @@ -56,10 +56,9 @@ func TestSaveAttachment(t *testing.T) { w := httptest.NewRecorder() createAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } createAttachmentHandler.ServeHTTP(w, req) @@ -90,10 +89,9 @@ func TestSaveAttachment(t *testing.T) { }) getAttachmentHandler := http.AttachmentGetHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } getAttachmentHandler.ServeHTTP(getW, getReq) @@ -127,13 +125,12 @@ func TestSaveAttachmentDisabled(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) saveAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } // Don't need real file to upload since we expect error first @@ -164,15 +161,14 @@ func TestSaveAttachmentLockedAccount(t *testing.T) { os.Setenv(api.AttachmentsEnabled, "1") services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) // Lock account account.Status = api.StatusSubmitted saveAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } // Don't need file to upload since we expect to error first @@ -203,13 +199,12 @@ func TestSaveAttachmentNoFile(t *testing.T) { os.Setenv(api.AttachmentsEnabled, "1") services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) saveAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } w, indexReq := standardResponseAndRequest("GET", "/me/attachments/", nil, account) @@ -239,7 +234,7 @@ func TestSaveAttachmentTooBig(t *testing.T) { os.Setenv(api.FileMaximumSize, "1") services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) certificationPath := "../testdata/attachments/signature-form.pdf" @@ -250,10 +245,9 @@ func TestSaveAttachmentTooBig(t *testing.T) { w := httptest.NewRecorder() createAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } createAttachmentHandler.ServeHTTP(w, req) @@ -292,7 +286,7 @@ func TestCreateAttachmentError(t *testing.T) { var mockStore errorCreateAttachmentStore services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) certificationPath := "../testdata/attachments/signature-form.pdf" @@ -303,10 +297,9 @@ func TestCreateAttachmentError(t *testing.T) { w := httptest.NewRecorder() createAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: &mockStore, + Env: services.env, + Log: services.log, + Store: &mockStore, } createAttachmentHandler.ServeHTTP(w, req) @@ -335,7 +328,7 @@ func TestGetAttachmentsDisabled(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) certificationPath := "../testdata/attachments/signature-form.pdf" @@ -346,10 +339,9 @@ func TestGetAttachmentsDisabled(t *testing.T) { w := httptest.NewRecorder() createAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } createAttachmentHandler.ServeHTTP(w, req) @@ -381,10 +373,9 @@ func TestGetAttachmentsDisabled(t *testing.T) { }) getAttachmentHandler := http.AttachmentGetHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } getAttachmentHandler.ServeHTTP(getW, getReq) @@ -412,7 +403,7 @@ func TestGetAttachmentBadID(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) certificationPath := "../testdata/attachments/signature-form.pdf" @@ -423,10 +414,9 @@ func TestGetAttachmentBadID(t *testing.T) { w := httptest.NewRecorder() createAttachmentHandler := http.AttachmentSaveHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } createAttachmentHandler.ServeHTTP(w, req) @@ -453,10 +443,9 @@ func TestGetAttachmentBadID(t *testing.T) { }) getAttachmentHandler := http.AttachmentGetHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } getAttachmentHandler.ServeHTTP(getW, getReq) diff --git a/api/integration/save_sections_test.go b/api/integration/save_sections_test.go index 65779ee8d..4255e5a15 100644 --- a/api/integration/save_sections_test.go +++ b/api/integration/save_sections_test.go @@ -11,7 +11,7 @@ import ( func TestSaveSection(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) tests := []struct { path string @@ -153,7 +153,7 @@ func TestSaveSection(t *testing.T) { func TestSaveMultipleSections(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) employmentSection := readTestData(t, "../testdata/history/history-employment-full.json") @@ -230,7 +230,7 @@ func TestDeleteApplication(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) section := readTestData(t, "../testdata/identification/identification-birthplace-full.json") diff --git a/api/integration/session_test.go b/api/integration/session_test.go index 2e4deadbf..744260f3a 100644 --- a/api/integration/session_test.go +++ b/api/integration/session_test.go @@ -6,7 +6,6 @@ import ( "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/session" - "github.com/18F/e-QIP-prototype/api/simplestore" ) func TestFullSessionFlow(t *testing.T) { @@ -15,13 +14,13 @@ func TestFullSessionFlow(t *testing.T) { defer services.closeDB() // actually *returning* clean test services // create a user - testUser := createTestAccount(t, services.db) + testUser := createTestAccount(t, services.store) // get a session service sessionService := session.NewSessionService(5*time.Second, services.store, services.log) // create a session for the user - sessionKey, userAuthdErr := sessionService.UserDidAuthenticate(testUser.ID, simplestore.NullString()) + sessionKey, userAuthdErr := sessionService.UserDidAuthenticate(testUser.ID, api.NullString()) if userAuthdErr != nil { t.Fatal("Failed to authenticate user", userAuthdErr) } diff --git a/api/integration/submission_test.go b/api/integration/submission_test.go index 9c5d6bee2..177daff51 100644 --- a/api/integration/submission_test.go +++ b/api/integration/submission_test.go @@ -89,7 +89,7 @@ func checkInfoRelease(t *testing.T, release api.Attachment) { func TestSubmitter(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) mockClock := clock.NewMock() const base = 1536570831 // Epoch seconds for September 10, 2018 PDT @@ -97,12 +97,11 @@ func TestSubmitter(t *testing.T) { xmlService := xml.NewXMLServiceWithMockClock("../templates/", mockClock) pdfService := pdf.NewPDFService("../pdf/templates/") - submitter := admin.NewSubmitter(services.db, services.store, xmlService, pdfService) + submitter := admin.NewSubmitter(services.store, xmlService, pdfService) submitHandler := http.SubmitHandler{ Env: services.env, Log: services.log, - Database: services.db, Store: services.store, Submitter: submitter, } @@ -138,10 +137,9 @@ func TestSubmitter(t *testing.T) { // Check that the generated PDFs can be retrieved via the API listAttachmentHandler := http.AttachmentListHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } // Enable Attachments @@ -184,10 +182,9 @@ func TestSubmitter(t *testing.T) { }) getAttachmentHandler := http.AttachmentGetHandler{ - Env: services.env, - Log: services.log, - Database: services.db, - Store: services.store, + Env: services.env, + Log: services.log, + Store: services.store, } getAttachmentHandler.ServeHTTP(w, req) @@ -218,7 +215,7 @@ func TestSubmitter(t *testing.T) { func TestLockedAccountSubmitter(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) account.Status = api.StatusSubmitted mockClock := clock.NewMock() @@ -227,12 +224,11 @@ func TestLockedAccountSubmitter(t *testing.T) { xmlService := xml.NewXMLServiceWithMockClock("../templates/", mockClock) pdfService := pdf.NewPDFService("../pdf/templates/") - submitter := admin.NewSubmitter(services.db, services.store, xmlService, pdfService) + submitter := admin.NewSubmitter(services.store, xmlService, pdfService) submitHandler := http.SubmitHandler{ Env: services.env, Log: services.log, - Database: services.db, Store: services.store, Submitter: submitter, } diff --git a/api/integration/validation_test.go b/api/integration/validation_test.go index 9dbdbfaa1..a61ae6b7f 100644 --- a/api/integration/validation_test.go +++ b/api/integration/validation_test.go @@ -15,7 +15,7 @@ import ( func TestCanValidateLocation(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) api.Geocode = mock.Geocoder{} location := readTestData(t, "../testdata/location.json") @@ -39,7 +39,7 @@ func TestCanValidateLocation(t *testing.T) { func TestCanNotValidateSomethingElse(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) api.Geocode = mock.Geocoder{} location := readTestData(t, "../testdata/history/history-employment.json") @@ -63,7 +63,7 @@ func TestCanNotValidateSomethingElse(t *testing.T) { func TestValidateHandlerInvalidAddress(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) api.Geocode = mock.Geocoder{} // Pass a bad address @@ -97,7 +97,7 @@ func TestValidateHandlerInvalidAddress(t *testing.T) { func TestValidateHandlerBadEntity(t *testing.T) { services := cleanTestServices(t) defer services.closeDB() - account := createTestAccount(t, services.db) + account := createTestAccount(t, services.store) api.Geocode = mock.Geocoder{} // Pass a non-JSON address diff --git a/api/log.go b/api/log.go index e792c0814..50454d7ae 100644 --- a/api/log.go +++ b/api/log.go @@ -36,6 +36,7 @@ const ( SessionExpired = "Auth failed because of an expired session" SessionDoesNotExist = "Auth failed because of an invalid session" SessionUnexpectedError = "An unexpected error occured while checking the session." + SessionCreationFailed = "An unexpected error occured creating a session" CSRFTokenMissing = "The state modifying request is missing a CSRF Token" CSRFTokenInvalid = "The state modifying request has an invalid CSRF Token" RequestIsMissingSessionCookie = "Unauthorized: Request is missing a session cookie" diff --git a/api/mock/database.go b/api/mock/database.go deleted file mode 100644 index ca824fbac..000000000 --- a/api/mock/database.go +++ /dev/null @@ -1,75 +0,0 @@ -package mock - -// DatabaseService mock implementation. -type DatabaseService struct { - SelectFn func(query interface{}) error - SelectCount int -} - -// Raw executes a string of SQL. -func (service *DatabaseService) Raw(query interface{}, params ...interface{}) error { - return nil -} - -// Find will check if the model exists and run the additional functionality. -func (service *DatabaseService) Find(query interface{}, callback func(query interface{})) { -} - -// FindAll instances of a type of model. -func (service *DatabaseService) FindAll(model interface{}) error { - return nil -} - -// Where is a conditional selection based on the model in the data store. -func (service *DatabaseService) Where(model interface{}, condition string, params ...interface{}) error { - return nil -} - -// ColumnsWhere is a Conditional selection based on the model in the data store only returning specific quoted columns. -func (service *DatabaseService) ColumnsWhere(model interface{}, columns []string, condition string, params ...interface{}) error { - return nil -} - -// Count return the number of rows found. -func (service *DatabaseService) Count(model interface{}, condition string, params ...interface{}) int { - return 0 -} - -// CountExpr return the number of rows found with an expression. -func (service *DatabaseService) CountExpr(model interface{}, expr string, retval interface{}, condition string, params ...interface{}) { -} - -// Array fills an array from the model and expression. -func (service *DatabaseService) Array(model interface{}, expr string, retval interface{}, condition string, params ...interface{}) { -} - -// Insert persists the new model in the data store -func (service *DatabaseService) Insert(query ...interface{}) error { - return nil -} - -// Update persists the existing model in the data store -func (service *DatabaseService) Update(query interface{}) error { - return nil -} - -// Save persists the model in the data store -func (service *DatabaseService) Save(query ...interface{}) error { - return nil -} - -// Delete removes the model from the data store -func (service *DatabaseService) Delete(query interface{}) error { - return nil -} - -// Select returns the model from the data store -func (service *DatabaseService) Select(query interface{}) error { - service.SelectCount++ - return service.SelectFn(query) -} - -// Close closes the db connection -func (service *DatabaseService) Close() error { - return nil -} diff --git a/api/mock/store.go b/api/mock/store.go index d0b207144..516cb33ee 100644 --- a/api/mock/store.go +++ b/api/mock/store.go @@ -75,6 +75,27 @@ func (s *StorageService) ExtendAndFetchSessionAccount(sessionKey string, session return api.Account{}, api.Session{}, nil } +func (s *StorageService) CreateAccount(account *api.Account) error { + return nil +} + +func (s *StorageService) FetchAccountByUsername(username string) (api.Account, error) { + return api.Account{}, nil +} + +func (s *StorageService) FetchAccountByExternalID(externalID string) (api.Account, error) { + return api.Account{}, nil +} + +func (s *StorageService) FetchAccountWithPasswordHash(username string) (api.Account, error) { + return api.Account{}, nil +} + +// UpdateAccountStatus updates an account +func (s *StorageService) UpdateAccountStatus(account *api.Account) error { + return nil +} + // Close closes the db connection func (s *StorageService) Close() error { return nil diff --git a/api/postgresql/db.go b/api/postgresql/db.go deleted file mode 100644 index 97dbb1a28..000000000 --- a/api/postgresql/db.go +++ /dev/null @@ -1,205 +0,0 @@ -package postgresql - -import ( - "crypto/tls" - "fmt" - "net/url" - "strings" - "time" - - "github.com/go-pg/pg" - - "github.com/18F/e-QIP-prototype/api" -) - -// Service to help abstract the technical implementation per driver used. -type Service struct { - Log api.LogService - Env api.Settings - database *pg.DB -} - -// DBConfig contains everything neccecary to setup a postgres db connection -type DBConfig struct { - User string - Password string - Address string - DBName string - SSLMode string -} - -// PostgresConnectURI creates a connection string, which is used by the golang sql Open() function -func PostgresConnectURI(conf DBConfig) string { - // By user (+ password) + database + host - uri := &url.URL{Scheme: "postgres"} - username := conf.User - - // Check if there is a password set. If not then we need to create - // the Userinfo structure in a different way so we don't include - // exta colons (:). - pw := conf.Password - if pw == "" { - uri.User = url.User(username) - } else { - uri.User = url.UserPassword(username, pw) - } - - // The database name will be part of the URI path so it needs - // a prefix of "/" - database := conf.DBName - uri.Path = fmt.Sprintf("/%s", database) - - // Host can be either "address + port" or just "address" - host := conf.Address - uri.Host = host - - if conf.SSLMode != "" { - params := url.Values{} - params.Set("sslmode", conf.SSLMode) - uri.RawQuery = params.Encode() - } - - return uri.String() -} - -// NewPostgresService returns a configured postgres service -func NewPostgresService(config DBConfig, logger api.LogService) *Service { - service := Service{ - Log: logger, - } - - options := &pg.Options{ - User: config.User, - Password: config.Password, - Addr: config.Address, - Database: config.DBName, - } - - if config.SSLMode != "" { - // Copied from the implementation of go-pg /sigh - switch config.SSLMode { - case "allow", "prefer", "require": - options.TLSConfig = &tls.Config{InsecureSkipVerify: true} - case "disable": - options.TLSConfig = nil - default: - service.Log.Warn("Unknown PG sslmdode specified, ignoring", api.LogFields{ - "sslmode": config.SSLMode, - }) - } - } - - db := pg.Connect(options) - - // Add logging - db.OnQueryProcessed(func(event *pg.QueryProcessedEvent) { - query, err := event.FormattedQuery() - if err == nil { - service.Log.Debug("Executed database query", api.LogFields{ - "elapsed": time.Since(event.StartTime), - "query": query, - }) - } - }) - - service.database = db - - return &service -} - -// Raw executes a string of SQL. -func (service *Service) Raw(query interface{}, params ...interface{}) error { - _, err := service.database.Exec(query, params...) - return err -} - -// Find will check if the model exists and run the additional functionality. -func (service *Service) Find(query interface{}, callback func(query interface{})) { - if count, err := service.database.Model(query).Count(); count > 0 && err == nil { - service.Select(query) - callback(query) - } -} - -// FindAll instances of a type of model. -func (service *Service) FindAll(query interface{}) error { - return explicitNotFoundError(service.database.Model(query).Select()) -} - -// Where is a conditional selection based on the model in the data store. -func (service *Service) Where(model interface{}, condition string, params ...interface{}) error { - return explicitNotFoundError(service.database.Model(model).Where(condition, params...).Select()) -} - -// ColumnsWhere is a conditional selection based on the model in the data store only returning specific quoted columns. -func (service *Service) ColumnsWhere(model interface{}, columns []string, condition string, params ...interface{}) error { - return explicitNotFoundError(service.database.Model(model).Column(columns...).Where(condition, params...).Select()) -} - -// Count return the number of rows found. -func (service *Service) Count(model interface{}, condition string, params ...interface{}) int { - count, _ := service.database.Model(model).Where(condition, params...).Count() - return count -} - -// CountExpr return the number of rows found with an expression. -func (service *Service) CountExpr(model interface{}, expr string, retval interface{}, condition string, params ...interface{}) { - service.database.Model(model).ColumnExpr(expr).Where(condition, params...).Select(retval) -} - -// Array fills an array from the model and expression. -func (service *Service) Array(model interface{}, expr string, retval interface{}, condition string, params ...interface{}) { - service.database.Model(model).ColumnExpr(expr).Where(condition, params...).Select(pg.Array(retval)) -} - -// Insert persists the new model in the data store -func (service *Service) Insert(query ...interface{}) error { - return service.database.Insert(query...) -} - -// Update persists the existing model in the data store -func (service *Service) Update(query interface{}) error { - return service.database.Update(query) -} - -// Save persists the model in the data store -func (service *Service) Save(query ...interface{}) error { - for _, q := range query { - err := service.Insert(q) - if err != nil { - // oh dear god, only try and update if it fails because of a duplicated primary key. - if strings.HasPrefix(err.Error(), "ERROR #23505 duplicate key value violates unique constraint") && strings.HasSuffix(err.Error(), `_pkey"`) { - err = service.Update(q) - } - } - - // If there were no rows found we already handle this. - if err != nil && err != pg.ErrNoRows { - return err - } - } - - return nil -} - -// Delete removes the model from the data store -func (service *Service) Delete(query interface{}) error { - return service.database.Delete(query) -} - -// Select returns the model from the data store -func (service *Service) Select(query interface{}) error { - return explicitNotFoundError(service.database.Select(query)) -} - -func explicitNotFoundError(err error) error { - if err == pg.ErrNoRows { - return api.DatabaseErrorNotFound("NOT_FOUND") - } - return err -} - -// Close closes the DB connection -func (service *Service) Close() error { - return service.database.Close() -} diff --git a/api/postgresql/db_test.go b/api/postgresql/db_test.go deleted file mode 100644 index 19337788c..000000000 --- a/api/postgresql/db_test.go +++ /dev/null @@ -1,94 +0,0 @@ -package postgresql - -import ( - "database/sql" - "testing" - - "github.com/18F/e-QIP-prototype/api" - "github.com/18F/e-QIP-prototype/api/env" - "github.com/18F/e-QIP-prototype/api/mock" -) - -func TestAccountPersistence(t *testing.T) { - settings := env.Native{} - settings.Configure() - - logger := &mock.LogService{Off: true} - - dbConf := DBConfig{ - User: settings.String(api.DatabaseUser), - Password: settings.String(api.DatabasePassword), - Address: settings.String(api.DatabaseHost), - DBName: settings.String(api.TestDatabaseName), - SSLMode: settings.String(api.DatabaseSSLMode), - } - - service := NewPostgresService(dbConf, logger) - defer service.Close() - - api.Geocode = mock.Geocoder{} - - account := api.Account{ - Username: "buzz1@example.com", - Email: sql.NullString{"buzz1@example.com", true}, - FormType: "SF86", - FormVersion: "2017-07", - Status: api.StatusIncomplete, - ExternalID: "ccc21c9d-20c4-47fa-83ed-2f1bd26fac7d", - } - - _, err := account.Get(service, -1) - if err != nil { - if api.IsDatabaseErrorNotFound(err) { - _, err := account.Save(service, -1) - if err != nil { - t.Fatal(err) - } - } else { - t.Fatal(err) - } - } - - // modify and save again to trigger the Update behavior - account.Email = sql.NullString{"buzz2@example.com", true} - - _, saveAgainErr := account.Save(service, -1) - if saveAgainErr != nil { - t.Fatal(saveAgainErr) - } - - fetchedAccount := api.Account{ - ID: account.ID, - } - - _, getErr := fetchedAccount.Get(service, account.ID) - if getErr != nil { - t.Fatal(getErr) - } - - if fetchedAccount.Username != account.Username { - t.Log("Should have gotten matching Usernames", fetchedAccount.Username, account.Username) - t.Fail() - } - - if fetchedAccount.Email != account.Email { - t.Log("Should have gotten matching Emails", fetchedAccount.Email, account.Email) - t.Fail() - } - - if fetchedAccount.FormType != account.FormType { - t.Log("Should have gotten matching FormTypes", fetchedAccount.FormType, account.FormType) - t.Fail() - } - - if fetchedAccount.FormVersion != account.FormVersion { - t.Log("Should have gotten matching FormVersions", fetchedAccount.FormVersion, account.FormVersion) - t.Fail() - } - - if fetchedAccount.ExternalID != account.ExternalID { - t.Log("Should have gotten matching ExternalIDs", fetchedAccount.ExternalID, account.ExternalID) - t.Fail() - } - -} diff --git a/api/postgresql/testdata b/api/postgresql/testdata deleted file mode 120000 index 4f7c11684..000000000 --- a/api/postgresql/testdata +++ /dev/null @@ -1 +0,0 @@ -../testdata \ No newline at end of file diff --git a/api/session/session_test.go b/api/session/session_test.go index fd12fc283..f51890b73 100644 --- a/api/session/session_test.go +++ b/api/session/session_test.go @@ -2,20 +2,64 @@ package session import ( "fmt" + "math/rand" "os" "testing" "time" + "github.com/google/uuid" "github.com/pkg/errors" "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/env" "github.com/18F/e-QIP-prototype/api/log" "github.com/18F/e-QIP-prototype/api/mock" - "github.com/18F/e-QIP-prototype/api/postgresql" "github.com/18F/e-QIP-prototype/api/simplestore" ) +// randomEmail an example.com email address with 10 random characters +func randomEmail() string { + + rand.Seed(time.Now().UTC().UnixNano()) + + len := 10 + bytes := make([]byte, len) + for i := 0; i < len; i++ { + aint := int('a') + zint := int('z') + char := aint + rand.Intn(zint-aint) + bytes[i] = byte(char) + } + + email := string(bytes) + "@example.com" + + return email + +} + +func createTestAccount(t *testing.T, store api.StorageService) api.Account { + t.Helper() + + email := randomEmail() + + account := api.Account{ + Username: email, + Email: api.NonNullString(email), + FormType: "SF86", + FormVersion: "2017-07", + Status: api.StatusIncomplete, + ExternalID: uuid.New().String(), + } + + createErr := store.CreateAccount(&account) + if createErr != nil { + t.Fatal(createErr) + } + + return account + +} + func getSimpleStore() api.StorageService { env := &env.Native{} os.Setenv(api.LogLevel, "info") @@ -23,14 +67,14 @@ func getSimpleStore() api.StorageService { log := &log.Service{Log: log.NewLogger()} - dbConf := postgresql.DBConfig{ + dbConf := simplestore.DBConfig{ User: env.String(api.DatabaseUser), Password: env.String(api.DatabasePassword), Address: env.String(api.DatabaseHost), DBName: env.String(api.TestDatabaseName), } - connString := postgresql.PostgresConnectURI(dbConf) + connString := simplestore.PostgresConnectURI(dbConf) serializer := simplestore.JSONSerializer{} @@ -50,7 +94,7 @@ func TestAuthExists(t *testing.T) { sessionLog := &mock.LogService{} session := NewSessionService(timeout, store, sessionLog) - session.UserDidAuthenticate(0, simplestore.NullString()) + session.UserDidAuthenticate(0, api.NullString()) } func TestLogSessionCreatedDestroyed(t *testing.T) { @@ -62,9 +106,9 @@ func TestLogSessionCreatedDestroyed(t *testing.T) { session := NewSessionService(timeout, store, sessionLog) - account := simplestore.CreateTestAccount(t, store.(simplestore.SimpleStore)) + account := createTestAccount(t, store.(simplestore.SimpleStore)) - sessionKey, authErr := session.UserDidAuthenticate(account.ID, simplestore.NullString()) + sessionKey, authErr := session.UserDidAuthenticate(account.ID, api.NullString()) if authErr != nil { t.Fatal(authErr) } @@ -136,9 +180,9 @@ func TestLogSessionExpired(t *testing.T) { session := NewSessionService(timeout, store, sessionLog) - account := simplestore.CreateTestAccount(t, store.(simplestore.SimpleStore)) + account := createTestAccount(t, store.(simplestore.SimpleStore)) - sessionKey, authErr := session.UserDidAuthenticate(account.ID, simplestore.NullString()) + sessionKey, authErr := session.UserDidAuthenticate(account.ID, api.NullString()) if authErr != nil { t.Fatal(authErr) } @@ -168,7 +212,7 @@ func TestLogSessionExpired(t *testing.T) { } // make sure you can re-auth after ending a session - _, newAuthErr := session.UserDidAuthenticate(account.ID, simplestore.NullString()) + _, newAuthErr := session.UserDidAuthenticate(account.ID, api.NullString()) if newAuthErr != nil { t.Fatal(newAuthErr) } @@ -185,9 +229,9 @@ func TestLogConcurrentSession(t *testing.T) { session := NewSessionService(timeout, store, sessionLog) - account := simplestore.CreateTestAccount(t, store.(simplestore.SimpleStore)) + account := createTestAccount(t, store.(simplestore.SimpleStore)) - _, authErr := session.UserDidAuthenticate(account.ID, simplestore.NullString()) + _, authErr := session.UserDidAuthenticate(account.ID, api.NullString()) if authErr != nil { t.Fatal(authErr) } @@ -198,7 +242,7 @@ func TestLogConcurrentSession(t *testing.T) { } // Now login again: - _, authAgainErr := session.UserDidAuthenticate(account.ID, simplestore.NullString()) + _, authAgainErr := session.UserDidAuthenticate(account.ID, api.NullString()) if authAgainErr != nil { t.Fatal(authAgainErr) } diff --git a/api/simplestore/accounts.go b/api/simplestore/accounts.go index b1c3167e5..d32025305 100644 --- a/api/simplestore/accounts.go +++ b/api/simplestore/accounts.go @@ -4,6 +4,7 @@ import ( "database/sql" "fmt" "strings" + "time" "github.com/18F/e-QIP-prototype/api" "github.com/pkg/errors" @@ -97,6 +98,27 @@ func (s SimpleStore) FetchAccountWithPasswordHash(username string) (api.Account, return account, nil } +// SetAccountPasswordHash sets the password hash for an account +func (s SimpleStore) SetAccountPasswordHash(account api.Account) error { + + // delete an existing one, if it exists + deleteQuery := `DELETE FROM basic_auth_memberships WHERE account_id = $1` + _, delErr := s.db.Exec(deleteQuery, account.ID) + if delErr != nil { + return errors.Wrap(delErr, "failed to delete old passwords") + } + + // create a new one. + insertQuery := `INSERT INTO basic_auth_memberships (account_id, password_hash, created) VALUES ($1, $2, $3)` + + _, insertErr := s.db.Exec(insertQuery, account.ID, account.PasswordHash, time.Now().UTC()) + if insertErr != nil { + return errors.Wrap(insertErr, "Failed to create a new password") + } + + return nil +} + // UpdateAccountStatus updates the status of an account. That should be the only thing we update post creation. func (s SimpleStore) UpdateAccountStatus(account *api.Account) error { @@ -118,3 +140,42 @@ func (s SimpleStore) UpdateAccountStatus(account *api.Account) error { return nil } + +// These functions are used by commands, and so are *not* part of the StorageService interface + +// ListAccounts returns a list of all accounts. This is only used by Commands +func (s SimpleStore) ListAccounts() ([]api.Account, error) { + + fetchQuery := `SELECT * from accounts` + + var accounts []api.Account + + fetchErr := s.db.Select(&accounts, fetchQuery) + if fetchErr != nil { + return []api.Account{}, errors.Wrap(fetchErr, "Couldn't list Accounts") + } + + return accounts, nil +} + +// UpdateAccountInfo updates the FormType and FormVersion for an account +func (s SimpleStore) UpdateAccountInfo(account *api.Account) error { + + updateQuery := `UPDATE accounts SET form_type = $1, form_version = $2 WHERE id = $3` + + result, updateErr := s.db.Exec(updateQuery, account.FormType, account.FormVersion, account.ID) + if updateErr != nil { + return errors.Wrap(updateErr, "Couldn't update Account") + } + + rows, affectedErr := result.RowsAffected() + if affectedErr != nil { + return errors.Wrap(affectedErr, "Bizzarely unable to read affected rows") + } + + if rows != 1 { + return api.ErrAccountDoesNotExist + } + + return nil +} diff --git a/api/simplestore/accounts_test.go b/api/simplestore/accounts_test.go index 38cfbd582..6ea308a23 100644 --- a/api/simplestore/accounts_test.go +++ b/api/simplestore/accounts_test.go @@ -3,6 +3,7 @@ package simplestore import ( "database/sql" "fmt" + "strings" "testing" "github.com/18F/e-QIP-prototype/api" @@ -13,7 +14,7 @@ func TestFetchAccountEmail(t *testing.T) { store := getSimpleStore() defer store.Close() - newAccount := CreateTestAccount(t, store) + newAccount := createTestAccount(t, store) if newAccount.ID <= 0 { t.Fatal("didn't get a valid ID") @@ -79,11 +80,63 @@ func TestFetchAccountWithPassword(t *testing.T) { fmt.Println(fetchedAccount.PasswordHash.String) } +func TestAccountPasswordHash(t *testing.T) { + store := getSimpleStore() + defer store.Close() + + account := createTestAccount(t, store) + + firstPasswordHash := "deadbeef" + account.PasswordHash = api.NonNullString(firstPasswordHash) + + setErr := store.SetAccountPasswordHash(account) + if setErr != nil { + t.Fatal(setErr) + } + + // Can we fetch the same account? + fetchedAccount, fetchErr := store.FetchAccountWithPasswordHash(account.Username) + if fetchErr != nil { + t.Fatal(fetchErr) + } + + if !fetchedAccount.PasswordHash.Valid { + t.Fatal("Invalid password hash!") + } + + if fetchedAccount.PasswordHash.String != firstPasswordHash { + t.Fatal("wrong password hash", fetchedAccount.PasswordHash.String) + } + + secondPasswordHash := "beefbaby" + account.PasswordHash = api.NonNullString(secondPasswordHash) + + setAgainErr := store.SetAccountPasswordHash(account) + if setAgainErr != nil { + t.Fatal(setAgainErr) + } + + // Can we fetch the same account? + fetchedAgainAccount, fetchAgainErr := store.FetchAccountWithPasswordHash(account.Username) + if fetchAgainErr != nil { + t.Fatal(fetchAgainErr) + } + + if !fetchedAgainAccount.PasswordHash.Valid { + t.Fatal("Invalid password hash!") + } + + if fetchedAgainAccount.PasswordHash.String != secondPasswordHash { + t.Fatal("wrong password hash", fetchedAgainAccount.PasswordHash.String) + } + +} + func TestFetchAccountExternalID(t *testing.T) { store := getSimpleStore() defer store.Close() - newAccount := CreateTestAccount(t, store) + newAccount := createTestAccount(t, store) if newAccount.ID <= 0 { t.Fatal("didn't get a valid ID") @@ -136,7 +189,7 @@ func TestFetchNoPassword(t *testing.T) { store := getSimpleStore() defer store.Close() - newAccount := CreateTestAccount(t, store) + newAccount := createTestAccount(t, store) if newAccount.ID <= 0 { t.Fatal("didn't get a valid ID") @@ -153,7 +206,7 @@ func TestUpdateAccountStatus(t *testing.T) { store := getSimpleStore() defer store.Close() - newAccount := CreateTestAccount(t, store) + newAccount := createTestAccount(t, store) success := newAccount.Submit() if !success { @@ -178,6 +231,38 @@ func TestUpdateAccountStatus(t *testing.T) { } +func TestUpdateAccountInfo(t *testing.T) { + store := getSimpleStore() + defer store.Close() + + newAccount := createTestAccount(t, store) + + newAccount.FormType = "SF85" + newAccount.FormVersion = "2017-12-draft7" + + updateErr := store.UpdateAccountInfo(&newAccount) + if updateErr != nil { + t.Fatal(updateErr) + } + + // Can we fetch the same newAccount? + fetchedAccount, fetchErr := store.FetchAccountByUsername(newAccount.Username) + if fetchErr != nil { + t.Fatal(fetchErr) + } + + if fetchedAccount.FormType != "SF85" { + t.Log("Should have got back the new FormType") + t.Fail() + } + + if fetchedAccount.FormVersion != "2017-12-draft7" { + t.Log("Should have got back the new FormVersion") + t.Fail() + } + +} + func TestCreateErr(t *testing.T) { store := getSimpleStore() defer store.Close() @@ -215,6 +300,11 @@ func TestFetchUnknownError(t *testing.T) { if fetchErr == nil && fetchErr != api.ErrAccountDoesNotExist { t.Fatal("Should have gotten an unknown error") } + + _, fetchErr = store.ListAccounts() + if fetchErr == nil { + t.Fatal("Should have gotten an unknown error") + } } func TestDoesntExist(t *testing.T) { @@ -246,6 +336,11 @@ func TestDoesntExist(t *testing.T) { if updateErr != api.ErrAccountDoesNotExist { t.Fatal(updateErr) } + + updateErr = store.UpdateAccountInfo(&account) + if updateErr != api.ErrAccountDoesNotExist { + t.Fatal(updateErr) + } } type badResult struct { @@ -270,16 +365,83 @@ func (db *badResultDB) Exec(query string, args ...interface{}) (sql.Result, erro func TestUpdateErrors(t *testing.T) { store := getErrorStore() - updateErr := store.UpdateAccountStatus(&api.Account{}) - if updateErr == nil { + updateStatusErr := store.UpdateAccountStatus(&api.Account{}) + if updateStatusErr == nil { + t.Fatal("Should have gotten an error") + } + + updateInfoErr := store.UpdateAccountInfo(&api.Account{}) + if updateInfoErr == nil { t.Fatal("Should have gotten an error") } store.db = &badResultDB{} badResultErr := store.UpdateAccountStatus(&api.Account{}) - if badResultErr == nil || badResultErr == updateErr { + if badResultErr == nil || badResultErr == updateStatusErr { + t.Fatal("Should have gotten a different error. ") + } + + badResultErr = store.UpdateAccountInfo(&api.Account{}) + if badResultErr == nil || badResultErr == updateInfoErr { t.Fatal("Should have gotten a different error. ") } } + +type goodDelDB struct { + errorDB +} + +func (db *goodDelDB) Exec(query string, args ...interface{}) (sql.Result, error) { + if strings.HasPrefix(query, "DELETE") { + return nil, nil + } + return db.errorDB.Exec(query, args...) +} +func TestPasswordErrors(t *testing.T) { + store := getErrorStore() + + delErr := store.SetAccountPasswordHash(api.Account{}) + if delErr == nil { + t.Fatal("Should have gotten an error") + } + + store.db = &goodDelDB{} + + insertErr := store.SetAccountPasswordHash(api.Account{}) + if insertErr == nil || insertErr == delErr { + t.Fatal("Should have gotten a different error") + } + +} + +func TestListAccounts(t *testing.T) { + store := getSimpleStore() + defer store.Close() + + accountOne := createTestAccount(t, store) + accountTwo := createTestAccount(t, store) + + byAllAccounts, listErr := store.ListAccounts() + if listErr != nil { + t.Fatal(listErr) + } + + foundAccount1 := false + foundAccount2 := false + + for _, account := range byAllAccounts { + if account == accountOne { + foundAccount1 = true + } + + if account == accountTwo { + foundAccount2 = true + } + } + + if !(foundAccount1 && foundAccount2) { + t.Fatal("didn't get back both accounts") + } +} diff --git a/api/simplestore/applications_test.go b/api/simplestore/applications_test.go index 228bcc343..062c038fe 100644 --- a/api/simplestore/applications_test.go +++ b/api/simplestore/applications_test.go @@ -12,7 +12,7 @@ func TestSaveSection(t *testing.T) { store := getSimpleStore() defer store.Close() - account := CreateTestAccount(t, store) + account := createTestAccount(t, store) sectionFilename := "../testdata/identification/identification-name.json" @@ -93,7 +93,7 @@ func TestStoreErrors(t *testing.T) { store := getSimpleStore() defer store.Close() - account := CreateTestAccount(t, store) + account := createTestAccount(t, store) sectionFilename := "../testdata/identification/identification-name.json" diff --git a/api/simplestore/attachments_test.go b/api/simplestore/attachments_test.go index 9f4af79b1..244c7c9f8 100644 --- a/api/simplestore/attachments_test.go +++ b/api/simplestore/attachments_test.go @@ -12,7 +12,7 @@ func TestSingleAttachment(t *testing.T) { store := getSimpleStore() defer store.Close() - account := CreateTestAccount(t, store) + account := createTestAccount(t, store) certificationPath := "../testdata/attachments/signature-form.pdf" certificationBytes, readErr := ioutil.ReadFile(certificationPath) @@ -88,7 +88,7 @@ func TestListAttachments(t *testing.T) { store := getSimpleStore() defer store.Close() - account := CreateTestAccount(t, store) + account := createTestAccount(t, store) testAttachments := []string{ "../testdata/attachments/signature-form.pdf", diff --git a/api/simplestore/error_db.go b/api/simplestore/error_db.go index e2c81fcde..e24b41aa2 100644 --- a/api/simplestore/error_db.go +++ b/api/simplestore/error_db.go @@ -31,6 +31,10 @@ func (db *errorDB) Queryx(query string, args ...interface{}) (*sqlx.Rows, error) return nil, errors.New("MOCK ERR") } +func (db *errorDB) Select(dest interface{}, query string, args ...interface{}) error { + return errors.New("MOCK ERR") +} + func (db *errorDB) Close() error { return errors.New("MOCK ERR") } diff --git a/api/simplestore/helpers.go b/api/simplestore/helpers.go index 037e27f09..63cc0c9c0 100644 --- a/api/simplestore/helpers.go +++ b/api/simplestore/helpers.go @@ -14,7 +14,6 @@ import ( "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/env" "github.com/18F/e-QIP-prototype/api/log" - "github.com/18F/e-QIP-prototype/api/postgresql" ) func getSimpleStore() SimpleStore { @@ -24,7 +23,7 @@ func getSimpleStore() SimpleStore { log := &log.Service{Log: log.NewLogger()} - dbConf := postgresql.DBConfig{ + dbConf := DBConfig{ User: env.String(api.DatabaseUser), Password: env.String(api.DatabasePassword), Address: env.String(api.DatabaseHost), @@ -32,7 +31,7 @@ func getSimpleStore() SimpleStore { SSLMode: "disable", } - connString := postgresql.PostgresConnectURI(dbConf) + connString := PostgresConnectURI(dbConf) serializer := JSONSerializer{} @@ -104,9 +103,9 @@ func areEqualJSON(t *testing.T, s1, s2 []byte) bool { return reflect.DeepEqual(o1, o2) } -// CreateTestAccount is exported for now because there is no simplestore way +// createTestAccount is exported for now because there is no simplestore way // of creating an account yet. Once the db stuff is ripped out I bet we can un-export this again -func CreateTestAccount(t *testing.T, store SimpleStore) api.Account { +func createTestAccount(t *testing.T, store SimpleStore) api.Account { t.Helper() email := randomEmail() @@ -114,7 +113,7 @@ func CreateTestAccount(t *testing.T, store SimpleStore) api.Account { account := api.Account{ Username: email, - Email: NonNullString(email), + Email: api.NonNullString(email), Status: api.StatusIncomplete, FormType: "SF86", FormVersion: "2017-07", diff --git a/api/simplestore/sessions.go b/api/simplestore/sessions.go index e37faff3a..16d833f91 100644 --- a/api/simplestore/sessions.go +++ b/api/simplestore/sessions.go @@ -61,18 +61,6 @@ func (s SimpleStore) DeleteSession(sessionKey string) error { return nil } -// Helper methods for sql.NullString - -// NonNullString returns a valid sql.NullString -func NonNullString(value string) sql.NullString { - return sql.NullString{Valid: true, String: value} -} - -// NullString returns an invalid sql.NullString -func NullString() sql.NullString { - return sql.NullString{} -} - type sessionAccountRow struct { api.Session api.Account diff --git a/api/simplestore/sessions_test.go b/api/simplestore/sessions_test.go index 8bc6d9cef..17a0c5b67 100644 --- a/api/simplestore/sessions_test.go +++ b/api/simplestore/sessions_test.go @@ -12,7 +12,7 @@ import ( func getTestObjects(t *testing.T) (SimpleStore, api.Account, string) { ss := getSimpleStore() - account := CreateTestAccount(t, ss) + account := createTestAccount(t, ss) UUID := uuid.New().String() return ss, account, UUID } @@ -31,7 +31,7 @@ func TestFetchExistingSessionToOverwrite(t *testing.T) { store, account, firstSessionKey := getTestObjects(t) expirationDuration := 5 * time.Minute - firstCreateErr := store.CreateSession(account.ID, firstSessionKey, NullString(), expirationDuration) + firstCreateErr := store.CreateSession(account.ID, firstSessionKey, api.NullString(), expirationDuration) if firstCreateErr != nil { t.Fatal(firstCreateErr) } @@ -57,7 +57,7 @@ func TestFetchExistingSessionToOverwrite(t *testing.T) { } secondSessionKey := uuid.New().String() - secondCreateErr := store.CreateSession(account.ID, secondSessionKey, NullString(), expirationDuration) + secondCreateErr := store.CreateSession(account.ID, secondSessionKey, api.NullString(), expirationDuration) if secondCreateErr != nil { t.Fatal(secondCreateErr) } @@ -81,7 +81,7 @@ func TestFetchSessionReturnsAccountAndSessionOnValidSession(t *testing.T) { store, account, sessionKey := getTestObjects(t) expirationDuration := 5 * time.Minute - createErr := store.CreateSession(account.ID, sessionKey, NullString(), expirationDuration) + createErr := store.CreateSession(account.ID, sessionKey, api.NullString(), expirationDuration) if createErr != nil { t.Fatal(createErr) } @@ -95,7 +95,7 @@ func TestFetchSessionReturnsAccountAndSessionOnValidSession(t *testing.T) { t.Fatal(fmt.Sprintf("actual returned account does not match expected returned account:\n%v\n%v", actualAccount, account)) } - if !(actualSession.AccountID == account.ID && actualSession.SessionKey == sessionKey && actualSession.SessionIndex == NullString()) { + if !(actualSession.AccountID == account.ID && actualSession.SessionKey == sessionKey && actualSession.SessionIndex == api.NullString()) { t.Fatal("Didn't get the expected session values back", actualSession) } @@ -111,7 +111,7 @@ func TestFetchSessionExtendsValidSession(t *testing.T) { shortInitialDuration := 5 * time.Minute - createErr := store.CreateSession(account.ID, sessionKey, NullString(), shortInitialDuration) + createErr := store.CreateSession(account.ID, sessionKey, api.NullString(), shortInitialDuration) if createErr != nil { t.Fatal(createErr) } @@ -143,7 +143,7 @@ func TestFetchSessionReturnsErrorOnExpiredSession(t *testing.T) { store, account, sessionKey := getTestObjects(t) expirationDuration := -10 * time.Minute - createErr := store.CreateSession(account.ID, sessionKey, NullString(), expirationDuration) + createErr := store.CreateSession(account.ID, sessionKey, api.NullString(), expirationDuration) if createErr != nil { t.Fatal(createErr) } @@ -157,7 +157,7 @@ func TestFetchSessionReturnsErrorOnExpiredSession(t *testing.T) { func TestDeleteSessionRemovesRecord(t *testing.T) { store, account, sessionKey := getTestObjects(t) expirationDuration := 5 * time.Minute - store.CreateSession(account.ID, sessionKey, NullString(), expirationDuration) + store.CreateSession(account.ID, sessionKey, api.NullString(), expirationDuration) fetchQuery := `SELECT * FROM sessions WHERE session_key = $1` row := api.Session{} @@ -216,7 +216,7 @@ func TestSessionDBConstraints(t *testing.T) { } // nil sessionkey - _, createErr = s.db.Exec(justCreateQuery, NullString(), account.ID, sessionIndex, expirationDate) + _, createErr = s.db.Exec(justCreateQuery, api.NullString(), account.ID, sessionIndex, expirationDate) if createErr == nil { t.Log("Should not have created a bogus session: missing sessionkey") t.Fail() @@ -231,7 +231,7 @@ func TestSessionDBConstraints(t *testing.T) { } // nil sessionIndex, this creates a record we can check UNIQE against - _, createErr = s.db.Exec(justCreateQuery, sessionKey, account.ID, NullString(), expirationDate) + _, createErr = s.db.Exec(justCreateQuery, sessionKey, account.ID, api.NullString(), expirationDate) if createErr != nil { t.Log("Should have created a session without a sessionIndex") t.Fail() @@ -239,15 +239,15 @@ func TestSessionDBConstraints(t *testing.T) { // duplicate accountid differentSessionKey := uuid.New().String() - _, createErr = s.db.Exec(justCreateQuery, differentSessionKey, account.ID, NullString(), expirationDate) + _, createErr = s.db.Exec(justCreateQuery, differentSessionKey, account.ID, api.NullString(), expirationDate) if createErr == nil { t.Log("Should not have created a session with a duplicate account ID") t.Fail() } // duplicate sessionkey - differentAccount := CreateTestAccount(t, s) - _, createErr = s.db.Exec(justCreateQuery, sessionKey, differentAccount.ID, NullString(), expirationDate) + differentAccount := createTestAccount(t, s) + _, createErr = s.db.Exec(justCreateQuery, sessionKey, differentAccount.ID, api.NullString(), expirationDate) if createErr == nil { t.Log("Should not have created a session with a duplicate SessionKey") t.Fail() @@ -259,7 +259,7 @@ func TestDeleteAccountDeletesSession(t *testing.T) { store, account, sessionKey := getTestObjects(t) expirationDuration := -5 * time.Minute - firstCreateErr := store.CreateSession(account.ID, sessionKey, NullString(), expirationDuration) + firstCreateErr := store.CreateSession(account.ID, sessionKey, api.NullString(), expirationDuration) if firstCreateErr != nil { t.Fatal(firstCreateErr) } diff --git a/api/simplestore/simple_store.go b/api/simplestore/simple_store.go index 870d71305..d34658110 100644 --- a/api/simplestore/simple_store.go +++ b/api/simplestore/simple_store.go @@ -2,6 +2,8 @@ package simplestore import ( "database/sql" + "fmt" + "net/url" "github.com/jmoiron/sqlx" _ "github.com/lib/pq" // pg is required for the sqlx package to work @@ -26,9 +28,53 @@ type sqlxConnection interface { MustExec(query string, args ...interface{}) sql.Result Beginx() (*sqlx.Tx, error) Queryx(query string, args ...interface{}) (*sqlx.Rows, error) + Select(dest interface{}, query string, args ...interface{}) error Close() error } +// DBConfig contains everything neccecary to setup a postgres db connection +type DBConfig struct { + User string + Password string + Address string + DBName string + SSLMode string +} + +// PostgresConnectURI creates a connection string, which is used by the golang sql Open() function +func PostgresConnectURI(conf DBConfig) string { + // By user (+ password) + database + host + uri := &url.URL{Scheme: "postgres"} + username := conf.User + + // Check if there is a password set. If not then we need to create + // the Userinfo structure in a different way so we don't include + // exta colons (:). + pw := conf.Password + if pw == "" { + uri.User = url.User(username) + } else { + uri.User = url.UserPassword(username, pw) + } + + // The database name will be part of the URI path so it needs + // a prefix of "/" + database := conf.DBName + uri.Path = fmt.Sprintf("/%s", database) + + // Host can be either "address + port" or just "address" + host := conf.Address + uri.Host = host + + if conf.SSLMode != "" { + params := url.Values{} + params.Set("sslmode", conf.SSLMode) + uri.RawQuery = params.Encode() + } + + return uri.String() +} + // SimpleStore saves JSON in the db for applications type SimpleStore struct { db sqlxConnection diff --git a/api/transmission.go b/api/transmission.go index a36c65aaa..20954dd5d 100644 --- a/api/transmission.go +++ b/api/transmission.go @@ -17,24 +17,24 @@ type Transmission struct { Modified time.Time } -// Get transmission record from the database. -func (transmission *Transmission) Get(context DatabaseService) error { - return context.Select(transmission) -} +// // Get transmission record from the database. +// func (transmission *Transmission) Get(context DatabaseService) error { +// return context.Select(transmission) +// } -// Save transmission record to the database. -func (transmission *Transmission) Save(context DatabaseService) error { - var err error - if transmission.ID == 0 { - err = context.Insert(transmission) - } else { - err = context.Update(transmission) - } +// // Save transmission record to the database. +// func (transmission *Transmission) Save(context DatabaseService) error { +// var err error +// if transmission.ID == 0 { +// err = context.Insert(transmission) +// } else { +// err = context.Update(transmission) +// } - return err -} +// return err +// } -// Find is not used for transmissions. Please use the `Get` method. -func (transmission *Transmission) Find(context DatabaseService) error { - return nil -} +// // Find is not used for transmissions. Please use the `Get` method. +// func (transmission *Transmission) Find(context DatabaseService) error { +// return nil +// } From 407346fec00278da9e5ed51f50f2cdc8a234beb9 Mon Sep 17 00:00:00 2001 From: MacRae Linton <55759+macrael@users.noreply.github.com> Date: Fri, 6 Sep 2019 00:50:14 -0700 Subject: [PATCH 4/8] remove go-pg deps --- api/Gopkg.lock | 35 +---------------------------------- api/Gopkg.toml | 4 ---- 2 files changed, 1 insertion(+), 38 deletions(-) diff --git a/api/Gopkg.lock b/api/Gopkg.lock index c4cf2fb42..f41ede8ef 100644 --- a/api/Gopkg.lock +++ b/api/Gopkg.lock @@ -52,21 +52,6 @@ revision = "f920e9562d5f951cbf11785728f67258c38a10d0" version = "v1.17.0" -[[projects]] - digest = "1:0a2260bcfa15237979ef496f03fb6af215c247ad998881a4d039992228876ae3" - name = "github.com/go-pg/pg" - packages = [ - ".", - "internal", - "internal/parser", - "internal/pool", - "orm", - "types", - ] - pruneopts = "UT" - revision = "ae5d5e7df4b2e598390e10b66b849c6af94f092b" - version = "v6.15.1" - [[projects]] digest = "1:582b704bebaa06b48c29b0cec224a6058a09c86883aaddabde889cd1a5f73e1b" name = "github.com/google/uuid" @@ -107,14 +92,6 @@ revision = "e59506cc896acb7f7bf732d4fdf5e25f7ccd8983" version = "v1.1.1" -[[projects]] - branch = "master" - digest = "1:fd97437fbb6b7dce04132cf06775bd258cce305c44add58eb55ca86c6c325160" - name = "github.com/jinzhu/inflection" - packages = ["."] - pruneopts = "UT" - revision = "04140366298a54a039076d798123ffa108fff46c" - [[projects]] digest = "1:6c41d4f998a03b6604227ccad36edaed6126c397e5d78709ef4814a1145a6757" name = "github.com/jmoiron/sqlx" @@ -220,12 +197,11 @@ revision = "0f6081814ce7f59da72e48a6748bf9b5c444e813" [[projects]] - digest = "1:7310f5459b88177d24e26c5ec7deb100b78ec03c0437d7a7aaec3c7eac333a55" + digest = "1:001a4e7a40e50ff2ef32e2556bca50c4f77daa457db3ac6afc8bea9bb2122cfb" name = "golang.org/x/crypto" packages = [ "bcrypt", "blowfish", - "pbkdf2", "ssh/terminal", ] pruneopts = "UT" @@ -279,14 +255,6 @@ revision = "f21a4dfb5e38f5895301dc265a8def02365cc3d0" version = "v0.3.0" -[[projects]] - digest = "1:999d566ad1ae4303c456af5a155f7b42401cca4fd33552567ed9dd997b107a08" - name = "mellium.im/sasl" - packages = ["."] - pruneopts = "UT" - revision = "e58e780e1378aa4df5dc705364ff5ae9a4dd1e04" - version = "v0.2.1" - [solve-meta] analyzer-name = "dep" analyzer-version = 1 @@ -296,7 +264,6 @@ "github.com/antchfx/xmlquery", "github.com/benbjohnson/clock", "github.com/cloudfoundry-community/go-cfenv", - "github.com/go-pg/pg", "github.com/google/uuid", "github.com/gorilla/csrf", "github.com/gorilla/mux", diff --git a/api/Gopkg.toml b/api/Gopkg.toml index 2321490f1..e10b4e2df 100644 --- a/api/Gopkg.toml +++ b/api/Gopkg.toml @@ -37,10 +37,6 @@ name = "github.com/cloudfoundry-community/go-cfenv" version = "1.14.0" -[[constraint]] - name = "github.com/go-pg/pg" - version = "6.4.9" - [[constraint]] name = "github.com/gorilla/mux" version = "1.1.0" From a839add7a9bf52b102026f6674f106a101cb96f0 Mon Sep 17 00:00:00 2001 From: MacRae Linton <55759+macrael@users.noreply.github.com> Date: Fri, 6 Sep 2019 01:02:54 -0700 Subject: [PATCH 5/8] fix comments for linter --- api/mock/store.go | 7 ++++++- api/store.go | 5 ++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/api/mock/store.go b/api/mock/store.go index 516cb33ee..f8dc4126d 100644 --- a/api/mock/store.go +++ b/api/mock/store.go @@ -75,23 +75,28 @@ func (s *StorageService) ExtendAndFetchSessionAccount(sessionKey string, session return api.Account{}, api.Session{}, nil } +// CreateAccount creates a new account func (s *StorageService) CreateAccount(account *api.Account) error { return nil } +// FetchAccountByUsername gets an account with the given username func (s *StorageService) FetchAccountByUsername(username string) (api.Account, error) { return api.Account{}, nil } +// FetchAccountByExternalID gets an account with the given externalID func (s *StorageService) FetchAccountByExternalID(externalID string) (api.Account, error) { return api.Account{}, nil } +// FetchAccountWithPasswordHash returns an account with the PasswordHash field filled out +// it raises an error if the given account has no password func (s *StorageService) FetchAccountWithPasswordHash(username string) (api.Account, error) { return api.Account{}, nil } -// UpdateAccountStatus updates an account +// UpdateAccountStatus updates the given account func (s *StorageService) UpdateAccountStatus(account *api.Account) error { return nil } diff --git a/api/store.go b/api/store.go index 2f3bf421c..932f1b48f 100644 --- a/api/store.go +++ b/api/store.go @@ -61,14 +61,13 @@ type StorageService interface { // Can fetch sessions, that's good. // CreateAccount creates a new account CreateAccount(account *Account) error - // FetchAccountByUsername updates the given account + // FetchAccountByUsername gets an account with the given username FetchAccountByUsername(username string) (Account, error) - // FetchAccountByExternalID updates the given account + // FetchAccountByExternalID gets an account with the given externalID FetchAccountByExternalID(externalID string) (Account, error) // FetchAccountWithPasswordHash returns an account with the PasswordHash field filled out // it raises an error if the given account has no password FetchAccountWithPasswordHash(username string) (Account, error) - // UpdateAccountStatus updates the given account UpdateAccountStatus(account *Account) error From f6ba042535a0baeadb1c7a4ed054ac4d93004023 Mon Sep 17 00:00:00 2001 From: MacRae Linton <55759+macrael@users.noreply.github.com> Date: Tue, 10 Sep 2019 15:49:50 -0500 Subject: [PATCH 6/8] Validate accounts on creation --- api/account.go | 4 ++-- api/simplestore/accounts.go | 5 +++++ api/simplestore/accounts_test.go | 14 +++++++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/api/account.go b/api/account.go index 7f7008ab3..27e0fb54f 100644 --- a/api/account.go +++ b/api/account.go @@ -66,9 +66,9 @@ func NullString() sql.NullString { return sql.NullString{} } -// validate returns nil if the account is valid or an error describing what's wrong otherwise. +// CheckIsValid returns nil if the account is valid or an error describing what's wrong otherwise. // This is intended to ensure data integrity by not saving invalid data to the database -func (entity Account) validate() error { +func (entity Account) CheckIsValid() error { // the required fields are Username, FormType, and FormVersion errorString := "Account is invalid. " diff --git a/api/simplestore/accounts.go b/api/simplestore/accounts.go index d32025305..ea00d74dc 100644 --- a/api/simplestore/accounts.go +++ b/api/simplestore/accounts.go @@ -13,6 +13,11 @@ import ( // CreateAccount creates a new account func (s SimpleStore) CreateAccount(account *api.Account) error { + validErr := account.CheckIsValid() + if validErr != nil { + return errors.Wrap(validErr, "failed to create invalid Account") + } + createQuery := `INSERT INTO accounts (username, email, status, form_type, form_version, external_id) VALUES ($1, $2, $3, $4, $5, $6) RETURNING id` diff --git a/api/simplestore/accounts_test.go b/api/simplestore/accounts_test.go index 6ea308a23..71e08d841 100644 --- a/api/simplestore/accounts_test.go +++ b/api/simplestore/accounts_test.go @@ -268,7 +268,9 @@ func TestCreateErr(t *testing.T) { defer store.Close() newAccount := api.Account{ - Username: "joe", + Username: "joe", + FormType: "SF86", + FormVersion: "2017-07", } // There is a unique constraint on username, so we can trigger an error there. @@ -281,6 +283,16 @@ func TestCreateErr(t *testing.T) { } } + invalidAccount := api.Account{ + Username: "invalid", + FormType: "SF86", + FormVersion: "nonsense", + } + invalidErr := store.CreateAccount(&invalidAccount) + if invalidErr == nil { + t.Fatal("Shouldn't have created an invalid account") + } + } func TestFetchUnknownError(t *testing.T) { From 6dfc299ac885f7e59008debb75645f8610ef734f Mon Sep 17 00:00:00 2001 From: MacRae Linton <55759+macrael@users.noreply.github.com> Date: Wed, 11 Sep 2019 01:42:20 -0400 Subject: [PATCH 7/8] fix build error in account test --- api/account_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/account_test.go b/api/account_test.go index d2e2cc10b..0beab9107 100644 --- a/api/account_test.go +++ b/api/account_test.go @@ -82,7 +82,7 @@ func TestAccountUsernameValidation(t *testing.T) { FormType: "SF86", FormVersion: "2017-07", } - err := account.validate() + err := account.CheckIsValid() if err == nil { t.Errorf("expected a Missing Username error") t.Fail() @@ -97,7 +97,7 @@ func TestAccountFormTypeValidation(t *testing.T) { FormType: "Dogs", FormVersion: "2017-07", } - err := account.validate() + err := account.CheckIsValid() if err == nil { t.Errorf("expected a Known Form error") t.Fail() From 27c2f3ba959861e6de9ad8b0ccab31b3aa6dc5df Mon Sep 17 00:00:00 2001 From: Ryan Hofschneider Date: Wed, 2 Oct 2019 08:47:39 -0700 Subject: [PATCH 8/8] Remove unused package and re-format --- api/session/session_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/session/session_test.go b/api/session/session_test.go index 54bb5075f..3999fbe81 100644 --- a/api/session/session_test.go +++ b/api/session/session_test.go @@ -8,7 +8,6 @@ import ( "time" "github.com/google/uuid" - "github.com/pkg/errors" "github.com/18F/e-QIP-prototype/api" "github.com/18F/e-QIP-prototype/api/env" @@ -107,7 +106,7 @@ func TestLogSessionCreatedDestroyed(t *testing.T) { sessionLog := &mock.LogRecorder{} session := NewSessionService(timeout, store, sessionLog) - account := createTestAccount(t, store.(simplestore.SimpleStore)) + account := createTestAccount(t, store.(simplestore.SimpleStore)) sessionKey, authErr := session.UserDidAuthenticate(account.ID, api.NullString()) if authErr != nil {