Skip to content

Draft: [Proof of Concept] removing weak passwords from specs

Nick Malcolm requested to merge 360030-rebase-test into master

See Use random passwords in specs (#360030 - closed)

This MR is to get more specs run so I can play whack-a-mole and unblock Blocks weak passwords on sign up or password ch... (!86310 - merged)

We expect tests to fail where they might be setting a weak password, due to a temporary addition to User#password_allowed?. Next step after whacking moles is to remove the below and hopefully we can merge.

  def password_allowed?(password)
    password_allowed = true

    DISALLOWED_PASSWORDS.each do |disallowed_password|
      password_allowed = false if Devise.secure_compare(password, disallowed_password)
    end

    # TODO - REMOVE - this is just to help identify specs which might
    # be setting weak passwords
    if Rails.env.test?
      password_allowed = false if password.nil? || (password.length < Devise.friendly_token.length)
    end

    password_allowed
  end

TODO

  • rspec ./spec/tasks/gitlab/password_rake_spec.rb:27 # gitlab:password rake tasks :reset when all inputs are correct updates the password properly using 'secretpassword' in https://gitlab.com/gitlab-org/gitlab/-/jobs/2737340592
  • rspec ./ee/spec/features/users/login_spec.rb:14 # Login creates a security event for an invalid password login in https://gitlab.com/gitlab-org/gitlab/-/jobs/2737341130
  • bcrypt ones are known "failures" since the random password is short, but they'll pass once we remove the above TODO code
    • decided to just adopt the pattern here too, so we're setting random passwords everywhere.

Replaces Draft: [Proof of Concept] removing weak passwor... (!90104 - closed) which had some weird yaml pipeline error thing.

Edited by Nick Malcolm

Merge request reports