Add IsKerberos field to FileSystemId

This change adds an additional encoded field to the FileSystemId string
for Smb Mounts when mounting with ChromAD Kerberos. This encodes
whether the share was initially mounted using ChromAD Kerberos and
allows us to remount ChromAD Kerberos users' shares
automatically.

Bug: chromium:757625
Test: unittests
Change-Id: Ic22e9bb6f26e5b526984388785b1ae9b1d8ca1fa
Reviewed-on: https://chromium-review.googlesource.com/1093954
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568268}

chrome/browser/chromeos/smb_client/smb_file_system_id.cc

@@ -9,46 +9,66 @@
 #include "base/files/file_path.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 
 namespace chromeos {
 namespace smb_client {
 namespace {
 
 constexpr char kDelimiter[] = "@@";
-constexpr size_t kDelimiterLen = arraysize(kDelimiter) - 1;
+constexpr char kKerberosSymbol[] = "kerberos_chromad";
 
-size_t GetDelimLocation(const std::string& file_system_id) {
-  const size_t delim_location = file_system_id.find(kDelimiter);
-  DCHECK(delim_location != std::string::npos);
+std::vector<std::string> GetComponents(const std::string& file_system_id) {
+  const std::vector<std::string> components =
+      SplitString(file_system_id, kDelimiter, base::TRIM_WHITESPACE,
+                  base::SPLIT_WANT_NONEMPTY);
 
-  return delim_location;
+  DCHECK_GE(components.size(), 2u);
+  DCHECK_LE(components.size(), 3u);
+
+  return components;
 }
 
 }  // namespace.
 
 std::string CreateFileSystemId(int32_t mount_id,
                                const base::FilePath& share_path) {
-  return base::StrCat(
+  return CreateFileSystemId(mount_id, share_path, false /* is_kerberos */);
+}
+
+std::string CreateFileSystemId(int32_t mount_id,
+                               const base::FilePath& share_path,
+                               bool is_kerberos_chromad) {
+  const std::string file_system_id = base::StrCat(
       {base::NumberToString(mount_id), kDelimiter, share_path.value()});
+  if (is_kerberos_chromad) {
+    return base::StrCat({file_system_id, kDelimiter, kKerberosSymbol});
+  }
+  return file_system_id;
 }
 
 int32_t GetMountIdFromFileSystemId(const std::string& file_system_id) {
-  const size_t delim_location = GetDelimLocation(file_system_id);
-
-  const std::string mount_id_string = file_system_id.substr(0, delim_location);
+  const std::vector<std::string> components = GetComponents(file_system_id);
+  DCHECK_GE(components.size(), 1u);
 
   int32_t mount_id;
-  bool result = base::StringToInt(mount_id_string, &mount_id);
+  const bool result = base::StringToInt(components[0], &mount_id);
   DCHECK(result);
 
   return mount_id;
 }
 
 base::FilePath GetSharePathFromFileSystemId(const std::string& file_system_id) {
-  const size_t delim_location = GetDelimLocation(file_system_id);
-  const size_t share_path_start = delim_location + kDelimiterLen;
+  const std::vector<std::string> components = GetComponents(file_system_id);
+  DCHECK_GE(components.size(), 1u);
+
+  return base::FilePath(components[1]);
+}
+
+bool IsKerberosChromadFileSystemId(const std::string& file_system_id) {
+  const std::vector<std::string> components = GetComponents(file_system_id);
 
-  return base::FilePath(file_system_id.substr(share_path_start));
+  return components.size() >= 3 && components[2] == kKerberosSymbol;
 }
 
 }  // namespace smb_client

chrome/browser/chromeos/smb_client/smb_file_system_id.h

@@ -13,7 +13,10 @@ namespace chromeos {
 namespace smb_client {
 
 // Creates a FileSystemId by concatenating |mount_id| and |share_path| with a
-// delimiter.
+// delimiter. If |is_kerberos_chromad| is set, an additional symbol is appended.
+std::string CreateFileSystemId(int32_t mount_id,
+                               const base::FilePath& share_path,
+                               bool is_kerberos_chromad);
 std::string CreateFileSystemId(int32_t mount_id,
                                const base::FilePath& share_path);
 
@@ -25,6 +28,10 @@ int32_t GetMountIdFromFileSystemId(const std::string& file_system_id);
 // be well-formed (e.g. 2@@smb://192.168.1.1/testShare).
 base::FilePath GetSharePathFromFileSystemId(const std::string& file_system_id);
 
+// Returns whether |file_system_id| corresponds to a share that was mounted
+// using ChromAD Kerberos.
+bool IsKerberosChromadFileSystemId(const std::string& file_system_id);
+
 }  // namespace smb_client
 }  // namespace chromeos
 

chrome/browser/chromeos/smb_client/smb_file_system_id_test.cc

@@ -26,20 +26,41 @@ TEST_F(SmbFileSystemIdTest, ShouldCreateFileSystemIdCorrectly) {
   const int32_t mount_id = 12;
 
   EXPECT_EQ("12@@smb://192.168.0.0/test",
-            CreateFileSystemId(mount_id, share_path));
+            CreateFileSystemId(mount_id, share_path, false /* is_kerberos */));
+  EXPECT_EQ("12@@smb://192.168.0.0/test@@kerberos_chromad",
+            CreateFileSystemId(mount_id, share_path, true /* is_kerberos */));
 }
 
 TEST_F(SmbFileSystemIdTest, ShouldParseMountIdCorrectly) {
-  const std::string file_system_id = "12@@smb://192.168.0.0/test";
+  const std::string file_system_id_1 = "12@@smb://192.168.0.0/test";
+  const std::string file_system_id_2 =
+      "13@@smb://192.168.0.1/test@@kerberos_chromad";
 
-  EXPECT_EQ(12, GetMountIdFromFileSystemId(file_system_id));
+  EXPECT_EQ(12, GetMountIdFromFileSystemId(file_system_id_1));
+  EXPECT_EQ(13, GetMountIdFromFileSystemId(file_system_id_2));
 }
 
 TEST_F(SmbFileSystemIdTest, ShouldParseSharePathCorrectly) {
-  const std::string file_system_id = "12@@smb://192.168.0.0/test";
-  const base::FilePath expected_share_path("smb://192.168.0.0/test");
+  const std::string file_system_id_1 = "12@@smb://192.168.0.0/test";
+  const base::FilePath expected_share_path_1("smb://192.168.0.0/test");
 
-  EXPECT_EQ(expected_share_path, GetSharePathFromFileSystemId(file_system_id));
+  const std::string file_system_id_2 =
+      "13@@smb://192.168.0.1/test@@kerberos_chromad";
+  const base::FilePath expected_share_path_2("smb://192.168.0.1/test");
+
+  EXPECT_EQ(expected_share_path_1,
+            GetSharePathFromFileSystemId(file_system_id_1));
+  EXPECT_EQ(expected_share_path_2,
+            GetSharePathFromFileSystemId(file_system_id_2));
+}
+
+TEST_F(SmbFileSystemIdTest, IsKerberosChromadReturnsCorrectly) {
+  const std::string kerberos_file_system_id =
+      "13@@smb://192.168.0.1/test@@kerberos_chromad";
+  const std::string non_kerberos_file_system_id = "12@@smb://192.168.0.0/test";
+
+  EXPECT_TRUE(IsKerberosChromadFileSystemId(kerberos_file_system_id));
+  EXPECT_FALSE(IsKerberosChromadFileSystemId(non_kerberos_file_system_id));
 }
 
 }  // namespace smb_client