diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 256759e..1cd735c 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -19,9 +19,7 @@ jobs: - name: Setup .NET uses: actions/setup-dotnet@v5 with: - dotnet-version: | - 8.0.x - 10.0.x + dotnet-version: 10.0.x - name: Restore run: dotnet restore PasswordGenerator.Benchmarks/PasswordGenerator.Benchmarks.csproj @@ -32,7 +30,7 @@ jobs: - name: Run benchmarks run: | cd PasswordGenerator.Benchmarks - dotnet run -c Release --no-build -f net8.0 -- --filter '*' + dotnet run -c Release --no-build -f net10.0 -- --filter '*' - name: Publish results to workflow summary if: always() diff --git a/CHANGELOG.md b/CHANGELOG.md index 991b5c3..6ebb94a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,9 @@ See the [v2 → v3 migration guide](docs/migration-v2-to-v3.md). - **Symbol injection:** `ForPassphrase(..., includeSymbol: true)` attaches a random symbol to one randomly chosen word, so passphrases satisfy "needs a number and a symbol" rules while staying memorable. Entropy estimation now accounts for both the trailing number and the symbol. +- **Optional passphrase separator:** the separator is now a `char?` — pass `separator: null` (or an + empty string when binding from configuration) to concatenate words with no separator. This does not + affect entropy. - **`ForMemorable()` preset:** capitalized words sized to at least 80 bits of entropy. - **Passphrases via dependency injection:** set `PasswordOptions.Passphrase` (a `PassphraseOptions`) in code or bind a `Passphrase` section from configuration to resolve a passphrase @@ -58,5 +61,4 @@ See the [v2 → v3 migration guide](docs/migration-v2-to-v3.md). ## 2.1.0 and earlier -See the project history and the original review in -[`docs/archive/V3_REVIEW_AND_DOCUMENTATION.md`](docs/archive/V3_REVIEW_AND_DOCUMENTATION.md). +See the project history in the Git log and the [GitHub releases](https://github.com/prjseal/PasswordGenerator/releases). diff --git a/PasswordGenerator.Benchmarks/PasswordGenerator.Benchmarks.csproj b/PasswordGenerator.Benchmarks/PasswordGenerator.Benchmarks.csproj index f9c107c..5e6d7fe 100644 --- a/PasswordGenerator.Benchmarks/PasswordGenerator.Benchmarks.csproj +++ b/PasswordGenerator.Benchmarks/PasswordGenerator.Benchmarks.csproj @@ -2,7 +2,7 @@ Exe - net8.0;net10.0 + net10.0 enable latest false diff --git a/PasswordGenerator.Benchmarks/Program.cs b/PasswordGenerator.Benchmarks/Program.cs index d28536e..d31fb94 100644 --- a/PasswordGenerator.Benchmarks/Program.cs +++ b/PasswordGenerator.Benchmarks/Program.cs @@ -12,11 +12,9 @@ public static void Main(string[] args) { // DefaultConfig already supplies the GitHub markdown exporter (MarkdownExporter-github), // which produces the *-report-github.md files the workflow drops into the step summary. - // Every benchmark is run on both runtimes so the reports compare .NET 8 against .NET 10 - // side by side (BenchmarkDotNet adds a "Runtime" column). + // Benchmarks run on .NET 10, the runtime the library is built and shipped against. var config = DefaultConfig.Instance .AddDiagnoser(MemoryDiagnoser.Default) - .AddJob(Job.Default.WithRuntime(CoreRuntime.Core80)) .AddJob(Job.Default.WithRuntime(CoreRuntime.Core10_0)); BenchmarkSwitcher diff --git a/PasswordGenerator.Tests/DocumentationSnippetTests.cs b/PasswordGenerator.Tests/DocumentationSnippetTests.cs index d9a3140..7f438a1 100644 --- a/PasswordGenerator.Tests/DocumentationSnippetTests.cs +++ b/PasswordGenerator.Tests/DocumentationSnippetTests.cs @@ -9,7 +9,7 @@ namespace PasswordGenerator.Tests { /// - /// Compile-and-run guards for the snippets in Readme.md and the v2->v3 migration guide, so the + /// Compile-and-run guards for the snippets in README.md and the v2->v3 migration guide, so the /// documentation cannot drift from the public API. /// public class DocumentationSnippetTests diff --git a/PasswordGenerator.Tests/PassphraseTests.cs b/PasswordGenerator.Tests/PassphraseTests.cs index 5465ce3..25f1492 100644 --- a/PasswordGenerator.Tests/PassphraseTests.cs +++ b/PasswordGenerator.Tests/PassphraseTests.cs @@ -212,5 +212,77 @@ public void Di_BindsPassphraseFromConfiguration() var parts = generator.Next().Split('.'); Assert.That(parts.Length, Is.EqualTo(5)); // 5 words, no trailing number } + + [Test] + public void Next_WithNullSeparator_ConcatenatesWordsDirectly() + { + var rng = new FixedRandomSource(0, 1, 2, 3); + var generator = new PassphraseGenerator(4, separator: null, capitalize: false, + includeNumber: false, includeSymbol: false, minimumEntropyBits: 0, randomSource: rng); + + var expected = string.Concat( + WordList.Words[0], WordList.Words[1], WordList.Words[2], WordList.Words[3]); + Assert.That(generator.Next(), Is.EqualTo(expected)); + } + + [Test] + public void Next_WithNullSeparator_AppendsNumberWithoutSeparator() + { + // Draw order: one index per word, then the trailing number (NextInt(90) + 10). + var rng = new FixedRandomSource(0, 1, 5); + var generator = new PassphraseGenerator(2, separator: null, capitalize: false, + includeNumber: true, includeSymbol: false, minimumEntropyBits: 0, randomSource: rng); + + var expected = WordList.Words[0] + WordList.Words[1] + "15"; // 5 % 90 + 10 + Assert.That(generator.Next(), Is.EqualTo(expected)); + } + + [Test] + public void Next_WithNullSeparator_StillCapitalizesEachWord() + { + var rng = new FixedRandomSource(0, 1); + var generator = new PassphraseGenerator(2, separator: null, capitalize: true, + includeNumber: false, includeSymbol: false, minimumEntropyBits: 0, randomSource: rng); + + static string Cap(string w) => char.ToUpperInvariant(w[0]) + w.Substring(1); + var expected = Cap(WordList.Words[0]) + Cap(WordList.Words[1]); + Assert.That(generator.Next(), Is.EqualTo(expected)); + } + + [Test] + public void ForPassphrase_AcceptsNullSeparator() + { + var generator = (PassphraseGenerator)Password.ForPassphrase(4, separator: null); + Assert.That(generator.Separator, Is.Null); + } + + [Test] + public void NullSeparator_DoesNotChangeEntropy() + { + var withSeparator = Password.ForPassphrase(6, separator: '-').EstimateEntropyBits(); + var withoutSeparator = Password.ForPassphrase(6, separator: null).EstimateEntropyBits(); + Assert.That(withoutSeparator, Is.EqualTo(withSeparator)); + } + + [Test] + public void Di_BindsEmptySeparatorAsNull() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Passphrase:WordCount"] = "3", + ["Passphrase:Separator"] = "", + ["Passphrase:IncludeNumber"] = "false" + }) + .Build(); + + var services = new ServiceCollection(); + services.AddPasswordGenerator(configuration); + + using var provider = services.BuildServiceProvider(); + var generator = (PassphraseGenerator)provider.GetRequiredService(); + + Assert.That(generator.Separator, Is.Null); + } } } diff --git a/PasswordGenerator.Tests/PasswordGenerator.Tests.csproj b/PasswordGenerator.Tests/PasswordGenerator.Tests.csproj index 40a0cae..6963aa7 100644 --- a/PasswordGenerator.Tests/PasswordGenerator.Tests.csproj +++ b/PasswordGenerator.Tests/PasswordGenerator.Tests.csproj @@ -11,7 +11,7 @@ - + diff --git a/PasswordGenerator/IPassword.cs b/PasswordGenerator/IPassword.cs index a778230..49bb1ef 100644 --- a/PasswordGenerator/IPassword.cs +++ b/PasswordGenerator/IPassword.cs @@ -30,6 +30,7 @@ public interface IPassword IPassword IncludeSpecial(string specialCharactersToInclude); /// Replaces the pool with an explicit set of characters (no forced composition). + /// is . IPassword WithCharacters(string characters); /// Uses every printable ASCII character as the pool (no forced composition). @@ -39,6 +40,8 @@ public interface IPassword IPassword ExcludeAmbiguous(); /// Requires at least characters from the given class. + /// is negative. + /// A custom character pool is in use (per-class minimums cannot be combined with it). IPassword RequireAtLeast(CharacterClass characterClass, int count); /// Sets the required password length. diff --git a/PasswordGenerator/IPasswordGenerator.cs b/PasswordGenerator/IPasswordGenerator.cs index 69c17a5..8f37e51 100644 --- a/PasswordGenerator/IPasswordGenerator.cs +++ b/PasswordGenerator/IPasswordGenerator.cs @@ -28,12 +28,14 @@ public interface IPasswordGenerator IReadOnlyList Generate(); /// Generates passwords. + /// is negative. IReadOnlyList Generate(int count); /// Generates the default number of passwords, observing . ValueTask> GenerateAsync(CancellationToken cancellationToken = default); /// Generates passwords, observing . + /// is negative. ValueTask> GenerateAsync(int count, CancellationToken cancellationToken = default); /// Estimates the strength, in bits, of the output produced by this generator. diff --git a/PasswordGenerator/IPasswordSettings.cs b/PasswordGenerator/IPasswordSettings.cs index ddd2b97..62b93ec 100644 --- a/PasswordGenerator/IPasswordSettings.cs +++ b/PasswordGenerator/IPasswordSettings.cs @@ -71,6 +71,7 @@ public interface IPasswordSettings IPasswordSettings AddSpecial(string specialCharactersToAdd); /// Replaces the entire pool with an explicit set of characters (no forced composition). + /// is . IPasswordSettings UseCharacters(string characters); /// Uses every printable ASCII character as the pool (no forced composition). @@ -80,6 +81,8 @@ public interface IPasswordSettings IPasswordSettings ExcludeAmbiguousCharacters(); /// Requires at least characters from the given class, enabling it if needed. + /// is negative. + /// A custom character pool is in use (per-class minimums cannot be combined with it). IPasswordSettings RequireAtLeast(CharacterClass characterClass, int count); /// The special characters used when special characters are included. diff --git a/PasswordGenerator/PassphraseGenerator.cs b/PasswordGenerator/PassphraseGenerator.cs index 2f16631..8d9aa25 100644 --- a/PasswordGenerator/PassphraseGenerator.cs +++ b/PasswordGenerator/PassphraseGenerator.cs @@ -19,9 +19,10 @@ public class PassphraseGenerator : IPasswordGenerator, IDisposable /// Creates a passphrase generator. /// The number of words in each passphrase; must be at least one. /// - /// The character placed between words (and before the trailing number). Note that a few EFF - /// words contain a hyphen (e.g. "t-shirt"), so if you need to split the output back into words - /// choose a separator that does not occur in any word, such as '.' or a space. + /// The character placed between words (and before the trailing number), or + /// for no separator at all (the words are concatenated directly). Note that a few EFF words + /// contain a hyphen (e.g. "t-shirt"), so if you need to split the output back into words choose a + /// separator that does not occur in any word, such as '.' or a space. /// /// Whether to capitalize the first letter of each word. /// Whether to append a random two-digit number. @@ -39,7 +40,7 @@ public class PassphraseGenerator : IPasswordGenerator, IDisposable /// /// is less than one. /// The estimated entropy is below . - public PassphraseGenerator(int wordCount = 4, char separator = '-', bool capitalize = false, + public PassphraseGenerator(int wordCount = 4, char? separator = '-', bool capitalize = false, bool includeNumber = true, bool includeSymbol = false, double minimumEntropyBits = 0, IRandomSource? randomSource = null) { @@ -66,8 +67,8 @@ public PassphraseGenerator(int wordCount = 4, char separator = '-', bool capital /// The number of words in each passphrase. public int WordCount { get; } - /// The character placed between words. - public char Separator { get; } + /// The character placed between words, or for no separator. + public char? Separator { get; } /// Whether the first letter of each word is capitalized. public bool Capitalize { get; } @@ -104,7 +105,7 @@ public string Next() for (var i = 0; i < WordCount; i++) { - if (i > 0) sb.Append(Separator); + if (i > 0 && Separator is char sep) sb.Append(sep); var word = WordList.Words[_random.NextInt(WordList.Words.Length)]; if (Capitalize && word.Length > 0) @@ -122,7 +123,7 @@ public string Next() if (IncludeNumber) { - sb.Append(Separator); + if (Separator is char numSep) sb.Append(numSep); sb.Append((_random.NextInt(90) + 10).ToString(CultureInfo.InvariantCulture)); } diff --git a/PasswordGenerator/PassphraseOptions.cs b/PasswordGenerator/PassphraseOptions.cs index c5cf6f5..5212747 100644 --- a/PasswordGenerator/PassphraseOptions.cs +++ b/PasswordGenerator/PassphraseOptions.cs @@ -10,8 +10,11 @@ public class PassphraseOptions /// The number of words in each passphrase. Defaults to 4. public int WordCount { get; set; } = 4; - /// The character placed between words. Defaults to '-'. - public char Separator { get; set; } = '-'; + /// + /// The character placed between words. Defaults to '-'. Set to + /// (or an empty string in configuration) for no separator. + /// + public char? Separator { get; set; } = '-'; /// Whether the first letter of each word is capitalized. public bool Capitalize { get; set; } diff --git a/PasswordGenerator/Password.cs b/PasswordGenerator/Password.cs index d01f022..64445d5 100644 --- a/PasswordGenerator/Password.cs +++ b/PasswordGenerator/Password.cs @@ -97,6 +97,7 @@ public Password(bool includeLowercase, bool includeUppercase, bool includeNumeri /// Creates a password generator with an explicit random source. The caller owns the /// supplied and is responsible for disposing it. /// + /// is . public Password(IPasswordSettings settings, IRandomSource randomSource) { Settings = settings; @@ -458,7 +459,7 @@ public static IPassword ForEnvironmentName(int length = 12) /// Diceware-style passphrase built from the EFF Large Wordlist. /// The number of words in the passphrase. - /// The character placed between words. + /// The character placed between words, or for no separator. /// Whether to capitalize the first letter of each word. /// Whether to append a random two-digit number. /// Whether to attach a random symbol to one randomly chosen word. @@ -466,7 +467,7 @@ public static IPassword ForEnvironmentName(int length = 12) /// An optional entropy floor; when greater than zero the configuration is rejected if it /// falls below this many bits. /// - public static IPasswordGenerator ForPassphrase(int words = 4, char separator = '-', + public static IPasswordGenerator ForPassphrase(int words = 4, char? separator = '-', bool capitalize = false, bool includeNumber = true, bool includeSymbol = false, double minimumEntropyBits = 0) { @@ -479,11 +480,11 @@ public static IPasswordGenerator ForPassphrase(int words = 4, char separator = ' /// The word count is derived from the word-list size, and the same value is enforced as a floor. /// /// The minimum entropy in bits (defaults to 80, a strong target). - /// The character placed between words. + /// The character placed between words, or for no separator. /// Whether to capitalize the first letter of each word. /// Whether to append a random two-digit number. /// Whether to attach a random symbol to one randomly chosen word. - public static IPasswordGenerator ForPassphraseWithEntropy(double targetBits = 80, char separator = '-', + public static IPasswordGenerator ForPassphraseWithEntropy(double targetBits = 80, char? separator = '-', bool capitalize = false, bool includeNumber = true, bool includeSymbol = false) { var words = PassphraseGenerator.WordCountForEntropy(targetBits, includeNumber); diff --git a/PasswordGenerator/PasswordGenerator.csproj b/PasswordGenerator/PasswordGenerator.csproj index 903a0f3..ed49533 100644 --- a/PasswordGenerator/PasswordGenerator.csproj +++ b/PasswordGenerator/PasswordGenerator.csproj @@ -4,10 +4,15 @@ net8.0;net10.0 enable latest + true true + + true + 3.0.0 3.0.0.0 @@ -15,8 +20,9 @@ PasswordGenerator Paul Seal + Paul Seal A cross-platform .NET library that generates cryptographically secure random passwords, passphrases, OTPs, API keys and readable identifiers. Configurable via a fluent API, presets (OWASP/NIST) and dependency injection, with async support and entropy estimation. - Copyright 2026 + Copyright © 2026 Paul Seal https://github.com/prjseal/PasswordGenerator/ https://github.com/prjseal/PasswordGenerator/ Git @@ -37,7 +43,7 @@ - + @@ -46,8 +52,8 @@ - - + + diff --git a/PasswordGenerator/PasswordGeneratorServiceCollectionExtensions.cs b/PasswordGenerator/PasswordGeneratorServiceCollectionExtensions.cs index ac67552..baffb74 100644 --- a/PasswordGenerator/PasswordGeneratorServiceCollectionExtensions.cs +++ b/PasswordGenerator/PasswordGeneratorServiceCollectionExtensions.cs @@ -23,6 +23,7 @@ public static IServiceCollection AddPasswordGenerator(this IServiceCollection se /// Registers the generator, binding options from configuration (e.g. appSettings.json) and then /// applying an optional code override. Resolution order is configure (code) > configuration > default. /// + /// is . public static IServiceCollection AddPasswordGenerator(this IServiceCollection services, IConfiguration configuration, Action? configure = null) { @@ -30,6 +31,13 @@ public static IServiceCollection AddPasswordGenerator(this IServiceCollection se var options = new PasswordOptions(); configuration.Bind(options); + + // The configuration binder skips empty string values, so an explicit empty separator + // (the way a config file asks for "no separator") would otherwise be lost and the + // default '-' kept. Honor it explicitly as null. + if (options.Passphrase != null && configuration["Passphrase:Separator"] == string.Empty) + options.Passphrase.Separator = null; + configure?.Invoke(options); return AddCore(services, options); } diff --git a/Readme.md b/README.md similarity index 67% rename from Readme.md rename to README.md index f4cb17b..ecfcdfc 100644 --- a/Readme.md +++ b/README.md @@ -1,6 +1,6 @@ # Password Generator -![Password Logo](https://github.com/prjseal/PasswordGenerator/blob/dev/v2/passwordgeneratorlogo.png "Password Logo") +![Password Logo](https://raw.githubusercontent.com/prjseal/PasswordGenerator/master/passwordgeneratorlogo.png "Password Logo") A cross-platform .NET library that generates cryptographically secure random passwords, passphrases, OTPs, API keys and readable identifiers. Configure it with a fluent API, ready-made presets @@ -17,12 +17,15 @@ Install via NuGet: ``` Install-Package PasswordGenerator ``` It targets `net8.0` and `net10.0`, so it requires .NET 8 or later. If you need to run on .NET Framework or other older runtimes, use the 2.x line (which targets `netstandard2.0`). -> **Upgrading from 2.x?** See the [v2 → v3 migration guide](docs/migration-v2-to-v3.md). +> **Upgrading from 2.x?** See the [v2 → v3 migration guide](https://github.com/prjseal/PasswordGenerator/blob/master/docs/migration-v2-to-v3.md). > The v2 API still works; the one behavioural change is that invalid settings now **throw** (or use > `TryNext`) instead of returning an error string as the "password". ## Basic usage +> The examples below assume `using PasswordGenerator;` (and, for the dependency-injection section, +> `using Microsoft.Extensions.DependencyInjection;`). + ```csharp // By default, all character types are available and the length is 16. // Returns a random password with the default settings. @@ -80,8 +83,8 @@ var password = pwd.Next(); ## Presets Ready-made starting points; later fluent calls still override them. See the -[standards mapping](docs/migration-v2-to-v3.md#6-standards-mapping-for-the-presets) for the -OWASP/NIST rationale. +[standards mapping](https://github.com/prjseal/PasswordGenerator/blob/master/docs/migration-v2-to-v3.md#6-standards-mapping-for-the-presets) +for the OWASP/NIST rationale. ```csharp string strong = Password.ForOwasp().Next(); // full printable-ASCII pool, length 16 @@ -90,7 +93,7 @@ string otp = Password.ForOtp(6).Next(); // 6-digit one-time code string apiKey = Password.ForApiKey(32).Next(); // URL-safe token string envName = Password.ForEnvironmentName(12).Next();// readable id, no look-alike characters string phrase = Password.ForPassphrase(4).Next(); // e.g. "maple-river-quartz-bloom-42" -string strong = Password.ForPassphraseWithEntropy(80).Next(); // word count derived to clear 80 bits +string strongPhrase = Password.ForPassphraseWithEntropy(80).Next(); // word count derived to clear 80 bits string memorable = Password.ForMemorable().Next(); // capitalized, ~80+ bits, e.g. "Maple-River-Quartz-Bloom-Glade-Vivid-42" ``` @@ -102,13 +105,20 @@ For sites that require a digit and a symbol, pass `includeSymbol: true`. A rando to one randomly chosen word (e.g. `maple-river#-quartz-bloom-42`), so the phrase passes composition rules while staying memorable. +To omit the separator entirely, pass `separator: null` (or an empty string when binding from +configuration); the words are concatenated directly, e.g. +`Password.ForPassphrase(4, separator: null).Next()` → `"mapleriverquartzbloom42"`. This does not +change the passphrase's entropy — the separator is a fixed character and never contributes to +strength — it only affects readability. + ## Quality controls ```csharp // Remove look-alike characters (I l 1 O 0 o) var readable = new Password(20).ExcludeAmbiguous().Next(); -// Guarantee at least N characters from a class +// Guarantee at least N characters from a class. +// CharacterClass values: Lowercase, Uppercase, Numeric, Special. var pwd = new Password(16).RequireAtLeast(CharacterClass.Numeric, 2).Next(); // Use a custom pool, or every printable ASCII character @@ -132,10 +142,19 @@ if (new Password(16).TryNext(out var result)) ## Async and batches +`Generate(count)` returns a batch synchronously; the `Async` overloads return a `ValueTask` and honour +a `CancellationToken`. The parameterless `Generate()` returns `DefaultBatchCount` passwords (1 by +default; set the property to change it). + ```csharp -string password = await pwd.NextAsync(cancellationToken); -IReadOnlyList ten = pwd.Generate(10); -IReadOnlyList ten2 = await pwd.GenerateAsync(10, cancellationToken); +async Task ExampleAsync(CancellationToken cancellationToken) +{ + var pwd = new Password(16); + + string password = await pwd.NextAsync(cancellationToken); + IReadOnlyList ten = pwd.Generate(10); + IReadOnlyList ten2 = await pwd.GenerateAsync(10, cancellationToken); +} ``` ## Dependency injection @@ -164,17 +183,35 @@ services.AddPasswordGenerator(o => o.Passphrase = new PassphraseOptions { WordCount = 6, Capitalize = true }); ``` +You can also bind from `appSettings.json` (code configuration still takes precedence over bound +values, which in turn take precedence over the defaults): + +```jsonc +{ + "PasswordGenerator": { + "Length": 20, + "IncludeSpecial": true, + "ExcludeAmbiguous": true, + "DefaultBatchCount": 5 + } +} +``` + +```csharp +services.AddPasswordGenerator(config.GetSection("PasswordGenerator")); +``` + ## Documentation -- [v2 → v3 migration guide](docs/migration-v2-to-v3.md) -- [Changelog](CHANGELOG.md) -- [Design & architecture docs](docs/README.md) +- [v2 → v3 migration guide](https://github.com/prjseal/PasswordGenerator/blob/master/docs/migration-v2-to-v3.md) +- [Changelog](https://github.com/prjseal/PasswordGenerator/blob/master/CHANGELOG.md) +- [Design & architecture docs](https://github.com/prjseal/PasswordGenerator/blob/master/docs/README.md) ## License & attribution -PasswordGenerator is licensed under the [MIT License](License.md). +PasswordGenerator is licensed under the [MIT License](https://github.com/prjseal/PasswordGenerator/blob/master/License.md). Passphrases are generated from the **EFF Large Wordlist** (7,776 words) by the [Electronic Frontier Foundation](https://www.eff.org/dice), used under the [Creative Commons Attribution 3.0 US](https://creativecommons.org/licenses/by/3.0/us/) -license. See [THIRD-PARTY-NOTICES.md](THIRD-PARTY-NOTICES.md) for details. +license. See [THIRD-PARTY-NOTICES.md](https://github.com/prjseal/PasswordGenerator/blob/master/THIRD-PARTY-NOTICES.md) for details. diff --git a/benchmarks/v3.0.0.md b/benchmarks/v3.0.0.md index 135c43d..4757832 100644 --- a/benchmarks/v3.0.0.md +++ b/benchmarks/v3.0.0.md @@ -1,143 +1,107 @@ # PasswordGenerator v3.0.0 — Benchmark Results Generated with [BenchmarkDotNet](https://benchmarkdotnet.org/) from -`PasswordGenerator.Benchmarks`, running every scenario on both **.NET 8.0** and -**.NET 10.0** in a single process. +`PasswordGenerator.Benchmarks`, running every scenario on **.NET 10.0** — the +runtime the library is built and shipped against. ## Environment ``` -BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat) -Intel Xeon Processor 2.80GHz, 1 CPU, 4 logical and 4 physical cores -[Host] : .NET 8.0.27, X64 RyuJIT x86-64-v4 -.NET 8 : .NET 8.0.27 (8.0.2726.22922) -.NET 10 : .NET 10.0.8 (10.0.826.23019) +BenchmarkDotNet v0.15.8, Windows 11 (10.0.26100.8390/24H2/2024Update/HudsonValley) +12th Gen Intel Core i7-12700H 2.30GHz, 1 CPU, 20 logical and 14 physical cores +.NET SDK 10.0.204 + [Host] : .NET 10.0.8 (10.0.8, 10.0.826.23019), X64 RyuJIT x86-64-v3 + Job-GVKUBM : .NET 10.0.8 (10.0.8, 10.0.826.23019), X64 RyuJIT x86-64-v3 + +Runtime=.NET 10.0 ``` -> **Note on precision:** this run used reduced sampling -> (`WarmupCount=1 IterationCount=3 LaunchCount=1`) to keep wall-time short, so -> the error margins are wide — treat the means as indicative rather than -> publication-grade. The `Benchmarks` GitHub Actions workflow runs the full -> default job for higher-precision numbers and publishes them to the run summary. +> **Precision:** this run uses the full default BenchmarkDotNet job (multiple +> warmup and measurement iterations per case), so the error margins are tight and +> the means are publication-grade. The `Benchmarks` GitHub Actions workflow runs +> the same job and publishes the numbers to the run summary. ## SingleGenerationBenchmarks — `Next()` by password length -| Method | Runtime | Length | Mean | Allocated | -|--------|---------|-------:|-----:|----------:| -| Next | .NET 10.0 | 8 | 25.64 μs | 80 B | -| Next | .NET 8.0 | 8 | 25.81 μs | 80 B | -| Next | .NET 10.0 | 16 | 57.56 μs | 112 B | -| Next | .NET 8.0 | 16 | 56.08 μs | 112 B | -| Next | .NET 10.0 | 32 | 119.35 μs | 176 B | -| Next | .NET 8.0 | 32 | 121.63 μs | 176 B | -| Next | .NET 10.0 | 64 | 252.17 μs | 304 B | -| Next | .NET 8.0 | 64 | 250.51 μs | 304 B | -| Next | .NET 10.0 | 128 | 494.68 μs | 560 B | -| Next | .NET 8.0 | 128 | 486.25 μs | 560 B | +| Method | Length | Mean | Error | StdDev | Gen0 | Allocated | +|------- |------- |------------:|----------:|----------:|-------:|----------:| +| Next | 8 | 882.9 ns | 11.21 ns | 10.48 ns | 0.0057 | 80 B | +| Next | 16 | 1,888.3 ns | 26.48 ns | 23.48 ns | 0.0076 | 112 B | +| Next | 32 | 4,023.4 ns | 32.95 ns | 30.82 ns | 0.0076 | 176 B | +| Next | 64 | 7,926.3 ns | 53.32 ns | 49.88 ns | 0.0153 | 304 B | +| Next | 128 | 16,818.8 ns | 323.96 ns | 332.68 ns | 0.0305 | 560 B | ## BatchGenerationBenchmarks — `Generate(n)` -| Method | Runtime | Count | Mean | Allocated | -|--------|---------|------:|-----:|----------:| -| Generate | .NET 10.0 | 1 | 56.11 μs | 176 B | -| Generate | .NET 8.0 | 1 | 56.63 μs | 176 B | -| Generate | .NET 10.0 | 10 | 559.63 μs | 1256 B | -| Generate | .NET 8.0 | 10 | 559.66 μs | 1256 B | -| Generate | .NET 10.0 | 100 | 5,623 μs | 12056 B | -| Generate | .NET 8.0 | 100 | 5,732 μs | 12056 B | -| Generate | .NET 10.0 | 1000 | 55,664 μs | 120056 B | -| Generate | .NET 8.0 | 1000 | 55,856 μs | 120056 B | -| Generate | .NET 10.0 | 10000 | 568,403 μs | 1200056 B | -| Generate | .NET 8.0 | 10000 | 554,511 μs | 1200056 B | +| Method | Count | Mean | Error | StdDev | Gen0 | Gen1 | Allocated | +|--------- |------ |--------------:|------------:|------------:|--------:|--------:|----------:| +| Generate | 1 | 1.928 μs | 0.0059 μs | 0.0055 μs | 0.0114 | - | 176 B | +| Generate | 10 | 19.074 μs | 0.3652 μs | 0.3237 μs | 0.0916 | - | 1256 B | +| Generate | 100 | 190.177 μs | 0.8045 μs | 0.7525 μs | 0.7324 | - | 12056 B | +| Generate | 1000 | 1,876.541 μs | 8.7535 μs | 8.1880 μs | 7.8125 | - | 120056 B | +| Generate | 10000 | 19,034.998 μs | 155.0712 μs | 145.0537 μs | 93.7500 | 62.5000 | 1200056 B | ## AsyncBenchmarks — `NextAsync()` / `GenerateAsync(n)` -`NextAsync` ignores `Count` (it generates one password), so it stays flat ~56 μs. - -| Method | Runtime | Count | Mean | Allocated | -|--------|---------|------:|-----:|----------:| -| NextAsync | .NET 10.0 | 1 | 56.42 μs | 184 B | -| GenerateAsync | .NET 10.0 | 1 | 58.68 μs | 248 B | -| NextAsync | .NET 8.0 | 1 | 56.57 μs | 184 B | -| GenerateAsync | .NET 8.0 | 1 | 57.43 μs | 248 B | -| NextAsync | .NET 10.0 | 10 | 56.58 μs | 184 B | -| GenerateAsync | .NET 10.0 | 10 | 559.01 μs | 1328 B | -| NextAsync | .NET 8.0 | 10 | 56.72 μs | 184 B | -| GenerateAsync | .NET 8.0 | 10 | 569.33 μs | 1328 B | -| NextAsync | .NET 10.0 | 100 | 57.71 μs | 184 B | -| GenerateAsync | .NET 10.0 | 100 | 5,591 μs | 12128 B | -| NextAsync | .NET 8.0 | 100 | 55.70 μs | 184 B | -| GenerateAsync | .NET 8.0 | 100 | 5,707 μs | 12128 B | -| NextAsync | .NET 10.0 | 1000 | 55.90 μs | 184 B | -| GenerateAsync | .NET 10.0 | 1000 | 55,939 μs | 120128 B | -| NextAsync | .NET 8.0 | 1000 | 56.24 μs | 184 B | -| GenerateAsync | .NET 8.0 | 1000 | 56,175 μs | 120128 B | -| NextAsync | .NET 10.0 | 10000 | 55.76 μs | 184 B | -| GenerateAsync | .NET 10.0 | 10000 | 563,644 μs | 1200128 B | -| NextAsync | .NET 8.0 | 10000 | 56.03 μs | 184 B | -| GenerateAsync | .NET 8.0 | 10000 | 560,427 μs | 1200128 B | +`NextAsync` ignores `Count` (it generates one password), so it stays flat ~1.9 μs. + +| Method | Count | Mean | Error | StdDev | Gen0 | Gen1 | Allocated | +|-------------- |------ |--------------:|------------:|------------:|--------:|--------:|----------:| +| NextAsync | 1 | 1.950 μs | 0.0100 μs | 0.0094 μs | 0.0076 | - | 112 B | +| GenerateAsync | 1 | 1.899 μs | 0.0258 μs | 0.0241 μs | 0.0134 | - | 176 B | +| NextAsync | 10 | 1.960 μs | 0.0074 μs | 0.0066 μs | 0.0076 | - | 112 B | +| GenerateAsync | 10 | 19.377 μs | 0.3787 μs | 0.4924 μs | 0.0916 | - | 1256 B | +| NextAsync | 100 | 1.886 μs | 0.0140 μs | 0.0131 μs | 0.0076 | - | 112 B | +| GenerateAsync | 100 | 189.833 μs | 1.1967 μs | 1.1194 μs | 0.7324 | - | 12056 B | +| NextAsync | 1000 | 1.901 μs | 0.0205 μs | 0.0181 μs | 0.0076 | - | 112 B | +| GenerateAsync | 1000 | 1,958.834 μs | 36.2342 μs | 37.2098 μs | 7.8125 | - | 120056 B | +| NextAsync | 10000 | 1.880 μs | 0.0212 μs | 0.0188 μs | 0.0076 | - | 112 B | +| GenerateAsync | 10000 | 19,434.837 μs | 155.4862 μs | 137.8344 μs | 93.7500 | 62.5000 | 1200056 B | ## PresetBenchmarks -| Method | Runtime | Mean | Allocated | -|--------|---------|-----:|----------:| -| ForOwasp | .NET 10.0 | 50.58 μs | 112 B | -| ForNist | .NET 10.0 | 38.46 μs | 96 B | -| ForOtp | .NET 10.0 | 19.24 μs | 80 B | -| ForApiKey | .NET 10.0 | 88.12 μs | 176 B | -| ForEnvironmentName | .NET 10.0 | 33.24 μs | 560 B | -| ForPassphrase | .NET 10.0 | 8.29 μs | 287 B | -| ForOwasp | .NET 8.0 | 50.19 μs | 112 B | -| ForNist | .NET 8.0 | 38.61 μs | 96 B | -| ForOtp | .NET 8.0 | 19.39 μs | 80 B | -| ForApiKey | .NET 8.0 | 87.03 μs | 176 B | -| ForEnvironmentName | .NET 8.0 | 33.72 μs | 560 B | -| ForPassphrase | .NET 8.0 | 8.54 μs | 287 B | +| Method | Mean | Error | StdDev | Gen0 | Allocated | +|------------------- |-----------:|---------:|---------:|-------:|----------:| +| ForOwasp | 1,635.2 ns | 4.08 ns | 3.62 ns | 0.0076 | 112 B | +| ForNist | 1,240.7 ns | 9.00 ns | 8.42 ns | 0.0076 | 96 B | +| ForOtp | 653.5 ns | 3.13 ns | 2.93 ns | 0.0057 | 80 B | +| ForApiKey | 2,895.3 ns | 11.08 ns | 10.37 ns | 0.0114 | 176 B | +| ForEnvironmentName | 1,288.7 ns | 25.02 ns | 28.82 ns | 0.0439 | 560 B | +| ForPassphrase | 297.1 ns | 4.86 ns | 4.55 ns | 0.0310 | 394 B | ## InstantiationBenchmarks — DI vs direct (baseline = `new Password()`) -DI resolution is essentially free on time and allocates ~9× less, because the +DI resolution is fractionally faster on time and allocates ~9× less, because the registered generator is a singleton and is reused on each resolve. -| Method | Runtime | Mean | Ratio | Allocated | Alloc Ratio | -|--------|---------|-----:|------:|----------:|------------:| -| DirectInstantiation | .NET 10.0 | 56.78 μs | 1.00 | 1032 B | 1.00 | -| ResolveFromContainer | .NET 10.0 | 55.92 μs | 0.98 | 112 B | 0.11 | -| DirectInstantiation | .NET 8.0 | 56.38 μs | 1.00 | 1032 B | 1.00 | -| ResolveFromContainer | .NET 8.0 | 56.04 μs | 0.99 | 112 B | 0.11 | +| Method | Mean | Error | StdDev | Ratio | Gen0 | Allocated | Alloc Ratio | +|--------------------- |---------:|----------:|----------:|------:|-------:|----------:|------------:| +| DirectInstantiation | 2.015 μs | 0.0027 μs | 0.0022 μs | 1.00 | 0.0801 | 1032 B | 1.00 | +| ResolveFromContainer | 1.886 μs | 0.0026 μs | 0.0025 μs | 0.94 | 0.0076 | 112 B | 0.11 | ## VersionComparisonBenchmarks — loop `Next()` (baseline) vs `Generate(n)` -| Method | Runtime | Count | Mean | Ratio | Allocated | Alloc Ratio | -|--------|---------|------:|-----:|------:|----------:|------------:| -| LoopNext | .NET 10.0 | 1 | 55.73 μs | 1.00 | 112 B | 1.00 | -| BatchGenerate | .NET 10.0 | 1 | 56.48 μs | 1.01 | 176 B | 1.57 | -| LoopNext | .NET 8.0 | 1 | 56.26 μs | 1.00 | 112 B | 1.00 | -| BatchGenerate | .NET 8.0 | 1 | 56.28 μs | 1.00 | 176 B | 1.57 | -| LoopNext | .NET 10.0 | 10 | 566.68 μs | 1.00 | 1120 B | 1.00 | -| BatchGenerate | .NET 10.0 | 10 | 562.27 μs | 0.99 | 1256 B | 1.12 | -| LoopNext | .NET 8.0 | 10 | 561.85 μs | 1.00 | 1120 B | 1.00 | -| BatchGenerate | .NET 8.0 | 10 | 562.08 μs | 1.00 | 1256 B | 1.12 | -| LoopNext | .NET 10.0 | 100 | 5,498 μs | 1.00 | 11200 B | 1.00 | -| BatchGenerate | .NET 10.0 | 100 | 5,566 μs | 1.01 | 12056 B | 1.08 | -| LoopNext | .NET 8.0 | 100 | 5,543 μs | 1.00 | 11200 B | 1.00 | -| BatchGenerate | .NET 8.0 | 100 | 5,576 μs | 1.01 | 12056 B | 1.08 | -| LoopNext | .NET 10.0 | 1000 | 56,380 μs | 1.00 | 112000 B | 1.00 | -| BatchGenerate | .NET 10.0 | 1000 | 56,404 μs | 1.00 | 120056 B | 1.07 | -| LoopNext | .NET 8.0 | 1000 | 56,727 μs | 1.00 | 112000 B | 1.00 | -| BatchGenerate | .NET 8.0 | 1000 | 56,752 μs | 1.00 | 120056 B | 1.07 | -| LoopNext | .NET 10.0 | 10000 | 555,818 μs | 1.00 | 1120000 B | 1.00 | -| BatchGenerate | .NET 10.0 | 10000 | 551,998 μs | 0.99 | 1200056 B | 1.07 | -| LoopNext | .NET 8.0 | 10000 | 555,037 μs | 1.00 | 1120000 B | 1.00 | -| BatchGenerate | .NET 8.0 | 10000 | 561,637 μs | 1.01 | 1200056 B | 1.07 | +| Method | Count | Mean | Error | StdDev | Ratio | RatioSD | Gen0 | Gen1 | Allocated | Alloc Ratio | +|-------------- |------ |--------------:|------------:|------------:|------:|--------:|--------:|--------:|----------:|------------:| +| LoopNext | 1 | 1.904 μs | 0.0380 μs | 0.0296 μs | 1.00 | 0.02 | 0.0076 | - | 112 B | 1.00 | +| BatchGenerate | 1 | 1.916 μs | 0.0048 μs | 0.0045 μs | 1.01 | 0.01 | 0.0114 | - | 176 B | 1.57 | +| LoopNext | 10 | 19.165 μs | 0.0691 μs | 0.0613 μs | 1.00 | 0.00 | 0.0610 | - | 1120 B | 1.00 | +| BatchGenerate | 10 | 18.812 μs | 0.0972 μs | 0.0909 μs | 0.98 | 0.01 | 0.0916 | - | 1256 B | 1.12 | +| LoopNext | 100 | 188.629 μs | 1.3250 μs | 1.4727 μs | 1.00 | 0.01 | 0.7324 | - | 11200 B | 1.00 | +| BatchGenerate | 100 | 189.877 μs | 0.4995 μs | 0.4171 μs | 1.01 | 0.01 | 0.7324 | - | 12056 B | 1.08 | +| LoopNext | 1000 | 1,918.328 μs | 27.4106 μs | 25.6399 μs | 1.00 | 0.02 | 7.8125 | - | 112000 B | 1.00 | +| BatchGenerate | 1000 | 1,952.976 μs | 11.2302 μs | 9.3778 μs | 1.02 | 0.01 | 7.8125 | - | 120056 B | 1.07 | +| LoopNext | 10000 | 19,202.571 μs | 59.6791 μs | 55.8239 μs | 1.00 | 0.00 | 62.5000 | - | 1120000 B | 1.00 | +| BatchGenerate | 10000 | 19,030.222 μs | 133.3657 μs | 118.2252 μs | 0.99 | 0.01 | 93.7500 | 62.5000 | 1200056 B | 1.07 | ## Takeaways -- **.NET 8 vs .NET 10:** within noise across every scenario — runtime is - dominated by `RandomNumberGenerator` crypto calls, not managed code, so the - newer JIT makes little difference here. +- **Cost scales linearly** with password length and batch count, as expected — + runtime is dominated by `RandomNumberGenerator` crypto calls. - **`Generate(n)` vs a manual `Next()` loop:** identical timing; the batch path only adds the result `List` allocation (~7% more memory). -- **Cost scales linearly** with password length and batch count, as expected. -- **DI has no measurable overhead** and allocates far less per call than - constructing a new `Password` each time. +- **`NextAsync` is flat ~1.9 μs** regardless of `Count`, because it generates a + single password; `GenerateAsync(n)` tracks `Generate(n)` exactly. +- **DI has no measurable overhead** and allocates ~9× less per call than + constructing a new `Password` each time, since the registered generator is a + reused singleton. diff --git a/docs/README.md b/docs/README.md index 33d9e0a..1e034eb 100644 --- a/docs/README.md +++ b/docs/README.md @@ -15,39 +15,22 @@ flowchart LR D5[migration-v2-to-v3.md] D6[v3-local-nuget-test.md] end - subgraph Archive["archive/ — historical"] - A1[V3_REVIEW_AND_DOCUMENTATION.md] - A2[V3_VERIFICATION.md] - A3[current-state/ v2.1.0 snapshot] - A4[before-after.md] - A5[roadmap.md] - A6[implementation-plan.md] - end - Docs -. superseded by .-> Archive ``` ## Reading order -1. **`architecture.md`** — type relationships, the random source, and the multi-targeting strategy. -2. **`generation-flow.md`** — how `Next()`/`Generate()` build a password, plus the async path. -3. **`api-surface.md`** — the public fluent surface, presets, and batch/async APIs. -4. **`configuration-and-di.md`** — `PasswordOptions`, settings resolution, and DI registration. -5. **`migration-v2-to-v3.md`** — the user-facing upgrade guide from 2.x. -6. **`v3-local-nuget-test.md`** — local `dotnet pack` / install verification procedure. +Each page ends with a navigation footer, so you can read straight through from start to finish. + +1. [**Architecture**](architecture.md) — type relationships, the random source, and the multi-targeting strategy. +2. [**Generation Flow**](generation-flow.md) — how `Next()`/`Generate()` build a password, plus the async path. +3. [**Public API Surface**](api-surface.md) — the public fluent surface, presets, and batch/async APIs. +4. [**Configuration & DI**](configuration-and-di.md) — `PasswordOptions`, settings resolution, and DI registration. +5. [**Migrating from v2.x to v3.0**](migration-v2-to-v3.md) — the user-facing upgrade guide from 2.x. +6. [**Local NuGet test report**](v3-local-nuget-test.md) — local `dotnet pack` / install verification procedure. + +**Start reading → [Architecture](architecture.md)** ## Conventions - These docs describe **v3.0.0** as shipped: targets `net8.0;net10.0`, nullable enabled. - Each doc ends with a **Why this is better** note. - -## Archive - -`archive/` keeps the material that led to v3 but no longer describes the current state: - -- **`V3_REVIEW_AND_DOCUMENTATION.md`** / **`V3_VERIFICATION.md`** — the original review and - issue-by-issue verification of the v2.1.0 code. -- **`current-state/`** — the diagrammed snapshot of the v2.1.0 (`netstandard2.0`) code that v3 replaced. -- **`roadmap.md`** / **`implementation-plan.md`** / **`before-after.md`** — the v3 planning documents. - -These are point-in-time records; where they recommend or describe `netstandard2.0` support, note that -v3 dropped it (see the root [`CHANGELOG.md`](../CHANGELOG.md)). diff --git a/docs/api-surface.md b/docs/api-surface.md index 8a47a15..8adfd39 100644 --- a/docs/api-surface.md +++ b/docs/api-surface.md @@ -4,8 +4,7 @@ Keeps the familiar fluent feel; adds safety, presets, batch, async, and custom p > The fluent builder is `IPassword` (there is no separate `IPasswordBuilder`/`Build()` split). > `Password` implements both `IPassword` and the generation contract `IPasswordGenerator`. -> Passphrases return an `IPasswordGenerator` (`PassphraseGenerator`). See -> `archive/implementation-plan.md` for how the shipped surface diverged from the early proposal. +> Passphrases return an `IPasswordGenerator` (`PassphraseGenerator`). ## API map @@ -73,6 +72,11 @@ Passphrases are built from the **EFF Large Wordlist** (7,776 words, ~12.9 bits/w - `ForPassphraseWithEntropy(targetBits)` derives the word count needed to clear a target and enforces it as a floor; `ForPassphrase(..., minimumEntropyBits)` enforces a floor for an explicit word count. + The derivation is exposed directly as the static `PassphraseGenerator.WordCountForEntropy(targetBits, + includeNumber)` if you want the word count without building a generator. +- The separator is a `char?` — pass `separator: null` (or an empty string when binding from + configuration) to concatenate the words with no separator. This does not change entropy; the + separator is a fixed character and never contributes to strength. - `includeSymbol: true` attaches a random symbol to one randomly chosen word, satisfying "needs a number and a symbol" composition rules while staying memorable. - `EstimateEntropyBits()` (on `IPasswordGenerator`) reports the estimated strength. @@ -105,6 +109,10 @@ flowchart TD > of async would be an anti-pattern and would spam every consumer with build warnings. Async exists > for ergonomics and cancellation only. -**Why this is better:** every gap noted in the v2.1.0 review (`archive/current-state/api-surface.md`) is closed +**Why this is better:** every gap from the v2.1.0 API is closed (`TryNext`/async/DI/presets/appSettings/custom pools), failures become explicit, and existing single `.Next()` users still work unchanged, giving a gentle upgrade path. + +--- + +**Docs:** [← Generation Flow](generation-flow.md) · [Index](README.md) · Next → [Configuration & DI](configuration-and-di.md) diff --git a/docs/architecture.md b/docs/architecture.md index 27961e9..a5ddb58 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -140,3 +140,7 @@ rejection-sampling fallback, was dropped in v3 — see the [changelog](../CHANGE **Why this is better:** removes the `static`/undisposed RNG, makes randomness unbiased and testable (inject a deterministic `IRandomSource` in unit tests), and uses the fast, allocation-free built-in crypto API on every supported runtime. + +--- + +**Docs:** [Index](README.md) · Next → [Generation Flow](generation-flow.md) diff --git a/docs/archive/V3_REVIEW_AND_DOCUMENTATION.md b/docs/archive/V3_REVIEW_AND_DOCUMENTATION.md deleted file mode 100644 index 1c56c1c..0000000 --- a/docs/archive/V3_REVIEW_AND_DOCUMENTATION.md +++ /dev/null @@ -1,330 +0,0 @@ -# PasswordGenerator — Full Package Documentation & v3 Review - -> **Archived / historical.** Review of the v2.1.0 source written to plan v3, kept for reference. -> Where it recommends multi-targeting `netstandard2.0`, note that v3 dropped `netstandard2.0` and -> targets `net8.0;net10.0`. See the root [`CHANGELOG.md`](../../CHANGELOG.md). - -> Purpose: a single, self-contained reference for the `PasswordGenerator` NuGet package as it -> stands today (v2.1.0). Written so it can be pasted into a Claude chat to plan v3. It covers -> what the package is, every public API, the internal implementation, confirmed bugs, design -> smells, build/test output, and a prioritised list of v3 candidate features. -> -> Date: 2026-05-24 · Current published version: 2.1.0 · Target framework: `netstandard2.0` -> Repo: https://github.com/prjseal/PasswordGenerator · Author: Paul Seal · License: MIT - ---- - -## 1. What the package is - -`PasswordGenerator` is a small .NET Standard 2.0 class library that generates random passwords -(and short numeric codes) according to configurable rules: which character classes to include -(lowercase, uppercase, numeric, special), the length, custom special-character sets, and a cap on -generation attempts. It is marketed as helping meet "OWASP requirements" and is widely used in -the Umbraco / .NET community. - -- **Package id:** `PasswordGenerator` -- **Single dependency-free assembly** (no third-party runtime dependencies). -- **Distribution:** NuGet (`Install-Package PasswordGenerator`). -- **Randomness source:** `System.Security.Cryptography.RandomNumberGenerator` (CSPRNG). - -### Solution layout - -``` -PasswordGenerator.sln -├── PasswordGenerator/ (the library, packable) -│ ├── IPassword.cs public fluent interface -│ ├── Password.cs main implementation -│ ├── IPasswordSettings.cs settings interface -│ ├── PasswordSettings.cs settings implementation -│ ├── PasswordGenerator.cs [Obsolete] back-compat wrapper class -│ ├── PasswordGeneratorSettings.cs [Obsolete] back-compat settings subclass -│ ├── PasswordGenerator.csproj SDK-style, PackageVersion 2.1.0, netstandard2.0 -│ ├── PasswordGenerator.nuspec STALE legacy nuspec (says 2.0.5) — see §7 -│ └── readme.txt ASCII-art readme bundled in older package -├── PasswordGenerator.Tests/ (NUnit tests, netcoreapp2.2) -│ ├── BasicTests.cs 16 tests against Password -│ └── ObsoleteTests.cs 8 tests against the obsolete PasswordGenerator -├── Readme.md GitHub readme (has stale docs — see §6) -├── appveyor.yml CI: AppVeyor, VS2017 image, publish_nuget: true -├── License.md, *.png -``` - ---- - -## 2. Public API (what callers can do today) - -### 2.1 `IPassword` (the contract) - -```csharp -public interface IPassword -{ - IPassword IncludeLowercase(); - IPassword IncludeUppercase(); - IPassword IncludeNumeric(); - IPassword IncludeSpecial(); - IPassword IncludeSpecial(string specialCharactersToInclude); - IPassword LengthRequired(int passwordLength); - string Next(); - IEnumerable NextGroup(int numberOfPasswordsToGenerate); -} -``` - -### 2.2 `Password` constructors - -| Constructor | Behaviour | -|---|---| -| `Password()` | All four classes on, length 16, maxAttempts 10000, `usingDefaults = true` | -| `Password(IPasswordSettings settings)` | Caller-supplied settings | -| `Password(int passwordLength)` | All four classes on, given length | -| `Password(bool lower, bool upper, bool numeric, bool special)` | Explicit classes, length 16, `usingDefaults = false` | -| `Password(bool…, int passwordLength)` | + length | -| `Password(bool…, int passwordLength, int maximumAttempts)` | + attempts cap | - -Defaults: `DefaultPasswordLength = 16`, `DefaultMaxPasswordAttempts = 10000`, all `Include*` default `true`. - -### 2.3 Fluent builders - -`IncludeLowercase() / IncludeUppercase() / IncludeNumeric() / IncludeSpecial() / IncludeSpecial(string) / LengthRequired(int)` — each returns `this` for chaining. The first fluent -`Include*`/`Add*` call after a defaulted `Password()` flips `usingDefaults` off and **clears the -character set**, so `new Password().IncludeNumeric()` yields a numeric-only password (not -"defaults plus numeric"). - -### 2.4 Generation - -- `string Next()` — returns one password, OR a human-readable error **string** on failure (see §5.1). -- `IEnumerable NextGroup(int n)` — calls `Next()` n times; **does not de-duplicate**. - -### 2.5 Settings (`IPasswordSettings` / `PasswordSettings`) - -Character pools (constants in `PasswordSettings`): - -``` -Lowercase = "abcdefghijklmnopqrstuvwxyz" -Uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" -Numeric = "0123456789" -Special = @"!#$%&*@\" // DEFAULT special set (8 chars only) -MinLength = 4 MaxLength = 256 -``` - -`Add*` methods mutate and return the same instance; `AddSpecial(string)` overrides the special set. - -### 2.6 Usage examples (from Readme) - -```csharp -var pwd = new Password(); var p = pwd.Next(); // 16 chars, all classes -var pwd = new Password(32); // length 32 -var pwd = new Password(true,true,false,false,21); // letters only, len 21 -var pwd = new Password().IncludeNumeric(); // numeric only, len 16 -var pwd = new Password().IncludeLowercase().IncludeUppercase().IncludeSpecial(); -var pwd = new Password(4).IncludeNumeric(); // 4-digit OTP -var pwd = new Password().IncludeLowercase().IncludeUppercase().IncludeNumeric().IncludeSpecial("[]{}^_="); -``` - ---- - -## 3. How generation actually works (internal flow) - -`Next()` → validate requested length is in `[Min,Max]` → loop up to `MaximumAttempts`: -`GenerateRandomPassword(settings)` then `PasswordIsValid(settings, pwd)` → return first valid, else `"Try again"`. - -`GenerateRandomPassword`: -1. Takes `settings.CharacterSet`, **shuffles** it via `OrderBy(Guid.NewGuid())`. -2. For each position, picks a char at `GetRandomNumberInRange(0, characterSetLength - 1)`. -3. Rejects a char that would make **3 identical in a row** (only checked from position > 2). - -`GetRandomNumberInRange(min, max)`: -```csharp -var data = new byte[sizeof(int)]; -_rng.GetBytes(data); -var randomNumber = BitConverter.ToInt32(data, 0); -return (int)Math.Floor((double)(min + Math.Abs(randomNumber % (max - min)))); -``` - -`PasswordIsValid`: regex-checks at least one lowercase/uppercase/numeric is present (when required), -checks at least one special char from the configured set is present (when required & set non-empty), -and re-checks length. It does **not** verify the password contains *only* allowed characters. - ---- - -## 4. Build & test results (verified in this environment) - -Built with the .NET 8 SDK (8.0.421). The library targets `netstandard2.0`; tests target -`netcoreapp2.2`. - -### 4.1 Library build — **succeeds, 5 warnings** - -All five are `CS0108` member-hiding warnings on the obsolete `PasswordGenerator` class, because its -`IncludeLowercase/Uppercase/Numeric/Special` and `LengthRequired` methods hide the inherited -`Password` members without the `new` keyword: - -``` -PasswordGenerator.cs(36,34): warning CS0108: 'PasswordGenerator.IncludeLowercase()' hides inherited member 'Password.IncludeLowercase()'. -… (same for IncludeUppercase, IncludeNumeric, IncludeSpecial, LengthRequired) -``` - -### 4.2 Test project build — **succeeds, with vulnerability + obsolete warnings** - -- `NU1903` (high) / `NU1902` (moderate): `Microsoft.NETCore.App` **2.2.0** has known - high/moderate-severity vulnerabilities. The `netcoreapp2.2` target is **out of support**. -- Many `CS0618`: the `ObsoleteTests` intentionally exercise the obsolete `PasswordGenerator` class. - -### 4.3 Tests — **24/24 pass** - -`netcoreapp2.2` runtime is not installable (EOL), so tests were re-run on `net8.0` (NUnit 3.14). -Result: `Failed: 0, Passed: 24, Skipped: 0, Total: 24`. (16 in `BasicTests`, 8 in `ObsoleteTests`.) - -> Note: the test names assert intent the code doesn't fully guarantee, e.g. -> `…10Passwords_ShouldReturn10DifferentPasswords` only asserts `Count() == 10`, never uniqueness. - ---- - -## 5. Confirmed bugs & correctness issues - -### 5.1 `Next()` returns error text *as if it were a password* (API design bug) -On invalid length it returns `"Password length invalid. Must be between 4 and 256 characters long"`; -if no valid password is produced within `MaximumAttempts` it returns `"Try again"`. A caller that -doesn't special-case these strings will happily store an error message as a user's password. There -is no exception, no `bool TryNext(out …)`, no `Result` type. **This is the single most important -correctness/safety issue.** - -### 5.2 Off-by-one: the last character of the shuffled set is never selected (verified) -`GetRandomNumberInRange(0, characterSetLength - 1)` computes `Math.Abs(r % (max - min))` = -`r % (characterSetLength - 1)`, which yields `0 … characterSetLength-2`. The highest index is -**never** reachable. Empirically confirmed: for a 10-element range, index 9 is never produced. -Because the set is reshuffled per `GenerateRandomPassword` call, no single character is permanently -excluded across passwords, but **within each password the effective alphabet is one char smaller** -and the distribution is skewed. - -### 5.3 Modulo bias (non-uniform distribution) -`r % n` over a full-range `Int32` is not uniform unless `n` divides 2^32. Character selection is -therefore slightly biased. For a security-focused generator that advertises a CSPRNG, the correct -approach is rejection sampling (e.g. `RandomNumberGenerator.GetInt32` on modern TFMs). - -### 5.4 "Max 2 identical in a row" rule is mis-guarded -The check is `characterPosition > maximumIdenticalConsecutiveChars` (i.e. `> 2`), so it only starts -at position 3. The first three characters can be identical (e.g. a password starting `aaa…`). The -rule itself also reduces entropy intentionally — debatable whether it belongs in a password -generator at all. - -### 5.5 Non-cryptographic, non-uniform shuffle -`Shuffle` uses `from item in items orderby Guid.NewGuid()`. `Guid.NewGuid()` is **not** a CSPRNG and -`OrderBy` over a random key is not a uniform (Fisher–Yates) shuffle. This undermines the -"cryptographically secure" positioning. (The per-character pick uses the CSPRNG, but the shuffle -layered on top adds weak, biased randomness.) - -### 5.6 `_rng` is `static`, reassigned per instance, never disposed -`private static RandomNumberGenerator _rng;` is reassigned inside **every** constructor. Constructing -many `Password` objects repeatedly replaces the shared static field and leaks `IDisposable` RNG -instances (never disposed). It's also a surprising shared-state design for a class that otherwise -looks instance-scoped. - -### 5.7 Dead code referencing the removed provider -`GetRngCryptoSeed(RNGCryptoServiceProvider rng)` is private, unused, and still references the legacy -`RNGCryptoServiceProvider` (the migration commit claimed to remove that usage). Should be deleted. - -### 5.8 `IncludeSpecial` with an empty/whitespace custom set silently never validates -If `IncludeSpecial` is true but `SpecialCharacters` is null/whitespace, `specialIsValid` stays -`false`, so every attempt fails and `Next()` returns `"Try again"`. No guard / no error explaining why. - -### 5.9 `Math.Abs(int.MinValue)` footgun (latent, NOT currently reachable) -Standalone `Math.Abs(int.MinValue)` throws `OverflowException` (verified). In this code it is **not** -reachable because `% (max - min)` is applied *before* `Math.Abs`, bounding the operand. Worth noting -so a v3 refactor doesn't accidentally expose it. - -### 5.10 `NextGroup` does not guarantee uniqueness -Despite the test name implying "different passwords", duplicates are possible (astronomically -unlikely at length 16, but real for short numeric OTPs like a 4-digit code). - ---- - -## 6. Documentation defects - -- **Readme length claims are wrong.** `Readme.md` repeatedly says length "Must be between 8 and 128", - but the code enforces **4 and 256** (`DefaultMinPasswordLength = 4`, `DefaultMaxPasswordLength = 256`). -- Code samples are fenced as ```javascript``` although they are C#. -- Readme logo points at branch `dev/v2`; compatibility image at `master`. Brittle. -- `IncludeSpecial(string)` exists on `Password`/`IPassword` but is **missing** from the obsolete - `PasswordGenerator` wrapper — minor inconsistency. - ---- - -## 7. Packaging / project hygiene - -- **Version drift:** `PasswordGenerator.csproj` declares `2.1.0` (and `Version`, `AssemblyVersion`, - `FileVersion` all 2.1.0). The legacy `PasswordGenerator.nuspec` still says **2.0.5** with 2019 - copyright and lists `RNGCryptoServiceProvider` in tags/notes. The nuspec appears stale/unused - (SDK-style `.csproj` packs the package) and is misleading — decide whether to delete it. -- `Copyright 2022` in csproj vs `Copyright 2019` in nuspec. -- **No `README` packed into the NuGet package** via the modern `` mechanism; only - the old `readme.txt` ASCII-art file is referenced by the nuspec. -- **No SourceLink, no deterministic build, no symbol package (`snupkg`), no ``** (uses - the deprecated ``). -- **CI is AppVeyor on the VS2017 image** (`appveyor.yml`) — very old; no GitHub Actions. -- Tests target EOL `netcoreapp2.2` and pull a vulnerable `Microsoft.NETCore.App 2.2.0`. - ---- - -## 8. What the package does NOT do (feature gaps) - -- No **passphrase / word-list** generation (e.g. diceware / xkcd-style). -- No **"exclude ambiguous characters"** option (e.g. `0/O`, `1/l/I`). -- No **per-class minimum counts** (e.g. "at least 2 digits and 1 special"). -- No **"require at least one of each included class"** guarantee — it relies on retry+validate, - which is probabilistic and can return `"Try again"`. -- No **entropy/strength estimate** for a generated password. -- No **pronounceable / memorable** password mode. -- No **`Span`/allocation-efficient** API; everything is `string`/`char[]`/LINQ. -- No **async** API (not really needed, but absent). -- No **dependency-injection helpers** (`AddPasswordGenerator()` / `IServiceCollection` extension). -- No **`TryNext`/`Result`** pattern — failures are encoded as magic strings. -- No **custom character pools** beyond special chars (can't, say, supply a full custom alphabet). -- No **uniqueness guarantee** in `NextGroup`. -- No **`net6/net8` target** — `netstandard2.0` only (works everywhere, but misses - `RandomNumberGenerator.GetInt32`, `GetItems`, etc.). -- No **nullable reference type** annotations. - ---- - -## 9. Suggested v3 direction (prioritised, for discussion) - -**Tier 1 — correctness & security (do these regardless):** -1. Replace the error-string returns with proper failure handling: throw `ArgumentException` for - invalid config and add `bool TryNext(out string password)` and/or a `PasswordResult` type. (§5.1) -2. Fix character selection: drop the Guid shuffle + biased modulo; use unbiased rejection sampling - (`RandomNumberGenerator.GetInt32` on modern TFMs, manual rejection on `netstandard2.0`). Fixes - §5.2, §5.3, §5.5 at once. -3. Guarantee included classes deterministically (seed one of each required class, then fill & shuffle) - instead of generate-and-retry; removes `"Try again"` and `MaximumAttempts` entirely. -4. Remove dead code (`GetRngCryptoSeed`) and fix the `static`/undisposed `_rng` design. (§5.6, §5.7) - -**Tier 2 — API & packaging modernisation:** -5. Multi-target `netstandard2.0;net8.0` (and maybe `net6.0`); add nullable annotations. -6. Add DI extension `services.AddPasswordGenerator()`. -7. Fix versioning/packaging: delete or regenerate the stale nuspec, add ``, - SourceLink, deterministic builds, `snupkg`, ``. Move CI to GitHub Actions. -8. Update tests to a supported TFM (`net8.0`) and NUnit 4, add uniqueness/entropy/edge-case tests. - -**Tier 3 — new features:** -9. "Exclude ambiguous characters" option and per-class minimum counts. -10. Passphrase/diceware mode with a bundled word list. -11. Entropy / strength estimate on the result. -12. `NextGroup` uniqueness option. - -**Breaking-change note:** items 1 and 3 change the failure contract and remove `MaximumAttempts` -semantics — appropriate for a major (v3) bump. Decide whether to keep the obsolete -`PasswordGenerator`/`PasswordGeneratorSettings` classes or finally drop them in v3. - ---- - -## 10. Quick reference — files & key line anchors - -- `Password.cs:114` `Next()` (error-string returns at lines 119, 131) -- `Password.cs:157` `GenerateRandomPassword` (shuffle at 163; 3-in-a-row guard at 172–177) -- `Password.cs:183` `GetRandomNumberInRange` (off-by-one + modulo bias) -- `Password.cs:195` `GetRngCryptoSeed` (dead code, references `RNGCryptoServiceProvider`) -- `Password.cs:208` `PasswordIsValid` (special-char empty-set edge case at 221–229) -- `Password.cs:247` `Shuffle` (Guid-based, non-uniform) -- `PasswordSettings.cs:10-15` character pools + min/max length (4/256) -- `PasswordSettings.cs:102` `StopUsingDefaults` (clears set on first fluent call) -- `PasswordGenerator.cs:5` `[Obsolete]` wrapper (source of the 5 CS0108 warnings) -- `PasswordGenerator.csproj:5,21` version 2.1.0 vs `PasswordGenerator.nuspec:5` version 2.0.5 diff --git a/docs/archive/V3_VERIFICATION.md b/docs/archive/V3_VERIFICATION.md deleted file mode 100644 index 35f8da2..0000000 --- a/docs/archive/V3_VERIFICATION.md +++ /dev/null @@ -1,204 +0,0 @@ -# PasswordGenerator v3 — Verification Report - -> **Archived / historical (2026-05-24).** Point-in-time analysis of the v2.1.0 source, kept for -> reference. Its target-framework recommendation (multi-target `netstandard2.0;net8.0`) was **not** -> followed: v3 dropped `netstandard2.0` and targets `net8.0;net10.0`. See the root -> [`CHANGELOG.md`](../../CHANGELOG.md). - -> Companion to `V3_REVIEW_AND_DOCUMENTATION.md` and the v3 Planning Addendum. -> This document does the verification the addendum asked for: every item from the original -> review's bug list (§5) and feature gaps (§8) was re-checked against the **current source**, -> and marked **Confirmed**, **Already Fixed**, or **Partially Fixed** with a code reference. -> -> Verification date: 2026-05-24. -> **Code state verified:** the source files on this branch are byte-identical to `origin/master` -> (`git diff origin/master -- PasswordGenerator/ PasswordGenerator.Tests/` is empty). The latest -> source commit is `36f2b58` *"removed usage of RNG Crypto Provider and replaced with -> RandomNumberGenerator"*. So the code reviewed here **is** the current default-branch state. - ---- - -## 0a. Which branch is current? (`master` vs `dev/v2`) - -`master` is the **most up-to-date** branch. `dev/v2` is the **older** v2.0.0 line — it has -diverged but is behind master on everything substantive: - -| Aspect | `dev/v2` (v2.0.0, `netstandard1.2`) | `master` (v2.1.0, `netstandard2.0`) | -|---|---|---| -| Randomness | `new Random()` — **insecure** (`Password.cs:148` on dev/v2) | `RandomNumberGenerator` CSPRNG (`Password.cs:189`) | -| Guid shuffle | present (`:204`) | **also present** (`:247-249`) | -| Length range | 8–128 | 4–256 | -| Custom special chars | absent | present (`IncludeSpecial(string)`) | -| Bug-fix tests | absent | present (`077b798`) | - -`dev/v2` carries a handful of commits master lacks, but they are all non-substantive -(readme/logo/nuspec/appveyor tweaks + an early "passwordservice" refactor). **Verify against -`master`** — which equals this branch's source. - -Key implication: the **Guid shuffle exists on BOTH branches and was never removed anywhere**, so the -"already fixed" recollection does not hold on any branch (see §0). What was actually fixed — only on -master — was `Random` → `RNGCryptoServiceProvider` → `RandomNumberGenerator` for *selection*. - ---- - -## 0. Headline correction (read this first) - -The addendum states the **Guid-based shuffle (original §5.5) is "ALREADY FIXED — remove from v3 -scope."** That is **not** what the code shows. - -- **What commit `36f2b58` actually changed:** the *character-selection* randomness. `GetRandomNumberInRange` - now draws from the CSPRNG — `_rng.GetBytes(data)` at `Password.cs:189`, where - `_rng = RandomNumberGenerator.Create()`. ✅ This part of the author's recollection is correct: the - **output's randomness now comes from a CSPRNG**, not from `Random` or from Guids. -- **What was NOT changed:** the `Shuffle` helper still uses `orderby Guid.NewGuid()` — - `Password.cs:247-249`, called at `Password.cs:163`. The Guid shuffle is **still in the code**. - -**Net verdict: Partially Fixed.** The Guid shuffle is no longer the source of the password's -randomness (so it is not a meaningful security hole anymore), but it is still present as -**redundant, non-uniform dead-weight** that reshuffles the pool before the CSPRNG indexes into it. -Recommendation: **keep a small cleanup task in v3** to delete `Shuffle` (and its call site) — do -**not** drop it from scope entirely. Selection via `GetRandomNumberInRange` alone already provides -the randomness; the shuffle adds nothing but a non-crypto code path. - ---- - -## 1. Bug list (§5) — verification - -| # | Original issue | Verdict | Evidence (current code) | -|---|---|---|---| -| 5.1 | `Next()` returns error text as a password (`"Try again"`, length message) | **Confirmed** | `Password.cs:119-120` (length message) and `Password.cs:131` (`… ? password : "Try again"`). No exception, no `TryNext`, no result type. | -| 5.2 | Off-by-one: top index of the pool never selected | **Confirmed** | `Password.cs:170` calls `GetRandomNumberInRange(0, characterSetLength - 1)`; `Password.cs:192` computes `… % (max - min)` = `% (characterSetLength - 1)` → range `0 … len-2`. Empirically reproduced earlier (index 9 never produced for a 10-element range). | -| 5.3 | Modulo bias (non-uniform selection) | **Confirmed** | `Password.cs:192` `randomNumber % (max - min)` over a full-range `Int32`. Should be rejection sampling / `RandomNumberGenerator.GetInt32`. | -| 5.4 | "Max 2 identical in a row" rule mis-guarded; first 3 chars can be identical | **Confirmed** | `Password.cs:173` guard is `characterPosition > maximumIdenticalConsecutiveChars` (i.e. `> 2`), so the check only starts at position 3. | -| 5.5 | Non-cryptographic, non-uniform Guid shuffle | **Partially Fixed** | Output randomness now from CSPRNG (`Password.cs:189`), but Guid shuffle still present at `Password.cs:247-249` (used at `:163`). See §0. Reclassify as **cleanup**, not security. | -| 5.6 | `_rng` is `static`, reassigned in every ctor, never disposed | **Confirmed** | `static` field `Password.cs:20`; reassigned in all six constructors (`:28, :35, :43, :51, :60, :69`); never disposed (it is `IDisposable`). | -| 5.7 | Dead code `GetRngCryptoSeed` referencing `RNGCryptoServiceProvider` | **Confirmed** | `Password.cs:195-200`. Unused; still references the legacy provider the commit message claimed to remove. | -| 5.8 | `IncludeSpecial` with empty/whitespace custom set silently never validates → `"Try again"` | **Confirmed** | `PasswordIsValid` `Password.cs:221-229`: `specialIsValid` only becomes `true` when `IncludeSpecial && !IsNullOrWhiteSpace(SpecialCharacters)` and a match is found; otherwise stays `false`. | -| 5.9 | `Math.Abs(int.MinValue)` overflow | **Confirmed (latent, NOT reachable)** | `Password.cs:192` applies `% (max - min)` *before* `Math.Abs`, bounding the operand. Standalone overflow verified earlier, but not reachable here. Keep in mind for the rewrite. | -| 5.10 | `NextGroup` does not de-duplicate | **Confirmed** | `Password.cs:138-149` simply loops `Next()` and adds to a `List`. Test `…ShouldReturn10DifferentPasswords` only asserts count. | - -### Documentation defects (§6) — still present -- Readme still says length "Must be between 8 and 128"; code enforces **4 and 256** - (`PasswordSettings.cs:14-15`). **Confirmed.** -- C# samples still fenced as ```javascript```. **Confirmed.** -- `IncludeSpecial(string)` still absent from the obsolete `PasswordGenerator` wrapper. **Confirmed.** - -### Packaging (§7) — still present -- `PasswordGenerator.csproj:5,21` = `2.1.0`; stale `PasswordGenerator.nuspec:5` = `2.0.5`. **Confirmed.** -- `PackageIconUrl` deprecation (`NU5048`) and missing `PackageReadmeFile` confirmed by `dotnet pack` - output during CI work. **Confirmed.** -- The 5 `CS0108` member-hiding warnings on the obsolete wrapper are still emitted. **Confirmed.** - ---- - -## 2. Feature gaps (§8) — verification - -All confirmed **absent** in current code (no hidden implementations found across the whole -`PasswordGenerator/` project — the only public surface is `IPassword`/`Password`/`IPasswordSettings`/ -`PasswordSettings` plus the two obsolete wrappers): - -| Gap | Verdict | Note | -|---|---|---| -| Passphrase / word-list (diceware) | **Confirmed gap** | No word list, no passphrase path. | -| Exclude ambiguous characters (`0/O`, `1/l/I`) | **Confirmed gap** | No such option. | -| Per-class minimum counts (e.g. "≥2 digits") | **Confirmed gap** | Only presence is checked, not counts. | -| Guarantee at least one of each included class | **Confirmed gap** | Probabilistic: relies on the generate-and-validate retry loop (`Password.cs:124-131`), hence `"Try again"`. | -| Entropy / strength estimate | **Confirmed gap** | None. | -| Pronounceable / memorable mode | **Confirmed gap** | None. | -| `Span` / low-allocation API | **Confirmed gap** | Uses `string`/`char[]`/LINQ throughout. | -| Async API | **Confirmed gap** | None. | -| DI helper (`AddPasswordGenerator()`) | **Confirmed gap** | None. | -| `TryNext` / `Result` pattern | **Confirmed gap** | Failures are magic strings (§5.1). | -| Custom full alphabet beyond special chars | **Partially achievable today** | No first-class API, but a caller *can* abuse `IncludeSpecial("…")` with the other classes off to supply an arbitrary pool (`PasswordSettings.cs:79-86`). v3 should add a clean `WithCharacters(...)`/`WithAllAscii()`. | -| `NextGroup` uniqueness | **Confirmed gap** | Dup of 5.10. | -| `net6`/`net8` target | **Confirmed gap** | `PasswordGenerator.csproj:4` is `netstandard2.0` only. | -| Nullable reference annotations | **Confirmed gap** | No `enable` in the csproj. | - ---- - -## 3. Adjusted v3 plan (reconciling the original review + the addendum + this verification) - -### Tier 1 — Correctness & security -1. **Replace error-string returns** with exceptions + a `TryNext`/`PasswordResult` pattern. (5.1) — *Confirmed, keep.* -2. **Fix selection: unbiased rejection sampling** (`RandomNumberGenerator.GetInt32` on modern TFMs; - manual rejection on `netstandard2.0`). Fixes 5.2 + 5.3 in one change. — *Confirmed, keep.* -3. **Delete the Guid `Shuffle`** (and its call site at `Password.cs:163`). — **Keep as a small - cleanup task** (the addendum's "remove from scope" is based on an inaccurate belief that the code - was already removed — it was not; see §0). Reclassified from "security" to "cleanup". -4. **Guarantee included classes deterministically** (seed one of each required class, then fill & - shuffle with the CSPRNG) → removes `"Try again"` and the `MaximumAttempts` retry loop. — *Keep.* - - Addendum nuance: retry behaviour, where it remains, must be **configurable** (fluent / - appSettings / library default) and must **throw** on exhaustion, never return a string. -5. **Remove dead code** `GetRngCryptoSeed` (5.7) and **fix the `static`/undisposed `_rng`** design - (5.6) — make the RNG an instance field (or use the static `RandomNumberGenerator.Fill`/`GetInt32` - static APIs and hold no field at all). — *Confirmed, keep.* -6. **Fix the empty-custom-special-set trap** (5.8) — validate the configuration up front and throw a - clear exception instead of silently failing. — *Confirmed, keep.* - -### Tier 2 — Modernisation -7. **Multi-target** `netstandard2.0;net8.0` (see open-question recommendation below); add nullable - annotations. -8. **Async API**: add `NextAsync()` / `GenerateAsync()`; mark sync methods `[Obsolete]` pointing to - async equivalents (gentle deprecation). *(addendum)* -9. **DI support**: `services.AddPasswordGenerator()` extension, opt-in (not auto-registered); wires up - the RNG; fluent API behaves identically whether `new`'d or resolved. *(addendum, confirmed gap)* -10. **BenchmarkDotNet** project alongside tests (sync vs async, batch sizes 1/100/1000/10000, - allocations); include comparative benchmark numbers in every release note going forward. *(addendum)* -11. **Packaging hygiene**: delete/regenerate the stale nuspec, add ``, replace - `PackageIconUrl` with `` (clears `NU5048`), add SourceLink + deterministic build + - `snupkg`. Fix the 5 `CS0108` warnings (or drop the obsolete wrappers — see open questions). -12. **Tests**: retarget to `net8.0` + NUnit 4 (current `netcoreapp2.2` is EOL and pulls vulnerable - `Microsoft.NETCore.App 2.2.0`); add uniqueness, entropy, and edge-case tests. This also makes the - AppVeyor `dotnet test` step reliable. - -### Tier 3 — New features *(all confirmed absent today)* -13. Fluent character-pool control incl. `.WithAllAscii()` / `WithCharacters(...)`; keep existing - `Include*` methods (library is also used for OTPs, env names, API keys). **Do not** impose a - global 12-char minimum. -14. Use-case / compliance presets: `.ForOwasp()`, `.ForNist()`, `.ForHipaa()`, `.ForPciDss()`, - `.ForOtp()`, `.ForPassphrase()`, `.ForApiKey()`, `.ForEnvironmentName()`. -15. `appSettings` configuration with resolution order fluent > appSettings > library default - (separate, opt-in step). -16. `.Generate()` / `.GenerateAsync()` batch API (count overloads + `.Count(n)` chaining + - appSettings default); keep `.Next()` for single (mirrors `Random.Next()`). -17. Exclude-ambiguous option, per-class minimum counts, entropy estimate, `NextGroup`/`Generate` - uniqueness option. - -### Tier 4 — Documentation *(addendum)* -18. v2→v3 migration guide (direct→DI, sync→async, error-string→exceptions, presets/appSettings). -19. Clarify broader purpose (OTPs, env names, API keys), document preset↔standard mapping with - OWASP/NIST links. - ---- - -## 4. Recommendations on the open questions - -1. **Drop the `[Obsolete] PasswordGenerator`/`PasswordGeneratorSettings` wrappers in v3?** - Recommend **dropping them**. They have carried `[Obsolete]` since v2, they are the sole source of - the 5 `CS0108` warnings, and v3 is a major version (the natural removal point). If you prefer - maximum caution, the fallback is to keep them for one more major but add the `new` keyword to - silence the warnings — but a clean removal is the better long-term call. -2. **Minimum target — `netstandard2.0` vs `net8.0;net10.0` only?** - Recommend **multi-targeting `netstandard2.0;net8.0`** (optionally add `net10.0`). Dropping - `netstandard2.0` would cut off .NET Framework / older consumers, which matters for this package's - Umbraco-heavy audience. Multi-targeting lets the modern TFM use `RandomNumberGenerator.GetInt32` - / `GetItems` (fixing the bias cleanly) while `netstandard2.0` keeps a manual rejection-sampling - fallback. -3. **DI overload taking an `IConfiguration` section for one-line appSettings binding?** - **Yes.** Provide both `AddPasswordGenerator(Action configure)` and - `AddPasswordGenerator(IConfiguration section)` so consumers can bind their policy from - `appSettings.json` in a single line, consistent with the fluent > appSettings > default order. - ---- - -## 5. Summary - -- **9 of 10** original §5 bugs are **Confirmed still present**; **§5.5 (Guid shuffle) is Partially - Fixed** — the addendum's premise that it was fully removed is inaccurate (`Password.cs:247-249`), - though it is now redundant rather than a security hole. -- **§5.9** remains a **latent, non-reachable** footgun. -- **All §8 feature gaps confirmed absent**, except a custom alphabet is *hackily* achievable via - `IncludeSpecial(string)` today. -- The adjusted plan keeps the Guid-shuffle removal as a **cleanup** task (not dropped), folds in all - addendum additions (async, DI, benchmarks, presets, appSettings, `.Generate()`, migration guide), - and answers the three open questions with recommendations. diff --git a/docs/archive/before-after.md b/docs/archive/before-after.md deleted file mode 100644 index e3ece99..0000000 --- a/docs/archive/before-after.md +++ /dev/null @@ -1,127 +0,0 @@ -# v3 Target — Before / After - -> **Archived / historical.** A v3 planning document, kept for reference and superseded by the shipped -> v3 docs in [`../`](../README.md). The "after" column reflects the early plan; note that v3 ultimately -> **dropped `netstandard2.0`** (targets `net8.0;net10.0`). - -Side-by-side of the things that change most, each tied to a verified issue. - -## 1. Failure handling - -```mermaid -flowchart LR - subgraph Before["v2.1.0 (§5.1)"] - b1["pwd.Next()"] --> b2["string — might be
'Try again' or
'Password length invalid...'"] - b2 --> b3["caller may store
an ERROR as a password"] - end - subgraph After["v3"] - a1["gen.Next()"] --> a2["valid password
OR throws ArgumentException"] - a1b["gen.TryNext(out pwd)"] --> a2b["bool + real password"] - end - classDef bad fill:#ffe6e6,stroke:#cc0000; - classDef good fill:#e6ffe6,stroke:#009900; - class b2,b3 bad; - class a2,a2b good; -``` - -```csharp -// Before — silent footgun -var pwd = new Password(3).Next(); // = "Password length invalid. Must be between 4 and 256..." - -// After — explicit -try { var pwd = gen.Next(); } // throws ArgumentException for length 3 -catch (ArgumentException ex) { /* handle */ } -if (gen.TryNext(out var p)) { /* use p */ } -``` - -## 2. Randomness & character selection - -```mermaid -flowchart LR - subgraph Before2["v2.1.0"] - x1["Guid.NewGuid() shuffle (§5.5)"] --> x2["pick rnd % (len-1)
top index never used (§5.2)
modulo bias (§5.3)"] - end - subgraph After2["v3"] - y1["IRandomSource (CSPRNG)"] --> y2["GetInt32 / rejection sampling
uniform, full range"] - y2 --> y3["crypto Fisher-Yates shuffle"] - end - classDef bad fill:#ffe6e6,stroke:#cc0000; - classDef good fill:#e6ffe6,stroke:#009900; - class x1,x2 bad; - class y1,y2,y3 good; -``` - -## 3. Guaranteeing required character classes - -```mermaid -flowchart LR - subgraph Before3["v2.1.0"] - g1["generate random"] --> g2["validate"] --> g3{"ok?"} - g3 -- no --> g1 - g3 -- "no, 10000x" --> g4["'Try again' (§5.1)"] - end - subgraph After3["v3"] - h1["seed one of each
required class"] --> h2["fill + crypto-shuffle"] --> h3["valid by construction"] - end - classDef bad fill:#ffe6e6,stroke:#cc0000; - classDef good fill:#e6ffe6,stroke:#009900; - class g4 bad; - class h3 good; -``` - -## 4. Configuration & wiring - -| Concern | v2.1.0 | v3 | -|---|---|---| -| Sources | constructor args + fluent only | fluent **>** appSettings **>** default | -| DI | none | opt-in `AddPasswordGenerator(...)` (+ `IConfiguration` overload) | -| RNG ownership | `static`, reassigned per ctor, never disposed (§5.6) | injected `IRandomSource`, disposable-aware | -| Presets | none | `ForOwasp/ForNist/ForOtp/ForPassphrase/ForApiKey/ForEnvironmentName` | - -## 5. Targets, tests, packaging - -```mermaid -flowchart LR - subgraph BeforeP["v2.1.0"] - p1["netstandard2.0 only"] - p2["tests on EOL netcoreapp2.2
(vulnerable 2.2.0)"] - p3["stale nuspec 2.0.5, NU5048,
no readme in package"] - p4["5x CS0108 from obsolete wrappers"] - end - subgraph AfterP["v3"] - q1["netstandard2.0 + net8.0"] - q2["tests on net8.0, NUnit 4
+ uniqueness/entropy/edge cases"] - q3["clean pack: PackageReadmeFile,
PackageIcon, SourceLink, snupkg"] - q4["obsolete wrappers removed → no CS0108"] - q5["BenchmarkDotNet numbers in release notes"] - end - classDef good fill:#e6ffe6,stroke:#009900; - class q1,q2,q3,q4,q5 good; -``` - -## Net effect - -```mermaid -mindmap - root((v3 better)) - Safety - exceptions not strings - TryNext - guaranteed classes - Correctness - unbiased CSPRNG - no off-by-one - no modulo bias - Reach - multi-target - nullable - DI + appSettings - Capability - presets - custom pools / WithAllAscii - batch Generate + async - Trust - modern tests - benchmarks in release notes - clean packaging -``` diff --git a/docs/archive/current-state/api-surface.md b/docs/archive/current-state/api-surface.md deleted file mode 100644 index 84b1089..0000000 --- a/docs/archive/current-state/api-surface.md +++ /dev/null @@ -1,77 +0,0 @@ -# Current State — Public API Surface (v2.1.0) - -> **Historical.** This describes v2.1.0. The issues noted here are resolved in v3 — see the root -> [`CHANGELOG.md`](../../../CHANGELOG.md) and the [migration guide](../../migration-v2-to-v3.md). - -What a caller can do today, and how configuration is resolved. - -## API map - -```mermaid -flowchart TD - subgraph Construct["Construction (6 constructors)"] - c0["Password()"] - c1["Password(int length)"] - c2["Password(IPasswordSettings)"] - c3["Password(bool l, u, n, s)"] - c4["Password(bool l,u,n,s, int length)"] - c5["Password(bool l,u,n,s, int length, int maxAttempts)"] - end - subgraph Fluent["Fluent builders (return this)"] - f1["IncludeLowercase()"] - f2["IncludeUppercase()"] - f3["IncludeNumeric()"] - f4["IncludeSpecial()"] - f5["IncludeSpecial(string)"] - f6["LengthRequired(int)"] - end - subgraph Generate["Generation"] - g1["Next() : string"] - g2["NextGroup(int) : IEnumerable~string~"] - end - Construct --> Fluent --> Generate -``` - -## Configuration resolution (today) - -```mermaid -flowchart LR - A["Constructor args
or defaults"] --> S[(PasswordSettings)] - B["Fluent Include*/LengthRequired"] --> S - S --> G["Next()"] - note1["First fluent call on a defaulted
Password() clears the pool
(StopUsingDefaults)"] - B -.-> note1 -``` - -There are only two sources: constructor arguments (or built-in defaults) and fluent calls. There is -**no** external configuration (`appSettings`), **no** DI, and **no** presets. - -Defaults: all four classes on, length 16, `MaximumAttempts` 10000, length bounds 4–256, default -special set `!#$%&*@\` (8 chars). - -## Key behaviour quirks (verified) - -```mermaid -flowchart TD - Q1["new Password().IncludeNumeric()"] --> R1["numeric ONLY, length 16
(first fluent call clears defaults)"] - Q2["Next() on bad config"] --> R2["returns an ERROR STRING (§5.1)"] - Q3["NextGroup(n)"] --> R3["n passwords, NOT de-duplicated (§5.10)"] - Q4["IncludeSpecial(empty string)"] --> R4["always 'Try again' (§5.8)"] - classDef bad fill:#ffe6e6,stroke:#cc0000; - class R2,R3,R4 bad; -``` - -## What the surface does NOT offer (verified gaps, §8) - -| Missing today | Confirmed | -|---|---| -| `TryNext` / result type (failures are strings) | ✅ | -| Async API | ✅ | -| DI registration helper | ✅ | -| Presets (OWASP/NIST/OTP/passphrase/API-key/env-name) | ✅ | -| `appSettings` configuration | ✅ | -| First-class custom alphabet (`WithAllAscii`/`WithCharacters`) | ✅ (only hackable via `IncludeSpecial(string)`) | -| Exclude-ambiguous, per-class minimums, entropy estimate | ✅ | -| `netstandard2.0` + `net8.0` multi-target / nullable | ✅ (netstandard2.0 only) | - -These gaps define the v3 surface in `../../api-surface.md`. diff --git a/docs/archive/current-state/architecture.md b/docs/archive/current-state/architecture.md deleted file mode 100644 index 3de1bcf..0000000 --- a/docs/archive/current-state/architecture.md +++ /dev/null @@ -1,100 +0,0 @@ -# Current State — Architecture (v2.1.0) - -> **Historical.** This describes v2.1.0. The issues noted here are resolved in v3 — see the root -> [`CHANGELOG.md`](../../../CHANGELOG.md) and the [migration guide](../../migration-v2-to-v3.md). - -`master` @ v2.1.0 · target `netstandard2.0` · no third-party runtime dependencies. - -## Type relationships - -```mermaid -classDiagram - class IPassword { - <> - +IncludeLowercase() IPassword - +IncludeUppercase() IPassword - +IncludeNumeric() IPassword - +IncludeSpecial() IPassword - +IncludeSpecial(string) IPassword - +LengthRequired(int) IPassword - +Next() string - +NextGroup(int) IEnumerable~string~ - } - class IPasswordSettings { - <> - +bool IncludeLowercase - +bool IncludeUppercase - +bool IncludeNumeric - +bool IncludeSpecial - +int PasswordLength - +string CharacterSet - +int MaximumAttempts - +int MinimumLength - +int MaximumLength - +string SpecialCharacters - +AddLowercase() IPasswordSettings - +AddUppercase() IPasswordSettings - +AddNumeric() IPasswordSettings - +AddSpecial() IPasswordSettings - +AddSpecial(string) IPasswordSettings - } - class Password { - -static RandomNumberGenerator _rng - +IPasswordSettings Settings - +Next() string - +NextGroup(int) IEnumerable~string~ - -GenerateRandomPassword(settings)$ string - -GetRandomNumberInRange(min, max)$ int - -PasswordIsValid(settings, pwd)$ bool - -Shuffle(items)$ IEnumerable - -GetRngCryptoSeed(rng)$ int - } - class PasswordSettings { - +BuildCharacterSet(...) - -StopUsingDefaults() - } - class PasswordGenerator { - <> - } - class PasswordGeneratorSettings { - <> - } - - IPassword <|.. Password - IPasswordSettings <|.. PasswordSettings - Password o-- IPasswordSettings : Settings - Password <|-- PasswordGenerator : inherits - PasswordSettings <|-- PasswordGeneratorSettings : inherits -``` - -Notes: -- `PasswordGenerator` / `PasswordGeneratorSettings` are `[Obsolete]` back-compat wrappers. The five - `CS0108` build warnings come from `PasswordGenerator` hiding `Password` methods without `new`. -- `_rng` is a **`static`** field on `Password` (`Password.cs:20`), reassigned in **every** constructor - and never disposed (verified issue §5.6). -- `GetRngCryptoSeed` (`Password.cs:195`) is dead code still referencing the legacy - `RNGCryptoServiceProvider` (verified issue §5.7). - -## Runtime composition - -```mermaid -flowchart TD - Caller["Caller code"] -->|new Password / fluent| P[Password] - P --> S[PasswordSettings
character pools, length, attempts] - P -->|reads CharacterSet| S - P --> RNG["static RandomNumberGenerator (CSPRNG)"] - P -->|orderby Guid.NewGuid| SH["Shuffle helper (non-crypto)"] - classDef warn fill:#ffe6e6,stroke:#cc0000; - class SH warn; -``` - -The output's randomness comes from the CSPRNG via `GetRandomNumberInRange` (`Password.cs:189`). The -`Shuffle` helper (`Password.cs:247-249`) reshuffles the pool first using `Guid.NewGuid()` — a -non-crypto, non-uniform sort that is now **redundant** (verified issue §5.5, reclassified as cleanup). - -## Packaging / build snapshot - -- Single packable project `PasswordGenerator.csproj`, version `2.1.0`. -- Stale `PasswordGenerator.nuspec` declares `2.0.5` (verified issue §7). -- `dotnet pack` emits `NU5048` (deprecated `PackageIconUrl`) and "missing readme". -- Tests target EOL `netcoreapp2.2` (pulls vulnerable `Microsoft.NETCore.App 2.2.0`). diff --git a/docs/archive/current-state/generation-flow.md b/docs/archive/current-state/generation-flow.md deleted file mode 100644 index ac54286..0000000 --- a/docs/archive/current-state/generation-flow.md +++ /dev/null @@ -1,94 +0,0 @@ -# Current State — Generation Flow (v2.1.0) - -> **Historical.** This describes v2.1.0. The issues noted here are resolved in v3 — see the root -> [`CHANGELOG.md`](../../../CHANGELOG.md) and the [migration guide](../../migration-v2-to-v3.md). - -How `Next()` produces a password today (`Password.cs:114-193`). - -## `Next()` control flow - -```mermaid -flowchart TD - Start([Next called]) --> LenOK{"length in
[Min, Max]?"} - LenOK -- no --> ErrStr["return ERROR STRING:
'Password length invalid...'"] - LenOK -- yes --> Gen["GenerateRandomPassword(settings)"] - Gen --> Valid{"PasswordIsValid?"} - Valid -- yes --> RetPwd([return password]) - Valid -- no --> Attempts{"attempts <
MaximumAttempts?"} - Attempts -- yes --> Gen - Attempts -- no --> TryAgain["return ERROR STRING:
'Try again'"] - - classDef bad fill:#ffe6e6,stroke:#cc0000; - class ErrStr,TryAgain bad; -``` - -**Verified problem (§5.1):** the two red nodes return human-readable **error strings in the same -`string` return slot as a real password**. A caller that does not special-case them will store an -error message as the user's password. There is no exception and no `TryNext`/result type. - -## Inside `GenerateRandomPassword` - -```mermaid -flowchart TD - A["pool = settings.CharacterSet"] --> B["pool = Shuffle(pool)
orderby Guid.NewGuid (non-crypto)"] - B --> C["for each position 0..length-1"] - C --> D["idx = GetRandomNumberInRange(0, len-1)
= rnd % (len-1) → range 0..len-2"] - D --> E["password[pos] = pool[idx]"] - E --> F{"pos > 2 AND
3 identical in a row?"} - F -- yes --> G["pos-- (redo this position)"] - F -- no --> H["next position"] - G --> C - H --> C - - classDef bad fill:#ffe6e6,stroke:#cc0000; - class B,D,F bad; -``` - -Verified problems in this loop: -- **§5.5** — `Shuffle` is a non-crypto `Guid.NewGuid()` sort (redundant; randomness really comes from - `GetRandomNumberInRange`). -- **§5.2 (off-by-one)** — `GetRandomNumberInRange(0, len-1)` computes `% (len-1)`, so the **top index - is never selected**; the effective alphabet is one char short per password. -- **§5.3 (modulo bias)** — `rnd % n` over a full-range `Int32` is not uniform. -- **§5.4** — the "no 3 identical in a row" guard only starts at position > 2, so the **first three - characters can be identical**. - -## How validity is decided (`PasswordIsValid`, `Password.cs:208`) - -```mermaid -flowchart LR - P[password] --> L{"lower required?
→ regex match"} - P --> U{"upper required?
→ regex match"} - P --> N{"numeric required?
→ regex match"} - P --> S{"special required?
→ any special char present"} - P --> Len{"length in range?"} - L & U & N & S & Len --> AND{{"all true?"}} - AND -- yes --> OK([valid]) - AND -- no --> NO([invalid → retry]) -``` - -**Verified problem (§5.8):** if `IncludeSpecial` is true but the custom special set is empty/whitespace, -`specialIsValid` stays `false` forever, so every attempt fails and `Next()` silently returns -`"Try again"`. - -## Why this design is fragile - -```mermaid -stateDiagram-v2 - [*] --> Configured - Configured --> Generating: Next() - Generating --> Generating: invalid (retry up to MaximumAttempts) - Generating --> Success: valid password - Generating --> FailureString: attempts exhausted - Configured --> FailureString: length invalid - note right of FailureString - Failure is a magic STRING, - not an exception. Caller may - not notice. (§5.1) - end note - Success --> [*] - FailureString --> [*] -``` - -The whole correctness contract hinges on probabilistic retry + string sentinels — the core thing v3 -replaces (see `../../generation-flow.md`). diff --git a/docs/archive/implementation-plan.md b/docs/archive/implementation-plan.md deleted file mode 100644 index 56d0299..0000000 --- a/docs/archive/implementation-plan.md +++ /dev/null @@ -1,344 +0,0 @@ -# v3 Target — Implementation Plan (phased) - -> **Archived / historical.** This is a v3 planning document, kept for reference and superseded by the -> shipped v3 docs in [`../`](../README.md). Note that v3 ultimately **dropped `netstandard2.0`** -> (targets `net8.0;net10.0`), contrary to the multi-target plan described here. - -> Actionable, phase-by-phase plan to deliver the v3 design in `../architecture.md`, -> `../generation-flow.md`, `../api-surface.md`, `../configuration-and-di.md` and `before-after.md`. -> Sequencing follows `roadmap.md`; issue numbers (§5.x / §8) reference `V3_VERIFICATION.md`. - -## Working principles - -- **One phase = one PR** (or a small stack), each independently green and reviewable. -- **Commit and push at the end of every phase** — no phase spans an uncommitted working tree. Each - phase ends with its own commit (suggested messages below) pushed to the working branch. -- **Tests must pass before each phase's commit.** A phase is not "done" until the appropriate test - suite is green (`dotnet test` exits 0). Never commit a phase with failing or skipped-for- - convenience tests. -- **Keep `master` shippable.** Behaviour-breaking changes (exceptions, removed wrappers) land behind - the v3 major and are called out in the migration guide. -- **Test-first for correctness work** — write the failing test that encodes the bug, then fix it. -- **Verify every phase with the SDK** (see Phase 0) before committing. - -## Definition of done — applies to EVERY phase - -Each phase repeats the same loop and only advances once it closes: - -```mermaid -flowchart LR - A["implement phase tasks"] --> B["add/update tests
for this phase"] - B --> C{"dotnet build
+ dotnet test
green?"} - C -- no --> A - C -- yes --> D["commit + push
(one commit per phase)"] - D --> E["open / update PR"] - E --> F["next phase"] - classDef gate fill:#fff5e6,stroke:#cc6600; - classDef good fill:#e6ffe6,stroke:#009900; - class C gate; - class D good; -``` - -A phase's checklist is complete only when **all** of the following hold: -1. The phase's tasks are implemented. -2. Tests covering the phase's changes exist and **pass** (`dotnet test` returns 0). -3. The build is green at the warning level the phase targets (e.g. Phase 3 must show zero `CS0108`). -4. The work is **committed and pushed** as that phase's commit. - -## Phase map - -```mermaid -flowchart TD - P0["Phase 0 — Toolchain & baseline"] --> P1["Phase 1 — Correctness & security core"] - P1 --> P2["Phase 2 — Targets & test modernisation"] - P2 --> P3["Phase 3 — API: async, DI, builder split"] - P3 --> P4["Phase 4 — New features"] - P4 --> P5["Phase 5 — Packaging & release"] - P5 --> P6["Phase 6 — Documentation & migration"] - classDef setup fill:#eee,stroke:#666; - classDef core fill:#ffe6e6,stroke:#cc0000; - classDef mod fill:#fff5e6,stroke:#cc6600; - classDef feat fill:#e6f0ff,stroke:#0066cc; - classDef rel fill:#e6ffe6,stroke:#009900; - class P0 setup; - class P1 core; - class P2,P3 mod; - class P4 feat; - class P5,P6 rel; -``` - ---- - -## Phase 0 — Toolchain & baseline - -**Objective:** a reproducible build/test environment and a known-green starting point. The remote / -CI containers do **not** ship the .NET SDK, so installing it is the first task of any work session. - -**Tasks** -1. **Install dotnet via bash** (verified working in this environment): - ```bash - cd /tmp - curl -fsSL https://dot.net/v1/dotnet-install.sh -o dotnet-install.sh - chmod +x dotnet-install.sh - ./dotnet-install.sh --channel 8.0 --install-dir /tmp/dotnet - export PATH="/tmp/dotnet:$PATH" - export DOTNET_CLI_TELEMETRY_OPTOUT=1 - dotnet --version # expect 8.0.4xx - ``` - (Add `--channel 10.0` as a second install once we multi-target to `net10.0`.) -2. Establish the baseline: - ```bash - dotnet build PasswordGenerator/PasswordGenerator.csproj -c Release # expect 5x CS0108 warnings - dotnet build PasswordGenerator.Tests/PasswordGenerator.Tests.csproj -c Release - ``` - Tests currently target EOL `netcoreapp2.2` and cannot run on a modern-only runtime; record this as - the reason Phase 2 retargets them. (Baseline behaviour: 24 tests, all passing when run on net8.) -3. Confirm CI is on the dotnet CLI (already done: `appveyor.yml` uses `dotnet restore/build/test/pack`, - `deploy: off`). - -**Verification / exit criteria** -- `dotnet --version` prints an 8.0.x SDK. -- Library builds (warnings only); CI build is green. -- **Tests:** the existing suite (24 tests) runs and **passes** (run on net8 in this environment, since - the `netcoreapp2.2` runtime is EOL) — this is the green baseline every later phase is measured against. -- A `docs/`-referenced note records the baseline warning set so later phases can show them clearing. -- **Commit & push** this phase, e.g. `chore: establish v3 toolchain and green baseline`. - -**Closes:** nothing yet (setup). - ---- - -## Phase 1 — Correctness & security core (Tier 1) - -**Objective:** make generation correct, unbiased, and fail-loud — without changing target frameworks -yet (stay on `netstandard2.0`, use manual rejection sampling; the optimised `net8` path arrives in -Phase 2). - -**Tasks** -1. **Introduce `IRandomSource` + `CryptoRandomSource`** wrapping `RandomNumberGenerator`. Provide - `int NextInt(int maxExclusive)` using **rejection sampling** (uniform, no modulo bias, no - off-by-one). Remove the `static` RNG field. *(closes §5.2, §5.3, §5.6)* -2. **Delete the Guid `Shuffle`**; replace pool randomisation with a crypto Fisher–Yates using - `IRandomSource`. *(closes §5.5 cleanup)* -3. **Delete dead `GetRngCryptoSeed`** and the `RNGCryptoServiceProvider` reference. *(closes §5.7)* -4. **Deterministic class-seeding:** place one char per required class first, fill the rest, then - shuffle — so output is valid by construction. Remove the validate-and-retry loop and - `MaximumAttempts` gamble. *(closes the probabilistic-guarantee gap)* -5. **Fail-loud contract:** invalid configuration throws `ArgumentException`; add - `bool TryNext(out string)`. No method ever returns `"Try again"` / a length-error string. - *(closes §5.1)* -6. **Up-front config validation** including the empty/whitespace custom-special-set case. - *(closes §5.8)* -7. **Fix the consecutive-char rule** (or drop it deliberately) so it can't allow 3 identical leading - chars. *(closes §5.4)* - -**Files:** `Password.cs`, `PasswordSettings.cs`, new `IRandomSource.cs` / `CryptoRandomSource.cs`, -plus tests. - -**Verification / exit criteria** -- New unit tests with a **deterministic `IRandomSource` stub** prove uniform selection, the seeding - guarantee, and exception/`TryNext` behaviour. -- **Tests green:** `dotnet test` returns 0, including the new correctness tests and the existing - suite; a statistical test confirms every pool index is reachable. -- **Commit & push** this phase, e.g. `feat: unbiased CSPRNG selection, fail-loud contract (§5.1-5.8)`. - -**Closes:** §5.1, §5.2, §5.3, §5.4, §5.5, §5.6, §5.7, §5.8. - ---- - -## Phase 2 — Targets & test modernisation (Tier 2a) - -**Objective:** broaden reach and put correctness work under a modern, fast test+benchmark harness. - -**Tasks** -1. **Multi-target** `netstandard2.0;net8.0` (optionally `net10.0`); enable `enable`. -2. In `CryptoRandomSource`, add a `#if NET8_0_OR_GREATER` path using - `RandomNumberGenerator.GetInt32` / `GetItems`; keep rejection sampling for `netstandard2.0`. -3. **Retarget tests** to `net8.0`, upgrade to **NUnit 4** (update classic asserts), drop the - vulnerable `Microsoft.NETCore.App 2.2.0`. -4. Add edge-case + property tests: uniqueness, length bounds, per-class guarantees, custom pools. -5. **Add a BenchmarkDotNet project** covering sync vs async and batch sizes 1/100/1000/10000 + - allocations. - -**Verification / exit criteria** -- **Tests green on `net8.0`** with NUnit 4: `dotnet test` returns 0 with **no `NU1903/NU1902`** - warnings (the full migrated suite passes, not a subset). -- `dotnet build` produces both TFMs; nullable warnings triaged to zero. -- Benchmarks run and emit a baseline report. -- **Commit & push** this phase, e.g. `build: multi-target net8.0, migrate tests to NUnit4, add benchmarks`. - -**Closes:** §8 multi-target/nullable; unblocks reliable CI `dotnet test`. - ---- - -## Phase 3 — API: async, DI, remove v2 wrappers (Tier 2b) - -**Objective:** the modern generation surface from `api-surface.md`, additively (no churn for existing -callers). - -**Decisions taken during implementation** (differ from the earlier draft): -- **Async is added but sync is NOT marked `[Obsolete]`.** Generation is CPU-bound, so obsoleting sync - in favour of async would be an anti-pattern and would spam every consumer with build warnings. - Async methods exist for ergonomics/cancellation only. -- **DI lives in the core package** (chosen over a separate `PasswordGenerator.DependencyInjection` - package), adding `Microsoft.Extensions.DependencyInjection.Abstractions` and - `Microsoft.Extensions.Configuration.Binder` dependencies. -- **The full `IPasswordBuilder` split is deferred.** The existing `IPassword` remains the fluent - builder; `IPasswordGenerator` is added as the generation contract and is what DI hands out. - -**Tasks** -1. Introduce `IPasswordGenerator` (`Next`/`TryNext`/`NextAsync`/`Generate`/`GenerateAsync`); - `Password` implements it alongside `IPassword`. -2. Add **async** methods (`NextAsync`/`GenerateAsync`) that honour `CancellationToken`; keep sync fully - supported. -3. **DI**: `AddPasswordGenerator(Action)` **and** - `AddPasswordGenerator(IConfiguration section)` (opt-in; wires `IRandomSource`). `new` vs DI produce - identical results. -4. **Remove the `[Obsolete] PasswordGenerator` / `PasswordGeneratorSettings` wrappers** (and their - tests) — clears the 5 `CS0108` warnings. - -**Verification / exit criteria** -- Build has **zero `CS0108`** (and zero warnings overall); DI resolves and generates. -- **Tests green:** new tests cover async, cancellation, batch `Generate`, and DI-resolved equivalence; - `dotnet test` returns 0. -- **Commit & push** this phase, e.g. `feat: async API, DI registration, remove obsolete v2 wrappers`. - -**Closes:** §8 async/DI; removes the obsolete-wrapper warnings. - ---- - -## Phase 4 — New features (Tier 3) - -**Objective:** the capability set that makes v3 worth the major bump. - -**Decisions taken during implementation:** -- **`ForPassphrase` uses a small built-in word list** (`WordList`, ~280 common words), not a full - EFF/diceware list — avoids bundling ~70KB and an external attribution. Entropy is reported honestly - by `PassphraseGenerator.EstimateEntropyBits()`. -- **Batch API is `Generate(count)` plus a parameterless `Generate()`** that uses a configurable - `DefaultBatchCount` (bindable from appSettings). The `.Count(n)` fluent-chaining shape from the - design doc was **not** added (no new return type); optional batch uniqueness was not implemented. -- The existing fluent `IPassword` remains the builder (no separate `IPasswordBuilder`); the new - methods/presets hang off it. Passphrases return an `IPasswordGenerator` (they have no char classes). - -**Tasks** -1. **Custom pools:** `WithCharacters(string)` and `WithAllAscii()`; keep `Include*`. -2. **Presets:** `ForOwasp`, `ForNist`, `ForOtp`, `ForPassphrase`, `ForApiKey`, `ForEnvironmentName` - (static factories; later fluent calls still override). -3. **`appSettings` configuration** with resolution order **code-configure > appSettings > default**, - realised by the `AddPasswordGenerator(IConfiguration, Action)` overload. -4. **`Generate()` batch API:** `Generate(count)` + parameterless `Generate()` using `DefaultBatchCount` - from appSettings. *(closes §5.10)* -5. **Quality options:** `ExcludeAmbiguous()`, `RequireAtLeast(class, count)`, and an - `IEntropyEstimator` (`PoolEntropyEstimator`) returning strength in bits. - -**Verification / exit criteria** -- Preset outputs match documented standards. -- **Tests green:** tests for ambiguity exclusion, minimum counts, batch uniqueness, appSettings - precedence, and entropy bounds **pass** (`dotnet test` returns 0). -- **Commit & push** this phase, e.g. `feat: presets, custom pools, appSettings, batch Generate, entropy`. - -**Closes:** §8 presets/appSettings/custom-pools/exclude-ambiguous/min-counts/entropy; §5.10. - ---- - -## Phase 5 — Packaging & release (Tier 2c) - -**Objective:** a clean, modern NuGet package and a disciplined release. - -**Decisions taken during implementation:** -- The stale `PasswordGenerator.nuspec` was **deleted** (not regenerated) — SDK-style `dotnet pack` - derives the nuspec from the csproj, which is now the single source of version truth (`Version`, - `AssemblyVersion`, `FileVersion` only; the duplicate `PackageVersion` was removed). -- README is the repo root `Readme.md`, packed to the package root as `README.md`. -- **SourceLink emits one warning in the web sandbox only** ("Source control information is not - available") because the sandbox clone's `origin` is a local HTTP proxy, not `github.com`. Packing - against a `github.com` remote is fully warning-free, so the config is correct for real CI. - -**Tasks** -1. Delete or regenerate the stale `PasswordGenerator.nuspec` (2.0.5); single source of version truth - in the csproj, bumped to **3.0.0**. -2. Add ``, replace `PackageIconUrl` with `` (clears `NU5048`), add - **SourceLink**, deterministic build, and a `.snupkg` symbol package. -3. Confirm `dotnet pack` is warning-free; artifact still produced by CI (no auto-publish; keep - `deploy: off` until an intentional release). -4. Release notes include **comparative BenchmarkDotNet numbers** (discipline to repeat every release). - -**Verification / exit criteria** -- `dotnet pack -c Release` produces `PasswordGenerator.3.0.0.nupkg` + `.snupkg` with **no NU5048 / no - missing-readme** warnings. -- **Tests stay green:** `dotnet test` returns 0 after the packaging/version changes (a regression - check that retargeting/version bumps broke nothing). -- **Commit & push** this phase, e.g. `build: clean packaging, SourceLink, snupkg, bump to 3.0.0`. - -**Closes:** §7 packaging issues. - ---- - -## Phase 6 — Documentation & migration (Tier 4) - -**Objective:** make the upgrade obvious and the broader use cases discoverable. - -**Decisions taken during implementation:** -- The migration guide drops the "`[Obsolete]` still working" framing: the **entire v2 surface is - intact** (no members were obsoleted), so the only behavioural change to flag is error-string → - exception/`TryNext`. Async/DI/presets are presented as **opt-in additions**. -- Standards mapping and the "beyond passwords" use cases live in - [`migration-v2-to-v3.md`](migration-v2-to-v3.md); the root `Readme.md` links to them. -- A root [`CHANGELOG.md`](../../CHANGELOG.md) captures the v3 changes; `current-state/` docs get a - "historical / resolved in v3" banner rather than being deleted. -- Readme/migration snippets are backed by `DocumentationSnippetTests` so docs can't drift from the API. - -**Tasks** -1. **v2→v3 migration guide:** direct→DI, sync→async (sync kept, not obsoleted), - error-string→exception/`TryNext`, preset/appSettings adoption — before/after snippets. -2. Document the **broader purpose** (OTPs, environment names, API keys, identifiers). -3. **OWASP/NIST mapping** for presets with links. -4. Fix the **stale Readme** length claim (8–128 → 4–256 is itself superseded by v3 docs) and the - ```javascript``` fences; link the root `Readme.md` into this `docs/` section. -5. Update `current-state/` notes to reflect that the documented issues are now resolved (or move them - to a CHANGELOG). - -**Verification / exit criteria** -- Docs build/render; all mermaid diagrams validated. -- **Tests green:** migration-guide snippets are backed by compiling sample/test code and the full - suite still passes (`dotnet test` returns 0) — docs changes must not land on a red tree. -- **Commit & push** this phase, e.g. `docs: v2->v3 migration guide, standards mapping, readme refresh`. - -**Closes:** §6 documentation defects; addendum Tier 4. - ---- - -## Issue → phase traceability - -| Issue / gap | Phase | -|---|---| -| §5.1 error strings → exceptions/`TryNext` | 1 | -| §5.2 off-by-one, §5.3 modulo bias | 1 | -| §5.4 consecutive-char guard | 1 | -| §5.5 Guid shuffle removal | 1 | -| §5.6 static/undisposed RNG | 1 | -| §5.7 dead code | 1 | -| §5.8 empty special set | 1 | -| §5.10 NextGroup/Generate uniqueness | 4 | -| §8 multi-target + nullable | 2 | -| §8 async, DI | 3 | -| §8 presets, appSettings, custom pools, exclude-ambiguous, min-counts, entropy | 4 | -| §7 packaging, CS0108 wrapper removal | 3 (warnings), 5 (package) | -| §6 docs defects | 6 | - -## Per-session checklist - -```bash -# 1. install SDK (Phase 0) -cd /tmp && curl -fsSL https://dot.net/v1/dotnet-install.sh -o dotnet-install.sh \ - && chmod +x dotnet-install.sh && ./dotnet-install.sh --channel 8.0 --install-dir /tmp/dotnet -export PATH="/tmp/dotnet:$PATH"; export DOTNET_CLI_TELEMETRY_OPTOUT=1 -# 2. build + test before and after changes -dotnet build PasswordGenerator.sln -c Release -dotnet test PasswordGenerator.Tests/PasswordGenerator.Tests.csproj -c Release -# 3. pack check (Phase 5) -dotnet pack PasswordGenerator/PasswordGenerator.csproj -c Release -o artifacts -# 4. only once tests are green, commit + push this phase (one commit per phase) -git add -A && git commit -m "" && git push -u origin -``` diff --git a/docs/archive/roadmap.md b/docs/archive/roadmap.md deleted file mode 100644 index 5a95746..0000000 --- a/docs/archive/roadmap.md +++ /dev/null @@ -1,88 +0,0 @@ -# v3 Target — Roadmap - -> **Archived / historical.** This is a v3 planning document, kept for reference. It is superseded by -> the shipped v3 docs in [`../`](../README.md). Note that v3 ultimately **dropped `netstandard2.0`** -> (targets `net8.0;net10.0`), contrary to the multi-target recommendation below. - -Tiered delivery from the adjusted plan in `V3_VERIFICATION.md` §3. Sequencing only — not committed -dates. (Delivered in v3.0.0; see `implementation-plan.md` for where the shipped code diverged.) - -## Tiers as phases - -```mermaid -flowchart TD - T1["Tier 1 — Correctness & Security
exceptions+TryNext · unbiased CSPRNG · delete Guid shuffle
· guarantee classes · fix static RNG · empty-special guard"] - T2["Tier 2 — Modernisation
multi-target+nullable · async (sync kept, not obsoleted) · opt-in DI
· BenchmarkDotNet · packaging hygiene · tests→net8/NUnit4"] - T3["Tier 3 — New Features
WithAllAscii/WithCharacters · presets · appSettings
· Generate batch · exclude-ambiguous · min-counts · entropy"] - T4["Tier 4 — Documentation
v2→v3 migration guide · broader-purpose docs · OWASP/NIST mapping"] - T1 --> T2 --> T3 --> T4 - classDef t1 fill:#ffe6e6,stroke:#cc0000; - classDef t2 fill:#fff5e6,stroke:#cc6600; - classDef t3 fill:#e6f0ff,stroke:#0066cc; - classDef t4 fill:#e6ffe6,stroke:#009900; - class T1 t1; - class T2 t2; - class T3 t3; - class T4 t4; -``` - -## Indicative sequencing - -```mermaid -gantt - title v3 indicative sequencing (relative, not dated) - dateFormat X - axisFormat %s - section Tier 1 Correctness - IRandomSource + unbiased selection :t1a, 0, 3 - Exceptions + TryNext :t1b, 0, 2 - Guarantee classes (seed+shuffle) :t1c, after t1a, 2 - Remove static RNG + dead code :t1d, after t1a, 1 - section Tier 2 Modernisation - Multi-target + nullable :t2a, after t1c, 2 - Async (sync kept, not obsoleted) :t2b, after t2a, 2 - Opt-in DI + appSettings bind :t2c, after t2a, 2 - Tests net8 + NUnit4 + BenchmarkDotNet :t2d, after t1c, 3 - Packaging hygiene :t2e, after t2a, 1 - section Tier 3 Features - Custom pools + WithAllAscii :t3a, after t2c, 2 - Presets :t3b, after t3a, 2 - Generate batch + uniqueness :t3c, after t2b, 2 - Exclude-ambiguous + min-counts + entropy :t3d, after t3a, 3 - section Tier 4 Docs - Migration guide + standards mapping :t4a, after t3b, 2 -``` - -## Dependency rationale - -```mermaid -flowchart LR - RNG["IRandomSource"] --> Classes["guarantee classes"] - RNG --> Multi["multi-target"] - Multi --> Async["async"] - Multi --> DI["DI + appSettings"] - DI --> Presets["presets"] - Async --> Batch["Generate batch"] - Presets --> Docs["migration guide"] -``` - -`IRandomSource` is the keystone: the correctness fixes, multi-targeting, and testability all build on -it, so it lands first. - -## Decision gates (resolve before/within the tier) - -```mermaid -flowchart TD - D1{"Drop [Obsolete] v2 wrappers?"} -->|recommended: yes| G1["clears 5x CS0108; Tier 2"] - D2{"Min target?"} -->|recommended: netstandard2.0 + net8.0| G2["Tier 2"] - D3{"IConfiguration DI overload?"} -->|recommended: yes| G3["Tier 2 DI"] - classDef q fill:#fff5e6,stroke:#cc6600; - class D1,D2,D3 q; -``` - -See `V3_VERIFICATION.md` §4 for the reasoning behind each recommendation. - -## Release-note discipline - -Every v3.x release includes comparative **BenchmarkDotNet** numbers (sync vs async; batch sizes 1 / -100 / 1000 / 10000; allocations) so performance trends are visible across versions. diff --git a/docs/configuration-and-di.md b/docs/configuration-and-di.md index 31ed2fc..cb0119e 100644 --- a/docs/configuration-and-di.md +++ b/docs/configuration-and-di.md @@ -32,6 +32,26 @@ applies. `appSettings` binding is an **opt-in, separate step** — it is never a } ``` +## Example `appSettings.json` (passphrase) + +Setting a `Passphrase` section makes the registered generator produce passphrases instead of +character passwords (the character-pool options above are then ignored). An empty `Separator` +(`""`) means *no separator* — the configuration binder skips empty values, so the DI registration +maps an explicit empty string to `null` for you. + +```jsonc +{ + "PasswordGenerator": { + "Passphrase": { + "WordCount": 6, + "Separator": "", + "Capitalize": true, + "IncludeNumber": true + } + } +} +``` + ## DI registration (opt-in, not auto-registered on install) ```mermaid @@ -75,3 +95,7 @@ builder does*. **Why this is better:** teams can centralise password policy in `appSettings` without forcing it on every call site, the RNG dependency is wired once, and unit tests can swap `IRandomSource` for a deterministic stub. + +--- + +**Docs:** [← Public API Surface](api-surface.md) · [Index](README.md) · Next → [Migrating from v2.x to v3.0](migration-v2-to-v3.md) diff --git a/docs/generation-flow.md b/docs/generation-flow.md index 90d9f96..c3a7021 100644 --- a/docs/generation-flow.md +++ b/docs/generation-flow.md @@ -79,3 +79,7 @@ stateDiagram-v2 **Why this is better:** failure is impossible to ignore (exception or `bool`), output is always a real password, randomness is unbiased and fully covered by deterministic-RNG unit tests, and the slowest part of the old design (validate-and-retry) is gone. + +--- + +**Docs:** [← Architecture](architecture.md) · [Index](README.md) · Next → [Public API Surface](api-surface.md) diff --git a/docs/migration-v2-to-v3.md b/docs/migration-v2-to-v3.md index 809e4b9..dcb874d 100644 --- a/docs/migration-v2-to-v3.md +++ b/docs/migration-v2-to-v3.md @@ -152,3 +152,7 @@ string apiKey = Password.ForApiKey(32).Next(); // URL-safe token string envName = Password.ForEnvironmentName(12).Next(); // readable, no look-alikes string phrase = Password.ForPassphrase(4).Next(); // "maple-river-quartz-bloom-42" ``` + +--- + +**Docs:** [← Configuration & DI](configuration-and-di.md) · [Index](README.md) · Next → [Local NuGet test report](v3-local-nuget-test.md) diff --git a/docs/v3-local-nuget-test.md b/docs/v3-local-nuget-test.md index 9f6b377..d9fe20b 100644 --- a/docs/v3-local-nuget-test.md +++ b/docs/v3-local-nuget-test.md @@ -65,8 +65,8 @@ info : Package 'PasswordGenerator' is compatible with all the specified framewor For the dependency-injection / `appsettings.json` scenarios two more packages were added: ```bash -dotnet add package Microsoft.Extensions.DependencyInjection --version 8.0.0 -dotnet add package Microsoft.Extensions.Configuration.Json --version 8.0.0 +dotnet add package Microsoft.Extensions.DependencyInjection --version 10.0.8 +dotnet add package Microsoft.Extensions.Configuration.Json --version 10.0.8 ``` ### Resulting `PgTestApp.csproj` @@ -85,8 +85,8 @@ dotnet add package Microsoft.Extensions.Configuration.Json --version 8.0.0 - - + + @@ -395,3 +395,7 @@ NuGet feed, and the public API — fluent builder, presets, quality controls, er async, batch generation, and both dependency-injection registration paths — all behave as documented in the Readme and CHANGELOG. The only non-fatal note during the whole run was the empty-SourceLink build warning, which is expected when packing outside CI. + +--- + +**Docs:** [← Migrating from v2.x to v3.0](migration-v2-to-v3.md) · [Index](README.md)