From ef7429e27beabca3b8b619019f92ba1240bb569d Mon Sep 17 00:00:00 2001 From: Julia Nguyen Date: Thu, 22 Nov 2018 14:49:18 -0800 Subject: [PATCH] Bug fix: do not prompt password change for Google auth (#1279) --- app/models/concerns/password_validator.rb | 9 ++- app/views/pages/home.html.erb | 6 +- spec/concerns/password_validator_spec.rb | 94 ++++++++++++----------- 3 files changed, 58 insertions(+), 51 deletions(-) diff --git a/app/models/concerns/password_validator.rb b/app/models/concerns/password_validator.rb index e7e9a047b9..cba36afb6a 100644 --- a/app/models/concerns/password_validator.rb +++ b/app/models/concerns/password_validator.rb @@ -6,13 +6,16 @@ module PasswordValidator PASSWORD_VALIDITY_MONTHS = 12 def password_needs_update? - no_histories? || out_dated_password? + return false if google_oauth2_enabled? + + no_histories? || outdated_password? end private - def out_dated_password? - return false unless (updated_on = password_histories.last.try(:created_at)) + def outdated_password? + updated_on = password_histories.last.try(:created_at) + return false unless updated_on (updated_on + PASSWORD_VALIDITY_MONTHS.months) < Time.zone.now end diff --git a/app/views/pages/home.html.erb b/app/views/pages/home.html.erb index 662c484c3f..a6b45e214f 100644 --- a/app/views/pages/home.html.erb +++ b/app/views/pages/home.html.erb @@ -1,8 +1,9 @@ <% if user_signed_in? && @stories.any? %> -<% title t('pages.home.stories') %> + <% title t('pages.home.stories') %> <% elsif user_signed_in? %> -<% title t('pages.home.signed_in_empty.main_message_one', { name: current_user.name }) %> + <% title t('pages.home.signed_in_empty.main_message_one', { name: current_user.name }) %> <% end %> + <% if !user_signed_in? %> <%= render partial: 'not_signed_in' %> <% elsif @stories.any? %> @@ -13,7 +14,6 @@ <% end %> <% end %> - <%= paginate @stories %> <% else %> <%= render partial: 'signed_in_empty' %> diff --git a/spec/concerns/password_validator_spec.rb b/spec/concerns/password_validator_spec.rb index 302e0cb634..9992d6d13b 100644 --- a/spec/concerns/password_validator_spec.rb +++ b/spec/concerns/password_validator_spec.rb @@ -1,9 +1,8 @@ # frozen_string_literal: true describe PasswordValidator, type: :model do + let(:user) { build(:user, password: nil) } describe '#password_validations' do - let(:user) { build(:user, password: nil) } - context 'when password is blank' do it 'throws password error only from devise' do expect(user.valid?).to be false @@ -13,8 +12,10 @@ end end - context 'when oauth is enabled' do - it 'doesnt throw any errors even if the password strength is less' do + context 'when Google auth is enabled' do + let(:user) { build(:user_oauth) } + + it "doesn't throw any errors even if the password strength is less" do user.password = 'warsdasdf' user.token = 'access token' expect(user.valid?).to be true @@ -22,7 +23,7 @@ end context 'when password is valid' do - it 'doesnt throw any errors' do + it "doesn't throw any errors" do user.password = 'waspAr$0' expect(user.valid?).to be true end @@ -33,7 +34,6 @@ ['waspar$0', 'waspaRs0', 'waspar$o', 'WASPAR$0', 'Was$0'].each do |password| user.password = password expect(user.valid?).to be false - expect(user).to have(1).error_on(:password) end end @@ -80,54 +80,58 @@ 'Expected user to be valid if the password is not previous password' end end + end - context 'password_needs_update' do - # As password_histories feature was not present initially, all existing users will not have passoword_histories. - # User who are created after adding this feature will get a password history automatically, after creation. - context 'without any histories' do - before do - allow_any_instance_of(User).to receive(:out_dated_password?) - .and_return(false) - end - - it 'returns true when there are no password_histories' do - allow(user).to receive(:password_histories).and_return([]) - - expect(user.password_needs_update?).to be (true) - end + context '#password_needs_update' do + # As password_histories feature was not present initially, all existing users will not have passoword_histories. + # User who are created after adding this feature will get a password history automatically, after creation. + context 'without any histories' do + before do + allow_any_instance_of(User).to receive(:outdated_password?) + .and_return(false) + end - it 'returns false if there are any password_histories' do - allow(user).to receive(:password_histories).and_return([PasswordHistory.new]) + it 'returns true when there are no password_histories' do + allow(user).to receive(:password_histories).and_return([]) + expect(user.password_needs_update?).to be (true) + end - expect(user.password_needs_update?).to be (false) - end + it 'returns false if there are any password_histories' do + allow(user).to receive(:password_histories).and_return([PasswordHistory.new]) + expect(user.password_needs_update?).to be (false) end + end - context 'with out_dated_password' do - before do - user.password = 'Password@1' - user.save + context 'with Google auth user' do + let(:user) { build(:user_oauth) } - allow_any_instance_of(User).to receive(:no_histories?) - .and_return(false) - end + it 'returns false' do + expect(user.password_needs_update?).to be (false) + end + end - it 'returns true if crossed password_validity months months' do - allow_any_instance_of(PasswordHistory).to receive(:created_at) - .and_return( - Time.now - (PasswordValidator::PASSWORD_VALIDITY_MONTHS.months + 10.minute) - ) + context 'with an outdated password' do + before do + user.password = 'Password@1' + user.save + allow_any_instance_of(User).to receive(:no_histories?) + .and_return(false) + end - expect(user.password_needs_update?).to be (true) - end + it 'returns true when password was updated more than 12 months ago' do + allow_any_instance_of(PasswordHistory).to receive(:created_at) + .and_return( + Time.now - (PasswordValidator::PASSWORD_VALIDITY_MONTHS.months + 10.minute) + ) + expect(user.password_needs_update?).to be (true) + end - it 'returns false if did not cross 5 months' do - allow_any_instance_of(PasswordHistory).to receive(:created_at) - .and_return( - Time.now - (PasswordValidator::PASSWORD_VALIDITY_MONTHS.months - 10.minute) - ) - expect(user.password_needs_update?).to be (false) - end + it 'returns false when password updated less than 12 months ago' do + allow_any_instance_of(PasswordHistory).to receive(:created_at) + .and_return( + Time.now - (PasswordValidator::PASSWORD_VALIDITY_MONTHS.months - 10.minute) + ) + expect(user.password_needs_update?).to be (false) end end end