Skip to content

Commit

Permalink
Fix password validator for short domains (#10201)
Browse files Browse the repository at this point in the history
* password validator now ignores parts of email domain that are under 4 characters

* test fixes

* added-releasenotes

* EDITS/Tests-Release_Notes
  • Loading branch information
JoonasAapro committed Feb 21, 2023
1 parent 166a61f commit 41ceb47
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 10 deletions.
16 changes: 16 additions & 0 deletions RELEASE_NOTES.md
Expand Up @@ -93,6 +93,22 @@ And define your own services in the environment variable `DECIDIM_SOCIAL_SHARE_S
With this change you can also define your own services. See [documentation for social share services customization](https://docs.decidim.org/en/customize/social_shares/).
### 4.3. Password validator configuration
Decidim implements several password strength checks that ensure the platforms participants and admins are not using weak passwords. One of these validation rules includes checking the user's password against the domain parts of the website, such as `foo.example.org`. The validation ensures that in this case the user's password does not contain the words `foo` or `example`.
This check turned out to be problematic for short subdomains, such as the one in the presented example. Because of this, a new configuration was added to configure the minimum length of a domain part to match against the user's password. The default configuration for this is four characters meaning any domain part shorter than this limit will not be included in this validation rule.
The default value is 4 characters, to change this value you can change the configuration:
```ruby
# In config/initializers/decidim.org
Decidim.configure do |config|
config.password_similarity_length = 4
end
```
## 5. Changes in APIs
### 5.1. Tailwind CSS instead of Foundation
Expand Down
22 changes: 13 additions & 9 deletions decidim-core/app/validators/password_validator.rb
Expand Up @@ -5,7 +5,7 @@ class PasswordValidator < ActiveModel::EachValidator
MINIMUM_LENGTH = 10
MAX_LENGTH = 256
MIN_UNIQUE_CHARACTERS = 5
IGNORE_SIMILARITY_SHORTER_THAN = 4
IGNORE_SIMILARITY_SHORTER_THAN = Decidim.config.password_similarity_length
ADMIN_MINIMUM_LENGTH = Decidim.config.admin_password_min_length
ADMIN_REPETITION_TIMES = Decidim.config.admin_password_repetition_times
VALIDATION_METHODS = [
Expand Down Expand Up @@ -106,22 +106,26 @@ def nickname_included_in_password?
value.include?(record.nickname)
end

def check_domain_length(domain)
domain.split(".").each do |part|
next if part.length < IGNORE_SIMILARITY_SHORTER_THAN

return true if value.downcase.include?(part.downcase)
end

false
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))
value.include?(name) || (domain && check_domain_length(domain))
end

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

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

return true if value.include?(part)
end
return true if value.include?(organization.host) || check_domain_length(organization.host)

false
end
Expand Down
5 changes: 5 additions & 0 deletions decidim-core/lib/decidim/core.rb
Expand Up @@ -481,6 +481,11 @@ def self.reset_all_column_information
[]
end

# Ignores strings similar to email / domain on password validation if too short
config_accessor :password_similarity_length do
4
end

# Defines if admins are required to have stronger passwords than other users
config_accessor :admin_password_strong do
true
Expand Down
47 changes: 46 additions & 1 deletion decidim-core/spec/validators/password_validator_spec.rb
Expand Up @@ -11,7 +11,7 @@
let(:record) do
double(
name: ::Faker::Name.name,
email: ::Faker::Internet.email,
email:,
nickname: ::Faker::Internet.username(specifier: 10..15),
current_organization: organization,
errors:,
Expand All @@ -22,6 +22,7 @@
)
end
let(:admin_record) { false }
let(:email) { ::Faker::Internet.email }
let(:previous_passwords) { [] }
let(:attribute) { "password" }
let(:options) do
Expand Down Expand Up @@ -140,6 +141,28 @@
end
end

describe "parts of email domain included in password" do
context "when less than 4 character parts" do
let(:email) { "john.doe@1.example.org" }
let(:value) { "Summer1Snoworg" }

it "ignores domain validation" do
expect(validator).to be(true)
expect(record.errors[attribute]).to be_empty
end
end

context "when 4 or more character parts" do
let(:email) { "john.doe@example.org" }
let(:value) { "Example1945" }

it "validates with domain" do
expect(validator).to be(false)
expect(record.errors[attribute]).to eq(["is too similar to your email"])
end
end
end

describe "name included in password" do
let(:value) { "foo#{record.name.delete(" ")}bar" }

Expand Down Expand Up @@ -167,6 +190,28 @@
end
end

describe "parts of organization host included in password" do
let(:organization) { create(:organization, host: "www.decidim.1.lvh.me") }

context "when less than 4 character parts" do
let(:value) { "Summer1Snowlvhme" }

it "ignores domain validation" do
expect(validator).to be(true)
expect(record.errors[attribute]).to be_empty
end
end

context "when 4 or more character parts" do
let(:value) { "Decidim1945" }

it "validates with domain" do
expect(validator).to be(false)
expect(record.errors[attribute]).to eq(["is too similar to this domain name"])
end
end
end

describe "common password" do
let(:value) { "qwerty12345" }

Expand Down
Expand Up @@ -396,6 +396,7 @@
config.follow_http_x_forwarded_host = Rails.application.secrets.decidim[:follow_http_x_forwarded_host].present?
config.maximum_conversation_message_length = Rails.application.secrets.decidim[:maximum_conversation_message_length].to_i
config.password_blacklist = Rails.application.secrets.decidim[:password_blacklist] if Rails.application.secrets.decidim[:password_blacklist].present?
config.password_similarity_length = Rails.application.secrets.decidim[:password_similarity_length] if Rails.application.secrets.decidim[:password_similarity_length].present?
config.allow_open_redirects = Rails.application.secrets.decidim[:allow_open_redirects] if Rails.application.secrets.decidim[:allow_open_redirects].present?
end

Expand Down
Expand Up @@ -39,6 +39,7 @@ decidim_default: &decidim_default
follow_http_x_forwarded_host: <%%= Decidim::Env.new("DECIDIM_FOLLOW_HTTP_X_FORWARDED_HOST").to_boolean_string %>
maximum_conversation_message_length: <%%= Decidim::Env.new("DECIDIM_MAXIMUM_CONVERSATION_MESSAGE_LENGTH", "1000").to_i %>
password_blacklist: <%%= Decidim::Env.new("DECIDIM_PASSWORD_BLACKLIST").to_array(separator: ", ").to_json %>
password_similarity_length: <%%= Decidim::Env.new("DECIDIM_PASSWORD_SIMILARITY_LENGTH", 4).to_i %>
allow_open_redirects: <%%= Decidim::Env.new("DECIDIM_ALLOW_OPEN_REDIRECTS").to_boolean_string %>
social_share_services: <%%= Decidim::Env.new("DECIDIM_SOCIAL_SHARE_SERVICES", "Twitter, Facebook, WhatsApp, Telegram").to_array.to_json %>
service_worker_enabled: <%%= Decidim::Env.new("DECIDIM_SERVICE_WORKER_ENABLED", Rails.env.exclude?("development")).to_boolean_string %>
Expand Down
Expand Up @@ -167,6 +167,7 @@
"DECIDIM_FOLLOW_HTTP_X_FORWARDED_HOST" => "true",
"DECIDIM_MAXIMUM_CONVERSATION_MESSAGE_LENGTH" => "1234",
"DECIDIM_PASSWORD_BLACKLIST" => "i-dont-like-this-password, i-dont,like,this,one,either, password123456",
"DECIDIM_PASSWORD_SIMILARITY_LENGTH" => "4",
"DECIDIM_ALLOW_OPEN_REDIRECTS" => "true",
"DECIDIM_ADMIN_PASSWORD_EXPIRATION_DAYS" => "93",
"DECIDIM_ADMIN_PASSWORD_MIN_LENGTH" => "18",
Expand Down Expand Up @@ -290,6 +291,7 @@
%w(decidim follow_http_x_forwarded_host) => false,
%w(decidim maximum_conversation_message_length) => 1000,
%w(decidim password_blacklist) => [],
%w(decidim password_similarity_length) => 4,
%w(decidim allow_open_redirects) => false,
%w(decidim admin_password expiration_days) => 90,
%w(decidim admin_password min_length) => 15,
Expand Down Expand Up @@ -394,6 +396,7 @@
%w(decidim follow_http_x_forwarded_host) => true,
%w(decidim maximum_conversation_message_length) => 1234,
%w(decidim password_blacklist) => ["i-dont-like-this-password", "i-dont,like,this,one,either", "password123456"],
%w(decidim password_similarity_length) => 4,
%w(decidim allow_open_redirects) => true,
%w(decidim admin_password expiration_days) => 93,
%w(decidim admin_password min_length) => 18,
Expand Down Expand Up @@ -488,6 +491,7 @@
"follow_http_x_forwarded_host" => false,
"maximum_conversation_message_length" => 1000,
"password_blacklist" => [],
"password_similarity_length" => 4,
"allow_open_redirects" => false,
"etherpad" => nil,
"maps" => nil
Expand Down Expand Up @@ -524,6 +528,7 @@
"follow_http_x_forwarded_host" => true,
"maximum_conversation_message_length" => 1234,
"password_blacklist" => ["i-dont-like-this-password", "i-dont,like,this,one,either", "password123456"],
"password_similarity_length" => 4,
"allow_open_redirects" => true,
"etherpad" => {
"server" => "http://a-etherpad-server.com",
Expand Down
5 changes: 5 additions & 0 deletions docs/modules/configure/pages/environment_variables.adoc
Expand Up @@ -639,6 +639,11 @@ Separate each item of the array with a comma AND a space, for instance:
|
|No

|DECIDIM_PASSWORD_SIMILARITY_LENGTH
|Amount of characters required by parts of a domain (site or email / separated by a dot) for them to be taken into account during a password validation
|4
|No

|DECIDIM_SOCIAL_SHARE_SERVICES
|The social networking services that will be available when using the "Share" feature on some resources.

Expand Down

0 comments on commit 41ceb47

Please sign in to comment.