Incorrect Read only connection string on development self-hosted environment (#5426)

src/Core/Settings/GlobalSettings.cs

@@ -241,7 +241,18 @@ public class GlobalSettings : IGlobalSettings
         public string ConnectionString
         {
             get => _connectionString;
-            set => _connectionString = value.Trim('"');
+            set
+            {
+                // On development environment, the self-hosted overrides would not override the read-only connection string, since it is already set from the non-self-hosted connection string.
+                // This causes a bug, where the read-only connection string is pointing to self-hosted database.
+                if (!string.IsNullOrWhiteSpace(_readOnlyConnectionString) &&
+                    _readOnlyConnectionString == _connectionString)
+                {
+                    _readOnlyConnectionString = null;
+                }
+
+                _connectionString = value.Trim('"');
+            }
         }
 
         public string ReadOnlyConnectionString

test/Core.Test/Auth/Services/AuthRequestServiceTests.cs

@@ -19,6 +19,7 @@ using Bit.Test.Common.AutoFixture.Attributes;
 using Bit.Test.Common.Helpers;
 using NSubstitute;
 using Xunit;
+using GlobalSettings = Bit.Core.Settings.GlobalSettings;
 
 #nullable enable
 
@@ -138,7 +139,7 @@ public class AuthRequestServiceTests
 
         sutProvider.GetDependency<IGlobalSettings>()
             .PasswordlessAuth
-            .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings());
+            .Returns(new GlobalSettings.PasswordlessAuthSettings());
 
         var foundAuthRequest = await sutProvider.Sut.GetValidatedAuthRequestAsync(authRequest.Id, authRequest.AccessCode);
 
@@ -513,7 +514,7 @@ public class AuthRequestServiceTests
 
         sutProvider.GetDependency<IGlobalSettings>()
             .PasswordlessAuth
-            .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings());
+            .Returns(new GlobalSettings.PasswordlessAuthSettings());
 
         var updateModel = new AuthRequestUpdateRequestModel
         {
@@ -582,7 +583,7 @@ public class AuthRequestServiceTests
 
         sutProvider.GetDependency<IGlobalSettings>()
             .PasswordlessAuth
-            .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings());
+            .Returns(new GlobalSettings.PasswordlessAuthSettings());
 
         sutProvider.GetDependency<IDeviceRepository>()
             .GetByIdentifierAsync(device.Identifier, authRequest.UserId)
@@ -736,7 +737,7 @@ public class AuthRequestServiceTests
 
         sutProvider.GetDependency<IGlobalSettings>()
             .PasswordlessAuth
-            .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings());
+            .Returns(new GlobalSettings.PasswordlessAuthSettings());
 
         var updateModel = new AuthRequestUpdateRequestModel
         {
@@ -803,7 +804,7 @@ public class AuthRequestServiceTests
 
         sutProvider.GetDependency<IGlobalSettings>()
             .PasswordlessAuth
-            .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings());
+            .Returns(new GlobalSettings.PasswordlessAuthSettings());
 
         var updateModel = new AuthRequestUpdateRequestModel
         {

test/Core.Test/Billing/Services/SubscriberServiceTests.cs

@@ -19,6 +19,7 @@ using Xunit;
 using static Bit.Core.Test.Billing.Utilities;
 using Address = Stripe.Address;
 using Customer = Stripe.Customer;
+using GlobalSettings = Bit.Core.Settings.GlobalSettings;
 using PaymentMethod = Stripe.PaymentMethod;
 using Subscription = Stripe.Subscription;
 
@@ -1446,7 +1447,7 @@ public class SubscriberServiceTests
             });
 
         sutProvider.GetDependency<IGlobalSettings>().BaseServiceUri
-            .Returns(new Settings.GlobalSettings.BaseServiceUriSettings(new Settings.GlobalSettings())
+            .Returns(new GlobalSettings.BaseServiceUriSettings(new GlobalSettings())
             {
                 CloudRegion = "US"
             });
@@ -1488,7 +1489,7 @@ public class SubscriberServiceTests
             });
 
         sutProvider.GetDependency<IGlobalSettings>().BaseServiceUri
-            .Returns(new Settings.GlobalSettings.BaseServiceUriSettings(new Settings.GlobalSettings())
+            .Returns(new GlobalSettings.BaseServiceUriSettings(new GlobalSettings())
             {
                 CloudRegion = "US"
             });

test/Core.Test/Services/LaunchDarklyFeatureServiceTests.cs

@@ -8,6 +8,7 @@ using Bit.Test.Common.AutoFixture.Attributes;
 using LaunchDarkly.Sdk.Server.Interfaces;
 using NSubstitute;
 using Xunit;
+using GlobalSettings = Bit.Core.Settings.GlobalSettings;
 
 namespace Bit.Core.Test.Services;
 
@@ -41,7 +42,7 @@ public class LaunchDarklyFeatureServiceTests
     [Theory, BitAutoData]
     public void DefaultFeatureValue_WhenSelfHost(string key)
     {
-        var sutProvider = GetSutProvider(new Settings.GlobalSettings { SelfHosted = true });
+        var sutProvider = GetSutProvider(new GlobalSettings { SelfHosted = true });
 
         Assert.False(sutProvider.Sut.IsEnabled(key));
     }
@@ -49,7 +50,7 @@ public class LaunchDarklyFeatureServiceTests
     [Fact]
     public void DefaultFeatureValue_NoSdkKey()
     {
-        var sutProvider = GetSutProvider(new Settings.GlobalSettings());
+        var sutProvider = GetSutProvider(new GlobalSettings());
 
         Assert.False(sutProvider.Sut.IsEnabled(_fakeFeatureKey));
     }
@@ -57,7 +58,7 @@ public class LaunchDarklyFeatureServiceTests
     [Fact(Skip = "For local development")]
     public void FeatureValue_Boolean()
     {
-        var settings = new Settings.GlobalSettings { LaunchDarkly = { SdkKey = _fakeSdkKey } };
+        var settings = new GlobalSettings { LaunchDarkly = { SdkKey = _fakeSdkKey } };
 
         var sutProvider = GetSutProvider(settings);
 
@@ -67,7 +68,7 @@ public class LaunchDarklyFeatureServiceTests
     [Fact(Skip = "For local development")]
     public void FeatureValue_Int()
     {
-        var settings = new Settings.GlobalSettings { LaunchDarkly = { SdkKey = _fakeSdkKey } };
+        var settings = new GlobalSettings { LaunchDarkly = { SdkKey = _fakeSdkKey } };
 
         var sutProvider = GetSutProvider(settings);
 
@@ -77,7 +78,7 @@ public class LaunchDarklyFeatureServiceTests
     [Fact(Skip = "For local development")]
     public void FeatureValue_String()
     {
-        var settings = new Settings.GlobalSettings { LaunchDarkly = { SdkKey = _fakeSdkKey } };
+        var settings = new GlobalSettings { LaunchDarkly = { SdkKey = _fakeSdkKey } };
 
         var sutProvider = GetSutProvider(settings);
 
@@ -87,7 +88,7 @@ public class LaunchDarklyFeatureServiceTests
     [Fact(Skip = "For local development")]
     public void GetAll()
     {
-        var sutProvider = GetSutProvider(new Settings.GlobalSettings());
+        var sutProvider = GetSutProvider(new GlobalSettings());
 
         var results = sutProvider.Sut.GetAll();
 

test/Core.Test/Services/UserServiceTests.cs

@@ -66,8 +66,8 @@ public class UserServiceTests
         user.EmailVerified = true;
         user.Email = userLicense.Email;
 
-        sutProvider.GetDependency<Settings.IGlobalSettings>().SelfHosted = true;
+        sutProvider.GetDependency<IGlobalSettings>().SelfHosted = true;
-        sutProvider.GetDependency<Settings.IGlobalSettings>().LicenseDirectory = tempDir.Directory;
+        sutProvider.GetDependency<IGlobalSettings>().LicenseDirectory = tempDir.Directory;
         sutProvider.GetDependency<ILicensingService>()
             .VerifyLicense(userLicense)
             .Returns(true);

test/Core.Test/Settings/GlobalSettingsTests.cs

@@ -0,0 +1,134 @@
+using Bit.Core.Settings;
+using Xunit;
+
+namespace Bit.Core.Test.Settings;
+
+public class GlobalSettingsTests
+{
+    public class SqlSettingsTests
+    {
+        private const string _testingConnectionString =
+            "Server=server;Database=database;User Id=user;Password=password;";
+
+        private const string _testingReadOnlyConnectionString =
+            "Server=server_read;Database=database_read;User Id=user_read;Password=password_read;";
+
+        [Fact]
+        public void ConnectionString_ValueInDoubleQuotes_Stripped()
+        {
+            var settings = new GlobalSettings.SqlSettings { ConnectionString = $"\"{_testingConnectionString}\"", };
+
+            Assert.Equal(_testingConnectionString, settings.ConnectionString);
+        }
+
+        [Fact]
+        public void ConnectionString_ValueWithoutDoubleQuotes_TheSameValue()
+        {
+            var settings = new GlobalSettings.SqlSettings { ConnectionString = _testingConnectionString };
+
+            Assert.Equal(_testingConnectionString, settings.ConnectionString);
+        }
+
+        [Fact]
+        public void ConnectionString_SetTwice_ReturnsSecondConnectionString()
+        {
+            var settings = new GlobalSettings.SqlSettings { ConnectionString = _testingConnectionString };
+
+            Assert.Equal(_testingConnectionString, settings.ConnectionString);
+
+            var newConnectionString = $"{_testingConnectionString}_new";
+            settings.ConnectionString = newConnectionString;
+
+            Assert.Equal(newConnectionString, settings.ConnectionString);
+        }
+
+        [Fact]
+        public void ReadOnlyConnectionString_ValueInDoubleQuotes_Stripped()
+        {
+            var settings = new GlobalSettings.SqlSettings
+            {
+                ReadOnlyConnectionString = $"\"{_testingReadOnlyConnectionString}\"",
+            };
+
+            Assert.Equal(_testingReadOnlyConnectionString, settings.ReadOnlyConnectionString);
+        }
+
+        [Fact]
+        public void ReadOnlyConnectionString_ValueWithoutDoubleQuotes_TheSameValue()
+        {
+            var settings = new GlobalSettings.SqlSettings
+            {
+                ReadOnlyConnectionString = _testingReadOnlyConnectionString
+            };
+
+            Assert.Equal(_testingReadOnlyConnectionString, settings.ReadOnlyConnectionString);
+        }
+
+        [Fact]
+        public void ReadOnlyConnectionString_NotSet_DefaultsToConnectionString()
+        {
+            var settings = new GlobalSettings.SqlSettings { ConnectionString = _testingConnectionString };
+
+            Assert.Equal(_testingConnectionString, settings.ReadOnlyConnectionString);
+        }
+
+        [Fact]
+        public void ReadOnlyConnectionString_Set_ReturnsReadOnlyConnectionString()
+        {
+            var settings = new GlobalSettings.SqlSettings
+            {
+                ConnectionString = _testingConnectionString,
+                ReadOnlyConnectionString = _testingReadOnlyConnectionString
+            };
+
+            Assert.Equal(_testingReadOnlyConnectionString, settings.ReadOnlyConnectionString);
+        }
+
+        [Fact]
+        public void ReadOnlyConnectionString_SetTwice_ReturnsSecondReadOnlyConnectionString()
+        {
+            var settings = new GlobalSettings.SqlSettings
+            {
+                ConnectionString = _testingConnectionString,
+                ReadOnlyConnectionString = _testingReadOnlyConnectionString
+            };
+
+            Assert.Equal(_testingReadOnlyConnectionString, settings.ReadOnlyConnectionString);
+
+            var newReadOnlyConnectionString = $"{_testingReadOnlyConnectionString}_new";
+            settings.ReadOnlyConnectionString = newReadOnlyConnectionString;
+
+            Assert.Equal(newReadOnlyConnectionString, settings.ReadOnlyConnectionString);
+        }
+
+        [Fact]
+        public void ReadOnlyConnectionString_NotSetAndConnectionStringSetTwice_ReturnsSecondConnectionString()
+        {
+            var settings = new GlobalSettings.SqlSettings { ConnectionString = _testingConnectionString };
+
+            Assert.Equal(_testingConnectionString, settings.ReadOnlyConnectionString);
+
+            var newConnectionString = $"{_testingConnectionString}_new";
+            settings.ConnectionString = newConnectionString;
+
+            Assert.Equal(newConnectionString, settings.ReadOnlyConnectionString);
+        }
+
+        [Fact]
+        public void ReadOnlyConnectionString_SetAndConnectionStringSetTwice_ReturnsReadOnlyConnectionString()
+        {
+            var settings = new GlobalSettings.SqlSettings
+            {
+                ConnectionString = _testingConnectionString,
+                ReadOnlyConnectionString = _testingReadOnlyConnectionString
+            };
+
+            Assert.Equal(_testingReadOnlyConnectionString, settings.ReadOnlyConnectionString);
+
+            var newConnectionString = $"{_testingConnectionString}_new";
+            settings.ConnectionString = newConnectionString;
+
+            Assert.Equal(_testingReadOnlyConnectionString, settings.ReadOnlyConnectionString);
+        }
+    }
+}

test/Core.Test/Tools/Services/SendServiceTests.cs

@@ -24,6 +24,8 @@ using Microsoft.AspNetCore.Identity;
 using NSubstitute;
 using Xunit;
 
+using GlobalSettings = Bit.Core.Settings.GlobalSettings;
+
 namespace Bit.Core.Test.Tools.Services;
 
 [SutProviderCustomize]
@@ -309,7 +311,7 @@ public class SendServiceTests
             .CanAccessPremium(user)
             .Returns(true);
 
-        sutProvider.GetDependency<Settings.GlobalSettings>()
+        sutProvider.GetDependency<GlobalSettings>()
             .SelfHosted = true;
 
         var badRequest = await Assert.ThrowsAsync<BadRequestException>(() =>
@@ -342,7 +344,7 @@ public class SendServiceTests
             .CanAccessPremium(user)
             .Returns(true);
 
-        sutProvider.GetDependency<Settings.GlobalSettings>()
+        sutProvider.GetDependency<GlobalSettings>()
             .SelfHosted = false;
 
         var badRequest = await Assert.ThrowsAsync<BadRequestException>(() =>