3
\$\begingroup\$

This is an update to my Simple Random Password Generator. It has been renamed to SecurePasswordGenerator and uses RandomNumberGenerator instead of using Random. If you would like to view the original source, it can be found here.

using System;
using System.Diagnostics;
using System.Collections;
using System.Collections.Generic;    
using System.Security.Cryptography;

public struct SecurePasswordGenerator(int PasswordLength, char[] LegalChars) : IEnumerable<char>
{
    //was used for test purposes calling from a static method
    //private readonly int PasswordLength => passwordLength;

    public readonly string NextPassword() => new(this.ToArray());

    public static IEnumerable<string> GeneratePasswords(int size, SecurePasswordGenerator generator) =>
        generator.Chunk(size).Select(chunk => new string(chunk));

    readonly IEnumerator<char> IEnumerable<char>.GetEnumerator()
    {            
        bool isInvalidChars = LegalChars == null || LegalChars.Length == 0;
        if (isInvalidChars || PasswordLength <= 0) yield break;
        using RandomNumberGenerator rng = RandomNumberGenerator.Create();
        byte[] buffer = new byte[PasswordLength * 4];
        rng.GetBytes(buffer);
        for (int i = 0; i < PasswordLength; i++)
        {
            int offset = i * 4;
            uint rand = BitConverter.ToUInt32(buffer, offset);
            yield return LegalChars![(int)(rand % LegalChars.Length)];
        }
    }
    
    [DebuggerHidden, DebuggerStepThrough]
    readonly IEnumerator IEnumerable.GetEnumerator()
    {
        throw new NotImplementedException();
    }
}
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

Rather nice! One small improvement I'd make, is change the char[] LegalChars to IEnumerable<char> LegalChars so, for example, a string of valid characters can be passed in. Here's how I've modified the struct:

public struct SecurePasswordGenerator(int PasswordLength, IEnumerable<char>? LegalChars) : IEnumerable<char>
{
    //was used for test purposes calling from a static method
    ////private readonly int PasswordLength => passwordLength;

    private readonly char[] _legalChars = LegalChars?.ToArray() ?? [];

    public readonly string NextPassword() => new(this.ToArray());

    public static IEnumerable<string> GeneratePasswords(int size, SecurePasswordGenerator generator) =>
        generator.Chunk(size).Select(chunk => new string(chunk));

    readonly IEnumerator<char> IEnumerable<char>.GetEnumerator()
    {
        // ReSharper disable once ComplexConditionExpression
        bool isInvalidChars = _legalChars.Length == 0;

        if (isInvalidChars || PasswordLength <= 0)
        {
            yield break;
        }

        using RandomNumberGenerator rng = RandomNumberGenerator.Create();

        byte[] buffer = new byte[sizeof(UInt32) * PasswordLength];

        rng.GetBytes(buffer);
        for (int i = 0; i < PasswordLength; i++)
        {
            int offset = sizeof(UInt32) * i;
            uint rand = BitConverter.ToUInt32(buffer, offset);

            yield return _legalChars[(int)(rand % _legalChars.Length)];
        }
    }

    [DebuggerHidden, DebuggerStepThrough]
    readonly IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable<char>)this).GetEnumerator();
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ That's awesome, switching LegalChars to IEnumerable<char>? is so much cleaner and flexible without the need of converting strings to .ToCharArray(). I appreciate the review, I will be adopting this pattern :) \$\endgroup\$ Commented Oct 17 at 14:57

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.