Fix hole in determining which password to store in credential record

Author: rastating

lib/wpxf/db/credentials.rb

@@ -7,12 +7,11 @@ module Wpxf::Db::Credentials
   # @param password [String] the password.
   # @param type [String] the type of string stored in the password field.
   # @return [Models::Credential] the newly created {Models::Credential}.
-  def store_credentials(username, password = nil, type = 'plain')
+  def store_credentials(username, password = '', type = 'plain')
     credential = Models::Credential.first(
       host: target_host,
       port: target_port,
       username: username,
-      password: nil,
       type: type,
       workspace: active_workspace
     )
@@ -21,10 +20,18 @@ def store_credentials(username, password = nil, type = 'plain')
     credential.host = target_host
     credential.port = target_port
     credential.username = username
-    credential.password = password
+    credential.password = _determine_password_to_store(credential, password)
     credential.type = type
     credential.workspace = active_workspace
 
     credential.save
   end
+
+  private
+
+  def _determine_password_to_store(model, new_password)
+    new_password = '' if new_password.nil?
+    return model.password if new_password == '' && !model.password.nil?
+    new_password
+  end
 end

spec/lib/wpxf/db/credentials_spec.rb

@@ -39,38 +39,59 @@
       expect(credential.type).to eql 'plain'
     end
 
+    it 'should store `nil` passwords as an empty string' do
+      subject.store_credentials 'foo', nil
+      credential = Models::Credential.first
+      expect(credential.password).to_not be_nil
+      expect(credential.password).to eql ''
+    end
+
     context 'if a record with the same host + username + type already exists' do
       context 'and the record is in the same workspace' do
         context 'and has no password' do
           it 'should overwrite the previous credential' do
-            subject.store_credentials 'foo', nil, 'test'
+            subject.store_credentials 'foo', '', 'test'
             subject.store_credentials 'foo', 'bar', 'test'
             expect(Models::Credential.count).to eql 1
 
             credential = Models::Credential.first
             expect(credential.password).to eql 'bar'
-            Models::Credential.truncate
-
-            subject.store_credentials 'foo', 'bar', 'unique'
-            subject.store_credentials 'foo', 'foo', 'unique2'
-            expect(Models::Credential.count).to eql 2
           end
         end
 
         context 'and has a password' do
-          it 'should add a new entry' do
-            Models::Credential.create(
-              host: subject.target_host,
-              port: subject.target_port,
-              username: 'foo',
-              password: 'bar',
-              type: 'plain',
-              workspace: Models::Workspace.first(name: 'default')
-            )
+          context 'if the password is the same' do
+            it 'should overwrite the previous credential' do
+              subject.store_credentials 'foo', 'bar', 'test'
+              subject.store_credentials 'foo', 'bar', 'test'
+              expect(Models::Credential.count).to eql 1
+            end
+          end
 
-            expect(Models::Credential.count).to eql 1
-            subject.store_credentials 'foo', 'foo'
-            expect(Models::Credential.count).to eql 2
+          context 'if the new password is blank' do
+            it 'should not overwrite the existing pasword' do
+              subject.store_credentials 'foo', 'bar', 'test'
+              subject.store_credentials 'foo', '', 'test'
+              expect(Models::Credential.count).to eql 1
+              expect(Models::Credential.first.password).to eql 'bar'
+            end
+          end
+
+          context 'if the password is different' do
+            it 'should add a new entry' do
+              Models::Credential.create(
+                host: subject.target_host,
+                port: subject.target_port,
+                username: 'foo',
+                password: 'bar',
+                type: 'plain',
+                workspace: Models::Workspace.first(name: 'default')
+              )
+
+              expect(Models::Credential.count).to eql 1
+              subject.store_credentials 'foo', 'foo'
+              expect(Models::Credential.count).to eql 2
+            end
           end
         end
       end