Bug fix: do not prompt password change for Google auth (#1279)

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

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 %>
   </div>
-
   <%= paginate @stories %>
 <% else %>
   <%= render partial: 'signed_in_empty' %>

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,16 +12,18 @@
       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
       end
     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