Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/controllers/sign_up/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,12 @@ def permitted_params

def process_successful_password_creation
@user.confirm
password = permitted_params[:password]
UpdateUser.new(
user: @user,
attributes: { reset_requested_at: nil, password: permitted_params[:password] }
attributes: { reset_requested_at: nil, password: password }
).call
PasswordMetricsIncrementer.new(password).increment_password_metrics
sign_in_and_redirect_user
end

Expand Down
5 changes: 5 additions & 0 deletions app/controllers/users/reset_passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,15 @@ def update_user
attributes = { password: user_params[:password] }
attributes[:confirmed_at] = Time.zone.now unless resource.confirmed?
UpdateUser.new(user: resource, attributes: attributes).call
increment_password_metrics

mark_profile_inactive
end

def increment_password_metrics
PasswordMetricsIncrementer.new(user_params[:password]).increment_password_metrics
end

def mark_profile_inactive
resource.active_profile&.deactivate(:password_reset)
end
Expand Down
5 changes: 5 additions & 0 deletions app/forms/update_user_password_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def process_valid_submission
update_user_password
email_user_about_password_change
encrypt_user_profile_if_active
increment_password_metrics
end

def update_user_password
Expand All @@ -42,6 +43,10 @@ def encrypt_user_profile_if_active
encryptor.call
end

def increment_password_metrics
PasswordMetricsIncrementer.new(password).increment_password_metrics
end

def encryptor
@_encryptor ||= ActiveProfileEncryptor.new(user, user_session, password)
end
Expand Down
28 changes: 28 additions & 0 deletions app/models/password_metric.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
class PasswordMetric < ApplicationRecord
enum metric: %i[length guesses_log10]

def self.increment(metric, value)
create_row_for_metric_category(metric, value)
query = <<-SQL
UPDATE password_metrics
SET count = count + 1
WHERE metric = ? AND value = ?
SQL
sanitized_query = sanitize_sql_array([query, metrics[metric.to_s], value])
connection.execute(sanitized_query)
end

private_class_method def self.create_row_for_metric_category(metric, value)
metric_key = metrics[metric.to_s]
# Insert a row with the count equal to 0 if a row does not already exist
query = <<-SQL
INSERT INTO password_metrics (metric, value, count)
SELECT ?, ?, 0
WHERE NOT EXISTS (
SELECT id FROM password_metrics WHERE metric = ? AND value = ?
)
SQL
sanitized_query = sanitize_sql_array([query, metric_key, value, metric_key, value])
connection.execute(sanitized_query)
end
end
19 changes: 19 additions & 0 deletions app/services/password_metrics_incrementer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
class PasswordMetricsIncrementer
def initialize(password)
@password = password
end

def increment_password_metrics
PasswordMetric.increment(:length, password.length)
PasswordMetric.increment(:guesses_log10, guesses_log10)
end

private

attr_reader :password

# Disable :reek:UncommunicativeMethodName b/c this method name ends with a number
def guesses_log10
Zxcvbn::Tester.new.test(password).guesses_log10.round(1)
end
end
12 changes: 12 additions & 0 deletions db/migrate/20180619145839_create_password_metrics.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class CreatePasswordMetrics < ActiveRecord::Migration[5.1]
def change
create_table :password_metrics do |t|
t.integer :metric, null: false
t.float :value, null: false
t.integer :count, null: false
t.index :metric
t.index :value
t.index [:metric, :value], unique: true
end
end
end
11 changes: 10 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20180607144007) do
ActiveRecord::Schema.define(version: 20180619145839) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -80,6 +80,15 @@
t.index ["updated_at"], name: "index_otp_requests_trackers_on_updated_at"
end

create_table "password_metrics", force: :cascade do |t|
t.integer "metric", null: false
t.float "value", null: false
t.integer "count", null: false
t.index ["metric", "value"], name: "index_password_metrics_on_metric_and_value", unique: true
t.index ["metric"], name: "index_password_metrics_on_metric"
t.index ["value"], name: "index_password_metrics_on_value"
end

create_table "profiles", force: :cascade do |t|
t.integer "user_id", null: false
t.boolean "active", default: false, null: false
Expand Down
10 changes: 10 additions & 0 deletions spec/controllers/sign_up/passwords_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@

post :create, params: { password_form: { password: 'NewVal' }, confirmation_token: token }
end

it 'saves password metrics' do
token = 'new token'
create(:user, confirmation_token: token, confirmation_sent_at: Time.zone.now)

post :create, params: { password_form: { password: 'saltypickles' }, confirmation_token: token }

expect(PasswordMetric.where(metric: 'length', value: 12, count: 1).count).to eq(1)
expect(PasswordMetric.where(metric: 'guesses_log10', value: 7.1, count: 1).count).to eq(1)
end
end

describe '#new' do
Expand Down
23 changes: 23 additions & 0 deletions spec/controllers/users/reset_passwords_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,29 @@
expect(response).to redirect_to new_user_session_path
end
end

it 'increments password metrics' do
raw_reset_token, db_confirmation_token =
Devise.token_generator.generate(User, :reset_password_token)

user = create(
:user,
:unconfirmed,
reset_password_token: db_confirmation_token,
reset_password_sent_at: Time.zone.now
)

stub_email_notifier(user)

password = 'saltypickles'
params = { password: password }

get :edit, params: { reset_password_token: raw_reset_token }
put :update, params: { reset_password_form: params }

expect(PasswordMetric.where(metric: 'length', value: 12, count: 1).count).to eq(1)
expect(PasswordMetric.where(metric: 'guesses_log10', value: 7.1, count: 1).count).to eq(1)
end
end

describe '#create' do
Expand Down
10 changes: 10 additions & 0 deletions spec/forms/update_user_password_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@

expect(email_notifier).to have_received(:send_password_changed_email)
end

it 'increments password metrics for the password' do
params[:password] = 'saltypickles'
stub_email_delivery

subject.submit(params)

expect(PasswordMetric.where(metric: 'length', value: 12, count: 1).count).to eq(1)
expect(PasswordMetric.where(metric: 'guesses_log10', value: 7.1, count: 1).count).to eq(1)
end
end

context 'when the user has an active profile' do
Expand Down
48 changes: 48 additions & 0 deletions spec/models/password_metric_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
require 'rails_helper'

describe PasswordMetric do
describe '.increment' do
context 'when a metric with the given metric and value does not exists' do
it 'creates the metric with a count of 1' do
PasswordMetric.increment(:length, 10)

expect(PasswordMetric.count).to eq(1)
expect(PasswordMetric.find_by(metric: :length, value: 10).count).to eq(1)
end
end

context 'when a metric with the same value exists' do
before do
PasswordMetric.create(
metric: 'length',
value: 9.0,
count: 2
)
end

it 'creates the metric with a value of 1' do
PasswordMetric.increment(:length, 10)

expect(PasswordMetric.count).to eq(2)
expect(PasswordMetric.find_by(metric: :length, value: 10).count).to eq(1)
end
end

context 'when a metric with the given category and value does exist' do
before do
PasswordMetric.create(
metric: 'length',
value: 10.0,
count: 1
)
end

it 'increments the value' do
PasswordMetric.increment(:length, 10)

expect(PasswordMetric.count).to eq(1)
expect(PasswordMetric.find_by(metric: :length, value: 10).count).to eq(2)
end
end
end
end
17 changes: 17 additions & 0 deletions spec/services/password_metrics_incrementer_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
require 'rails_helper'

describe PasswordMetricsIncrementer do
let(:password) { 'saltypickles' }
let(:guesses_log10) { 7.1 }

subject { described_class.new(password) }

describe '#increment_password_metrics' do
it 'increments password metrics for the length and guesses' do
subject.increment_password_metrics

expect(PasswordMetric.where(metric: 'length', value: 12, count: 1).count).to eq(1)
expect(PasswordMetric.where(metric: 'guesses_log10', value: 7.1, count: 1).count).to eq(1)
end
end
end