From 3c96a6310760d33bf829723a317d924b393a45e4 Mon Sep 17 00:00:00 2001 From: Ryan Kelly Date: Fri, 25 May 2018 12:41:40 +1000 Subject: [PATCH] fix(mysql): Set strict mode on all new db migrations. We've stored enforcing strict-mode on all connections to MySQL from the app at runtime. However, stored procedures remember the sql_mode that was in effect at the time they were created, and we create our stored procedures through an out-of-band connection that doesn't force strict mode. This change adds an explicit "enable strict mode" preamble to new migrations, and re-creates on of the existing stored procedures to make use of it. Once we're confident this will work on OK in practice, we'll need to re-create all stored procedures that might have been created without strict mode. --- lib/db/mysql.js | 2 +- lib/db/patch.js | 2 +- lib/db/schema/patch-080-081.sql | 89 +++++++++++++++++++++++++++++++++ lib/db/schema/patch-081-080.sql | 7 +++ scripts/migration.sh | 4 +- 5 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 lib/db/schema/patch-080-081.sql create mode 100644 lib/db/schema/patch-081-080.sql diff --git a/lib/db/mysql.js b/lib/db/mysql.js index 6a6c2157..899002e6 100644 --- a/lib/db/mysql.js +++ b/lib/db/mysql.js @@ -203,7 +203,7 @@ module.exports = function (log, error) { // Insert : accounts // Values : uid = $1, normalizedEmail = $2, email = $3, emailCode = $4, emailVerified = $5, kA = $6, wrapWrapKb = $7, authSalt = $8, verifierVersion = $9, verifyHash = $10, verifierSetAt = $11, createdAt = $12, locale = $13 - var CREATE_ACCOUNT = 'CALL createAccount_7(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' + var CREATE_ACCOUNT = 'CALL createAccount_8(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' MySql.prototype.createAccount = function (uid, data) { return this.write( diff --git a/lib/db/patch.js b/lib/db/patch.js index 15a68816..468d52da 100644 --- a/lib/db/patch.js +++ b/lib/db/patch.js @@ -4,4 +4,4 @@ // The expected patch level of the database. Update if you add a new // patch in the ./schema/ directory. -module.exports.level = 80 +module.exports.level = 81 diff --git a/lib/db/schema/patch-080-081.sql b/lib/db/schema/patch-080-081.sql new file mode 100644 index 00000000..a64875db --- /dev/null +++ b/lib/db/schema/patch-080-081.sql @@ -0,0 +1,89 @@ +SET SESSION sql_mode = CONCAT(@@sql_mode, ',STRICT_ALL_TABLES,NO_ENGINE_SUBSTITUTION'); +SET NAMES utf8mb4 COLLATE utf8mb4_bin; + +CREATE PROCEDURE `createAccount_8`( + IN `inUid` BINARY(16) , + IN `inNormalizedEmail` VARCHAR(255), + IN `inEmail` VARCHAR(255), + IN `inEmailCode` BINARY(16), + IN `inEmailVerified` TINYINT(1), + IN `inKA` BINARY(32), + IN `inWrapWrapKb` BINARY(32), + IN `inAuthSalt` BINARY(32), + IN `inVerifierVersion` TINYINT UNSIGNED, + IN `inVerifyHash` BINARY(32), + IN `inVerifierSetAt` BIGINT UNSIGNED, + IN `inCreatedAt` BIGINT UNSIGNED, + IN `inLocale` VARCHAR(255) +) +BEGIN + DECLARE EXIT HANDLER FOR SQLEXCEPTION + BEGIN + ROLLBACK; + RESIGNAL; + END; + + START TRANSACTION; + + -- Check to see if the normalizedEmail exists in the emails table before creating a new user + -- with this email. + SET @emailExists = 0; + SELECT COUNT(*) INTO @emailExists FROM emails WHERE normalizedEmail = inNormalizedEmail; + IF @emailExists > 0 THEN + SIGNAL SQLSTATE '45000' SET MYSQL_ERRNO = 1062, MESSAGE_TEXT = 'Unable to create user, email used belongs to another user.'; + END IF; + + INSERT INTO accounts( + uid, + normalizedEmail, + email, + emailCode, + emailVerified, + kA, + wrapWrapKb, + authSalt, + verifierVersion, + verifyHash, + verifierSetAt, + createdAt, + locale + ) + VALUES( + inUid, + LOWER(inNormalizedEmail), + inEmail, + inEmailCode, + inEmailVerified, + inKA, + inWrapWrapKb, + inAuthSalt, + inVerifierVersion, + inVerifyHash, + inVerifierSetAt, + inCreatedAt, + inLocale + ); + + INSERT INTO emails( + normalizedEmail, + email, + uid, + emailCode, + isVerified, + isPrimary, + createdAt + ) + VALUES( + LOWER(inNormalizedEmail), + inEmail, + inUid, + inEmailCode, + inEmailVerified, + true, + inCreatedAt + ); + + COMMIT; +END; + +UPDATE dbMetadata SET value = '81' WHERE name = 'schema-patch-level'; diff --git a/lib/db/schema/patch-081-080.sql b/lib/db/schema/patch-081-080.sql new file mode 100644 index 00000000..c7e82e8c --- /dev/null +++ b/lib/db/schema/patch-081-080.sql @@ -0,0 +1,7 @@ +-- SET SESSION sql_mode = CONCAT(@@sql_mode, ',STRICT_ALL_TABLES,NO_ENGINE_SUBSTITUTION'); +-- SET NAMES utf8mb4 COLLATE utf8mb4_bin; + +-- DROP PROCEDURE createAccount_8; + +-- UPDATE dbMetadata SET value = '80' WHERE name = 'schema-patch-level'; + diff --git a/scripts/migration.sh b/scripts/migration.sh index df5ae91a..32a9dd2e 100755 --- a/scripts/migration.sh +++ b/scripts/migration.sh @@ -32,11 +32,13 @@ fi printf "Generating migration boilerplate for patch level $NEW_LEVEL..." -echo "SET NAMES utf8mb4 COLLATE utf8mb4_bin;\n" > "$FWD_SCHEMA" +echo "SET SESSION sql_mode = CONCAT(@@sql_mode, ',STRICT_ALL_TABLES,NO_ENGINE_SUBSTITUTION');" > "$FWD_SCHEMA" +echo "SET NAMES utf8mb4 COLLATE utf8mb4_bin;\n" >> "$FWD_SCHEMA" echo "-- TODO: Implement your forward migration here\n" >> "$FWD_SCHEMA" echo "UPDATE dbMetadata SET value = '$NEW_LEVEL' WHERE name = 'schema-patch-level';\n" >> "$FWD_SCHEMA" echo '-- -- TODO: Implement your *commented-out* reverse migration here\n' > "$REV_SCHEMA" +echo "-- SET SESSION sql_mode = CONCAT(@@sql_mode, ',STRICT_ALL_TABLES,NO_ENGINE_SUBSTITUTION');" > "$REV_SCHEMA" echo "-- SET NAMES utf8mb4 COLLATE utf8mb4_bin;\n" >> "$REV_SCHEMA" echo "-- UPDATE dbMetadata SET value = '$PREV_LEVEL' WHERE name = 'schema-patch-level';\n" >> "$REV_SCHEMA"