From 7c771b28b5692b39176363257f33df1baeb5d227 Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 9 Oct 2023 09:16:08 +0200 Subject: [PATCH] Add password complexity --- app/assets/javascripts/managers.js | 29 +++++++++--- app/models/user.rb | 8 ++++ .../account/edit_password_manually.html.erb | 5 +- config/initializers/devise-security.rb | 2 +- config/secrets.yml.example | 4 ++ spec/models/user_spec.rb | 33 +++++++++++++ spec/system/users_auth_spec.rb | 47 +++++++++++++++++++ 7 files changed, 120 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/managers.js b/app/assets/javascripts/managers.js index b66914f2a..0308e4ba1 100644 --- a/app/assets/javascripts/managers.js +++ b/app/assets/javascripts/managers.js @@ -2,16 +2,33 @@ "use strict"; App.Managers = { generatePassword: function() { - var chars, possible_chars; - possible_chars = "aAbcdeEfghiJkmnpqrstuUvwxyz23456789"; - chars = Array.apply(null, { - length: 12 + var pass, possible_chars, possible_digits, possible_symbols, password_complexity; + password_complexity = $(".generate-random-value").data("password-complexity"); + possible_chars = "abcdefghijklmnopqrstuvwxyz"; + possible_digits = "123456789"; + possible_symbols = "-_.,;!?"; + + pass = Array.apply(null, { + length: 8 }).map(function() { var i; i = Math.floor(Math.random() * possible_chars.length); return possible_chars.charAt(i); - }); - return chars.join(""); + }).join(""); + + for (var i = 0; i < password_complexity.upper; i++) { + pass += possible_chars.charAt(Math.floor(Math.random() * possible_chars.length)).toUpperCase(); + } + + for (var i = 0; i < password_complexity.digit; i++) { + pass += possible_digits.charAt(Math.floor(Math.random() * possible_digits.length)); + } + + for (var i = 0; i < password_complexity.symbol; i++) { + pass += possible_symbols.charAt(Math.floor(Math.random() * possible_symbols.length)); + } + + return pass; }, togglePassword: function(type) { $("#user_password").prop("type", type); diff --git a/app/models/user.rb b/app/models/user.rb index ae195ec54..38eb7911e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -416,6 +416,14 @@ class User < ApplicationRecord update!(subscriptions_token: SecureRandom.base58(32)) if subscriptions_token.blank? end + def self.password_complexity + if Tenant.current_secrets.dig(:security, :password_complexity) + { digit: 1, lower: 1, symbol: 0, upper: 1 } + else + { digit: 0, lower: 0, symbol: 0, upper: 0 } + end + end + private def clean_document_number diff --git a/app/views/management/account/edit_password_manually.html.erb b/app/views/management/account/edit_password_manually.html.erb index 4f4ea6b4e..b86bafaa8 100644 --- a/app/views/management/account/edit_password_manually.html.erb +++ b/app/views/management/account/edit_password_manually.html.erb @@ -12,7 +12,10 @@ - <%= link_to t("management.account.edit.password.random"), "#", class: "generate-random-value float-right" %> + <%= link_to t("management.account.edit.password.random"), + "#", + class: "generate-random-value float-right", + data: { "password-complexity": User.password_complexity } %> <%= f.submit t("management.account.edit.password.save"), class: "button success" %> diff --git a/config/initializers/devise-security.rb b/config/initializers/devise-security.rb index dde3e34e3..6cc8edc25 100644 --- a/config/initializers/devise-security.rb +++ b/config/initializers/devise-security.rb @@ -8,7 +8,7 @@ Devise.setup do |config| # Need 1 char each of: A-Z, a-z, 0-9, and a punctuation mark or symbol # You may use "digits" in place of "digit" and "symbols" in place of # "symbol" based on your preference - config.password_complexity = { digit: 0, lower: 0, symbol: 0, upper: 0 } + config.password_complexity = { digit: 0, lower: 0, symbol: 0, upper: 0 } # Overwritten in User model # How many passwords to keep in archive # config.password_archiving_count = 5 diff --git a/config/secrets.yml.example b/config/secrets.yml.example index a5aa854c4..c2c736321 100644 --- a/config/secrets.yml.example +++ b/config/secrets.yml.example @@ -21,6 +21,7 @@ development: multitenancy: false security: last_sign_in: false + password_complexity: false secret_key_base: 56792feef405a59b18ea7db57b4777e855103882b926413d4afdfb8c0ea8aa86ea6649da4e729c5f5ae324c0ab9338f789174cf48c544173bc18fdc3b14262e4 <<: *maps @@ -54,6 +55,7 @@ staging: multitenancy: false security: last_sign_in: false + password_complexity: false tenants: # If you've enabled multitenancy, you can overwrite secrets for a # specific tenant with: @@ -92,6 +94,7 @@ preproduction: multitenancy: false security: last_sign_in: false + password_complexity: false tenants: # If you've enabled multitenancy, you can overwrite secrets for a # specific tenant with: @@ -135,6 +138,7 @@ production: multitenancy: false security: last_sign_in: false + password_complexity: false tenants: # If you've enabled multitenancy, you can overwrite secrets for a # specific tenant with: diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d9ce655ad..2ff3a9cb5 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -849,4 +849,37 @@ describe User do expect(user.subscriptions_token).to eq "already_set" end end + + describe ".password_complexity" do + it "returns no complexity when the secrets aren't configured" do + expect(User.password_complexity).to eq({ digit: 0, lower: 0, symbol: 0, upper: 0 }) + end + + context "when secrets are configured" do + before do + allow(Rails.application).to receive(:secrets).and_return(ActiveSupport::OrderedOptions.new.merge( + security: { + password_complexity: true + }, + tenants: { + tolerant: { + security: { + password_complexity: false + } + } + } + )) + end + + it "uses the general secrets for the main tenant" do + expect(User.password_complexity).to eq({ digit: 1, lower: 1, symbol: 0, upper: 1 }) + end + + it "uses the tenant secrets for a tenant" do + allow(Tenant).to receive(:current_schema).and_return("tolerant") + + expect(User.password_complexity).to eq({ digit: 0, lower: 0, symbol: 0, upper: 0 }) + end + end + end end diff --git a/spec/system/users_auth_spec.rb b/spec/system/users_auth_spec.rb index f5934515a..60f3c8877 100644 --- a/spec/system/users_auth_spec.rb +++ b/spec/system/users_auth_spec.rb @@ -755,4 +755,51 @@ describe "Users" do expect(page).to have_content "must be different than the current password." end + + context "Regular authentication with password complexity enabled" do + before do + allow(Rails.application).to receive(:secrets).and_return(ActiveSupport::OrderedOptions.new.merge( + security: { password_complexity: true } + )) + end + + context "Sign up" do + scenario "Success with password" do + message = "You have been sent a message containing a verification link. Please click on this" \ + " link to activate your account." + visit "/" + click_link "Register" + + fill_in "Username", with: "Manuela Carmena" + fill_in "Email", with: "manuela@consul.dev" + fill_in "Password", with: "ValidPassword1234" + fill_in "Confirm password", with: "ValidPassword1234" + check "user_terms_of_service" + + click_button "Register" + + expect(page).to have_content message + + confirm_email + + expect(page).to have_content "Your account has been confirmed." + end + + scenario "Errors on sign up" do + visit "/" + click_link "Register" + + fill_in "Username", with: "Manuela Carmena" + fill_in "Email", with: "manuela@consul.dev" + fill_in "Password", with: "invalid_password" + fill_in "Confirm password", with: "invalid_password" + check "user_terms_of_service" + + click_button "Register" + + expect(page).to have_content "must contain at least one digit, must contain at least one" \ + " upper-case letter" + end + end + end end