Skip to content

Commit

Permalink
Update password strength check (decidim#8455)
Browse files Browse the repository at this point in the history
  • Loading branch information
lahdeero authored Dec 20, 2021
1 parent 77d9423 commit a663377
Show file tree
Hide file tree
Showing 17 changed files with 128,812 additions and 15 deletions.
2 changes: 0 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ PATH
loofah (~> 2.3.1)
mini_magick (~> 4.9)
mustache (~> 1.1.0)
nobspw (~> 0.6.0)
omniauth (~> 2.0)
omniauth-facebook (~> 5.0)
omniauth-google-oauth2 (~> 1.0)
Expand Down Expand Up @@ -550,7 +549,6 @@ GEM
mustache (1.1.1)
netrc (0.11.0)
nio4r (2.5.8)
nobspw (0.6.2)
nokogiri (1.12.5)
mini_portile2 (~> 2.6.1)
racc (~> 1.4)
Expand Down
1 change: 1 addition & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ task :check_locale_completeness do
end

load "decidim-dev/lib/tasks/generators.rake"
load "lib/tasks/common_passwords_tasks.rake"

desc "Generates a dummy app for testing"
task test_app: "decidim:generate_external_test_app"
Expand Down
123 changes: 123 additions & 0 deletions decidim-core/app/validators/password_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
# frozen_string_literal: true

# Class is used to verify that the user's password is strong enough
class PasswordValidator < ActiveModel::EachValidator
MINIMUM_LENGTH = 10
MAX_LENGTH = 256
MIN_UNIQUE_CHARACTERS = 5
IGNORE_SIMILARITY_SHORTER_THAN = 4
VALIDATION_METHODS = [
:password_too_short?,
:password_too_long?,
:not_enough_unique_characters?,
:name_included_in_password?,
:nickname_included_in_password?,
:email_included_in_password?,
:domain_included_in_password?,
:password_too_common?,
:blacklisted?
].freeze

# Check if user's password is strong enough
#
# record - Instance of a form (e.g. Decidim::RegistrationForm) or model
# attribute - "password"
# value - Actual password
# Returns true if password is strong enough
def validate_each(record, attribute, value)
return false if value.blank?

@record = record
@attribute = attribute
@value = value
@weak_password_reasons = []

return true if strong?

@weak_password_reasons.each do |reason|
record.errors[attribute] << get_message(reason)
end

false
end

private

attr_reader :record, :attribute, :value

def get_message(reason)
I18n.t "password_validator.#{reason}"
end

def strong?
VALIDATION_METHODS.each do |method|
@weak_password_reasons << method.to_s.sub(/\?$/, "").to_sym if send(method.to_s)
end

@weak_password_reasons.empty?
end

def password_too_short?
value.length < MINIMUM_LENGTH
end

def password_too_long?
value.length > MAX_LENGTH
end

def not_enough_unique_characters?
value.chars.uniq.length < MIN_UNIQUE_CHARACTERS
end

def name_included_in_password?
return false if !record.respond_to?(:name) || record.name.blank?
return true if value.include?(record.name.delete(" "))

record.name.split(" ").each do |part|
next if part.length < IGNORE_SIMILARITY_SHORTER_THAN

return true if value.include?(part)
end

false
end

def nickname_included_in_password?
return false if !record.respond_to?(:nickname) || record.nickname.blank?

value.include?(record.nickname)
end

def email_included_in_password?
return false if !record.respond_to?(:email) || record.email.blank?

name, domain, _whatever = record.email.split("@")
value.include?(name) || (domain && value.include?(domain.split(".").first))
end

def domain_included_in_password?
return false unless record&.current_organization&.host
return true if value.include?(record.current_organization.host)

record.current_organization.host.split(".").each do |part|
next if part.length < IGNORE_SIMILARITY_SHORTER_THAN

return true if value.include?(part)
end

false
end

def blacklisted?
Array(Decidim.password_blacklist).each do |expression|
return true if expression.is_a?(Regexp) && value.match?(expression)
return true if expression.to_s == value
end

false
end

def password_too_common?
Decidim::CommonPasswords.instance.passwords.include?(value)
end
end
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<%= form.password_field :password, value: form.object.password, autocomplete: "off", help_text: t("devise.passwords.edit.password_help", minimun_characters: NOBSPW.configuration.min_password_length) %>
<%= form.password_field :password, value: form.object.password, autocomplete: "off", help_text: t("devise.passwords.edit.password_help", minimun_characters: ::PasswordValidator::MINIMUM_LENGTH) %>
<%= form.password_field :password_confirmation, value: form.object.password_confirmation, autocomplete: "off" %>
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<%= f.hidden_field :reset_password_token %>

<div class="field">
<%= f.password_field :password, autocomplete: "off", help_text: t("devise.passwords.edit.password_help", minimun_characters: NOBSPW.configuration.min_password_length) %>
<%= f.password_field :password, autocomplete: "off", help_text: t("devise.passwords.edit.password_help", minimun_characters: ::PasswordValidator::MINIMUM_LENGTH) %>
</div>

<div class="field">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
</div>

<div class="field">
<%= f.password_field :password, help_text: t(".password_help", minimun_characters: NOBSPW.configuration.min_password_length), autocomplete: "off" %>
<%= f.password_field :password, help_text: t(".password_help", minimun_characters: ::PasswordValidator::MINIMUM_LENGTH), autocomplete: "off" %>
</div>

<div class="field">
Expand Down
2 changes: 2 additions & 0 deletions decidim-core/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1669,10 +1669,12 @@ en:
name: English
name_with_error: English (error!)
password_validator:
blacklisted: is blacklisted
domain_included_in_password: is too similar to this domain name
email_included_in_password: is too similar to your email
fallback: is not valid
name_included_in_password: is too similar to your name
nickname_included_in_password: is too similar to your nickname
not_enough_unique_characters: does not have enough unique characters
password_not_allowed: is not allowed
password_too_common: is too common
Expand Down
1 change: 0 additions & 1 deletion decidim-core/decidim-core.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ Gem::Specification.new do |s|
s.add_dependency "loofah", "~> 2.3.1"
s.add_dependency "mini_magick", "~> 4.9"
s.add_dependency "mustache", "~> 1.1.0"
s.add_dependency "nobspw", "~> 0.6.0"
s.add_dependency "omniauth", "~> 2.0"
s.add_dependency "omniauth-facebook", "~> 5.0"
s.add_dependency "omniauth-google-oauth2", "~> 1.0"
Expand Down
56 changes: 56 additions & 0 deletions decidim-core/lib/decidim/common_passwords.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# frozen_string_literal: true

module Decidim
class CommonPasswords
include Singleton

attr_reader :passwords

URLS = %w(
https://raw.githubusercontent.com/danielmiessler/SecLists/master/Passwords/xato-net-10-million-passwords-1000000.txt
https://raw.githubusercontent.com/danielmiessler/SecLists/master/Passwords/darkweb2017-top10000.txt
https://raw.githubusercontent.com/danielmiessler/SecLists/master/Passwords/Common-Credentials/10-million-password-list-top-1000000.txt
).freeze

def initialize
raise FileNotFoundError unless File.exist?(self.class.common_passwords_path)

File.open(self.class.common_passwords_path, "r") do |file|
@passwords = file.read.split
end
end

def self.update_passwords!
File.open(common_passwords_path, "w") do |file|
common_password_list.each { |item| file.puts(item) }
end
end

def self.common_password_list
@common_password_list ||= begin
list = []
URLS.each do |url|
URI.open(url) do |data|
data.read.split.each do |line|
list << line if line.length >= min_length
end
end
end

list.uniq
end
end

def self.min_length
return ::PasswordValidator::MINIMUM_LENGTH if defined?(::PasswordValidator)

10
end

def self.common_passwords_path
File.join(__dir__, "db", "common-passwords.txt")
end

class FileNotFoundError < StandardError; end
end
end
6 changes: 6 additions & 0 deletions decidim-core/lib/decidim/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ module Decidim
autoload :RecordEncryptor, "decidim/record_encryptor"
autoload :AttachmentAttributes, "decidim/attachment_attributes"
autoload :CarrierWaveMigratorService, "decidim/carrier_wave_migrator_service"
autoload :CommonPasswords, "decidim/common_passwords"

include ActiveSupport::Configurable
# Loads seeds from all engines.
Expand Down Expand Up @@ -384,6 +385,11 @@ def self.seed!
"decidim-cc"
end

# Blacklisted passwords. Array may contain strings and regex entries.
config_accessor :password_blacklist do
[]
end

# This is an internal key that allow us to properly configure the caching key separator. This is useful for redis cache store
# as it creates some namespaces within the cached data.
# use `config.cache_key_separator = ":"` in your initializer to have namespaced data
Expand Down
5 changes: 0 additions & 5 deletions decidim-core/lib/decidim/core/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
require "kaminari"
require "doorkeeper"
require "doorkeeper-i18n"
require "nobspw"
require "batch-loader"
require "etherpad-lite"
require "diffy"
Expand Down Expand Up @@ -539,10 +538,6 @@ class Engine < ::Rails::Engine
end
end

initializer "nbspw" do
NOBSPW.configuration.use_ruby_grep = true
end

initializer "decidim.premailer" do
Premailer::Adapter.use = :decidim
end
Expand Down
Loading

0 comments on commit a663377

Please sign in to comment.