Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Short Faker::Internet.password are not including mix case chars. #2512

Closed
raivil opened this issue Jul 15, 2022 · 7 comments · Fixed by #2533
Closed

Short Faker::Internet.password are not including mix case chars. #2512

raivil opened this issue Jul 15, 2022 · 7 comments · Fixed by #2533

Comments

@raivil
Copy link

raivil commented Jul 15, 2022

Describe the bug

Short passwords generated with Faker::Internet.password does not respect the configs related to "mix_case".

To Reproduce

Generate several short passwords with mixed case and special characters. Eventually, a invalid password without mixed cased chars will be generated.
Example:

Faker::Internet.password(min_length: 7, max_length: 7, mix_case: true, special_characters: true)
=> "@!#!$!D"

Expected behavior

Passwords to have mixed case characters.

Additional context

Version:

faker (2.21.0)
@thdaraujo
Copy link
Contributor

thdaraujo commented Jul 25, 2022

adding a bit more context:

so maybe a regression?

adding a test/reproduction scripts that can reproduce this issue would be a nice addition.

@tiff-o
Copy link
Contributor

tiff-o commented Jul 31, 2022

it looks like this occurs when both mix_case & special_case is true as special_case can currently override the characters that were changed by mix_case. have a rough solution that seems to work so would be keen to contribute (once i've done more testing & refactored) with a possible fix soon if timing isn't a major issue :)

@stefannibrasil
Copy link
Contributor

stefannibrasil commented Aug 5, 2022

Hi @tiff-o Could you double check #2308 already solves this issue? Thanks!

@stefannibrasil
Copy link
Contributor

@thdaraujo and I were able to reproduce the error with the following script:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem 'faker', :git => 'https://github.com/faker-ruby/faker.git', :branch => 'master'
  gem 'minitest'
end

require "minitest/autorun"

class BugTest < Minitest::Test
  def test_password_with_special_chars_and_mixed_case
    @tester = Faker::Internet

    32.times do
      password = @tester.password(
        min_length: 7, max_length: 7, mix_case: true, special_characters: true
      )

      puts "does not have an uppercase letter: #{password}" unless /[A-Z]+/.match(password)
      puts "does not have a lowercase letter: #{password}" unless /[a-z]+/.match(password)

      assert /[A-Z]+/.match(password), "must have an uppercase letter"
      assert /[a-z]+/.match(password), "must have a lowercase letter"
      assert /[A-Z]/.match(password).size >= 1, "must have at least one uppercase letter"
      assert /[a-z]/.match(password).size >= 1, "must have at least one lowercase letter"
    end
  end
end

#2308 did not solve this issue. At the moment, the following scenarios are possible:

  • only special characters, i.e. %*@!!^$
  • only one uppercase OR one lowercase letter but not both.

What we want is make sure that we have at least one upper case AND one lower case, and the special characters. Examples:

  • !gX9yE3
  • ^*2DrTe
  • &@@hRtM

@tiff-o
Copy link
Contributor

tiff-o commented Aug 10, 2022

Hi @stefannibrasil - so sorry for the delay. Yes, I was able to reproduce the issue and found #2308 did not resolve. I haven't the chance to revisit this just yet but can get my PR up in the next 24 hours.

@stefannibrasil
Copy link
Contributor

hi @tiff-o no worries! Take your time. I was just trying to reproduce the issue and share what we found. Thanks for working on this 👍

@tiff-o
Copy link
Contributor

tiff-o commented Aug 14, 2022

thanks @stefannibrasil! no problem - am currently working on trying to simplify def password overall following feedback from the PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants