smbfs: Create a Mojo method to delete any stored credentials
When the user explicitly removes a SMB share, we need to clean up any
stored state. There is no explicit 'umount' request made to smbfs when
the share is removed, so add a Mojo method to ask for any files to be
deleted.
BUG=chromium:1054705
TEST=cros_run_unit_tests --board=chell --packages smbfs
TEST=Add a password-protected share using files app, look in
/run/daemon-store/smbfs for password file. Remove share using files
app, and see password file in /run/daemon-store/smbfs is deleted.
Change-Id: If791a9fbd7504f562cd9be090ff0ee1ea7607e6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2169117
Reviewed-by: Sergei Datsenko <dats@chromium.org>
Tested-by: Anand Mistry <amistry@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
diff --git a/smbfs/mojo_session.cc b/smbfs/mojo_session.cc
index 8da0aed..c0c2049 100644
--- a/smbfs/mojo_session.cc
+++ b/smbfs/mojo_session.cc
@@ -105,6 +105,10 @@
kerberos_sync_->SetupKerberos(std::move(callback));
}
+void MojoSession::OnPasswordFilePathSet(const base::FilePath& path) {
+ password_file_path_ = path;
+}
+
std::unique_ptr<SmbFilesystem> MojoSession::CreateSmbFilesystem(
SmbFilesystem::Options options) {
options.uid = uid_;
@@ -125,8 +129,8 @@
DCHECK(!fuse_session_);
DCHECK(chan_);
- smbfs_impl_ =
- std::make_unique<SmbFsImpl>(fs->GetWeakPtr(), std::move(smbfs_request));
+ smbfs_impl_ = std::make_unique<SmbFsImpl>(
+ fs->GetWeakPtr(), std::move(smbfs_request), password_file_path_);
smbfs_delegate_ = std::move(delegate_ptr);
smbfs_delegate_.set_connection_error_handler(
base::BindOnce(&MojoSession::DoShutdown, base::Unretained(this)));
diff --git a/smbfs/mojo_session.h b/smbfs/mojo_session.h
index 124a7cb..cb4a249 100644
--- a/smbfs/mojo_session.h
+++ b/smbfs/mojo_session.h
@@ -47,6 +47,7 @@
// SmbFsBootstrapImpl::Delegate overrides.
void SetupKerberos(mojom::KerberosConfigPtr kerberos_config,
base::OnceCallback<void(bool success)> callback) override;
+ void OnPasswordFilePathSet(const base::FilePath& path) override;
// SmbFilesystem::Delegate overrides.
void RequestCredentials(RequestCredentialsCallback callback) override;
@@ -78,6 +79,7 @@
base::OnceClosure shutdown_callback_;
std::unique_ptr<SmbFsBootstrapImpl> bootstrap_impl_;
+ base::FilePath password_file_path_;
std::unique_ptr<FuseSession> fuse_session_;
std::unique_ptr<KerberosArtifactSynchronizer> kerberos_sync_;
std::unique_ptr<SmbFsImpl> smbfs_impl_;
diff --git a/smbfs/mojom/smbfs.mojom b/smbfs/mojom/smbfs.mojom
index c270d2f..fe108f6 100644
--- a/smbfs/mojom/smbfs.mojom
+++ b/smbfs/mojom/smbfs.mojom
@@ -25,6 +25,8 @@
// Implemented by SmbFs, used from Chrome.
interface SmbFs {
+ // Deletes any credentials stored for this share mount.
+ RemoveSavedCredentials() => (bool success);
};
// Implemented by Chrome, used from SmbFs.
diff --git a/smbfs/smbfs_bootstrap_impl.cc b/smbfs/smbfs_bootstrap_impl.cc
index a4e13bc..cffe291 100644
--- a/smbfs/smbfs_bootstrap_impl.cc
+++ b/smbfs/smbfs_bootstrap_impl.cc
@@ -208,6 +208,7 @@
pass_file_path = GetUserDaemonStoreDirectory(
options->credential_storage_options->account_hash)
.Append(pass_file_name);
+ delegate_->OnPasswordFilePathSet(pass_file_path);
if (credential->password) {
// We obfuscate the password now because |credential| is moved into the
diff --git a/smbfs/smbfs_bootstrap_impl.h b/smbfs/smbfs_bootstrap_impl.h
index 7e12637..5b86a5c 100644
--- a/smbfs/smbfs_bootstrap_impl.h
+++ b/smbfs/smbfs_bootstrap_impl.h
@@ -32,6 +32,11 @@
virtual void SetupKerberos(
mojom::KerberosConfigPtr kerberos_config,
base::OnceCallback<void(bool success)> callback) = 0;
+
+ // Observer for when the password file path has been determined. The file
+ // at |path| may not exist at this point, but it will exist if bootstrap
+ // completes successfully.
+ virtual void OnPasswordFilePathSet(const base::FilePath& path) = 0;
};
// Factory function to create an SmbFilesystem instance.
diff --git a/smbfs/smbfs_bootstrap_impl_test.cc b/smbfs/smbfs_bootstrap_impl_test.cc
index c4917d5..076fc05 100644
--- a/smbfs/smbfs_bootstrap_impl_test.cc
+++ b/smbfs/smbfs_bootstrap_impl_test.cc
@@ -67,6 +67,7 @@
(mojom::KerberosConfigPtr,
base::OnceCallback<void(bool success)>),
(override));
+ MOCK_METHOD(void, OnPasswordFilePathSet, (const base::FilePath&), (override));
};
class MockSmbFsDelegate : public mojom::SmbFsDelegate {
@@ -514,6 +515,10 @@
.WillOnce(Return(SmbFilesystem::ConnectError::kOk));
return fs;
});
+ EXPECT_CALL(mock_delegate_, OnPasswordFilePathSet(_))
+ .WillOnce([user_directory](const base::FilePath& path) {
+ EXPECT_TRUE(user_directory.IsParent(path));
+ });
SmbFsBootstrapImpl boostrap_impl(mojo::MakeRequest(&boostrap_ptr),
fs_factory, &mock_delegate_,
daemon_store_dir_.GetPath());
@@ -562,6 +567,10 @@
.WillOnce(Return(SmbFilesystem::ConnectError::kOk));
return fs;
});
+ EXPECT_CALL(mock_delegate_, OnPasswordFilePathSet(_))
+ .WillOnce([user_directory](const base::FilePath& path) {
+ EXPECT_TRUE(user_directory.IsParent(path));
+ });
SmbFsBootstrapImpl boostrap_impl(mojo::MakeRequest(&boostrap_ptr),
fs_factory, &mock_delegate_,
daemon_store_dir_.GetPath());
diff --git a/smbfs/smbfs_impl.cc b/smbfs/smbfs_impl.cc
index 548ade4..534f312 100644
--- a/smbfs/smbfs_impl.cc
+++ b/smbfs/smbfs_impl.cc
@@ -6,16 +6,32 @@
#include <utility>
+#include <base/files/file_util.h>
#include <base/logging.h>
namespace smbfs {
SmbFsImpl::SmbFsImpl(base::WeakPtr<SmbFilesystem> fs,
- mojom::SmbFsRequest request)
- : fs_(fs), binding_(this, std::move(request)) {
+ mojom::SmbFsRequest request,
+ const base::FilePath& password_file_path)
+ : fs_(fs),
+ binding_(this, std::move(request)),
+ password_file_path_(password_file_path) {
DCHECK(fs_);
}
SmbFsImpl::~SmbFsImpl() = default;
+void SmbFsImpl::RemoveSavedCredentials(
+ RemoveSavedCredentialsCallback callback) {
+ if (password_file_path_.empty()) {
+ std::move(callback).Run(true /* success */);
+ return;
+ }
+
+ bool success = base::DeleteFile(password_file_path_, false /* recursive */);
+ LOG_IF(WARNING, !success) << "Unable to erase credential file";
+ std::move(callback).Run(success);
+}
+
} // namespace smbfs
diff --git a/smbfs/smbfs_impl.h b/smbfs/smbfs_impl.h
index 610ba2c..d1517ba 100644
--- a/smbfs/smbfs_impl.h
+++ b/smbfs/smbfs_impl.h
@@ -5,6 +5,7 @@
#ifndef SMBFS_SMBFS_IMPL_H_
#define SMBFS_SMBFS_IMPL_H_
+#include <base/files/file_path.h>
#include <base/macros.h>
#include <base/memory/weak_ptr.h>
#include <mojo/public/cpp/bindings/binding.h>
@@ -20,12 +21,17 @@
class SmbFsImpl : public mojom::SmbFs {
public:
explicit SmbFsImpl(base::WeakPtr<SmbFilesystem> fs,
- mojom::SmbFsRequest request);
+ mojom::SmbFsRequest request,
+ const base::FilePath& password_file_path);
~SmbFsImpl() override;
private:
+ // mojom::SmbFs overrides.
+ void RemoveSavedCredentials(RemoveSavedCredentialsCallback callback) override;
+
base::WeakPtr<SmbFilesystem> fs_;
mojo::Binding<mojom::SmbFs> binding_;
+ const base::FilePath password_file_path_;
DISALLOW_IMPLICIT_CONSTRUCTORS(SmbFsImpl);
};
