My first lib in Rust

[Sworzen1]

Hello folks,
I'm TypeScript developer and I want to start coding in Rust. I've already had my first course, I tried to create my first lib, if anyone has the time, I invite you to have a look and give me some suggestions on what could be changed / improved. Link below:
Lib to generate and balance passwords - GitHub
SecurePass - Crates

[derspiny]

Welcome to Rust!

I have a few items of feedback, which you can take or leave as you feel appropriate:

Alphabet constants

The UPPERCASE_CHARSET and LOWERCASE_CHARSET would be much easier to inspect and to ensure are correct if they were in lexical order (ABC…XYZ, abc…xyz) order, rather than keyboard order.

&mut String

Use &mut str, where you can; it's more general. However, you cannot change the length of a string through a &mut str reference, since str doesn't require that capability.

For methods that mutate a String in-place, consider rewriting them to return a new String, instead. This kind of parsimony, where you reuse an existing value instead of constructing a new one, is usually a false economy, and it makes it harder for callers to reason about the characteristics of the value. (If you're writing for environments where there is no allocator, obviously you can't allocate new strings, but you're coming from Typescript, which leads me to believe that you're free to allocate if you need to.)

You can annotate the functions with #[must_use] in order to ensure that the developer is warned if they don't use the result.

Free functions vs. methods

The functions generate_password, balance_password, and generate_charset all take a PasswordOptions and operate in the context of that value. It would be natural in Rust to express them as methods of PasswordOptions instead of as free functions, though the two are (mostly) equivalent functionally:

pub struct PasswordOptions {
    // …fields omitted…
}
  
impl PasswordOptions {
    pub fn generate_password(&self) -> String {
       // …body of generate_password, using `self` instead of `options`
    }
  
    pub fn balance_password(&self, password: &mut String) {
        // …
    }
  
    pub fn generate_charset(&self) {
        // …
    }
}

Callers would invoke these via

let password_options = PasswordOptions {
    // …
};
  
let generated = password_options.generate_password();

This may also suggest some naming changes; I might change generate_password to generate, for example, since it's now more obvious from context what it's generating.

Pervasive use of Option in PasswordOptions

I like the idea that callers don't need to specify every last parameter, and that any un-set parameters in your PasswordOptions struct have some kind of sensible default value. However, using Option here complicates the program and requires the caller to specify a bunch of mostly-meaningless Some wrappers.

I would instead be inclined to implement the Default trait:

pub struct PasswordOptions {
    pub length: u32,
    pub include_special_chars: bool,
    pub include_uppercase: bool,
    pub include_numbers: bool,
    pub with_balancing: bool,
    pub phrase: Option<String>,
}

impl Default for PasswordOptions {
    fn default() -> Self {
        Self {
            length: 12,
            include_special_chars: true,
            include_uppercase: true,
            include_numbers: true,
            with_balancing: true,
            phrase: None,
        }
    }
}

Callers can then create a partially-specified struct using constructs like

let password_options = PasswordOptions {
  length: 16,
  ..PasswordOptions::default()
}

and you can eliminate almost all of the the unwrap_or usage in functions like generate_password.

check_password_strength

I would strongly recommend that you use a published algorithm rather than inventing your own metric here. Your callers are unlikely to be able to determine for themselves how trustworthy this estimate is, and an inappropriate reliance on a poorly-chosen password strength estimate may pose a security risk for your callers.

In addition, the docs for this function should call out the specific algorithm used and lay out expectations for how to interpret the score.

[Sworzen1]

Thanks, I'll look at the code again and definitely take some of your advice.

[drmason13]

A good test of any password strength checker:

Password123!

This is a terrible password because it is far too common and easily guessed, but it'll test as Strong according to your checker.

Perhaps the security aspect perhaps wasn't the main focus of your project, since you're learning rust. However, you've published your project to crates.io... and named it securepass. So the security aspect is much harder for me to ignore!

Perhaps an entropy based approach to measuring password strength might be better. Checking off "criteria" without actually considering the content of the password as a whole is problematic. Lazy users and hackers alike will "game" any set of criteria you choose. (E.g. "oh it needs a special character, darn it I'll just throw a ! at the end" "A number too? Ugh fine, 123" and so on...)

[Sworzen1]

Yes I totally agree with you. I'll improve my code. <3

[Sworzen1]

I've made changes, in next update I'll work on calculating password entropy and I'll update check_password_strenght functions with entropy calculation and checking for most frequent passwords.