From 3a4aa11d84f054cb83b73ae0c995641e39a97558 Mon Sep 17 00:00:00 2001 From: Ari Timonen Date: Thu, 2 Apr 2020 03:08:19 +0300 Subject: [PATCH] Add scrypt --- .env.development | 2 +- .env.example | 5 + Gemfile | 1 + Gemfile.lock | 6 ++ app/controllers/users_controller.rb | 17 ++-- app/models/user.rb | 99 ++++++++++++++----- config/locales/en.yml | 1 + ...200401031046_add_password_hash_to_users.rb | 7 ++ ...200401031518_update_passwords_to_scrypt.rb | 14 +++ db/schema.rb | 3 +- spec/models/user_spec.rb | 35 +++---- 11 files changed, 141 insertions(+), 49 deletions(-) create mode 100644 db/migrate/20200401031046_add_password_hash_to_users.rb create mode 100644 db/migrate/20200401031518_update_passwords_to_scrypt.rb diff --git a/.env.development b/.env.development index 2e2816b..794bf50 100644 --- a/.env.development +++ b/.env.development @@ -6,7 +6,7 @@ RAILS_ENV=development APP_SECRET=fe837ea72667ec3d8ecb94cfba1a1bba DEPLOY_PATH=/var/www -FILES_PATH=/var/www/files +FILES_PATH=/var/www/public/files PUMA_WORKERS=0 PUMA_MIN_THREADS=1 diff --git a/.env.example b/.env.example index 454a8d9..923480e 100644 --- a/.env.example +++ b/.env.example @@ -8,6 +8,11 @@ RAILS_ENV=production # App secret for cookie encryption APP_SECRET=randomstringhere +# Use +# SCrypt::Engine.calibrate!(max_mem: 16 * 1024 * 1024) +# SCrypt::Engine.generate_salt +SCRYPT_SALT_OPTS= + # Since this is inside Docker container, it doesn't really matter DEPLOY_PATH=/var/www diff --git a/Gemfile b/Gemfile index ff5e4af..2b9b7b6 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,7 @@ gem 'unicorn' # Model plugins gem 'unread' +gem 'scrypt' # gem 'impressionist' # gem 'ratyrate' # gem "acts_as_rateable", :git => "git://github.com/anton-zaytsev/acts_as_rateable.git" diff --git a/Gemfile.lock b/Gemfile.lock index e570388..763ce45 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -200,6 +200,9 @@ GEM faraday (0.17.3) multipart-post (>= 1.2, < 3) ffi (1.12.2) + ffi-compiler (1.0.1) + ffi (>= 1.0.0) + rake font-awesome-sass (4.1.0) sass (~> 3.2) geckodriver-helper (0.24.0) @@ -393,6 +396,8 @@ GEM sprockets (>= 2.8, < 4.0) sprockets-rails (>= 2.0, < 4.0) tilt (>= 1.1, < 3) + scrypt (3.0.7) + ffi-compiler (>= 1.0, < 2.0) selenium-webdriver (3.142.7) childprocess (>= 0.5, < 4.0) rubyzip (>= 1.2.2) @@ -511,6 +516,7 @@ DEPENDENCIES rubocop sanitize sass-rails (~> 5.0.3) + scrypt selenium-webdriver signet (= 0.11.0) simplecov diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2cd613f..a3489ae 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -90,15 +90,18 @@ class UsersController < ApplicationController # FIXME: maybe move to session controller def login - if params[:login] && (u = User.authenticate(params[:login])) - if u.banned? Ban::TYPE_SITE - flash[:notice] = t(:accounts_locked) + if params[:login] + if (u = User.authenticate(params[:login])) + if u.banned? Ban::TYPE_SITE + flash[:notice] = t(:accounts_locked) + else + flash[:notice] = "%s (%s)" % [t(:login_successful), u.password_hash_s] + flash[:notice] << " \n%s" % I18n.t(:password_md5_scrypt) if u.password_hash_changed? + save_session u + end else - flash[:notice] = t(:login_successful) - save_session u + flash[:error] = t(:login_unsuccessful) end - else - flash[:error] = t(:login_unsuccessful) end # FIXME: check return on rails 6 if session[:return_to] diff --git a/app/models/user.rb b/app/models/user.rb index da3b2bb..cf801af 100755 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2,23 +2,24 @@ # # Table name: users # -# id :integer not null, primary key -# birthdate :date -# country :string(255) -# email :string(255) -# firstname :string(255) -# lastip :string(255) -# lastname :string(255) -# lastvisit :datetime -# password :string(255) -# public_email :boolean default(FALSE), not null -# steamid :string(255) -# time_zone :string(255) -# username :string(255) -# version :integer -# created_at :datetime -# updated_at :datetime -# team_id :integer +# id :integer not null, primary key +# birthdate :date +# country :string(255) +# email :string(255) +# firstname :string(255) +# lastip :string(255) +# lastname :string(255) +# lastvisit :datetime +# password :string(255) +# password_hash :integer default(0) +# public_email :boolean default(FALSE), not null +# steamid :string(255) +# time_zone :string(255) +# username :string(255) +# version :integer +# created_at :datetime +# updated_at :datetime +# team_id :integer # # Indexes # @@ -27,6 +28,7 @@ # require 'digest/md5' +require "scrypt" class SteamIdValidator < ActiveModel::Validator def validate(record) @@ -43,10 +45,15 @@ class User < ActiveRecord::Base VERIFICATION_TIME = 604800 + PASSWORD_SCRYPT = 0 + PASSWORD_MD5 = 1 + PASSWORD_MD5_SCRYPT = 2 + #attr_protected :id, :created_at, :updated_at, :lastvisit, :lastip, :password, :version - attr_accessor :raw_password + attr_accessor :raw_password, :password_updated attribute :lastvisit, :datetime, default: Time.now.utc + attribute :password_hash, :integer, default: PASSWORD_SCRYPT belongs_to :team, :optional => true has_one :profile, :dependent => :destroy @@ -121,8 +128,8 @@ class User < ActiveRecord::Base validates_uniqueness_of :username, :email, :steamid validates_length_of :firstname, :in => 1..15, :allow_blank => true validates_length_of :lastname, :in => 1..25, :allow_blank => true - validates_length_of :username, :in => 2..20 - validates_format_of :username, :with => /\A[A-Za-z0-9_\-\+]{2,20}\Z/ + validates_length_of :username, :in => 1..30 + validates_format_of :username, :with => /\A[A-Za-z0-9_\-\+]{1,30}\Z/ validates_presence_of :raw_password, :on => :create validates_length_of :email, :maximum => 50 validates_format_of :email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i @@ -131,6 +138,7 @@ class User < ActiveRecord::Base # validates_format_of :steamid, :with => /\A(STEAM_)?[0-5]:[01]:\d+\Z/ validates_length_of :time_zone, :maximum => 100, :allow_blank => true, :allow_nil => true validates_inclusion_of [:public_email], :in => [true, false], :allow_nil => true + # validates_inclusion_of :password_hash, in: => [User::PASSWORD_SCRYPT, User::PASSWORD_MD5, User::PASSWORD_MD5_SCRYPT] validate :validate_team before_create :init_variables @@ -152,12 +160,25 @@ class User < ActiveRecord::Base non_versioned_columns << 'birthdate' non_versioned_columns << 'time_zone' non_versioned_columns << 'public_email' + non_versioned_columns << 'password_hash' non_versioned_columns << 'created_at' def to_s username end + def password_hash_s + case self.password_hash + when User::PASSWORD_MD5 + "MD5" + when User::PASSWORD_SCRYPT + "Scrypt" + when User::PASSWORD_MD5_SCRYPT + "Scrypt+MD5" + else + end + end + def email_s email.gsub /@/, " (at) " end @@ -307,9 +328,17 @@ class User < ActiveRecord::Base self.time_zone = "Amsterdam" end - # Password should store password and password_hash shoulds store + # NOTE: function does not call save + # Maybe it should return to not waste save? def update_password - self.password = Digest::MD5.hexdigest(raw_password) if raw_password and raw_password.length > 0 + if raw_password and raw_password.length > 0 + self.password = SCrypt::Password.create(raw_password) + self.password_hash = User::PASSWORD_SCRYPT + elsif password_hash == User::PASSWORD_MD5 + # Scrypt(Md5(passsword)) + self.password = SCrypt::Password.create(password) + self.password_hash = User::PASSWORD_MD5_SCRYPT + end end def send_new_password @@ -339,7 +368,31 @@ class User < ActiveRecord::Base end def self.authenticate(login) - where("LOWER(username) = LOWER(?)", login[:username]).where(password: Digest::MD5.hexdigest(login[:password])).first + if (user = where("LOWER(username) = LOWER(?)", login[:username]).first) + case user.password_hash + when User::PASSWORD_SCRYPT + pass = SCrypt::Password.new(user.password) + return user if pass == login[:password] + when User::PASSWORD_MD5_SCRYPT + pass = SCrypt::Password.new(user.password) + # Match to Scrypt(Md5(password)) + if pass == Digest::MD5.hexdigest(login[:password]) + user.raw_password = login[:password] + user.update_password + user.save! + return user + end + # when User::PASSWORD_MD5 + else + if user.password == Digest::MD5.hexdigest(login[:password]) + user.raw_password = login[:password] + user.update_password + user.save! + return user + end + end + end + return nil end def self.get id diff --git a/config/locales/en.yml b/config/locales/en.yml index 5120c82..81b34e2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -84,6 +84,7 @@ en: login_out: "Logged out." login_status: "Logged in as" passwords_sent: "Password has been sent." + password_md5_scrypt: "Password has been upgraded to higher security level (MD5->SCRYPT)." incorrect_information: "Incorrect Information." weeks_create: "Week was successfully created." weeks_update: "Week was successfully updated." diff --git a/db/migrate/20200401031046_add_password_hash_to_users.rb b/db/migrate/20200401031046_add_password_hash_to_users.rb new file mode 100644 index 0000000..1da4456 --- /dev/null +++ b/db/migrate/20200401031046_add_password_hash_to_users.rb @@ -0,0 +1,7 @@ +class AddPasswordHashToUsers < ActiveRecord::Migration[6.0] + def change + change_table :users do |u| + u.integer :password_hash, default: User::PASSWORD_MD5 + end + end +end diff --git a/db/migrate/20200401031518_update_passwords_to_scrypt.rb b/db/migrate/20200401031518_update_passwords_to_scrypt.rb new file mode 100644 index 0000000..4610daf --- /dev/null +++ b/db/migrate/20200401031518_update_passwords_to_scrypt.rb @@ -0,0 +1,14 @@ +class UpdatePasswordsToScrypt < ActiveRecord::Migration[6.0] + require 'scrypt' + require 'user' + + def up + SCrypt::Engine.calibrate!(max_time: 0.03) + ActiveRecord::Base.transaction do + User.all.order(:id).each do |user| + user.update_password + user.save! + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 618d08e..eabd503 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_03_31_020637) do +ActiveRecord::Schema.define(version: 2020_04_01_031046) do create_table "article_versions", id: :integer, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| t.integer "article_id" @@ -805,6 +805,7 @@ ActiveRecord::Schema.define(version: 2020_03_31_020637) do t.string "time_zone" t.integer "version" t.boolean "public_email", default: false, null: false + t.integer "password_hash", default: 1 t.index ["lastvisit"], name: "index_users_on_lastvisit" t.index ["team_id"], name: "index_users_on_team_id" end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 31f5302..6489e11 100755 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2,23 +2,24 @@ # # Table name: users # -# id :integer not null, primary key -# birthdate :date -# country :string(255) -# email :string(255) -# firstname :string(255) -# lastip :string(255) -# lastname :string(255) -# lastvisit :datetime -# password :string(255) -# public_email :boolean default(FALSE), not null -# steamid :string(255) -# time_zone :string(255) -# username :string(255) -# version :integer -# created_at :datetime -# updated_at :datetime -# team_id :integer +# id :integer not null, primary key +# birthdate :date +# country :string(255) +# email :string(255) +# firstname :string(255) +# lastip :string(255) +# lastname :string(255) +# lastvisit :datetime +# password :string(255) +# password_hash :integer default(0) +# public_email :boolean default(FALSE), not null +# steamid :string(255) +# time_zone :string(255) +# username :string(255) +# version :integer +# created_at :datetime +# updated_at :datetime +# team_id :integer # # Indexes #