Skip to content
Permalink
Browse files

Merge pull request #2082 from matiaskorhonen/unpwn

Password validation using Unpwn
  • Loading branch information...
indirect committed Oct 31, 2019
2 parents db88e23 + c2d68e7 commit 8ce4bdac25732f53eec24df41f84edb6b802033d
@@ -44,6 +44,7 @@ gem "sprockets-rails"
gem "rack-attack"
gem "rqrcode"
gem "rotp"
gem "unpwn", "~> 0.3.0"

# Logging
gem "lograge"
@@ -72,6 +72,10 @@ GEM
aws-sigv4 (1.1.0)
aws-eventstream (~> 1.0, >= 1.0.2)
bcrypt (3.1.12)
bitarray (1.2.0)
bloomer (1.0.0)
bitarray
msgpack
bootsnap (1.4.3)
msgpack (~> 1.0)
builder (3.2.3)
@@ -255,7 +259,7 @@ GEM
minitest (5.12.2)
mocha (1.2.1)
metaclass (~> 0.0.1)
msgpack (1.2.10)
msgpack (1.3.1)
multi_json (1.13.1)
multipart-post (2.0.0)
netrc (0.11.0)
@@ -280,6 +284,7 @@ GEM
pry (~> 0.10)
psych (3.1.0)
public_suffix (3.1.0)
pwned (1.2.1)
rack (2.0.7)
rack-attack (6.0.0)
rack (>= 1.0, < 3)
@@ -406,6 +411,9 @@ GEM
unicorn (5.5.0.1.g6836)
kgio (~> 2.6)
raindrops (~> 0.7)
unpwn (0.3.0)
bloomer (~> 1.0)
pwned (~> 1.2)
validates_formatting_of (0.9.0)
activemodel
websocket-driver (0.7.1)
@@ -478,6 +486,7 @@ DEPENDENCIES
toxiproxy (~> 1.0.0)
uglifier (>= 1.0.3)
unicorn (~> 5.5.0.1.g6836)
unpwn (~> 0.3.0)
validates_formatting_of
xml-simple

@@ -43,7 +43,11 @@ class User < ApplicationRecord
}, allow_nil: true

validates :twitter_username, length: { within: 0..20 }, allow_nil: true
validates :password, length: { within: 10..200 }, allow_nil: true, unless: :skip_password_validation?
validates :password,
length: { within: 10..200 },
unpwn: true,
allow_nil: true,
unless: :skip_password_validation?
validate :unconfirmed_email_uniqueness

enum mfa_level: { disabled: 0, ui_only: 1, ui_and_api: 2 }, _prefix: :mfa
@@ -43,6 +43,9 @@ de:
email: E-Mail-Adresse
handle: Benutzername
password: Passwort
errors:
messages:
unpwn:
clearance_mailer:
change_password:
title:
@@ -51,6 +51,9 @@ en:
email: Email address
handle: Username
password: Password
errors:
messages:
unpwn: has previously appeared in a data breach and should not be used
clearance_mailer:
change_password:
title: CHANGE PASSWORD
@@ -51,6 +51,9 @@ es:
email: Dirección de correo
handle: Usuario
password: Contraseña
errors:
messages:
unpwn:
clearance_mailer:
change_password:
title: CAMBIAR CONTRASEÑA
@@ -43,6 +43,9 @@ fr:
email: Adresse e-mail
handle: Pseudonyme
password: Mot de passe
errors:
messages:
unpwn:
clearance_mailer:
change_password:
title:
@@ -43,6 +43,9 @@ ja:
email: Emailアドレス
handle: ユーザー名
password: パスワード
errors:
messages:
unpwn:
clearance_mailer:
change_password:
title:
@@ -43,6 +43,9 @@ nl:
email: E-mailadres
handle: Gebruikersnaam
password: Wachtwoord
errors:
messages:
unpwn:
clearance_mailer:
change_password:
title:
@@ -43,6 +43,9 @@ pt-BR:
email: Email
handle: Usuário
password: Senha
errors:
messages:
unpwn:
clearance_mailer:
change_password:
title:
@@ -43,6 +43,9 @@ zh-CN:
email: Email
handle: 账号
password: 密码
errors:
messages:
unpwn:
clearance_mailer:
change_password:
title:
@@ -43,6 +43,9 @@ zh-TW:
email: Email
handle: 帳號
password: 密碼
errors:
messages:
unpwn:
clearance_mailer:
change_password:
title:
@@ -0,0 +1,17 @@
require "unpwn"

class UnpwnValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
record.errors.add(attribute, :unpwn) unless unpwn.acceptable?(value)
rescue Pwned::TimeoutError # rubocop:disable Lint/HandleExceptions
# Do nothing if the HTTP call timesout, consider the value valid
rescue Pwned::Error # rubocop:disable Lint/HandleExceptions
# Do nothing if the HTTP call fails, consider the value valid
end

private

def unpwn
@unpwn ||= Unpwn.new(min: nil, max: nil, request_options: { read_timeout: 3 })
end
end
@@ -10,7 +10,7 @@
factory :user do
email
handle
password { "password12345" }
password { PasswordHelpers::SECURE_TEST_PASSWORD }
api_key { "secret123" }
email_confirmed { true }
end
@@ -127,7 +127,7 @@ class PasswordsControllerTest < ActionController::TestCase
put :update, params: {
user_id: @user.id,
token: @user.confirmation_token,
password_reset: { password: "password1234" }
password_reset: { password: PasswordHelpers::SECURE_TEST_PASSWORD }
}
end

@@ -145,7 +145,7 @@ class PasswordsControllerTest < ActionController::TestCase
put :update, params: {
user_id: @user.id,
token: @user.confirmation_token,
password_reset: { reset_api_key: "false", password: "password1234" }
password_reset: { reset_api_key: "false", password: PasswordHelpers::SECURE_TEST_PASSWORD }
}
end

@@ -163,7 +163,7 @@ class PasswordsControllerTest < ActionController::TestCase
put :update, params: {
user_id: @user.id,
token: @user.confirmation_token,
password_reset: { reset_api_key: "true", password: "password1234" }
password_reset: { reset_api_key: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD }
}
end

@@ -12,7 +12,7 @@ class UsersControllerTest < ActionController::TestCase
context "on POST to create" do
context "when email and password are given" do
should "create a user" do
post :create, params: { user: { email: "foo@bar.com", password: "secret12345" } }
post :create, params: { user: { email: "foo@bar.com", password: PasswordHelpers::SECURE_TEST_PASSWORD } }
assert User.find_by(email: "foo@bar.com")
end
end
@@ -27,7 +27,7 @@ class UsersControllerTest < ActionController::TestCase

context "when extra parameters given" do
should "create a user if parameters are ok" do
post :create, params: { user: { email: "foo@bar.com", password: "secret12345", handle: "foo" } }
post :create, params: { user: { email: "foo@bar.com", password: PasswordHelpers::SECURE_TEST_PASSWORD, handle: "foo" } }
assert_equal "foo", User.where(email: "foo@bar.com").pluck(:handle).first
end

@@ -39,7 +39,7 @@ class UsersControllerTest < ActionController::TestCase

context "confirmation mail" do
setup do
post :create, params: { user: { email: "foo@bar.com", password: "secretpassword", handle: "foo" } }
post :create, params: { user: { email: "foo@bar.com", password: PasswordHelpers::SECURE_TEST_PASSWORD, handle: "foo" } }
Delayed::Worker.new.work_off
end

@@ -0,0 +1,3 @@
module PasswordHelpers
SECURE_TEST_PASSWORD = "?98,TUESDAY,SHOWN,exactly,56?".freeze
end
@@ -37,15 +37,15 @@ def forgot_password_with(email)
expected_path = "/users/#{@user.id}/password/edit"
assert_equal expected_path, page.current_path, "removes confirmation token from url"

fill_in "Password", with: "secret54321"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Save this password"
assert_equal dashboard_path, page.current_path

click_link "Sign out"

visit sign_in_path
fill_in "Email or Username", with: @user.email
fill_in "Password", with: "secret54321"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Sign in"

assert page.has_content? "Sign out"
@@ -80,10 +80,10 @@ def forgot_password_with(email)

visit password_reset_link

fill_in "Password", with: "secret54321"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Save this password"

assert @user.reload.authenticated? "secret54321"
assert @user.reload.authenticated? PasswordHelpers::SECURE_TEST_PASSWORD
end

test "restting password when mfa is enabled" do
@@ -95,7 +95,7 @@ def forgot_password_with(email)
fill_in "otp", with: ROTP::TOTP.new(@user.mfa_seed).now
click_button "Authenticate"

fill_in "Password", with: "secret3210"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Save this password"

assert page.has_content?("Sign out")
@@ -2,7 +2,7 @@

class ProfileTest < SystemTest
setup do
@user = create(:user, email: "nick@example.com", password: "password12345", handle: "nick1", mail_fails: 1)
@user = create(:user, email: "nick@example.com", password: PasswordHelpers::SECURE_TEST_PASSWORD, handle: "nick1", mail_fails: 1)

page.driver.browser.set_cookie("mfa_feature=true")
end
@@ -37,7 +37,7 @@ def mfa_key

click_link "Edit Profile"
fill_in "Username", with: "nick2"
fill_in "Password", with: "password12345"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Update"

assert page.has_content? "nick2"
@@ -51,7 +51,7 @@ def mfa_key
click_link "Edit Profile"

fill_in "Username", with: "nick2"
fill_in "Password", with: "password12345"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Update"

assert page.has_content? "Username has already been taken"
@@ -63,7 +63,7 @@ def mfa_key
click_link "Edit Profile"

fill_in "Username", with: "nick1" * 10
fill_in "Password", with: "password12345"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Update"

assert page.has_content? "Username is too long (maximum is 40 characters)"
@@ -76,7 +76,7 @@ def mfa_key
click_link "Edit Profile"

fill_in "Email address", with: "nick2@example.com"
fill_in "Password", with: "password12345"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD

click_button "Update"

@@ -102,7 +102,7 @@ def mfa_key
visit profile_path("nick1")
click_link "Edit Profile"

fill_in "Password", with: "password12345"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
check "Hide email in public profile"
click_button "Update"

@@ -116,7 +116,7 @@ def mfa_key

click_link "Edit Profile"
fill_in "Twitter username", with: "nick1"
fill_in "Password", with: "password12345"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Update"

visit profile_path("nick1")
@@ -130,7 +130,7 @@ def mfa_key
click_link "Edit Profile"

click_button "Delete"
fill_in "Password", with: "password12345"
fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
click_button "Confirm"

assert page.has_content? "Your account deletion request has been enqueued."\
@@ -6,7 +6,7 @@ class RackAttackTest < ActionDispatch::IntegrationTest
Rails.cache.clear

@ip_address = "1.2.3.4"
@user = create(:user, email: "nick@example.com", password: "secret12345")
@user = create(:user, email: "nick@example.com", password: PasswordHelpers::SECURE_TEST_PASSWORD)
end

def exceeding_limit
@@ -7,9 +7,9 @@ def retrive_authenticity_token(path)
end

setup do
create(:user, handle: "johndoe", password: "chunkybacon")
create(:user, handle: "johndoe", password: PasswordHelpers::SECURE_TEST_PASSWORD)
@last_session_token = retrive_authenticity_token sign_in_path
post session_path(session: { who: "johndoe", password: "chunkybacon" })
post session_path(session: { who: "johndoe", password: PasswordHelpers::SECURE_TEST_PASSWORD })
ActionController::Base.allow_forgery_protection = true # default is false
end

@@ -19,7 +19,7 @@ def retrive_authenticity_token(path)

test "authenticity_token of guest session should be invalid in authenticated session" do
post session_path(
session: { who: "johndoe", password: "chunkybacon" },
session: { who: "johndoe", password: PasswordHelpers::SECURE_TEST_PASSWORD },
authenticity_token: @last_session_token
)

@@ -31,9 +31,9 @@ def retrive_authenticity_token(path)
@last_session_token = retrive_authenticity_token edit_profile_path
delete sign_out_path(authenticity_token: request.session[:_csrf_token])

create(:user, handle: "bob", password: "lovesunicorns")
create(:user, handle: "bob", password: PasswordHelpers::SECURE_TEST_PASSWORD)
post session_path(
session: { who: "bob", password: "lovesunicorns" },
session: { who: "bob", password: PasswordHelpers::SECURE_TEST_PASSWORD },
authenticity_token: request.session[:_csrf_token]
)

0 comments on commit 8ce4bda

Please sign in to comment.
You can’t perform that action at this time.