diff --git a/README.md b/README.md index 7be33b5..747aeb5 100644 --- a/README.md +++ b/README.md @@ -3,12 +3,12 @@ WaveBox A free, open source personal media server written in C# and available on all platforms thanks to the Mono framework. -Licensed under [The GNU General Public License v3.0](https://www.gnu.org/licenses/gpl.html "Permalink to The GNU General Public License v3.0 - GNU Project"), for more information please read the [LICENSE.md](https://github.com/einsteinx2/WaveBox/blob/main/LICENSE.md) file in this repository or visit the preceding link to the GNU website. +Licensed under [The GNU General Public License v3.0](https://www.gnu.org/licenses/gpl.html "Permalink to The GNU General Public License v3.0 - GNU Project"), for more information please read the [LICENSE.md](LICENSE.md) file in this repository or visit the preceding link to the GNU website. API --- -WaveBox features a very extensible JSON API, and is built with API developers in mind! Full API documentation can be found in [API_DOCS.md](https://github.com/einsteinx2/WaveBox/blob/main/API_DOCS.md). +WaveBox features a very extensible JSON API, and is built with API developers in mind! Full API documentation can be found in [API_DOCS.md](API_DOCS.md). Testing ------- diff --git a/WaveBox.Core/WaveBox.Core.csproj b/WaveBox.Core/WaveBox.Core.csproj index 8a54759..573aa66 100644 --- a/WaveBox.Core/WaveBox.Core.csproj +++ b/WaveBox.Core/WaveBox.Core.csproj @@ -6,6 +6,7 @@ + diff --git a/WaveBox.Core/src/BaseClasses.cs b/WaveBox.Core/src/BaseClasses.cs index 33c4529..2e67a64 100644 --- a/WaveBox.Core/src/BaseClasses.cs +++ b/WaveBox.Core/src/BaseClasses.cs @@ -107,6 +107,12 @@ public interface ISQLiteConnection : IDisposable { int Execute(string query, params object[] args); + /// + /// Executes every statement in a SQL script, for schema and migration files. Execute() + /// only ever runs the first statement it is given. + /// + void ExecuteScript(string script); + T ExecuteScalar(string query, params object[] args); List Query(string query, params object[] args) where T : new(); diff --git a/WaveBox.Core/src/Extensions/ByteExtensions.cs b/WaveBox.Core/src/Extensions/ByteExtensions.cs index 8a45b1d..2533bf0 100644 --- a/WaveBox.Core/src/Extensions/ByteExtensions.cs +++ b/WaveBox.Core/src/Extensions/ByteExtensions.cs @@ -9,27 +9,11 @@ namespace WaveBox.Core.Extensions { public static class ByteExtensions { /// - /// Generates a MD5 sum of a given byte array - /// Thanks: http://msdn.microsoft.com/en-us/library/s02tk69a.aspx + /// Generates a MD5 sum of a given byte array, as lowercase hex /// public static string MD5(this byte[] input) { - using (System.Security.Cryptography.MD5 md5 = System.Security.Cryptography.MD5.Create()) { - // Convert the input string to a byte array and compute the hash. - byte[] data = md5.ComputeHash(input); - - // Create a new Stringbuilder to collect the bytes - // and create a string. - StringBuilder sBuilder = new StringBuilder(); - - // Loop through each byte of the hashed data - // and format each one as a hexadecimal string. - for (int i = 0; i < data.Length; i++) { - sBuilder.Append(data[i].ToString("x2")); - } - - // Return the hexadecimal string. - return sBuilder.ToString(); - } + byte[] hash = System.Security.Cryptography.MD5.HashData(input); + return Convert.ToHexString(hash).ToLowerInvariant(); } } } diff --git a/WaveBox.Core/src/Extensions/StringExtensions.cs b/WaveBox.Core/src/Extensions/StringExtensions.cs index c7bab40..0eef794 100644 --- a/WaveBox.Core/src/Extensions/StringExtensions.cs +++ b/WaveBox.Core/src/Extensions/StringExtensions.cs @@ -38,15 +38,15 @@ public static bool IsTrue(this string boolString) { } /// - /// Generates a MD5 sum of a given string + /// Generates a MD5 sum of a given string, as lowercase hex /// public static string MD5(this string sumthis) { if (sumthis == "" || sumthis == null) { return ""; } - MD5CryptoServiceProvider md5 = new MD5CryptoServiceProvider(); - return BitConverter.ToString(md5.ComputeHash(System.Text.Encoding.ASCII.GetBytes(sumthis)), 0); + byte[] hash = System.Security.Cryptography.MD5.HashData(Encoding.ASCII.GetBytes(sumthis)); + return Convert.ToHexString(hash).ToLowerInvariant(); } /// @@ -84,15 +84,15 @@ public static int MonthForAbbreviation(this string abb) { } /// - /// Generates a SHA1 sum of a given string + /// Generates a SHA1 sum of a given string, as lowercase hex /// public static string SHA1(this string sumthis) { if (sumthis == "" || sumthis == null) { return ""; } - SHA1CryptoServiceProvider provider = new SHA1CryptoServiceProvider(); - return BitConverter.ToString(provider.ComputeHash(Encoding.ASCII.GetBytes(sumthis))).Replace("-", ""); + byte[] hash = System.Security.Cryptography.SHA1.HashData(Encoding.ASCII.GetBytes(sumthis)); + return Convert.ToHexString(hash).ToLowerInvariant(); } /// diff --git a/WaveBox.Core/src/Injection/Interfaces/IDatabase.cs b/WaveBox.Core/src/Injection/Interfaces/IDatabase.cs index 7683a3a..c0c2ce0 100644 --- a/WaveBox.Core/src/Injection/Interfaces/IDatabase.cs +++ b/WaveBox.Core/src/Injection/Interfaces/IDatabase.cs @@ -9,10 +9,12 @@ public interface IDatabase { int Version { get; } - string DatabaseTemplatePath { get; } + string DatabaseSchemaPath { get; } string DatabasePath { get; } - string QuerylogTemplatePath { get; } + string MigrationsPath { get; } + + string QuerylogSchemaPath { get; } string QuerylogPath { get; } void DatabaseSetup(); diff --git a/WaveBox.Core/src/Model/Playlist.cs b/WaveBox.Core/src/Model/Playlist.cs index a6db9d4..08ebc65 100644 --- a/WaveBox.Core/src/Model/Playlist.cs +++ b/WaveBox.Core/src/Model/Playlist.cs @@ -69,7 +69,7 @@ public string CalculateHash() { Injection.Get().CloseSqliteConnection(conn); } - return itemIds.ToString().MD5().Replace("-", string.Empty); + return itemIds.ToString().MD5(); } public void UpdateDatabase() { diff --git a/WaveBox.Core/src/Model/User.cs b/WaveBox.Core/src/Model/User.cs index 2554477..f8fac5f 100644 --- a/WaveBox.Core/src/Model/User.cs +++ b/WaveBox.Core/src/Model/User.cs @@ -1,8 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text; -using System.Security.Cryptography; using Cirrious.MvvmCross.Plugins.Sqlite; using System.Text.Json.Serialization; using WaveBox.Core.Extensions; @@ -12,9 +10,6 @@ namespace WaveBox.Core.Model { public class User : IGroupingItem { - // PBKDF2 iterations - public const int HashIterations = 2500; - private static readonly WaveBox.Core.Logging.ILog logger = WaveBox.Core.Logging.LogManager.GetLogger(); [JsonPropertyName("userId")] @@ -39,9 +34,6 @@ public class User : IGroupingItem { [JsonIgnore] public string PasswordHash { get; set; } - [JsonIgnore] - public string PasswordSalt { get; set; } - [JsonIgnore, IgnoreRead, IgnoreWrite] public string SessionId { get; set; } @@ -106,17 +98,18 @@ public IList ListOfSessions() { } public bool UpdatePassword(string password) { - string salt = GeneratePasswordSalt(); - string hash = ComputePasswordHash(password, salt); + string hash = PasswordHasher.Hash(password); + if (hash == null) { + return false; + } ISQLiteConnection conn = null; try { conn = Injection.Get().GetSqliteConnection(); - int affected = conn.Execute("UPDATE User SET PasswordHash = ?, PasswordSalt = ? WHERE UserId = ?", hash, salt, this.UserId); + int affected = conn.Execute("UPDATE User SET PasswordHash = ? WHERE UserId = ?", hash, this.UserId); if (affected > 0) { this.PasswordHash = hash; - this.PasswordSalt = salt; return Injection.Get().UpdateUserCache(this); } @@ -212,24 +205,9 @@ public bool UpdateRole(Role role) { return false; } - // Verify password, using timing attack resistant approach - // Credit to PHP5.5 Password API for this method + // Verify password; the comparison is timing attack resistant public bool Authenticate(string password) { - // Compute hash - string hash = ComputePasswordHash(password, this.PasswordSalt); - - // Ensure hashes are same length - if (hash.Length != this.PasswordHash.Length) { - return false; - } - - // Compare ASCII value of each character, bitwise OR any diff - int status = 0; - for (int i = 0; i < hash.Length; i++) { - status |= ((int)hash[i] ^ (int)this.PasswordHash[i]); - } - - return status == 0; + return PasswordHasher.Verify(password, this.PasswordHash); } public bool CreateSession(string password, string clientName) { @@ -281,30 +259,5 @@ public static int CompareUsersByName(User x, User y) { return StringComparer.OrdinalIgnoreCase.Compare(x.UserName, y.UserName); } - // Compute password hash using PBKDF2 - public static string ComputePasswordHash(string password, string salt, int iterations = HashIterations) { - // Convert salt to byte array - byte[] saltBytes = Encoding.UTF8.GetBytes(salt); - - // Hash using PBKDF2 with salt and predefined iterations - using (Rfc2898DeriveBytes pbkdf2 = new Rfc2898DeriveBytes(password, saltBytes, iterations)) { - var key = pbkdf2.GetBytes(64); - return Convert.ToBase64String(key); - } - } - - // Use RNG crypto service to generate random bytes for salt - public static string GeneratePasswordSalt() { - // Create byte array to store salt - byte[] salt = new byte[32]; - - // Fill array using RNG - using (RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider()) { - rng.GetBytes(salt); - } - - // Return string representation - return Convert.ToBase64String(salt); - } } } diff --git a/WaveBox.Core/src/Repository/UserRepository.cs b/WaveBox.Core/src/Repository/UserRepository.cs index 6be07d9..a5777d6 100644 --- a/WaveBox.Core/src/Repository/UserRepository.cs +++ b/WaveBox.Core/src/Repository/UserRepository.cs @@ -78,8 +78,10 @@ public User CreateUser(string userName, string password, Role role, long? delete return new User(); } - string salt = User.GeneratePasswordSalt(); - string hash = User.ComputePasswordHash(password, salt); + string hash = PasswordHasher.Hash(password); + if (hash == null) { + return null; + } var u = new User(); u.UserId = itemId; @@ -87,7 +89,6 @@ public User CreateUser(string userName, string password, Role role, long? delete u.Role = role; u.Password = password; u.PasswordHash = hash; - u.PasswordSalt = salt; u.CreateTime = DateTime.UtcNow.ToUnixTime(); u.DeleteTime = deleteTime; diff --git a/WaveBox.Core/src/SQLiteNet.cs b/WaveBox.Core/src/SQLiteNet.cs index e2ace2a..fa642a1 100644 --- a/WaveBox.Core/src/SQLiteNet.cs +++ b/WaveBox.Core/src/SQLiteNet.cs @@ -597,6 +597,15 @@ public ISQLiteCommand CreateCommand(string cmdText, params object[] ps) { /// /// The number of rows modified in the database as a result of this execution. /// + /// + /// WaveBox addition: executes every statement in a SQL script. Execute() above only ever + /// runs the first, since sqlite3_prepare_v2 compiles one statement at a time. + /// Takes no parameters -- this is for schema and migration scripts, not queries. + /// + public void ExecuteScript(string script) { + SQLite3.ExecuteScript(Handle, script); + } + public int Execute(string query, params object[] args) { var cmd = CreateCommand(query, args); @@ -990,13 +999,7 @@ private void DoSavePointExecute(string savepoint, string cmd) { if (Int32.TryParse(savepoint.Substring(firstLen + 1), out depth)) { // TODO: Mild race here, but inescapable without locking almost everywhere. if (0 <= depth && depth < _trasactionDepth) { -#if NETFX_CORE - Volatile.Write (ref _trasactionDepth, depth); -#elif SILVERLIGHT - _trasactionDepth = depth; -#else - Thread.VolatileWrite(ref _trasactionDepth, depth); -#endif + Volatile.Write(ref _trasactionDepth, depth); Execute(cmd + savepoint); return; } @@ -2403,6 +2406,87 @@ public static IntPtr Prepare2(IntPtr db, string query) { return stmt; } + // WaveBox addition: the overload above discards pzTail, so it only ever compiles the + // first statement of a script. This one hands back the pointer to the remaining SQL, + // which is what lets ExecuteScript walk a multi-statement file. + [DllImport("e_sqlite3", EntryPoint = "sqlite3_prepare_v2", CallingConvention = CallingConvention.Cdecl)] + public static extern Result Prepare2(IntPtr db, IntPtr sql, int numBytes, + out IntPtr stmt, out IntPtr pzTail); + + /// + /// WaveBox addition: runs every statement in a SQL script. + /// + /// SQLite's parser does the tokenizing, so semicolons inside string literals, comments + /// and trigger bodies are all handled correctly -- unlike splitting the text on ';'. + /// The script is marshalled as an explicit UTF-8 buffer and walked by byte offset, + /// avoiding the ANSI marshalling and char-vs-byte length bugs in the string overload + /// of Prepare2 above. + /// + /// Transactions are the caller's business; scripts should not contain BEGIN or COMMIT. + /// + public static void ExecuteScript(IntPtr db, string script) { + if (String.IsNullOrEmpty(script)) { + return; + } + + byte[] utf8 = System.Text.Encoding.UTF8.GetBytes(script); + // sqlite3_prepare_v2 wants a NUL-terminated buffer when it reads past numBytes + Array.Resize(ref utf8, utf8.Length + 1); + + GCHandle pin = GCHandle.Alloc(utf8, GCHandleType.Pinned); + try { + IntPtr start = pin.AddrOfPinnedObject(); + int offset = 0; + + // utf8.Length - 1 excludes the NUL we appended + while (offset < utf8.Length - 1) { + IntPtr stmt; + IntPtr tail; + Result r = Prepare2(db, start + offset, utf8.Length - 1 - offset, out stmt, out tail); + + if (r != Result.OK) { + throw SQLiteException.New(r, GetErrmsg(db) + "\n" + RemainingText(utf8, offset)); + } + + int nextOffset = (int)(tail.ToInt64() - start.ToInt64()); + + // A trailing run of whitespace or comments compiles to no statement at all + if (stmt == IntPtr.Zero) { + if (nextOffset <= offset) { + break; + } + offset = nextOffset; + continue; + } + + try { + Result step = Step(stmt); + if (step != Result.Done && step != Result.Row) { + throw SQLiteException.New(step, GetErrmsg(db) + "\n" + StatementText(utf8, offset, nextOffset)); + } + } finally { + Finalize(stmt); + } + + if (nextOffset <= offset) { + break; + } + offset = nextOffset; + } + } finally { + pin.Free(); + } + } + + private static string StatementText(byte[] utf8, int offset, int end) { + return System.Text.Encoding.UTF8.GetString(utf8, offset, Math.Max(0, end - offset)).Trim(); + } + + private static string RemainingText(byte[] utf8, int offset) { + // On a compile failure there is no tail to bound the statement, so show what is left + return System.Text.Encoding.UTF8.GetString(utf8, offset, utf8.Length - 1 - offset).Trim(); + } + [DllImport("e_sqlite3", EntryPoint = "sqlite3_step", CallingConvention = CallingConvention.Cdecl)] public static extern Result Step(IntPtr stmt); diff --git a/WaveBox.Core/src/Static/PasswordHasher.cs b/WaveBox.Core/src/Static/PasswordHasher.cs new file mode 100644 index 0000000..f7c5aad --- /dev/null +++ b/WaveBox.Core/src/Static/PasswordHasher.cs @@ -0,0 +1,69 @@ +using System; +using System.Text; + +namespace WaveBox.Core.Static { + /// + /// Password storage, wrapping bcrypt so the library stays out of the call sites. + /// + /// A bcrypt hash is self describing -- "$2a$<cost>$<22 char salt><31 char hash>" -- so the salt + /// and work factor travel with it and no separate salt column is needed. + /// + public static class PasswordHasher { + /// + /// bcrypt only consumes the first 72 bytes of a password and silently ignores the rest, so + /// longer passwords are rejected outright rather than quietly truncated. + /// + public const int MaxPasswordBytes = 72; + + /// + /// Work factor, as a power of two. OWASP's guidance is a minimum of 10, set as high as + /// verification performance allows while staying under a second on the slowest target. + /// + /// Measured on an M-series Mac: cost 10 ~115ms, 11 ~140ms, 12 ~279ms, 13 ~552ms. Pi-class + /// linux-arm64 hardware runs several times slower, which would put 12 over the one second + /// mark there, so 11 it is. Subsonic clients re-send credentials on every request, and + /// SubsonicAuth only caches a verification for ten minutes, so this sets a floor on + /// request latency for clients that don't keep connections alive. + /// + /// Tests dial this down to bcrypt's minimum, since they hash on nearly every fixture. + /// + internal static int WorkFactor = 11; + + /// + /// Hashes a password for storage, or returns null if it is unusable. bcrypt generates its + /// own salt, so calling this twice with the same password yields two different hashes. + /// + public static string Hash(string password) { + if (!IsAcceptable(password)) { + return null; + } + + return BCrypt.Net.BCrypt.HashPassword(password, WorkFactor); + } + + /// + /// Verifies a password against a stored hash, in time independent of how much of the hash + /// matched. Returns false for anything malformed rather than throwing. + /// + public static bool Verify(string password, string storedHash) { + if (String.IsNullOrEmpty(password) || String.IsNullOrEmpty(storedHash)) { + return false; + } + + try { + return BCrypt.Net.BCrypt.Verify(password, storedHash); + } catch (BCrypt.Net.SaltParseException) { + // Not a bcrypt hash at all + return false; + } + } + + /// + /// Whether a password can be stored, i.e. non-empty and within bcrypt's input limit. + /// + public static bool IsAcceptable(string password) { + return !String.IsNullOrEmpty(password) + && Encoding.UTF8.GetByteCount(password) <= MaxPasswordBytes; + } + } +} diff --git a/WaveBox.Server/WaveBox.Server.csproj b/WaveBox.Server/WaveBox.Server.csproj index c3253d5..ba39a6b 100644 --- a/WaveBox.Server/WaveBox.Server.csproj +++ b/WaveBox.Server/WaveBox.Server.csproj @@ -41,8 +41,11 @@ - - + + + + diff --git a/WaveBox.Server/res/migrations/README.md b/WaveBox.Server/res/migrations/README.md new file mode 100644 index 0000000..3c94b9b --- /dev/null +++ b/WaveBox.Server/res/migrations/README.md @@ -0,0 +1,39 @@ +Database migrations +=================== + +Ordered SQL scripts that bring an existing `wavebox.db` up to the current schema. The server +applies any whose version is above the database's own on startup, then records the new version in +the single-row `Version` table. + +`../wavebox.sql` is a **frozen baseline at version 0**. Do not edit it to add or change a column — +write a migration instead. A fresh database is the baseline plus every migration replayed in order, +so migrations are exercised by every fresh install and every CI run rather than only being tried +for the first time against somebody's real database. + +Naming +------ + + 00001_add_user_nickname.sql + 00002_index_song_release_year.sql + +Five zero-padded digits, an underscore, then a short description of what the migration does. The +padding is so lexical and numeric order agree when you list the directory; the server sorts +numerically either way. Gaps are fine — two branches can merge out of order — but two files sharing +a version number is an error, and so is a name that doesn't match the pattern. Both fail at startup +rather than being skipped silently. + +Writing one +----------- + +* **No `BEGIN`, `COMMIT` or `ROLLBACK`.** The server wraps each migration in a transaction together + with its version bump, so either the whole file applies or none of it does. +* Multiple statements per file are fine. Scripts run through SQLite's own parser, so semicolons + inside string literals, comments and trigger bodies are handled correctly. +* Write them to be safe against a partially-set-up database where you can (`IF NOT EXISTS`), but + don't contort the SQL for it — a failed migration stops startup with the file name in the log. +* SQLite's `ALTER TABLE` is limited. Anything beyond adding a column or renaming means the usual + dance: create the new table, `INSERT INTO ... SELECT`, drop the old, rename. That is several + statements in one file, which is supported. + +Once a migration has shipped in a release, treat it as immutable — editing it will not re-run on +databases that already recorded its version. diff --git a/WaveBox.Server/res/wavebox.db b/WaveBox.Server/res/wavebox.db deleted file mode 100644 index b0ebca0..0000000 Binary files a/WaveBox.Server/res/wavebox.db and /dev/null differ diff --git a/WaveBox.Server/res/wavebox.sql b/WaveBox.Server/res/wavebox.sql index 47e83bd..df2b13f 100644 --- a/WaveBox.Server/res/wavebox.sql +++ b/WaveBox.Server/res/wavebox.sql @@ -1,6 +1,4 @@ -PRAGMA foreign_keys=OFF; -BEGIN TRANSACTION; CREATE TABLE ItemType ( "ItemTypeId" INTEGER PRIMARY KEY ASC AUTOINCREMENT, "Name" TEXT UNIQUE @@ -136,7 +134,6 @@ CREATE TABLE "User" ( "UserName" TEXT UNIQUE NOT NULL, "Role" INTEGER NOT NULL, "PasswordHash" TEXT NOT NULL, - "PasswordSalt" TEXT NOT NULL, "LastfmSession" TEXT, "CreateTime" INTEGER NOT NULL, "DeleteTime" INTEGER, @@ -165,6 +162,8 @@ CREATE TABLE "Song" ( CREATE TABLE "Version" ( "VersionNumber" INTEGER NOT NULL ); +-- Schema baseline. This file is frozen; further changes go in res/migrations/. +INSERT INTO "Version" VALUES(0); CREATE TABLE "Favorite" ( "FavoriteId" INTEGER NOT NULL UNIQUE, "FavoriteUserId" INTEGER NOT NULL, @@ -210,4 +209,3 @@ CREATE INDEX "song_ItemId" ON "Song" ("ItemId"); CREATE INDEX "favorite_userId" ON "Favorite" ("FavoriteUserId"); CREATE UNIQUE INDEX "user_ApiKey" ON "User" ("ApiKey"); CREATE UNIQUE INDEX "album_AlbumNameArtistId" ON "Album" ("AlbumName", "AlbumArtistId"); -COMMIT; diff --git a/WaveBox.Server/res/wavebox_querylog.db b/WaveBox.Server/res/wavebox_querylog.db deleted file mode 100644 index 0d594cf..0000000 Binary files a/WaveBox.Server/res/wavebox_querylog.db and /dev/null differ diff --git a/WaveBox.Server/res/wavebox_querylog.sql b/WaveBox.Server/res/wavebox_querylog.sql new file mode 100644 index 0000000..a020171 --- /dev/null +++ b/WaveBox.Server/res/wavebox_querylog.sql @@ -0,0 +1,5 @@ +CREATE TABLE "QueryLog" ( + "QueryId" INTEGER NOT NULL PRIMARY KEY, + "QueryString" TEXT NOT NULL, + "ValuesString" TEXT NOT NULL +); diff --git a/WaveBox.Server/src/Extensions/StreamExtensions.cs b/WaveBox.Server/src/Extensions/StreamExtensions.cs index 6d14770..5a1e021 100644 --- a/WaveBox.Server/src/Extensions/StreamExtensions.cs +++ b/WaveBox.Server/src/Extensions/StreamExtensions.cs @@ -11,27 +11,11 @@ namespace WaveBox.Core.Extensions { public static class StreamExtensions { /// - /// Generates a MD5 sum of a given input Stream - /// Thanks: http://msdn.microsoft.com/en-us/library/s02tk69a.aspx + /// Generates a MD5 sum of a given input Stream, as lowercase hex /// public static string MD5(this Stream input) { - using (System.Security.Cryptography.MD5 md5 = System.Security.Cryptography.MD5.Create()) { - // Convert the input string to a byte array and compute the hash. - byte[] data = md5.ComputeHash(input); - - // Create a new Stringbuilder to collect the bytes - // and create a string. - StringBuilder sBuilder = new StringBuilder(); - - // Loop through each byte of the hashed data - // and format each one as a hexadecimal string. - for (int i = 0; i < data.Length; i++) { - sBuilder.Append(data[i].ToString("x2")); - } - - // Return the hexadecimal string. - return sBuilder.ToString(); - } + byte[] hash = System.Security.Cryptography.MD5.HashData(input); + return Convert.ToHexString(hash).ToLowerInvariant(); } /// diff --git a/WaveBox.Server/src/Static/Database.cs b/WaveBox.Server/src/Static/Database.cs index 135711b..df8d471 100644 --- a/WaveBox.Server/src/Static/Database.cs +++ b/WaveBox.Server/src/Static/Database.cs @@ -15,11 +15,16 @@ public class Database : IDatabase { private static readonly WaveBox.Core.Logging.ILog logger = WaveBox.Core.Logging.LogManager.GetLogger(); private static readonly string DATABASE_FILE_NAME = "wavebox.db"; - public string DatabaseTemplatePath { get { return ServerUtility.ExecutablePath() + "res" + Path.DirectorySeparatorChar + DATABASE_FILE_NAME; } } + private static readonly string DATABASE_SCHEMA_FILE_NAME = "wavebox.sql"; + public string DatabaseSchemaPath { get { return ServerUtility.ExecutablePath() + "res" + Path.DirectorySeparatorChar + DATABASE_SCHEMA_FILE_NAME; } } public string DatabasePath { get { return ServerUtility.RootPath() + DATABASE_FILE_NAME; } } + private static readonly string MIGRATIONS_DIR_NAME = "migrations"; + public string MigrationsPath { get { return ServerUtility.ExecutablePath() + "res" + Path.DirectorySeparatorChar + MIGRATIONS_DIR_NAME; } } + private static readonly string QUERY_LOG_FILE_NAME = "wavebox_querylog.db"; - public string QuerylogTemplatePath { get { return ServerUtility.ExecutablePath() + "res" + Path.DirectorySeparatorChar + QUERY_LOG_FILE_NAME; } } + private static readonly string QUERY_LOG_SCHEMA_FILE_NAME = "wavebox_querylog.sql"; + public string QuerylogSchemaPath { get { return ServerUtility.ExecutablePath() + "res" + Path.DirectorySeparatorChar + QUERY_LOG_SCHEMA_FILE_NAME; } } public string QuerylogPath { get { return ServerUtility.RootPath() + QUERY_LOG_FILE_NAME; } } private static readonly object dbBackupLock = new object(); @@ -54,73 +59,59 @@ public Database() { } public void DatabaseSetup() { - if (!File.Exists(DatabasePath)) { - try { - logger.IfInfo("Database file doesn't exist; Creating it : " + DATABASE_FILE_NAME); - - // new filestream on the template - FileStream dbTemplate = new FileStream(DatabaseTemplatePath, FileMode.Open); - - // a new byte array - byte[] dbData = new byte[dbTemplate.Length]; - - // read the template file into memory - dbTemplate.Read(dbData, 0, Convert.ToInt32(dbTemplate.Length)); - - // write it all out - System.IO.File.WriteAllBytes(DatabasePath, dbData); - - // close the template file - dbTemplate.Close(); - } catch (Exception e) { - logger.Error(e); - } - } - - // Upgrade databases created before newer columns existed (the bundled template's - // Version table is empty, so schema state is detected per-column instead) - this.UpgradeSchema(); - - if (!File.Exists(QuerylogPath)) { - try { - logger.IfInfo("Query log database file doesn't exist; Creating it : " + QUERY_LOG_FILE_NAME); - - // new filestream on the template - FileStream dbTemplate = new FileStream(QuerylogTemplatePath, FileMode.Open); - - // a new byte array - byte[] dbData = new byte[dbTemplate.Length]; - - // read the template file into memory - dbTemplate.Read(dbData, 0, Convert.ToInt32(dbTemplate.Length)); + ApplySchemaIfEmpty(DATABASE_FILE_NAME, DatabaseSchemaPath, GetSqliteConnection, CloseSqliteConnection); + ApplyMigrations(); - // write it all out - System.IO.File.WriteAllBytes(QuerylogPath, dbData); + // The query log is a single table that has never changed, so it has no migrations + ApplySchemaIfEmpty(QUERY_LOG_FILE_NAME, QuerylogSchemaPath, GetQueryLogSqliteConnection, CloseQueryLogSqliteConnection); + } - // close the template file - dbTemplate.Close(); - } catch (Exception e) { - logger.Error(e); - } + private void ApplyMigrations() { + ISQLiteConnection conn = null; + try { + conn = GetSqliteConnection(); + DatabaseMigrator.Apply(conn, MigrationsPath); + } catch (Exception e) { + // Same reasoning as the schema apply: a half-migrated database only produces + // confusing errors later, so surface it now + logger.Error(e); + throw; + } finally { + CloseSqliteConnection(conn); } } - private void UpgradeSchema() { + /// + /// Creates a database from its bundled schema script if it has no tables yet. Checking for + /// tables rather than for the file means an empty database left behind by an earlier + /// connection still gets its schema, and makes repeat calls a no-op. + /// + private void ApplySchemaIfEmpty(string name, string schemaPath, Func open, Action close) { ISQLiteConnection conn = null; try { - conn = GetSqliteConnection(); + conn = open(); + + if (conn.ExecuteScalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'table'") > 0) { + return; + } + + logger.IfInfo("Database " + name + " is empty; applying schema from " + schemaPath); - // Subsonic API keys: User.ApiKey column + unique index - int hasApiKey = conn.ExecuteScalar("SELECT COUNT(*) FROM pragma_table_info('User') WHERE name = 'ApiKey'"); - if (hasApiKey == 0) { - logger.IfInfo("Upgrading database schema: adding User.ApiKey"); - conn.Execute("ALTER TABLE User ADD COLUMN ApiKey TEXT"); + conn.BeginTransaction(); + try { + conn.ExecuteScript(File.ReadAllText(schemaPath)); + conn.Commit(); + } catch (Exception) { + conn.Rollback(); + throw; } - conn.Execute("CREATE UNIQUE INDEX IF NOT EXISTS user_ApiKey ON User(ApiKey)"); } catch (Exception e) { + // Log before rethrowing so the failure lands in the server log, then fail loudly: + // continuing with no schema only turns into confusing null references later on logger.Error(e); + throw; } finally { - CloseSqliteConnection(conn); + close(conn); } } diff --git a/WaveBox.Server/src/Static/DatabaseMigrator.cs b/WaveBox.Server/src/Static/DatabaseMigrator.cs new file mode 100644 index 0000000..cb8cc95 --- /dev/null +++ b/WaveBox.Server/src/Static/DatabaseMigrator.cs @@ -0,0 +1,132 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Text.RegularExpressions; +using Cirrious.MvvmCross.Plugins.Sqlite; +using WaveBox.Core.Extensions; + +namespace WaveBox.Static { + /// + /// Applies the ordered SQL migrations in res/migrations to a database, tracking how far it has + /// got in the single-row Version table. + /// + /// res/wavebox.sql is a frozen baseline at version 0; every schema change since is a migration, + /// so a fresh database is the seed plus every migration replayed in order. Keeping one path + /// means the migrations are exercised on every fresh install and every CI run, rather than + /// only ever being tried against a real database on someone's server. + /// + internal static class DatabaseMigrator { + private static readonly WaveBox.Core.Logging.ILog logger = WaveBox.Core.Logging.LogManager.GetLogger(); + + // 00001_add_something.sql -- zero padded so lexical and numeric order agree at a glance, + // though ordering below is numeric regardless + private static readonly Regex FileNamePattern = new Regex(@"^(\d{5})_.+\.sql$", RegexOptions.Compiled); + + internal sealed class Migration { + internal int Version { get; set; } + internal string Path { get; set; } + internal string Name { get; set; } + } + + /// + /// Reads the migrations directory, ordered by version. Throws on a malformed file name or a + /// duplicate version, so a typo fails loudly at startup instead of silently not running. + /// Gaps in numbering are fine -- branches merge out of order. + /// + internal static IList Discover(string directory) { + List migrations = new List(); + + if (!Directory.Exists(directory)) { + return migrations; + } + + Dictionary seen = new Dictionary(); + + foreach (string path in Directory.GetFiles(directory, "*.sql")) { + string name = System.IO.Path.GetFileName(path); + + Match match = FileNamePattern.Match(name); + if (!match.Success) { + throw new InvalidOperationException( + "Migration file name '" + name + "' is malformed; expected a 5 digit version, " + + "an underscore and a description, e.g. 00001_add_user_nickname.sql"); + } + + int version = Int32.Parse(match.Groups[1].Value, CultureInfo.InvariantCulture); + + string existing; + if (seen.TryGetValue(version, out existing)) { + throw new InvalidOperationException( + "Migrations '" + existing + "' and '" + name + "' share version " + version + + "; renumber one of them"); + } + seen[version] = name; + + migrations.Add(new Migration { Version = version, Path = path, Name = name }); + } + + migrations.Sort((a, b) => a.Version.CompareTo(b.Version)); + + return migrations; + } + + /// + /// Brings a database up to the newest bundled migration. Each migration and its version bump + /// share a transaction, so a failure part way through a chain leaves the database sitting + /// cleanly at the last migration that did succeed. + /// + internal static void Apply(ISQLiteConnection conn, string directory) { + IList migrations = Discover(directory); + + int current = CurrentVersion(conn); + int newest = migrations.Count > 0 ? migrations[migrations.Count - 1].Version : 0; + + if (current > newest) { + throw new InvalidOperationException( + "Database is at schema version " + current + " but this build only knows up to " + + newest + "; it was created by a newer version of WaveBox. Upgrade WaveBox, or " + + "delete the database to start fresh."); + } + + foreach (Migration migration in migrations) { + if (migration.Version <= current) { + continue; + } + + logger.IfInfo("Applying database migration " + migration.Name); + + conn.BeginTransaction(); + try { + conn.ExecuteScript(File.ReadAllText(migration.Path)); + SetVersion(conn, migration.Version); + conn.Commit(); + } catch (Exception e) { + conn.Rollback(); + logger.Error("Migration " + migration.Name + " failed; database left at version " + current, e); + throw; + } + + current = migration.Version; + } + } + + /// + /// The schema version, treating a missing row as 0. Databases created before versioning + /// existed have the table but no row in it. + /// + internal static int CurrentVersion(ISQLiteConnection conn) { + if (conn.ExecuteScalar("SELECT COUNT(*) FROM Version") == 0) { + return 0; + } + + return conn.ExecuteScalar("SELECT VersionNumber FROM Version LIMIT 1"); + } + + private static void SetVersion(ISQLiteConnection conn, int version) { + // Replace rather than update, so a database missing its row heals itself + conn.Execute("DELETE FROM Version"); + conn.Execute("INSERT INTO Version (VersionNumber) VALUES (?)", version); + } + } +} diff --git a/WaveBox.Server/src/WaveBoxMain.cs b/WaveBox.Server/src/WaveBoxMain.cs index f3f04e1..b22c05d 100644 --- a/WaveBox.Server/src/WaveBoxMain.cs +++ b/WaveBox.Server/src/WaveBoxMain.cs @@ -43,7 +43,17 @@ public void Start() { } // Perform initial setup of Settings, Database - Injection.Get().DatabaseSetup(); + try { + Injection.Get().DatabaseSetup(); + } catch (Exception e) { + // The database is unusable, so there is nothing to serve. Report it as a plain + // message rather than letting a stack trace be the whole story -- these failures + // are things an operator can act on, like a misnamed or broken migration. + logger.Error("Database setup failed: " + e.Message); + Console.Error.WriteLine("WaveBox cannot start: " + e.Message); + Environment.Exit(1); + } + Injection.Get().SettingsSetup(); // Start services diff --git a/tests/WaveBox.Core.Tests/ByteExtensionsTests.cs b/tests/WaveBox.Core.Tests/ByteExtensionsTests.cs index 6bbd595..2515bb3 100644 --- a/tests/WaveBox.Core.Tests/ByteExtensionsTests.cs +++ b/tests/WaveBox.Core.Tests/ByteExtensionsTests.cs @@ -6,9 +6,7 @@ namespace WaveBox.Core.Tests { public class ByteExtensionsTests { [Fact] - public void MD5_KnownVector_IsLowercaseHexWithoutDashes() { - // Note: byte[].MD5() is lowercase without dashes, unlike string.MD5() which is - // dash-separated uppercase — both formats are pinned deliberately + public void MD5_KnownVector_IsLowercaseHex() { byte[] input = Encoding.ASCII.GetBytes("test"); Assert.Equal("098f6bcd4621d373cade4e832627b4f6", input.MD5()); } diff --git a/tests/WaveBox.Core.Tests/DateTimeExtensionsTests.cs b/tests/WaveBox.Core.Tests/DateTimeExtensionsTests.cs index 78c82aa..4de27c0 100644 --- a/tests/WaveBox.Core.Tests/DateTimeExtensionsTests.cs +++ b/tests/WaveBox.Core.Tests/DateTimeExtensionsTests.cs @@ -63,8 +63,8 @@ public void ToETag_IsSha1OfRFC1123String() { DateTime dt = new DateTime(2001, 2, 3, 4, 5, 6, DateTimeKind.Utc); string etag = dt.ToETag(); Assert.Equal(dt.ToRFC1123().SHA1(), etag); - // 40 uppercase hex chars (SHA1 in this codebase's uppercase, dash-stripped format) - Assert.Matches(new Regex("^[0-9A-F]{40}$"), etag); + // 40 lowercase hex chars + Assert.Matches(new Regex("^[0-9a-f]{40}$"), etag); } } } diff --git a/tests/WaveBox.Core.Tests/StringExtensionsTests.cs b/tests/WaveBox.Core.Tests/StringExtensionsTests.cs index 377b73d..8011e97 100644 --- a/tests/WaveBox.Core.Tests/StringExtensionsTests.cs +++ b/tests/WaveBox.Core.Tests/StringExtensionsTests.cs @@ -1,4 +1,5 @@ using System; +using System.Text.RegularExpressions; using WaveBox.Core.Extensions; using Xunit; @@ -32,12 +33,15 @@ public void IsTrue_MatchesTruthyMatrix(string input, bool expected) { } [Fact] - public void MD5_KnownVector_IsDashSeparatedUppercaseHex() { - // Pinned: string.MD5() returns BitConverter.ToString output (dash-separated uppercase - // hex), unlike SHA1() which strips the dashes. The asymmetry is intentional pinning - // of current behavior — hashed values are compared against stored values in the same - // format, so changing it would break existing data. - Assert.Equal("09-8F-6B-CD-46-21-D3-73-CA-DE-4E-83-26-27-B4-F6", "test".MD5()); + public void MD5_KnownVector_IsLowercaseHex() { + Assert.Equal("098f6bcd4621d373cade4e832627b4f6", "test".MD5()); + } + + [Fact] + public void MD5_IsPlain32CharLowercaseHex() { + // Last.fm's api_sig must be "a 32-character hexadecimal md5 hash", and Lastfm.cs + // builds it straight from this method, so the exact shape is externally load-bearing + Assert.Matches(new Regex("^[0-9a-f]{32}$"), "any input at all".MD5()); } [Fact] @@ -47,8 +51,8 @@ public void MD5_EmptyOrNull_ReturnsEmptyString() { } [Fact] - public void SHA1_KnownVector_IsUppercaseHexWithoutDashes() { - Assert.Equal("A94A8FE5CCB19BA61C4C0873D391E987982FBBD3", "test".SHA1()); + public void SHA1_KnownVector_IsLowercaseHex() { + Assert.Equal("a94a8fe5ccb19ba61c4c0873d391e987982fbbd3", "test".SHA1()); } [Fact] diff --git a/tests/WaveBox.Core.Tests/TestPasswordWorkFactor.cs b/tests/WaveBox.Core.Tests/TestPasswordWorkFactor.cs new file mode 100644 index 0000000..981ae9e --- /dev/null +++ b/tests/WaveBox.Core.Tests/TestPasswordWorkFactor.cs @@ -0,0 +1,16 @@ +using System.Runtime.CompilerServices; +using WaveBox.Core.Static; + +namespace WaveBox.Core.Tests { + internal static class TestPasswordWorkFactor { + /// + /// Drops bcrypt to its minimum work factor for the duration of the test run. The production + /// value is tuned to take a noticeable fraction of a second per hash, and fixtures across + /// this assembly create users constantly, so leaving it alone would dominate the run time. + /// + [ModuleInitializer] + internal static void Reduce() { + PasswordHasher.WorkFactor = 4; + } + } +} diff --git a/tests/WaveBox.Core.Tests/UserCryptoTests.cs b/tests/WaveBox.Core.Tests/UserCryptoTests.cs index 4a3de4d..df82321 100644 --- a/tests/WaveBox.Core.Tests/UserCryptoTests.cs +++ b/tests/WaveBox.Core.Tests/UserCryptoTests.cs @@ -1,50 +1,80 @@ using System; using WaveBox.Core.Model; +using WaveBox.Core.Static; using Xunit; namespace WaveBox.Core.Tests { // Only User's static/pure members are tested here; everything else on User goes through // Injection and repositories, which are process-global and off limits for unit tests. - // PBKDF2 at 2500 iterations is deliberately slow, so hash computations are kept to a minimum. + // bcrypt is deliberately slow even at the reduced test work factor, so hash computations + // are kept to a minimum. public class UserCryptoTests { [Fact] - public void ComputePasswordHash_IsDeterministicForSameSaltAndDiffersAcrossSalts() { - string hash1 = User.ComputePasswordHash("password", "saltA"); - string hash2 = User.ComputePasswordHash("password", "saltA"); - string hash3 = User.ComputePasswordHash("password", "saltB"); - - Assert.Equal(hash1, hash2); - Assert.NotEqual(hash1, hash3); - - // 64-byte PBKDF2 output as base64: 88 chars - Assert.Equal(88, hash1.Length); - byte[] decoded = Convert.FromBase64String(hash1); - Assert.Equal(64, decoded.Length); + public void Hash_IsSelfSaltingSoRepeatsDiffer() { + // bcrypt generates its own salt per call, so unlike the old PBKDF2 scheme the same + // password never produces the same hash twice + string hash1 = PasswordHasher.Hash("password"); + string hash2 = PasswordHasher.Hash("password"); + + Assert.NotEqual(hash1, hash2); + Assert.True(PasswordHasher.Verify("password", hash1)); + Assert.True(PasswordHasher.Verify("password", hash2)); + } + + [Fact] + public void Hash_IsAModularCryptFormatBcryptString() { + string hash = PasswordHasher.Hash("password"); + + // "$2$$<22 char salt><31 char hash>" + Assert.StartsWith("$2", hash); + Assert.Equal(60, hash.Length); + } + + [Fact] + public void Verify_RejectsWrongPassword() { + string hash = PasswordHasher.Hash("password1"); + + Assert.False(PasswordHasher.Verify("password2", hash)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("not a bcrypt hash")] + [InlineData("$2a$truncated")] + public void Verify_ReturnsFalseForUnusableHashesInsteadOfThrowing(string storedHash) { + Assert.False(PasswordHasher.Verify("password", storedHash)); } [Fact] - public void ComputePasswordHash_DiffersAcrossPasswords() { - Assert.NotEqual( - User.ComputePasswordHash("password1", "saltA"), - User.ComputePasswordHash("password2", "saltA")); + public void Verify_ReturnsFalseForEmptyPassword() { + string hash = PasswordHasher.Hash("password"); + + Assert.False(PasswordHasher.Verify(null, hash)); + Assert.False(PasswordHasher.Verify("", hash)); } [Fact] - public void GeneratePasswordSalt_Is32RandomBytesBase64AndUnique() { - string salt1 = User.GeneratePasswordSalt(); - string salt2 = User.GeneratePasswordSalt(); + public void Hash_RejectsPasswordsBeyondBcryptsInputLimit() { + // bcrypt reads only the first 72 bytes and ignores the rest, so an over-long password + // must be refused rather than silently truncated to something weaker than it looks + Assert.Null(PasswordHasher.Hash(new string('a', PasswordHasher.MaxPasswordBytes + 1))); + Assert.NotNull(PasswordHasher.Hash(new string('a', PasswordHasher.MaxPasswordBytes))); + + // The limit is bytes, not characters: this is 73 bytes of UTF-8 in 25 characters + Assert.Null(PasswordHasher.Hash(new string('é', 36) + "a")); + } - Assert.Equal(32, Convert.FromBase64String(salt1).Length); - Assert.Equal(32, Convert.FromBase64String(salt2).Length); - Assert.NotEqual(salt1, salt2); + [Fact] + public void Hash_RejectsNullOrEmptyPassword() { + Assert.Null(PasswordHasher.Hash(null)); + Assert.Null(PasswordHasher.Hash("")); } [Fact] public void Authenticate_AcceptsCorrectPasswordAndRejectsWrongOne() { - string salt = "fixed-salt"; User user = new User { - PasswordSalt = salt, - PasswordHash = User.ComputePasswordHash("correct horse", salt) + PasswordHash = PasswordHasher.Hash("correct horse") }; Assert.True(user.Authenticate("correct horse")); diff --git a/tests/WaveBox.Server.Tests/DatabaseMigratorTests.cs b/tests/WaveBox.Server.Tests/DatabaseMigratorTests.cs new file mode 100644 index 0000000..858ce32 --- /dev/null +++ b/tests/WaveBox.Server.Tests/DatabaseMigratorTests.cs @@ -0,0 +1,216 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Cirrious.MvvmCross.Plugins.Sqlite; +using WaveBox.Static; +using Xunit; + +namespace WaveBox.Server.Tests { + /// + /// Exercises the migrator directly against a throwaway database and migrations directory, so + /// none of this depends on the bundled res/migrations content (which is empty today). + /// + public class DatabaseMigratorTests : IDisposable { + private readonly string workDir; + private readonly string migrationsDir; + private readonly string dbPath; + private readonly SQLite.SQLiteConnection conn; + + public DatabaseMigratorTests() { + workDir = Directory.CreateTempSubdirectory("wavebox-migrations-").FullName; + migrationsDir = Path.Combine(workDir, "migrations"); + Directory.CreateDirectory(migrationsDir); + + dbPath = Path.Combine(workDir, "test.db"); + conn = new SQLite.SQLiteConnection(dbPath); + conn.ExecuteScript("CREATE TABLE Version (VersionNumber INTEGER NOT NULL); INSERT INTO Version VALUES (0);"); + } + + public void Dispose() { + conn.Dispose(); + try { + Directory.Delete(workDir, true); + } catch (IOException) { + } + } + + private void WriteMigration(string name, string sql) { + File.WriteAllText(Path.Combine(migrationsDir, name), sql); + } + + private bool ColumnExists(string table, string column) { + return conn.ExecuteScalar( + "SELECT COUNT(*) FROM pragma_table_info('" + table + "') WHERE name = '" + column + "'") > 0; + } + + // --- Discover ------------------------------------------------------- + + [Fact] + public void DiscoverOrdersNumericallyNotLexically() { + WriteMigration("00010_ten.sql", "SELECT 1;"); + WriteMigration("00002_two.sql", "SELECT 1;"); + WriteMigration("00001_one.sql", "SELECT 1;"); + + IList found = DatabaseMigrator.Discover(migrationsDir); + + Assert.Equal(new int[] { 1, 2, 10 }, found.Select(m => m.Version).ToArray()); + } + + [Fact] + public void DiscoverIgnoresNonSqlFiles() { + WriteMigration("00001_one.sql", "SELECT 1;"); + File.WriteAllText(Path.Combine(migrationsDir, "README.md"), "not a migration"); + File.WriteAllText(Path.Combine(migrationsDir, "notes.txt"), "also not"); + + Assert.Single(DatabaseMigrator.Discover(migrationsDir)); + } + + [Fact] + public void DiscoverReturnsEmptyForMissingDirectory() { + Assert.Empty(DatabaseMigrator.Discover(Path.Combine(workDir, "does-not-exist"))); + } + + [Theory] + [InlineData("1_short_version.sql")] + [InlineData("000001_six_digits.sql")] + [InlineData("00001-wrong-separator.sql")] + [InlineData("00001_.sql")] + [InlineData("no_version_at_all.sql")] + public void DiscoverThrowsOnMalformedFileName(string name) { + WriteMigration(name, "SELECT 1;"); + + InvalidOperationException error = Assert.Throws( + () => DatabaseMigrator.Discover(migrationsDir)); + + Assert.Contains(name, error.Message); + } + + [Fact] + public void DiscoverThrowsOnDuplicateVersion() { + WriteMigration("00001_one.sql", "SELECT 1;"); + WriteMigration("00001_one_again.sql", "SELECT 1;"); + + InvalidOperationException error = Assert.Throws( + () => DatabaseMigrator.Discover(migrationsDir)); + + Assert.Contains("share version 1", error.Message); + } + + [Fact] + public void DiscoverAllowsGapsInNumbering() { + WriteMigration("00001_one.sql", "SELECT 1;"); + WriteMigration("00007_seven.sql", "SELECT 1;"); + + Assert.Equal(new int[] { 1, 7 }, DatabaseMigrator.Discover(migrationsDir).Select(m => m.Version).ToArray()); + } + + // --- Apply ---------------------------------------------------------- + + [Fact] + public void ApplyRunsPendingMigrationsInOrderAndRecordsVersion() { + conn.ExecuteScript("CREATE TABLE T (a INTEGER);"); + WriteMigration("00001_add_b.sql", "ALTER TABLE T ADD COLUMN b INTEGER;"); + WriteMigration("00002_add_c.sql", "ALTER TABLE T ADD COLUMN c INTEGER;"); + + DatabaseMigrator.Apply(conn, migrationsDir); + + Assert.True(ColumnExists("T", "b")); + Assert.True(ColumnExists("T", "c")); + Assert.Equal(2, DatabaseMigrator.CurrentVersion(conn)); + } + + [Fact] + public void ApplySkipsMigrationsAtOrBelowCurrentVersion() { + conn.ExecuteScript("CREATE TABLE T (a INTEGER); DELETE FROM Version; INSERT INTO Version VALUES (1);"); + // Would fail if it ran, since column a already exists + WriteMigration("00001_already_applied.sql", "ALTER TABLE T ADD COLUMN a INTEGER;"); + WriteMigration("00002_add_b.sql", "ALTER TABLE T ADD COLUMN b INTEGER;"); + + DatabaseMigrator.Apply(conn, migrationsDir); + + Assert.True(ColumnExists("T", "b")); + Assert.Equal(2, DatabaseMigrator.CurrentVersion(conn)); + } + + [Fact] + public void ApplyIsANoOpWhenAlreadyCurrent() { + conn.ExecuteScript("CREATE TABLE T (a INTEGER);"); + WriteMigration("00001_add_b.sql", "ALTER TABLE T ADD COLUMN b INTEGER;"); + + DatabaseMigrator.Apply(conn, migrationsDir); + DatabaseMigrator.Apply(conn, migrationsDir); + + Assert.Equal(1, DatabaseMigrator.CurrentVersion(conn)); + } + + [Fact] + public void ApplyRollsBackAFailingMigrationAndStopsAtTheLastGoodVersion() { + conn.ExecuteScript("CREATE TABLE T (a INTEGER);"); + WriteMigration("00001_add_b.sql", "ALTER TABLE T ADD COLUMN b INTEGER;"); + WriteMigration("00002_broken.sql", "ALTER TABLE T ADD COLUMN c INTEGER;\nINSERT INTO NotATable VALUES (1);"); + WriteMigration("00003_add_d.sql", "ALTER TABLE T ADD COLUMN d INTEGER;"); + + Assert.ThrowsAny(() => DatabaseMigrator.Apply(conn, migrationsDir)); + + Assert.True(ColumnExists("T", "b")); + // 00002 is atomic, so its first statement is rolled back too, and 00003 never runs + Assert.False(ColumnExists("T", "c")); + Assert.False(ColumnExists("T", "d")); + Assert.Equal(1, DatabaseMigrator.CurrentVersion(conn)); + } + + [Fact] + public void ApplyThrowsWhenTheDatabaseIsNewerThanTheBuild() { + conn.ExecuteScript("DELETE FROM Version; INSERT INTO Version VALUES (9);"); + WriteMigration("00001_one.sql", "SELECT 1;"); + + InvalidOperationException error = Assert.Throws( + () => DatabaseMigrator.Apply(conn, migrationsDir)); + + Assert.Contains("newer version of WaveBox", error.Message); + } + + [Fact] + public void ApplyTreatsAMissingVersionRowAsZero() { + conn.ExecuteScript("CREATE TABLE T (a INTEGER); DELETE FROM Version;"); + WriteMigration("00001_add_b.sql", "ALTER TABLE T ADD COLUMN b INTEGER;"); + + Assert.Equal(0, DatabaseMigrator.CurrentVersion(conn)); + + DatabaseMigrator.Apply(conn, migrationsDir); + + Assert.True(ColumnExists("T", "b")); + Assert.Equal(1, DatabaseMigrator.CurrentVersion(conn)); + } + + [Fact] + public void ApplyKeepsASingleVersionRow() { + conn.ExecuteScript("CREATE TABLE T (a INTEGER);"); + WriteMigration("00001_add_b.sql", "ALTER TABLE T ADD COLUMN b INTEGER;"); + WriteMigration("00002_add_c.sql", "ALTER TABLE T ADD COLUMN c INTEGER;"); + + DatabaseMigrator.Apply(conn, migrationsDir); + + Assert.Equal(1, conn.ExecuteScalar("SELECT COUNT(*) FROM Version")); + } + + [Fact] + public void ApplyHandlesMultiStatementMigrations() { + // The SQLite table-rebuild dance, which is the realistic shape of a non-trivial migration + conn.ExecuteScript("CREATE TABLE T (a INTEGER, b TEXT); INSERT INTO T VALUES (1, 'keep;me');"); + WriteMigration("00001_rebuild.sql", @" + CREATE TABLE T_new (a INTEGER, b TEXT, c INTEGER DEFAULT 0); + INSERT INTO T_new (a, b) SELECT a, b FROM T; + DROP TABLE T; + ALTER TABLE T_new RENAME TO T; + "); + + DatabaseMigrator.Apply(conn, migrationsDir); + + Assert.True(ColumnExists("T", "c")); + Assert.Equal("keep;me", conn.ExecuteScalar("SELECT b FROM T")); + Assert.Equal(1, DatabaseMigrator.CurrentVersion(conn)); + } + } +} diff --git a/tests/WaveBox.Server.Tests/DatabaseSetupTests.cs b/tests/WaveBox.Server.Tests/DatabaseSetupTests.cs index 9907d52..b360ea9 100644 --- a/tests/WaveBox.Server.Tests/DatabaseSetupTests.cs +++ b/tests/WaveBox.Server.Tests/DatabaseSetupTests.cs @@ -30,23 +30,41 @@ private static int Scalar(string sql) { } } - private static void Execute(string sql) { + private static int QuerylogScalar(string sql) { IDatabase db = Injection.Get(); ISQLiteConnection conn = null; try { - conn = db.GetSqliteConnection(); - conn.Execute(sql); + conn = db.GetQueryLogSqliteConnection(); + return conn.ExecuteScalar(sql); } finally { - db.CloseSqliteConnection(conn); + db.CloseQueryLogSqliteConnection(conn); } } [Fact] - public void SetupCopiesTemplateDatabasesIntoRoot() { + public void SetupCreatesDatabasesInRoot() { Assert.True(File.Exists(Path.Combine(harness.Root.Path, "wavebox.db"))); Assert.True(File.Exists(Path.Combine(harness.Root.Path, "wavebox_querylog.db"))); } + [Fact] + public void SetupAppliesFullSchemaFromScript() { + // The whole script has to run, not just its first statement: a partial apply would + // still produce a file and a User table, so assert on tables declared near the end + Assert.Equal(1, Scalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'User'")); + Assert.Equal(1, Scalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'Favorite'")); + Assert.Equal(1, Scalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'MusicBrainzCheckDate'")); + Assert.Equal(1, QuerylogScalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'QueryLog'")); + } + + [Fact] + public void SetupSeedsLookupTables() { + // These rows came from the old prebuilt wavebox.db template; they now come from the + // INSERTs in wavebox.sql, and nothing else would notice if they went missing + Assert.Equal(12, Scalar("SELECT COUNT(*) FROM ItemType")); + Assert.Equal(10, Scalar("SELECT COUNT(*) FROM FileType")); + } + [Fact] public void SetupAddsApiKeyColumnAndUniqueIndex() { Assert.Equal(1, Scalar("SELECT COUNT(*) FROM pragma_table_info('User') WHERE name = 'ApiKey'")); @@ -61,25 +79,29 @@ public void SetupIsIdempotent() { db.DatabaseSetup(); Assert.Equal(1, Scalar("SELECT COUNT(*) FROM pragma_table_info('User') WHERE name = 'ApiKey'")); + Assert.Equal(12, Scalar("SELECT COUNT(*) FROM ItemType")); Assert.True(File.Exists(Path.Combine(harness.Root.Path, "wavebox.db"))); } [Fact] - public void UpgradeSchemaRestoresApiKeyOnPreMigrationDatabase() { - // Simulate a database created before the ApiKey migration - Execute("DROP INDEX user_ApiKey"); - Execute("ALTER TABLE User DROP COLUMN ApiKey"); - Assert.Equal(0, Scalar("SELECT COUNT(*) FROM pragma_table_info('User') WHERE name = 'ApiKey'")); - - IDatabase db = Injection.Get(); - db.DatabaseSetup(); + public void SetupLeavesTheDatabaseAtTheNewestBundledMigration() { + // The seed is a frozen baseline at 0, so a fresh database ends up at whatever the + // highest migration in res/migrations is -- 0 while that directory is still empty + int expected = 0; + foreach (string path in Directory.GetFiles(Injection.Get().MigrationsPath, "*.sql")) { + expected = Math.Max(expected, Int32.Parse(Path.GetFileName(path).Substring(0, 5))); + } - Assert.Equal(1, Scalar("SELECT COUNT(*) FROM pragma_table_info('User') WHERE name = 'ApiKey'")); - Assert.Equal(1, Scalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'index' AND name = 'user_ApiKey'")); + Assert.Equal(1, Scalar("SELECT COUNT(*) FROM Version")); + Assert.Equal(expected, Scalar("SELECT VersionNumber FROM Version")); + } - // Running the migration again on an already-upgraded schema is a no-op - db.DatabaseSetup(); - Assert.Equal(1, Scalar("SELECT COUNT(*) FROM pragma_table_info('User') WHERE name = 'ApiKey'")); + [Fact] + public void SetupFindsTheBundledMigrationsDirectory() { + // Guards the csproj copy: if res/migrations stops being deployed next to the binary, + // migrations would silently never run rather than failing + Assert.True(Directory.Exists(Injection.Get().MigrationsPath), + "res/migrations was not copied to the output directory"); } [Fact] diff --git a/tests/WaveBox.Server.Tests/SqliteScriptTests.cs b/tests/WaveBox.Server.Tests/SqliteScriptTests.cs new file mode 100644 index 0000000..6dcd234 --- /dev/null +++ b/tests/WaveBox.Server.Tests/SqliteScriptTests.cs @@ -0,0 +1,140 @@ +using System; +using System.IO; +using Cirrious.MvvmCross.Plugins.Sqlite; +using Xunit; + +namespace WaveBox.Server.Tests { + /// + /// ExecuteScript runs a whole SQL script through SQLite's own parser. Every case below except + /// the first breaks a splitter that cuts the text on ';', which is what this replaced. + /// + public class SqliteScriptTests : IDisposable { + private readonly string dbPath; + private readonly SQLite.SQLiteConnection conn; + + public SqliteScriptTests() { + dbPath = Path.Combine(Path.GetTempPath(), "wavebox-script-" + Guid.NewGuid().ToString("N") + ".db"); + conn = new SQLite.SQLiteConnection(dbPath); + } + + public void Dispose() { + conn.Dispose(); + File.Delete(dbPath); + } + + [Fact] + public void RunsEveryStatementNotJustTheFirst() { + conn.ExecuteScript(@" + CREATE TABLE A (x INTEGER); + CREATE TABLE B (y INTEGER); + INSERT INTO A VALUES (1); + INSERT INTO A VALUES (2); + "); + + Assert.Equal(1, conn.ExecuteScalar("SELECT COUNT(*) FROM sqlite_master WHERE name = 'B'")); + Assert.Equal(2, conn.ExecuteScalar("SELECT COUNT(*) FROM A")); + } + + [Fact] + public void PreservesSemicolonsInsideStringLiterals() { + conn.ExecuteScript(@" + CREATE TABLE T (v TEXT); + INSERT INTO T VALUES ('a;b;c'); + INSERT INTO T VALUES ('it''s; quoted'); + "); + + Assert.Equal("a;b;c", conn.ExecuteScalar("SELECT v FROM T ORDER BY rowid LIMIT 1")); + Assert.Equal("it's; quoted", conn.ExecuteScalar("SELECT v FROM T ORDER BY rowid DESC LIMIT 1")); + } + + [Fact] + public void IgnoresSemicolonsInLineAndBlockComments() { + conn.ExecuteScript(@" + -- a comment; with a semicolon + CREATE TABLE T (x INTEGER); + /* another; one + spanning lines; too */ + INSERT INTO T VALUES (42); + "); + + Assert.Equal(42, conn.ExecuteScalar("SELECT x FROM T")); + } + + [Fact] + public void HandlesTriggerBodiesContainingSemicolons() { + conn.ExecuteScript(@" + CREATE TABLE Src (x INTEGER); + CREATE TABLE Log (x INTEGER); + CREATE TRIGGER SrcInsert AFTER INSERT ON Src + BEGIN + INSERT INTO Log VALUES (NEW.x); + UPDATE Log SET x = x * 2; + END; + INSERT INTO Src VALUES (5); + "); + + Assert.Equal(10, conn.ExecuteScalar("SELECT x FROM Log")); + } + + [Fact] + public void RoundTripsNonAsciiText() { + // The script is marshalled as UTF-8 by byte length; the older string overload passed a + // character count as a byte count, which truncates as soon as the text is not ASCII + conn.ExecuteScript(@" + CREATE TABLE T (v TEXT); + INSERT INTO T VALUES ('Björk – Jóga ⟨æ⟩'); + "); + + Assert.Equal("Björk – Jóga ⟨æ⟩", conn.ExecuteScalar("SELECT v FROM T")); + } + + [Fact] + public void ToleratesTrailingWhitespaceAndComments() { + conn.ExecuteScript("CREATE TABLE T (x INTEGER);\n-- trailing comment\n \n"); + + Assert.Equal(1, conn.ExecuteScalar("SELECT COUNT(*) FROM sqlite_master WHERE name = 'T'")); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" \n ")] + [InlineData("-- nothing but a comment")] + public void EmptyScriptsAreNoOps(string script) { + conn.ExecuteScript(script); + + Assert.Equal(0, conn.ExecuteScalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'table'")); + } + + [Fact] + public void ReportsTheFailingStatement() { + SQLite.SQLiteException error = Assert.Throws(() => conn.ExecuteScript(@" + CREATE TABLE T (x INTEGER); + INSERT INTO NotATable VALUES (1); + ")); + + Assert.Contains("NotATable", error.Message); + + // The statements before the failure still ran; the caller owns the transaction + Assert.Equal(1, conn.ExecuteScalar("SELECT COUNT(*) FROM sqlite_master WHERE name = 'T'")); + } + + [Fact] + public void RollsBackWhenTheCallerOwnsATransaction() { + conn.ExecuteScript("CREATE TABLE Keep (x INTEGER);"); + + conn.BeginTransaction(); + try { + conn.ExecuteScript(@" + CREATE TABLE Dropped (x INTEGER); + INSERT INTO NotATable VALUES (1); + "); + } catch (SQLite.SQLiteException) { + conn.Rollback(); + } + + Assert.Equal(1, conn.ExecuteScalar("SELECT COUNT(*) FROM sqlite_master WHERE name = 'Keep'")); + Assert.Equal(0, conn.ExecuteScalar("SELECT COUNT(*) FROM sqlite_master WHERE name = 'Dropped'")); + } + } +} diff --git a/tests/WaveBox.Server.Tests/TestPasswordWorkFactor.cs b/tests/WaveBox.Server.Tests/TestPasswordWorkFactor.cs new file mode 100644 index 0000000..24905f5 --- /dev/null +++ b/tests/WaveBox.Server.Tests/TestPasswordWorkFactor.cs @@ -0,0 +1,16 @@ +using System.Runtime.CompilerServices; +using WaveBox.Core.Static; + +namespace WaveBox.Server.Tests { + internal static class TestPasswordWorkFactor { + /// + /// Drops bcrypt to its minimum work factor for the duration of the test run. The production + /// value is tuned to take a noticeable fraction of a second per hash, and the integration + /// fixtures create a user per test method, so leaving it alone would dominate the run time. + /// + [ModuleInitializer] + internal static void Reduce() { + PasswordHasher.WorkFactor = 4; + } + } +} diff --git a/tests/WaveBox.Server.Tests/UserRepositoryTests.cs b/tests/WaveBox.Server.Tests/UserRepositoryTests.cs index 5434251..8fc1535 100644 --- a/tests/WaveBox.Server.Tests/UserRepositoryTests.cs +++ b/tests/WaveBox.Server.Tests/UserRepositoryTests.cs @@ -28,7 +28,6 @@ public void CreateUserRoundTripsThroughRepositoryAndAuthenticates() { Assert.NotNull(created.UserId); Assert.Equal("alice", created.UserName); Assert.NotNull(created.PasswordHash); - Assert.NotNull(created.PasswordSalt); Assert.NotNull(created.CreateTime); User fetched = users.UserForName("alice");