From 93e5f7d0fe569fdde1c3732cea851193a68f10ac Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> Date: Thu, 20 Feb 2025 20:21:50 +0100 Subject: [PATCH] Incorrect Read only connection string on development self-hosted environment (#5426) --- src/Core/Settings/GlobalSettings.cs | 13 +- .../Auth/Services/AuthRequestServiceTests.cs | 11 +- .../Services/SubscriberServiceTests.cs | 5 +- .../LaunchDarklyFeatureServiceTests.cs | 13 +- test/Core.Test/Services/UserServiceTests.cs | 4 +- .../Core.Test/Settings/GlobalSettingsTests.cs | 134 ++++++++++++++++++ .../Tools/Services/SendServiceTests.cs | 6 +- 7 files changed, 168 insertions(+), 18 deletions(-) create mode 100644 test/Core.Test/Settings/GlobalSettingsTests.cs diff --git a/src/Core/Settings/GlobalSettings.cs b/src/Core/Settings/GlobalSettings.cs index a1c7a4fac6..dbfc8543a3 100644 --- a/src/Core/Settings/GlobalSettings.cs +++ b/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 diff --git a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs b/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs index 8feef2facc..5e99ecf171 100644 --- a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs +++ b/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() .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() .PasswordlessAuth - .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings()); + .Returns(new GlobalSettings.PasswordlessAuthSettings()); var updateModel = new AuthRequestUpdateRequestModel { @@ -582,7 +583,7 @@ public class AuthRequestServiceTests sutProvider.GetDependency() .PasswordlessAuth - .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings()); + .Returns(new GlobalSettings.PasswordlessAuthSettings()); sutProvider.GetDependency() .GetByIdentifierAsync(device.Identifier, authRequest.UserId) @@ -736,7 +737,7 @@ public class AuthRequestServiceTests sutProvider.GetDependency() .PasswordlessAuth - .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings()); + .Returns(new GlobalSettings.PasswordlessAuthSettings()); var updateModel = new AuthRequestUpdateRequestModel { @@ -803,7 +804,7 @@ public class AuthRequestServiceTests sutProvider.GetDependency() .PasswordlessAuth - .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings()); + .Returns(new GlobalSettings.PasswordlessAuthSettings()); var updateModel = new AuthRequestUpdateRequestModel { diff --git a/test/Core.Test/Billing/Services/SubscriberServiceTests.cs b/test/Core.Test/Billing/Services/SubscriberServiceTests.cs index 9c25ffdc55..5b7a2cc8bd 100644 --- a/test/Core.Test/Billing/Services/SubscriberServiceTests.cs +++ b/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().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().BaseServiceUri - .Returns(new Settings.GlobalSettings.BaseServiceUriSettings(new Settings.GlobalSettings()) + .Returns(new GlobalSettings.BaseServiceUriSettings(new GlobalSettings()) { CloudRegion = "US" }); diff --git a/test/Core.Test/Services/LaunchDarklyFeatureServiceTests.cs b/test/Core.Test/Services/LaunchDarklyFeatureServiceTests.cs index a2c86b5a76..a173566b88 100644 --- a/test/Core.Test/Services/LaunchDarklyFeatureServiceTests.cs +++ b/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(); diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 88c214f471..880e79500d 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -66,8 +66,8 @@ public class UserServiceTests user.EmailVerified = true; user.Email = userLicense.Email; - sutProvider.GetDependency().SelfHosted = true; - sutProvider.GetDependency().LicenseDirectory = tempDir.Directory; + sutProvider.GetDependency().SelfHosted = true; + sutProvider.GetDependency().LicenseDirectory = tempDir.Directory; sutProvider.GetDependency() .VerifyLicense(userLicense) .Returns(true); diff --git a/test/Core.Test/Settings/GlobalSettingsTests.cs b/test/Core.Test/Settings/GlobalSettingsTests.cs new file mode 100644 index 0000000000..1f5aa494bb --- /dev/null +++ b/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); + } + } +} diff --git a/test/Core.Test/Tools/Services/SendServiceTests.cs b/test/Core.Test/Tools/Services/SendServiceTests.cs index 7ef6f915dd..cabb438b61 100644 --- a/test/Core.Test/Tools/Services/SendServiceTests.cs +++ b/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() + sutProvider.GetDependency() .SelfHosted = true; var badRequest = await Assert.ThrowsAsync(() => @@ -342,7 +344,7 @@ public class SendServiceTests .CanAccessPremium(user) .Returns(true); - sutProvider.GetDependency() + sutProvider.GetDependency() .SelfHosted = false; var badRequest = await Assert.ThrowsAsync(() =>