Merge pull request #464 from AyuntamientoMadrid/lock_verification-279

refactors lock tries

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]

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

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

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

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

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

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

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

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"

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"

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

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

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
@@ -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

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á"

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
 

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

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
 

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

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