diff --git a/Gemfile b/Gemfile index 26d6482a8..a879e99d4 100644 --- a/Gemfile +++ b/Gemfile @@ -32,7 +32,7 @@ gem 'net-sftp' gem 'newrelic_rpm' gem 'term-ansicolor', :require => "term/ansicolor" gem 'diff-lcs', :require => "diff/lcs/array" -gem 'bcrypt-ruby' +gem 'bcrypt-ruby', :require => "bcrypt" group :development do gem 'ruby-prof' diff --git a/app/logical/session_creator.rb b/app/logical/session_creator.rb index 0aa0a6ade..3997e8fb1 100644 --- a/app/logical/session_creator.rb +++ b/app/logical/session_creator.rb @@ -15,8 +15,8 @@ class SessionCreator user.update_column(:last_logged_in_at, Time.now) if remember.present? - cookies[:user_name] = {:expires => 1.year.from_now, :value => user.name} - cookies[:cookie_password_hash] = {:expires => 1.year.from_now, :value => user.cookie_password_hash} + cookies.permanent.signed[:user_name] = user.name + cookies.permanent.signed[:password_hash] = user.bcrypt_password_hash end session[:user_id] = user.id diff --git a/app/logical/session_loader.rb b/app/logical/session_loader.rb index 4513ed36e..46edc8177 100644 --- a/app/logical/session_loader.rb +++ b/app/logical/session_loader.rb @@ -32,7 +32,7 @@ private end def load_cookie_user - CurrentUser.user = User.find_by_name(cookies[:user_name]) + CurrentUser.user = User.find_by_name(cookies.signed[:user_name]) CurrentUser.ip_addr = request.remote_ip end @@ -41,7 +41,7 @@ private end def cookie_password_hash_valid? - cookies[:cookie_password_hash] && User.authenticate_cookie_hash(cookies[:user_name], cookies[:cookie_password_hash]) + cookies[:password_hash] && User.authenticate_cookie_hash(cookies.signed[:user_name], cookies.signed[:password_hash]) end def update_last_logged_in_at diff --git a/app/models/user.rb b/app/models/user.rb index 54cfdd8d1..fd42862c1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -121,19 +121,24 @@ class User < ActiveRecord::Base end module PasswordMethods + def bcrypt_password + BCrypt::Password.new(bcrypt_password_hash) + end + def encrypt_password_on_create - self.password_hash = User.sha1(password) + self.password_hash = "" + self.bcrypt_password_hash = User.bcrypt(password) end def encrypt_password_on_update return if password.blank? return if old_password.blank? - if User.sha1(old_password) == password_hash - self.password_hash = User.sha1(password) + if bcrypt_password == User.sha1(old_password) + self.bcrypt_password_hash = User.bcrypt(password) return true else - errors[:old_password] << "is incorrect" + errors[:old_password] = "is incorrect" return false end end @@ -149,7 +154,7 @@ class User < ActiveRecord::Base end pass << rand(100).to_s - update_column(:password_hash, User.sha1(pass)) + update_column(:bcrypt_password_hash, User.bcrypt(pass)) pass end @@ -168,29 +173,31 @@ class User < ActiveRecord::Base end def authenticate_hash(name, hash) - where(["lower(name) = ? AND password_hash = ?", name.downcase, hash]).first != nil + user = find_by_name(name) + if user && user.bcrypt_password == hash + user + else + nil + end end - + def authenticate_cookie_hash(name, hash) - user = User.find_by_name(name) - return nil if user.nil? - return hash == user.cookie_password_hash + user = find_by_name(name) + if user && user.bcrypt_password_hash == hash + user + else + nil + end + end + + def bcrypt(pass) + BCrypt::Password.create(sha1(pass)) end def sha1(pass) Digest::SHA1.hexdigest("#{Danbooru.config.password_salt}--#{pass}--") end end - - def cookie_password_hash - hash = password_hash - - (name.size + 8).times do - hash = User.sha1(hash) - end - - return hash - end end module FavoriteMethods @@ -456,7 +463,7 @@ class User < ActiveRecord::Base module ApiMethods def hidden_attributes - super + [:password_hash, :email, :email_verification_key] + super + [:password_hash, :bcrypt_password_hash, :email, :email_verification_key] end def serializable_hash(options = {}) diff --git a/config/application.rb b/config/application.rb index 9ad924f98..d3c023f34 100644 --- a/config/application.rb +++ b/config/application.rb @@ -1,5 +1,4 @@ require File.expand_path('../boot', __FILE__) - require 'rails/all' if defined?(Bundler) @@ -11,6 +10,7 @@ end module Danbooru class Application < Rails::Application + config.active_record.schema_format = :sql config.encoding = "utf-8" config.filter_parameters += [:password] @@ -25,4 +25,3 @@ module Danbooru config.log_tags = [lambda {|req| "PID:#{Process.pid}"}] end end - diff --git a/config/danbooru_default_config.rb b/config/danbooru_default_config.rb index b30fbfe02..3eb61329c 100644 --- a/config/danbooru_default_config.rb +++ b/config/danbooru_default_config.rb @@ -112,11 +112,6 @@ module Danbooru !user.is_privileged? end - # This is required for Rails 2.0. - def session_secret_key - "This should be at least 30 characters long" - end - # Users cannot search for more than X regular tags at a time. def base_tag_query_limit 6 diff --git a/config/initializers/secret_token.rb b/config/initializers/secret_token.rb index 38b12c5bd..924f5f907 100644 --- a/config/initializers/secret_token.rb +++ b/config/initializers/secret_token.rb @@ -1,13 +1,9 @@ -# Be sure to restart your server when you modify this file. +require File.expand_path('../../state_checker', __FILE__) -# Your secret key for verifying the integrity of signed cookies. -# If you change this key, all old signed cookies will become invalid! -# Make sure the secret is at least 30 characters and all random, -# no regular words or you'll be exposed to dictionary attacks. - -if File.exists?(File.expand_path("~/.danbooru/secret_token")) - Danbooru::Application.config.secret_token = File.read(File.expand_path("~/.danbooru/secret_token")) -else - Danbooru::Application.config.secret_token = SecureRandom.hex(64) -end +StateChecker.new.check! +Danbooru::Application.config.action_dispatch.session = { + :key => '_danbooru2_session', + :secret => File.read(File.expand_path("~/.danbooru/session_secret_key")) +} +Danbooru::Application.config.secret_token = File.read(File.expand_path("~/.danbooru/secret_token")) diff --git a/config/state_checker.rb b/config/state_checker.rb new file mode 100644 index 000000000..a6f4b0a61 --- /dev/null +++ b/config/state_checker.rb @@ -0,0 +1,36 @@ +class StateChecker + def check! + check_secret_token + check_session_secret_key + end + +private + + def secret_token_path + File.expand_path("~/.danbooru/secret_token") + end + + def check_secret_token + unless File.exists?(secret_token_path) + raise "You must create a file in #{secret_token_path} containing a secret key. It should be a string of at least 32 random characters." + end + + if File.stat(secret_token_path).world_readable? || File.stat(secret_token_path).world_writable? + raise "#{secret_token_path} must not be world readable or writable" + end + end + + def session_secret_key_path + File.expand_path("~/.danbooru/session_secret_key") + end + + def check_session_secret_key + unless File.exists?(session_secret_key_path) + raise "You must create a file in #{session_secret_key_path} containing a secret key. It should be a string of at least 32 random characters." + end + + if File.stat(session_secret_key_path).world_readable? || File.stat(session_secret_key_path).world_writable? + raise "#{session_secret_key_path} must not be world readable or writable" + end + end +end diff --git a/db/migrate/20130305005138_add_bcrypt_fields_to_users.rb b/db/migrate/20130305005138_add_bcrypt_fields_to_users.rb new file mode 100644 index 000000000..5e2b54bbf --- /dev/null +++ b/db/migrate/20130305005138_add_bcrypt_fields_to_users.rb @@ -0,0 +1,7 @@ +class AddBcryptFieldsToUsers < ActiveRecord::Migration + def change + execute "set statement_timeout = 0" + + add_column :users, :bcrypt_password_hash, :text + end +end diff --git a/db/structure.sql b/db/structure.sql index 32f8a2cb7..f486d06bb 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2596,7 +2596,8 @@ CREATE TABLE users ( default_image_size character varying(255) DEFAULT 'large'::character varying NOT NULL, favorite_tags text, blacklisted_tags text, - time_zone character varying(255) DEFAULT 'Eastern Time (US & Canada)'::character varying NOT NULL + time_zone character varying(255) DEFAULT 'Eastern Time (US & Canada)'::character varying NOT NULL, + bcrypt_password_hash text ); @@ -6207,4 +6208,6 @@ INSERT INTO schema_migrations (version) VALUES ('20130221032344'); INSERT INTO schema_migrations (version) VALUES ('20130221035518'); -INSERT INTO schema_migrations (version) VALUES ('20130221214811'); \ No newline at end of file +INSERT INTO schema_migrations (version) VALUES ('20130221214811'); + +INSERT INTO schema_migrations (version) VALUES ('20130305005138'); \ No newline at end of file diff --git a/script/fixes/004.rb b/script/fixes/004.rb index e346061ea..f95e4fe91 100644 --- a/script/fixes/004.rb +++ b/script/fixes/004.rb @@ -2,4 +2,6 @@ require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'config', 'environment')) -puts Post.count \ No newline at end of file +User.find_each do |user| + user.update_column(:bcrypt_password_hash, BCrypt::Password.create(user.password_hash)) +end diff --git a/test/functional/maintenance/user/password_resets_controller_test.rb b/test/functional/maintenance/user/password_resets_controller_test.rb index 8c03073a5..1ba087270 100644 --- a/test/functional/maintenance/user/password_resets_controller_test.rb +++ b/test/functional/maintenance/user/password_resets_controller_test.rb @@ -101,7 +101,7 @@ module Maintenance @user = FactoryGirl.create(:user) @nonce = FactoryGirl.create(:user_password_reset_nonce, :email => @user.email) ActionMailer::Base.deliveries.clear - @old_password = @user.password_hash + @old_password = @user.bcrypt_password_hash post :update, :email => @nonce.email, :key => @nonce.key end @@ -115,7 +115,7 @@ module Maintenance should "change the password" do @user.reload - assert_not_equal(@old_password, @user.password_hash) + assert_not_equal(@old_password, @user.bcrypt_password_hash) end should "delete the nonce" do diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 0f470a5b1..3b73e5b90 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -35,7 +35,7 @@ class UserTest < ActiveSupport::TestCase assert_difference("ModAction.count") do @user.invite!(User::Levels::CONTRIBUTOR) end - assert_equal("#{@user.id} level changed Member -> Contributor by #{CurrentUser.name}", ModAction.first.description) + assert_equal("#{@user.name} level changed Member -> Contributor by #{CurrentUser.name}", ModAction.last.description) end end @@ -199,21 +199,16 @@ class UserTest < ActiveSupport::TestCase end end - context "cookie password hash" do - setup do - @user = FactoryGirl.create(:user, :name => "albert", :password_hash => "1234") - end - - should "be correct" do - assert_equal("8ac3b1d04bdb95ba92f9e355897c880e0d88ac5a", @user.cookie_password_hash) - end - - should "validate" do - assert(User.authenticate_cookie_hash(@user.name, "8ac3b1d04bdb95ba92f9e355897c880e0d88ac5a")) - end - end - context "password" do + should "match the cookie hash" do + @user = FactoryGirl.create(:user) + @user.password = "zugzug5" + @user.password_confirmation = "zugzug5" + @user.save + @user.reload + assert(User.authenticate_cookie_hash(@user.name, @user.bcrypt_password_hash)) + end + should "match the confirmation" do @user = FactoryGirl.create(:user) @user.password = "zugzug5" @@ -248,25 +243,25 @@ class UserTest < ActiveSupport::TestCase should "not change the password if the password and old password are blank" do @user = FactoryGirl.create(:user, :password => "67890") @user.update_attributes(:password => "", :old_password => "") - assert_equal(User.sha1("67890"), @user.password_hash) + assert(@user.bcrypt_password == User.sha1("67890")) end should "not change the password if the old password is incorrect" do @user = FactoryGirl.create(:user, :password => "67890") @user.update_attributes(:password => "12345", :old_password => "abcdefg") - assert_equal(User.sha1("67890"), @user.password_hash) + assert(@user.bcrypt_password == User.sha1("67890")) end should "not change the password if the old password is blank" do @user = FactoryGirl.create(:user, :password => "67890") @user.update_attributes(:password => "12345", :old_password => "") - assert_equal(User.sha1("67890"), @user.password_hash) + assert(@user.bcrypt_password == User.sha1("67890")) end should "change the password if the old password is correct" do @user = FactoryGirl.create(:user, :password => "67890") @user.update_attributes(:password => "12345", :old_password => "67890") - assert_equal(User.sha1("12345"), @user.password_hash) + assert(@user.bcrypt_password == User.sha1("12345")) end context "in the json representation" do