Skip to content
Snippets Groups Projects
Unverified Commit 2d8035dc authored by Khemissi Amir's avatar Khemissi Amir Committed by GitHub
Browse files

GIT-3100: Reconsidered the password complexity check design (Closes #3100, Closes #3091). (#3101)

parent 920a72b7
No related branches found
No related tags found
No related merge requests found
......@@ -55,8 +55,8 @@ class PasswordResetsController < ApplicationController
elsif @user.update_attributes(user_params)
# Clear the user's social uid if they are switching from a social to a local account
@user.update_attribute(:social_uid, nil) if @user.social_uid.present?
# Mark password as secure and deactivate the reset digest in use.
@user.update(reset_digest: nil, reset_sent_at: nil, secure_password: true)
# Deactivate the reset digest in use disabling the reset link.
@user.update(reset_digest: nil, reset_sent_at: nil)
# Successfully reset password
return redirect_to root_path, flash: { success: I18n.t("password_reset_success") }
end
......
......@@ -98,7 +98,7 @@ class SessionsController < ApplicationController
end
return redirect_to edit_password_reset_path(user.create_reset_digest),
flash: { alert: I18n.t("registration.insecure_password") } unless user.secure_password
flash: { alert: I18n.t("registration.insecure_password") } unless User.secure_password?(session_params[:password])
login(user)
end
......
......@@ -45,9 +45,6 @@ class UsersController < ApplicationController
logger.info "Support: #{@user.email} user has been created."
# Mark password as secure
@user.update(secure_password: true)
# Set user to pending and redirect if Approval Registration is set
if approval_registration
@user.set_role :pending
......@@ -104,9 +101,6 @@ class UsersController < ApplicationController
if @user.update_attributes(user_params)
@user.update_attributes(email_verified: false) if user_params[:email] != @user.email
# Mark password as secure
@user.update(secure_password: true)
user_locale(@user)
if update_roles(params[:user][:role_id])
......
......@@ -21,6 +21,7 @@ require 'bbb_api'
class User < ApplicationRecord
include Deleteable
PASSWORD_PATTERN = /\A(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*(\W|_)).{8,}\z/
after_create :setup_user
before_save { email.try(:downcase!) }
......@@ -45,7 +46,7 @@ class User < ApplicationRecord
format: { with: /\A[\w+\-'.]+@[a-z\d\-.]+\.[a-z]+\z/i }
validates :password, length: { minimum: 8 },
format: /\A(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*-]).{8,}\z/,
format: PASSWORD_PATTERN,
confirmation: true,
if: :validate_password?
......@@ -114,6 +115,10 @@ class User < ApplicationRecord
search_param = "%#{sanitize_sql_like(string)}%"
where(search_query, search: search_param)
end
def secure_password?(pwd)
pwd.match? PASSWORD_PATTERN
end
end
# Returns a list of rooms ordered by last session (with nil rooms last)
......
# frozen_string_literal: true
class RemoveSecurePasswordFromUsers < ActiveRecord::Migration[5.2]
def change
remove_column :users, :secure_password, :boolean, null: false, default: false
end
end
......@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2021_12_15_001240) do
ActiveRecord::Schema.define(version: 2022_02_09_094148) do
create_table "active_storage_attachments", force: :cascade do |t|
t.string "name", null: false
......@@ -144,7 +144,6 @@ ActiveRecord::Schema.define(version: 2021_12_15_001240) do
t.boolean "deleted", default: false, null: false
t.integer "role_id"
t.datetime "last_login"
t.boolean "secure_password", default: false, null: false
t.integer "failed_attempts"
t.datetime "last_failed_attempt"
t.index ["created_at"], name: "index_users_on_created_at"
......
......@@ -288,15 +288,14 @@ describe SessionsController, type: :controller do
expect(@user1.last_login).to_not be_nil
end
it "redirects to reset password page if password is marked as insecure" do
it "redirects to reset password page if the password is insecure" do
allow_any_instance_of(User).to receive(:create_reset_digest).and_return("reset_token")
@user1.update(secure_password: false)
@user1.update_attribute(:password, "example")
expect(@user1.authenticate("example")).to be
post :create, params: {
session: {
email: @user1.email,
password: 'Example1!',
password: 'example',
},
}
......
......@@ -28,7 +28,6 @@ FactoryBot.define do
password_confirmation { password }
accepted_terms { true }
email_verified { true }
secure_password { true }
activated_at { Time.zone.now }
role { set_role(:user) }
end
......
......@@ -22,6 +22,8 @@ require 'bigbluebutton_api'
describe User, type: :model do
before do
@user = create(:user)
@secure_pwd = "#{Faker::Internet.password(min_length: 8, mix_case: true, special_characters: true)}1aB"
@insecure_pwd = Faker::Internet.password(min_length: 8, mix_case: true).to_s
end
context 'validations' do
......@@ -47,10 +49,16 @@ describe User, type: :model do
user = create(:user, email: "DOWNCASE@DOWNCASE.COM")
expect(user.email).to eq("downcase@downcase.com")
end
context 'is greenlight account' do
before { allow(subject).to receive(:greenlight_account?).and_return(true) }
it { should validate_length_of(:password).is_at_least(8) }
it { should validate_confirmation_of(:password) }
it "should validate password complexity" do
@user.update(password: @secure_pwd, password_confirmation: @secure_pwd)
expect(@user).to be_valid
@user.update(password: @insecure_pwd, password_confirmation: @insecure_pwd)
expect(@user).to be_invalid
end
end
context 'is not greenlight account' do
......@@ -268,4 +276,14 @@ describe User, type: :model do
expect(@user.reload.failed_attempts).to eq(0)
end
end
context 'class methods' do
context "#secure_password?" do
it "should return true for secure passwords" do
expect(User.secure_password?(@secure_pwd)).to be
end
it "should return false for insecure passwords" do
expect(User.secure_password?(@insecure_pwd)).not_to be
end
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment