diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4e36da811..f71ebf723 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -50,6 +50,12 @@ class ApplicationController < ActionController::Base Rails.application.secrets.http_basic_auth end + def verify_lock + if current_user.locked? + redirect_to account_path, alert: t('verification.alert.lock') + end + end + def set_locale if params[:locale] && I18n.available_locales.include?(params[:locale].to_sym) session[:locale] = params[:locale] diff --git a/app/controllers/verification/letter_controller.rb b/app/controllers/verification/letter_controller.rb index f1a919855..15d408a5f 100644 --- a/app/controllers/verification/letter_controller.rb +++ b/app/controllers/verification/letter_controller.rb @@ -2,7 +2,7 @@ class Verification::LetterController < ApplicationController before_action :authenticate_user! before_action :verify_resident! before_action :verify_phone! - before_action :verify_attemps_left! + before_action :verify_lock skip_authorization_check def new @@ -29,7 +29,7 @@ class Verification::LetterController < ApplicationController current_user.update(verified_at: Time.now) redirect_to account_path, notice: t('verification.letter.update.flash.success') else - @letter.increase_letter_verification_tries + Lock.increase_tries(@letter.user) render :edit end end @@ -46,9 +46,4 @@ class Verification::LetterController < ApplicationController end end - def verify_attemps_left! - if current_user.letter_verification_tries >= 2 - redirect_to account_path, alert: t('verification.letter.alert.verify_attemps_left') - end - end end \ No newline at end of file diff --git a/app/controllers/verification/residence_controller.rb b/app/controllers/verification/residence_controller.rb index bd4ab082a..df24454e6 100644 --- a/app/controllers/verification/residence_controller.rb +++ b/app/controllers/verification/residence_controller.rb @@ -1,6 +1,6 @@ class Verification::ResidenceController < ApplicationController before_action :authenticate_user! - before_action :verify_attemps_left!, only: [:new, :create] + before_action :verify_lock, only: [:new, :create] skip_authorization_check def new @@ -21,10 +21,4 @@ class Verification::ResidenceController < ApplicationController def residence_params params.require(:residence).permit(:document_number, :document_type, :date_of_birth, :postal_code, :terms_of_service) end - - def verify_attemps_left! - if current_user.residence_verification_tries >= 5 - redirect_to account_path, alert: t('verification.residence.alert.verify_attemps_left') - end - end end \ No newline at end of file diff --git a/app/controllers/verification/sms_controller.rb b/app/controllers/verification/sms_controller.rb index 264cca907..87604f726 100644 --- a/app/controllers/verification/sms_controller.rb +++ b/app/controllers/verification/sms_controller.rb @@ -1,7 +1,7 @@ class Verification::SmsController < ApplicationController before_action :authenticate_user! before_action :verify_resident! - before_action :verify_attemps_left!, only: [:new, :create] + before_action :verify_lock, only: [:new, :create] before_action :set_phone, only: :create skip_authorization_check @@ -68,10 +68,4 @@ class Verification::SmsController < ApplicationController end end - def verify_attemps_left! - if current_user.sms_confirmation_tries >= 3 - redirect_to account_path, notice: t('verification.sms.alert.verify_attemps_left') - end - end - end \ No newline at end of file diff --git a/app/models/lock.rb b/app/models/lock.rb new file mode 100644 index 000000000..3c043de79 --- /dev/null +++ b/app/models/lock.rb @@ -0,0 +1,30 @@ +class Lock < ActiveRecord::Base + belongs_to :user + + before_save :set_locked_until + + def locked? + locked_until > Time.now + end + + def set_locked_until + self.locked_until = lock_time if too_many_tries? + end + + def lock_time + Time.now + (2**tries).minutes + end + + def too_many_tries? + return false unless tries > 0 + tries % Lock.max_tries == 0 + end + + def self.increase_tries(user) + Lock.find_or_create_by(user: user).increment!(:tries) + end + + def self.max_tries + 5 + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 1099bc6c4..08dfd6609 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -16,6 +16,7 @@ class User < ActiveRecord::Base has_one :administrator has_one :moderator has_one :organization + has_one :lock has_many :flags has_many :identities, dependent: :destroy has_many :debates, -> { with_hidden }, foreign_key: :author_id @@ -128,11 +129,16 @@ class User < ActiveRecord::Base Comment.hide_all comments_ids end + def email_provided? !!(email && email !~ OMNIAUTH_EMAIL_REGEX) || !!(unconfirmed_email && unconfirmed_email !~ OMNIAUTH_EMAIL_REGEX) end + def locked? + Lock.find_or_create_by(user: self).locked? + end + def self.search(term) term.present? ? where("email = ? OR username ILIKE ?", term, "%#{term}%") : none end diff --git a/app/models/verification/residence.rb b/app/models/verification/residence.rb index e65ecf584..e07b88897 100644 --- a/app/models/verification/residence.rb +++ b/app/models/verification/residence.rb @@ -41,8 +41,8 @@ class Verification::Residence unless residency.valid? errors.add(:residence_in_madrid, false) - user.update(residence_verification_tries: user.residence_verification_tries += 1) store_failed_attempt + Lock.increase_tries(user) end self.date_of_birth = string_to_date(date_of_birth) end diff --git a/app/models/verification/sms.rb b/app/models/verification/sms.rb index 9027b597c..ba484c4f3 100644 --- a/app/models/verification/sms.rb +++ b/app/models/verification/sms.rb @@ -20,7 +20,7 @@ class Verification::Sms return false unless self.valid? update_user_phone_information send_sms - increase_sms_tries + Lock.increase_tries(user) end def update_user_phone_information @@ -31,10 +31,6 @@ class Verification::Sms SMSApi.new.sms_deliver(user.unconfirmed_phone, user.sms_confirmation_code) end - def increase_sms_tries - user.update(sms_confirmation_tries: user.sms_confirmation_tries += 1) - end - def verified? user.sms_confirmation_code == confirmation_code end diff --git a/config/locales/verification.en.yml b/config/locales/verification.en.yml index aa231298f..a333f78d7 100644 --- a/config/locales/verification.en.yml +++ b/config/locales/verification.en.yml @@ -4,6 +4,8 @@ en: step_1: "1. Residence" step_2: "2. Confirmation code" step_3: "3. Final verification" + alert: + lock: "You have reached the maximum number of verification tries. Please try again later." residence: new: title: "Verify residence" @@ -24,7 +26,6 @@ en: flash: success: "Residence verified" alert: - verify_attemps_left: "You have reached the maximum number of Census verification tries" unconfirmed_residency: "You have not yet confirmed your residence" sms: new: @@ -47,8 +48,6 @@ en: success: "Correct code. Your account is verified" level_two: success: "Correct code" - alert: - verify_attemps_left: "You have reached the maximum number of sms verification tries" email: show: flash: @@ -82,7 +81,6 @@ en: success: "Correct code. Your account is verified" alert: unconfirmed_code: "You have not yet enter the confirmation code" - verify_attemps_left: "You have reached the maximum number of letter verification tries" errors: letter_not_sent: "We have not sent you the letter with the code yet" incorect_code: "Incorrect confirmation code" diff --git a/config/locales/verification.es.yml b/config/locales/verification.es.yml index d5ebf8328..6c86abcfc 100644 --- a/config/locales/verification.es.yml +++ b/config/locales/verification.es.yml @@ -4,6 +4,8 @@ es: step_1: "1. Residencia" step_2: "2. Código de confirmación" step_3: "3. Verificación final" + alert: + lock: "Has llegado al máximo número de intentos. Por favor intentalo de nuevo más tarde." residence: new: title: "Verificar residencia" @@ -24,7 +26,6 @@ es: flash: success: "Residencia verificada" alert: - verify_attemps_left: "Has llegado al máximo número de intentos de verificar tu residencia." unconfirmed_residency: "Aún no has verificado tu residencia" sms: new: @@ -47,8 +48,6 @@ es: success: "Código correcto. Tu cuenta ya está verificada" level_two: success: "Código correcto" - alert: - verify_attemps_left: "Has llegado al máximo número de intentos de verificar tu teléfono." email: show: flash: @@ -82,7 +81,6 @@ es: success: "Código correcto. Tu cuenta ya está verificada" alert: unconfirmed_code: "Todavía no has introducido el código de confirmación" - verify_attemps_left: "Has llegado al máximo número de intentos de verificar tu carta." errors: letter_not_sent: "Aún no te hemos enviado la carta con el código" incorect_code: "Código de verificación incorrecto" diff --git a/db/migrate/20150910133047_create_locks.rb b/db/migrate/20150910133047_create_locks.rb new file mode 100644 index 000000000..03611d97a --- /dev/null +++ b/db/migrate/20150910133047_create_locks.rb @@ -0,0 +1,11 @@ +class CreateLocks < ActiveRecord::Migration + def change + create_table :locks do |t| + t.references :user, index: true, foreign_key: true + t.integer :tries, default: 0 + t.datetime :locked_until, null: false, default: Time.now + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20150910152734_remove_verification_tries_from_users.rb b/db/migrate/20150910152734_remove_verification_tries_from_users.rb new file mode 100644 index 000000000..cffaa971c --- /dev/null +++ b/db/migrate/20150910152734_remove_verification_tries_from_users.rb @@ -0,0 +1,7 @@ +class RemoveVerificationTriesFromUsers < ActiveRecord::Migration + def change + remove_column :users, :sms_confirmation_tries, :integer, default: 0 + remove_column :users, :residence_verification_tries, :integer, default: 0 + remove_column :users, :letter_verification_tries, :integer, default: 0 + end +end diff --git a/db/schema.rb b/db/schema.rb index bf4f9efc8..fde03fe30 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150910092713) do +ActiveRecord::Schema.define(version: 20150910152734) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -162,6 +162,16 @@ ActiveRecord::Schema.define(version: 20150910092713) do add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree + create_table "locks", force: :cascade do |t| + t.integer "user_id" + t.integer "tries", default: 0 + t.datetime "locked_until", default: '2015-09-10 13:46:11', null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "locks", ["user_id"], name: "index_locks_on_user_id", using: :btree + create_table "moderators", force: :cascade do |t| t.integer "user_id" end @@ -213,27 +223,27 @@ ActiveRecord::Schema.define(version: 20150910092713) do add_index "tags", ["name"], name: "index_tags_on_name", unique: true, using: :btree create_table "users", force: :cascade do |t| - t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false + t.string "email", default: "", null: false + t.string "encrypted_password", default: "", null: false t.string "reset_password_token" t.datetime "reset_password_sent_at" t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0, null: false + t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.string "confirmation_token" t.datetime "confirmed_at" t.datetime "confirmation_sent_at" t.string "unconfirmed_email" - t.boolean "email_on_debate_comment", default: false - t.boolean "email_on_comment_reply", default: false - t.string "phone_number", limit: 30 + t.boolean "email_on_debate_comment", default: false + t.boolean "email_on_comment_reply", default: false + t.string "phone_number", limit: 30 t.string "official_position" - t.integer "official_level", default: 0 + t.integer "official_level", default: 0 t.datetime "hidden_at" t.string "sms_confirmation_code" t.string "username", limit: 60 @@ -242,15 +252,12 @@ ActiveRecord::Schema.define(version: 20150910092713) do t.datetime "residence_verified_at" t.datetime "letter_sent_at" t.string "email_verification_token" - t.integer "sms_confirmation_tries", default: 0 - t.integer "residence_verification_tries", default: 0 t.datetime "verified_at" t.string "unconfirmed_phone" t.string "confirmed_phone" t.datetime "letter_requested_at" t.datetime "confirmed_hide_at" t.string "letter_verification_code" - t.integer "letter_verification_tries", default: 0 end add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree @@ -316,6 +323,7 @@ ActiveRecord::Schema.define(version: 20150910092713) do add_foreign_key "failed_census_calls", "users" add_foreign_key "flags", "users" add_foreign_key "identities", "users" + add_foreign_key "locks", "users" add_foreign_key "moderators", "users" add_foreign_key "organizations", "users" end diff --git a/spec/factories.rb b/spec/factories.rb index 211520aec..ff3f60540 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -1,4 +1,5 @@ FactoryGirl.define do + factory :user do sequence(:username) { |n| "Manuela#{n}" } sequence(:email) { |n| "manuela#{n}@madrid.es" } @@ -43,6 +44,12 @@ FactoryGirl.define do address end + factory :lock do + user + tries 0 + locked_until Time.now + end + factory :address do street_type "Calle" street "Alcalá" diff --git a/spec/features/verification/letter_spec.rb b/spec/features/verification/letter_spec.rb index 9233377f8..dd5e59b96 100644 --- a/spec/features/verification/letter_spec.rb +++ b/spec/features/verification/letter_spec.rb @@ -84,23 +84,23 @@ feature 'Verify Letter' do expect(URI.parse(current_url).path).to eq(new_sms_path) end - scenario '3 tries allowed' do + scenario '6 tries allowed' do user = create(:user, residence_verified_at: Time.now, confirmed_phone: "611111111") login_as(user) visit new_letter_path click_button 'Send me a letter with the code' - 3.times do + 6.times do fill_in 'letter_verification_code', with: "999999" click_button 'Send' end - expect(page).to have_content 'You have reached the maximum number of letter verification tries' + expect(page).to have_content "You have reached the maximum number of verification tries. Please try again later." expect(URI.parse(current_url).path).to eq(account_path) visit new_letter_path - expect(page).to have_content 'You have reached the maximum number of letter verification tries' + expect(page).to have_content "You have reached the maximum number of verification tries. Please try again later." expect(URI.parse(current_url).path).to eq(account_path) end diff --git a/spec/features/verification/residence_spec.rb b/spec/features/verification/residence_spec.rb index fa11fa48b..5091f9c7c 100644 --- a/spec/features/verification/residence_spec.rb +++ b/spec/features/verification/residence_spec.rb @@ -73,11 +73,11 @@ feature 'Residence' do end click_button 'Verify residence' - expect(page).to have_content 'You have reached the maximum number of Census verification tries' + expect(page).to have_content "You have reached the maximum number of verification tries. Please try again later." expect(URI.parse(current_url).path).to eq(account_path) visit new_residence_path - expect(page).to have_content 'You have reached the maximum number of Census verification tries' + expect(page).to have_content "You have reached the maximum number of verification tries. Please try again later." expect(URI.parse(current_url).path).to eq(account_path) end end \ No newline at end of file diff --git a/spec/features/verification/sms_spec.rb b/spec/features/verification/sms_spec.rb index e445b3a27..0d022d188 100644 --- a/spec/features/verification/sms_spec.rb +++ b/spec/features/verification/sms_spec.rb @@ -57,23 +57,23 @@ feature 'SMS Verification' do expect(URI.parse(current_url).path).to eq(new_residence_path) end - scenario '3 tries allowed' do + scenario '5 tries allowed' do user = create(:user, residence_verified_at: Time.now) login_as(user) visit new_sms_path - 3.times do + 5.times do fill_in 'sms_phone', with: "611111111" click_button 'Send' click_link 'Click here to send the confirmation code again' end - expect(page).to have_content 'You have reached the maximum number of sms verification tries' + expect(page).to have_content "You have reached the maximum number of verification tries. Please try again later." expect(URI.parse(current_url).path).to eq(account_path) visit new_sms_path - expect(page).to have_content 'You have reached the maximum number of sms verification tries' + expect(page).to have_content "You have reached the maximum number of verification tries. Please try again later." expect(URI.parse(current_url).path).to eq(account_path) end diff --git a/spec/models/lock_spec.rb b/spec/models/lock_spec.rb new file mode 100644 index 000000000..0c072e9c8 --- /dev/null +++ b/spec/models/lock_spec.rb @@ -0,0 +1,59 @@ +require 'rails_helper' + +describe Lock do + + let(:lock) { create(:lock) } + + describe "#locked?" do + it "returns true if locked_until is after the current time" do + lock.locked_until = 1.day.from_now + expect(lock.locked?).to be true + end + + it "return false if locked_until is before curren time" do + lock.locked_until = 1.day.ago + expect(lock.locked?).to be false + end + end + + describe "#lock_time" do + it "increases exponentially with number of tries" do + lock.tries = 5 + lock.save + expect(lock.reload.lock_time).to be_between(30.minutes.from_now, 35.minutes.from_now) + + lock.tries = 10 + lock.save + expect(lock.reload.lock_time).to be_between(16.hours.from_now, 18.hours.from_now) + + lock.tries = 15 + lock.save + expect(lock.reload.lock_time).to be_between(21.days.from_now, 23.days.from_now) + end + end + + describe "#too_many_tries?" do + it "returns true if number of tries is multiple of 5" do + lock.tries = 5 + expect(lock.too_many_tries?).to be true + + lock.tries = 10 + expect(lock.too_many_tries?).to be true + + lock.tries = 15 + expect(lock.too_many_tries?).to be true + end + + it "returns false if number of tries is not multiple of 5" do + lock.tries = 0 + expect(lock.too_many_tries?).to be false + + lock.tries = 1 + expect(lock.too_many_tries?).to be false + + lock.tries = 6 + expect(lock.too_many_tries?).to be false + end + end + +end diff --git a/spec/models/residence_spec.rb b/spec/models/residence_spec.rb index 177e8f20d..cb1f91ae0 100644 --- a/spec/models/residence_spec.rb +++ b/spec/models/residence_spec.rb @@ -64,15 +64,14 @@ describe Verification::Residence do describe "tries" do it "should increase tries after a call to the Census" do residence.postal_code = "12345" - - expect { residence.valid? }.to change{ - residence.user.residence_verification_tries }.from(0).to(1) + residence.valid? + expect(residence.user.lock.tries).to eq(1) end it "should not increase tries after a validation error" do residence.postal_code = "" - expect { residence.valid? }.to_not change{ - residence.user.residence_verification_tries } + residence.valid? + expect(residence.user.lock).to be nil end end