From b493503e09499eee3247a0f1b371b34cba244ea3 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Thu, 25 Apr 2019 10:07:48 -0700
Subject: [PATCH] Scrub SMB credential information from folder configuration

This prevents credential information from being persisted into the
local data directory which is used during subsequent runs to determine
folder definition changes.
---
 plugins/synced_folders/smb/synced_folder.rb   | 21 ++++++
 .../synced_folders/smb/synced_folder_test.rb  | 71 +++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/plugins/synced_folders/smb/synced_folder.rb b/plugins/synced_folders/smb/synced_folder.rb
index f382c36c9..f6b669f13 100644
--- a/plugins/synced_folders/smb/synced_folder.rb
+++ b/plugins/synced_folders/smb/synced_folder.rb
@@ -152,6 +152,15 @@ module VagrantPlugins
             guest: data[:guestpath].to_s))
           machine.guest.capability(
             :mount_smb_shared_folder, data[:smb_id], data[:guestpath], data)
+
+          clean_folder_configuration(data)
+        end
+      end
+
+      # Nothing to do here but ensure folder options are scrubbed
+      def disable(machine, folders, opts)
+        folders.each do |_, data|
+          clean_folder_configuration(data)
         end
       end
 
@@ -160,6 +169,18 @@ module VagrantPlugins
           machine.env.host.capability(:smb_cleanup, machine, opts)
         end
       end
+
+      protected
+
+      # Remove data that should not be persisted within folder
+      # specific configuration
+      #
+      # @param [Hash] data Folder configuration
+      def clean_folder_configuration(data)
+        return if !data.is_a?(Hash)
+        data.delete(:smb_password)
+        nil
+      end
     end
   end
 end
diff --git a/test/unit/plugins/synced_folders/smb/synced_folder_test.rb b/test/unit/plugins/synced_folders/smb/synced_folder_test.rb
index 493a427f5..e3383519d 100644
--- a/test/unit/plugins/synced_folders/smb/synced_folder_test.rb
+++ b/test/unit/plugins/synced_folders/smb/synced_folder_test.rb
@@ -229,6 +229,11 @@ describe VagrantPlugins::SyncedFolderSMB::SyncedFolder do
         expect(folders["/second/path"][:smb_host]).to eq("ADDR")
       end
 
+      it "should scrub folder configuration" do
+        expect(subject).to receive(:clean_folder_configuration).at_least(:once)
+        subject.enable(machine, folders, options)
+      end
+
       context "with smb_host option set" do
         let(:folders){ {"/first/path" => {smb_host: "ADDR"}, "/second/path" => {}} }
 
@@ -251,6 +256,59 @@ describe VagrantPlugins::SyncedFolderSMB::SyncedFolder do
           expect(folders["/second/path"][:group]).to eq("smbgroup")
         end
       end
+
+      context "with smb_username and smb_password set" do
+        let(:folders){ {
+          "/first/path" => {owner: "smbowner", smb_username: "user", smb_password: "pass"},
+          "/second/path" => {group: "smbgroup", smb_username: "user", smb_password: "pass"}
+        } }
+
+        it "should retain non password configuration options" do
+          subject.enable(machine, folders, options)
+          folder1 = folders["/first/path"]
+          folder2 = folders["/second/path"]
+          expect(folder1.key?(:owner)).to be_truthy
+          expect(folder1.key?(:smb_username)).to be_truthy
+          expect(folder2.key?(:group)).to be_truthy
+          expect(folder2.key?(:smb_username)).to be_truthy
+        end
+
+        it "should remove the smb_password option when set" do
+          subject.enable(machine, folders, options)
+          expect(folders["/first/path"].key?(:smb_password)).to be_falsey
+          expect(folders["/second/path"].key?(:smb_password)).to be_falsey
+        end
+      end
+    end
+  end
+
+  describe "#disable" do
+    it "should scrub folder configuration" do
+      expect(subject).to receive(:clean_folder_configuration).at_least(:once)
+      subject.disable(machine, folders, options)
+    end
+
+    context "with smb_username and smb_password set" do
+      let(:folders){ {
+        "/first/path" => {owner: "smbowner", smb_username: "user", smb_password: "pass"},
+        "/second/path" => {group: "smbgroup", smb_username: "user", smb_password: "pass"}
+      } }
+
+      it "should retain non password configuration options" do
+        subject.disable(machine, folders, options)
+        folder1 = folders["/first/path"]
+        folder2 = folders["/second/path"]
+        expect(folder1.key?(:owner)).to be_truthy
+        expect(folder1.key?(:smb_username)).to be_truthy
+        expect(folder2.key?(:group)).to be_truthy
+        expect(folder2.key?(:smb_username)).to be_truthy
+      end
+
+      it "should remove the smb_password option when set" do
+        subject.disable(machine, folders, options)
+        expect(folders["/first/path"].key?(:smb_password)).to be_falsey
+        expect(folders["/second/path"].key?(:smb_password)).to be_falsey
+      end
     end
   end
 
@@ -270,4 +328,17 @@ describe VagrantPlugins::SyncedFolderSMB::SyncedFolder do
       end
     end
   end
+
+  describe "#clean_folder_configuration" do
+    it "should remove smb_password if defined" do
+      data = {smb_password: "password"}
+      subject.send(:clean_folder_configuration, data)
+      expect(data.key?(:smb_password)).to be_falsey
+    end
+
+    it "should not error if non-hash value provided" do
+      expect { subject.send(:clean_folder_configuration, nil) }.
+        not_to raise_error
+    end
+  end
 end