Merge pull request #432 from drusepth/implement-bcrypt

Implement ActiveModel SecurePassword functionality
This commit is contained in:
Andrew Brown 2015-04-20 19:50:35 -05:00
commit df981fdc77
15 changed files with 359 additions and 196 deletions

View File

@ -2,12 +2,13 @@
source 'https://rubygems.org'
gem 'bcrypt-ruby', '~> 3.0.1'
gem 'rails', '4.1.0'
gem 'jquery-rails'
gem 'sass-rails', '~> 4.0.3', require: 'sass'
gem 'coffee-rails', '~> 4.0.0'
gem 'paperclip', '~> 4.2.0'
gem 'rmagick'
gem 'rmagick', '2.13.4'
gem 'aws-sdk', '~> 1.50'
group :production do

View File

@ -31,11 +31,13 @@ GEM
ast (2.0.0)
astrolabe (1.3.0)
parser (>= 2.2.0.pre.3, < 3.0)
aws-sdk (1.63.0)
aws-sdk-v1 (= 1.63.0)
aws-sdk-v1 (1.63.0)
aws-sdk (1.64.0)
aws-sdk-v1 (= 1.64.0)
aws-sdk-v1 (1.64.0)
json (~> 1.4)
nokogiri (>= 1.4.4)
bcrypt-ruby (3.0.1)
bcrypt-ruby (3.0.1-x86-mingw32)
bootplus-rails (1.0.0)
builder (3.2.2)
capybara (2.4.4)
@ -44,7 +46,7 @@ GEM
rack (>= 1.0.0)
rack-test (>= 0.5.4)
xpath (~> 2.0)
childprocess (0.5.5)
childprocess (0.5.6)
ffi (~> 1.0, >= 1.0.11)
climate_control (0.0.3)
activesupport (>= 3.0)
@ -53,23 +55,27 @@ GEM
coffee-rails (4.0.1)
coffee-script (>= 2.2.0)
railties (>= 4.0.0, < 5.0)
coffee-script (2.3.0)
coffee-script (2.4.1)
coffee-script-source
execjs
coffee-script-source (1.9.1)
coffee-script-source (1.9.1.1)
commonjs (0.2.7)
coveralls (0.7.11)
multi_json (~> 1.10)
rest-client (>= 1.6.8, < 2)
simplecov (~> 0.9.1)
term-ansicolor (~> 1.3)
thor (~> 0.19.1)
coveralls (0.7.1)
multi_json (~> 1.3)
rest-client
simplecov (>= 0.7)
term-ansicolor
thor
docile (1.1.5)
domain_name (0.5.24)
unf (>= 0.0.5, < 1.0.0)
erubis (2.7.0)
execjs (2.4.0)
execjs (2.5.2)
ffi (1.9.8)
ffi (1.9.8-x86-mingw32)
hike (1.2.3)
http-cookie (1.0.2)
domain_name (~> 0.5)
i18n (0.7.0)
jquery-rails (3.1.2)
railties (>= 3.0, < 5.0)
@ -88,7 +94,7 @@ GEM
treetop (~> 1.4.8)
mime-types (1.25.1)
mini_portile (0.6.2)
minitest (5.5.1)
minitest (5.6.0)
multi_json (1.11.0)
netrc (0.10.3)
nokogiri (1.6.6.2)
@ -100,7 +106,7 @@ GEM
activesupport (>= 3.0.0)
cocaine (~> 0.5.3)
mime-types
parser (2.2.0.3)
parser (2.2.2.1)
ast (>= 1.1, < 3.0)
pg (0.18.1)
pg (0.18.1-x86-mingw32)
@ -127,21 +133,23 @@ GEM
rainbow (2.0.0)
rake (10.4.2)
ref (1.0.5)
rest-client (1.7.3)
rest-client (1.8.0)
http-cookie (>= 1.0.2, < 2.0)
mime-types (>= 1.16, < 3.0)
netrc (~> 0.7)
rest-client (1.7.3-x86-mingw32)
rest-client (1.8.0-x86-mingw32)
ffi (~> 1.9)
http-cookie (>= 1.0.2, < 2.0)
mime-types (>= 1.16, < 3.0)
netrc (~> 0.7)
rmagick (2.13.4)
rubocop (0.29.1)
rubocop (0.30.0)
astrolabe (~> 1.3)
parser (>= 2.2.0.1, < 3.0)
powerpack (~> 0.1)
rainbow (>= 1.99.1, < 3.0)
ruby-progressbar (~> 1.4)
ruby-progressbar (1.7.1)
ruby-progressbar (1.7.5)
rubyzip (1.1.7)
sass (3.2.19)
sass-rails (4.0.5)
@ -154,11 +162,11 @@ GEM
multi_json (~> 1.0)
rubyzip (~> 1.0)
websocket (~> 1.0)
simplecov (0.9.2)
simplecov (0.10.0)
docile (~> 1.1.0)
multi_json (~> 1.0)
simplecov-html (~> 0.9.0)
simplecov-html (0.9.0)
json (~> 1.8)
simplecov-html (~> 0.10.0)
simplecov-html (0.10.0)
sprockets (2.12.3)
hike (~> 1.2)
multi_json (~> 1.0)
@ -172,7 +180,7 @@ GEM
sqlite3 (1.3.10-x86-mingw32)
term-ansicolor (1.3.0)
tins (~> 1.0)
therubyracer (0.12.1)
therubyracer (0.12.2)
libv8 (~> 3.16.14.0)
ref
thor (0.19.1)
@ -184,12 +192,16 @@ GEM
polyglot (>= 0.3.1)
tzinfo (1.2.2)
thread_safe (~> 0.1)
tzinfo-data (1.2015.2)
tzinfo-data (1.2015.3)
tzinfo (>= 1.0.0)
uglifier (2.7.1)
execjs (>= 0.3.0)
json (>= 1.8.0)
websocket (1.2.1)
unf (0.1.4)
unf_ext
unf_ext (0.0.7.1)
unf_ext (0.0.7.1-x86-mingw32)
websocket (1.2.2)
xpath (2.0.0)
nokogiri (~> 1.3)
@ -199,6 +211,7 @@ PLATFORMS
DEPENDENCIES
aws-sdk (~> 1.50)
bcrypt-ruby (~> 3.0.1)
bootplus-rails
capybara
coffee-rails (~> 4.0.0)
@ -209,7 +222,7 @@ DEPENDENCIES
paperclip (~> 4.2.0)
pg
rails (= 4.1.0)
rmagick
rmagick (= 2.13.4)
rubocop
sass-rails (~> 4.0.3)
selenium-webdriver

View File

@ -46,15 +46,26 @@ class ApplicationController < ActionController::Base
def create_anonymous_account_if_not_logged_in
return if logged_in?
@user = create_anonymous_user
if @user.save
session[:user] = @user.id
session[:anon_user] = true
else
# layman's collision detection
create_anonymous_account_if_not_logged_in
# layman's collision detection
10.times do
@user = create_anonymous_user
break if @user.save
end
return if @user.nil?
session[:user] = @user.id
session[:anon_user] = true
end
def create_anonymous_user
# TODO: guarantee anonymous id is random (or just let db assign it?)
id = rand(10_000_000).to_s + rand(10_000_000).to_s # lol
User.new(
name: 'Anonymous-' + id.to_s,
email: id.to_s + '@localhost',
password: id.to_s)
end
# Require ownership
@ -112,16 +123,6 @@ class ApplicationController < ActionController::Base
private
def create_anonymous_user
# TODO: guarantee anonymous id is random (or just let db assign it?)
id = rand(10_000_000).to_s + rand(10_000_000).to_s # lol
User.new(
name: 'Anonymous-' + id.to_s,
email: id.to_s + '@localhost',
password: id.to_s)
end
def redirect_if_not_owned(object_to_check, redirect_path)
return if owned_by_current_user? object_to_check
redirect_to redirect_path, notice: t(:no_do_permission)

View File

@ -50,9 +50,19 @@ class SessionsController < ApplicationController
def user_from_params
login = Session.new(session_params)
hash = SessionsController.create_password_digest(login.username,
login.password)
User.where(name: login.username, password: hash).first
user = User.find_by(name: login.username)
migrate_to_bcrypt(user, login.password) if user.old_password.present?
user.try(:authenticate, login.password)
end
def migrate_to_bcrypt(user, password)
hash = SessionsController.create_password_digest user.name, password
return unless user.old_password == hash
user.old_password = nil
user.password = password
user.save
end
def build_session_for(user)

View File

@ -75,6 +75,7 @@ class UsersController < ApplicationController
private
def user_params
params.require(:user).permit(:name, :password, :email)
params.require(:user).permit(:name, :email,
:password, :password_confirmation)
end
end

View File

@ -10,7 +10,7 @@ class Location < ActiveRecord::Base
include NilsBlankUniverse
has_attached_file :map, styles: { original: '1920x1080>', thumb: '200x200>' }
validates_attachment_content_type :map, content_type: /\Aimage\/.*\Z/
validates_attachment_content_type :map, content_type: %r{\Aimage\/.*\Z}
validates :name, presence: true

View File

@ -2,10 +2,10 @@
# a person using the Indent web application. Owns all other content.
class User < ActiveRecord::Base
validates :name, presence: true
validates :password, presence: true
validates :email, presence: true
before_save :hash_password
has_secure_password
before_save :hash_old_password
has_many :characters
has_many :equipment
@ -14,10 +14,13 @@ class User < ActiveRecord::Base
has_many :magics
has_many :universes
def hash_password
def hash_old_password
require 'digest'
self.password = Digest::MD5.hexdigest(
name + "'s password IS... " + password + ' (lol!)')
return if old_password.blank?
self.old_password = Digest::MD5.hexdigest(
name + "'s password IS... " + old_password + ' (lol!)')
end
def content

View File

@ -7,6 +7,7 @@ require 'rails/all'
Bundler.require(*Rails.groups)
module PlanCharacters
# Default application environment configurations
class Application < Rails::Application
# Settings in config/environments/* take precedence over those
# specified here.
@ -25,5 +26,9 @@ module PlanCharacters
# config.i18n.load_path += Dir[Rails.root.join(
# 'my', 'locales', '*.{rb,yml}').to_s]
# config.i18n.default_locale = :de
# Filter sensitive parameters out of logs
config.filter_parameters << :password
config.filter_parameters << :password_confirmation
end
end

View File

@ -0,0 +1,7 @@
class AddPasswordDigestToUsers < ActiveRecord::Migration
def change
rename_column :users, :password, :old_password
change_column_null :users, :old_password, true
add_column :users, :password_digest, :string
end
end

View File

@ -11,157 +11,160 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20_140_713_043_535) do
create_table 'characters', force: true do |t|
t.string 'name', null: false
t.string 'role'
t.string 'gender'
t.string 'age'
t.string 'height'
t.string 'weight'
t.string 'haircolor'
t.string 'hairstyle'
t.string 'facialhair'
t.string 'eyecolor'
t.string 'race'
t.string 'skintone'
t.string 'bodytype'
t.string 'identmarks'
t.text 'bestfriend'
t.text 'religion'
t.text 'politics'
t.text 'prejudices'
t.text 'occupation'
t.text 'pets'
t.text 'mannerisms'
t.text 'birthday'
t.text 'birthplace'
t.text 'education'
t.text 'background'
t.string 'fave_color'
t.string 'fave_food'
t.string 'fave_possession'
t.string 'fave_weapon'
t.string 'fave_animal'
t.text 'father'
t.text 'mother'
t.text 'spouse'
t.text 'siblings'
t.text 'archenemy'
t.text 'notes'
t.text 'private_notes'
t.integer 'user_id'
t.integer 'universe_id'
t.datetime 'created_at'
t.datetime 'updated_at'
ActiveRecord::Schema.define(version: 20150419134141) do
create_table "characters", force: true do |t|
t.string "name", null: false
t.string "role"
t.string "gender"
t.string "age"
t.string "height"
t.string "weight"
t.string "haircolor"
t.string "hairstyle"
t.string "facialhair"
t.string "eyecolor"
t.string "race"
t.string "skintone"
t.string "bodytype"
t.string "identmarks"
t.text "bestfriend"
t.text "religion"
t.text "politics"
t.text "prejudices"
t.text "occupation"
t.text "pets"
t.text "mannerisms"
t.text "birthday"
t.text "birthplace"
t.text "education"
t.text "background"
t.string "fave_color"
t.string "fave_food"
t.string "fave_possession"
t.string "fave_weapon"
t.string "fave_animal"
t.text "father"
t.text "mother"
t.text "spouse"
t.text "siblings"
t.text "archenemy"
t.text "notes"
t.text "private_notes"
t.integer "user_id"
t.integer "universe_id"
t.datetime "created_at"
t.datetime "updated_at"
end
create_table 'equipment', force: true do |t|
t.string 'name', null: false
t.string 'equip_type'
t.text 'description'
t.string 'weight'
t.string 'original_owner'
t.string 'current_owner'
t.text 'made_by'
t.text 'materials'
t.string 'year_made'
t.text 'magic'
t.text 'notes'
t.text 'private_notes'
t.integer 'user_id'
t.integer 'universe_id'
t.datetime 'created_at'
t.datetime 'updated_at'
create_table "equipment", force: true do |t|
t.string "name", null: false
t.string "equip_type"
t.text "description"
t.string "weight"
t.string "original_owner"
t.string "current_owner"
t.text "made_by"
t.text "materials"
t.string "year_made"
t.text "magic"
t.text "notes"
t.text "private_notes"
t.integer "user_id"
t.integer "universe_id"
t.datetime "created_at"
t.datetime "updated_at"
end
create_table 'languages', force: true do |t|
t.string 'name', null: false
t.text 'words'
t.string 'established_year'
t.string 'established_location'
t.text 'characters'
t.text 'locations'
t.text 'notes'
t.integer 'user_id'
t.integer 'universe_id'
t.datetime 'created_at'
t.datetime 'updated_at'
create_table "languages", force: true do |t|
t.string "name", null: false
t.text "words"
t.string "established_year"
t.string "established_location"
t.text "characters"
t.text "locations"
t.text "notes"
t.integer "user_id"
t.integer "universe_id"
t.datetime "created_at"
t.datetime "updated_at"
end
create_table 'locations', force: true do |t|
t.string 'name', null: false
t.string 'type_of'
t.text 'description'
t.string 'map_file_name'
t.string 'map_content_type'
t.integer 'map_file_size'
t.datetime 'map_updated_at'
t.string 'population'
t.string 'language'
t.string 'currency'
t.string 'motto'
t.text 'capital'
t.text 'largest_city'
t.text 'notable_cities'
t.text 'area'
t.text 'crops'
t.text 'located_at'
t.string 'established_year'
t.text 'notable_wars'
t.text 'notes'
t.text 'private_notes'
t.integer 'user_id'
t.integer 'universe_id'
t.datetime 'created_at'
t.datetime 'updated_at'
create_table "locations", force: true do |t|
t.string "name", null: false
t.string "type_of"
t.text "description"
t.string "map_file_name"
t.string "map_content_type"
t.integer "map_file_size"
t.datetime "map_updated_at"
t.string "population"
t.string "language"
t.string "currency"
t.string "motto"
t.text "capital"
t.text "largest_city"
t.text "notable_cities"
t.text "area"
t.text "crops"
t.text "located_at"
t.string "established_year"
t.text "notable_wars"
t.text "notes"
t.text "private_notes"
t.integer "user_id"
t.integer "universe_id"
t.datetime "created_at"
t.datetime "updated_at"
end
create_table 'magics', force: true do |t|
t.string 'name', null: false
t.string 'type_of'
t.text 'manifestation'
t.text 'symptoms'
t.string 'element'
t.string 'diety'
t.text 'harmfulness'
t.text 'helpfulness'
t.text 'neutralness'
t.text 'resource'
t.text 'skill_level'
t.text 'limitations'
t.text 'notes'
t.text 'private_notes'
t.integer 'user_id'
t.integer 'universe_id'
t.datetime 'created_at'
t.datetime 'updated_at'
create_table "magics", force: true do |t|
t.string "name", null: false
t.string "type_of"
t.text "manifestation"
t.text "symptoms"
t.string "element"
t.string "diety"
t.text "harmfulness"
t.text "helpfulness"
t.text "neutralness"
t.text "resource"
t.text "skill_level"
t.text "limitations"
t.text "notes"
t.text "private_notes"
t.integer "user_id"
t.integer "universe_id"
t.datetime "created_at"
t.datetime "updated_at"
end
create_table 'sessions', force: true do |t|
t.string 'username', null: false
t.string 'password', null: false
t.datetime 'created_at'
t.datetime 'updated_at'
create_table "sessions", force: true do |t|
t.string "username", null: false
t.string "password", null: false
t.datetime "created_at"
t.datetime "updated_at"
end
create_table 'universes', force: true do |t|
t.string 'name', null: false
t.text 'description'
t.text 'history'
t.text 'notes'
t.text 'private_notes'
t.string 'privacy'
t.integer 'user_id'
t.datetime 'created_at'
t.datetime 'updated_at'
create_table "universes", force: true do |t|
t.string "name", null: false
t.text "description"
t.text "history"
t.text "notes"
t.text "private_notes"
t.string "privacy"
t.integer "user_id"
t.datetime "created_at"
t.datetime "updated_at"
end
create_table 'users', force: true do |t|
t.string 'name', null: false
t.string 'email', null: false
t.string 'password', null: false
t.datetime 'created_at'
t.datetime 'updated_at'
create_table "users", force: true do |t|
t.string "name", null: false
t.string "email", null: false
t.string "old_password"
t.datetime "created_at"
t.datetime "updated_at"
t.string "password_digest"
end
end

View File

@ -2,6 +2,7 @@ source 'https://rubygems.org'
source 'https://rubygems.org'
gem 'bcrypt-ruby', '~> 3.0.1'
gem 'rails', '4.1.0'
gem 'jquery-rails'
gem 'sass-rails', '~> 4.0.3', :require => 'sass'

View File

@ -2,4 +2,28 @@ require 'test_helper'
# Tests for the SessionsController class
class SessionsControllerTest < ActionController::TestCase
test 'old password migrates to bcrypt' do
post :create, session: {
username: users(:martin).name,
password: 'HODOR'
}
migrated_martin = User.find_by(name: users(:martin).name)
.authenticate('HODOR')
assert_not_nil migrated_martin,
'Could not authenticate an older user using the '\
'new bcrypt scheme'
end
test 'old password is cleared when user is migrated' do
post :create, session: {
username: users(:martin).name,
password: 'HODOR'
}
old_password = User.find_by(name: users(:martin).name).old_password
assert old_password.blank?
end
end

View File

@ -1,4 +1,11 @@
tolkien:
name: JRRTolkien
email: tolkien@example.com
password: <%= SessionsController.create_password_digest "JRRTolkien", "Mellon" %>
password_digest: <%= BCrypt::Password.create("Mellon") %>
martin:
name: GRRMartin
email: martin@localhost
old_password: <%= require 'digest'; Digest::MD5.hexdigest("GRRMartin's password IS... HODOR (lol!)") %>

View File

@ -0,0 +1,86 @@
require 'test_helper'
##
# Tests the entire ActiveModel::SecurePassword API implemented by the User model
#
# Taken from this demonstration:
# http://api.rubyonrails.org/classes/ActiveModel/SecurePassword/ClassMethods.html#method-i-has_secure_password
class UserSecurePasswordTest < ActiveSupport::TestCase
setup do
@username = 'ChrisTolkien'
@email = 'chris@localhost'
@rightpass = 'HiDad'
@wrongpass = 'Mellon'
end
test 'cannot save user with mismatched password and confirmation' do
user = User.new(name: @username,
password: @rightpass,
password_confirmation: @wrongpass)
refute user.save,
'Was able to save user despite mismatched password and confirmation'
end
test 'user with correct password and confirmation is a valid model' do
user = User.new(name: @username,
email: @email,
password: @rightpass,
password_confirmation: @rightpass)
assert user.valid?, "User model was not valid with a matching password
and confirmation: #{user.errors.messages}"
end
test 'can save user with matching password and confirmation' do
user = User.new(name: @username,
email: @email,
password: @rightpass,
password_confirmation: @rightpass)
assert user.save,
'Was not able to save user despite matching password '\
'and confirmation'
end
test 'can save a user with correct password and no confirmation' do
user = User.new(name: @username,
email: @email,
password: @rightpass)
assert user.save, 'Was not able to save user despite having a password'
end
test 'cannot authenticate a user with the incorrect password' do
user = User.new(name: @username, password: @rightpass)
refute user.authenticate(@wrongpass),
'Was able to authenticate a user with an incorrect password'
end
test 'can authenticate a user with the correct password' do
user = User.new(name: @username,
email: @email,
password: @rightpass)
assert_not_nil user.authenticate(@rightpass),
'Was not able to authenticate a user with a correct password'
end
test 'can find a user and authenticate with the correct password' do
User.create(name: @username,
email: @email,
password: @rightpass)
assert_not_nil User.find_by(name: @username).try(:authenticate, @rightpass),
'Was not able to find & authenticate user with a '\
'correct password'
end
test 'cannot find and authenticate with an incorrect password' do
User.create(name: @username,
email: @email,
password: @rightpass)
refute User.find_by(name: @username).try(:authenticate, @wrongpass),
'Was able to find & authenticate a user with an incorrect password'
end
end

View File

@ -25,7 +25,8 @@ class UserTest < ActiveSupport::TestCase
test 'user fixture assumptions' do
assert_not_nil users(:tolkien), 'User fixture is unavailable'
assert users(:tolkien).valid?, 'User fixture is not a valid user'
assert users(:tolkien).valid?, "User fixture is not a valid user:
#{users(:tolkien).errors.messages}"
end
test 'can get user content' do