Dotmim.Sync: Switching server database at runtime exposes data to wrong user

Our customers are companies and each company has their own database on our server. We have rolled out a version of our app that uses DMS to sync SQLite on the client to their database via https.

The client passes a token in the header httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue(...

The sync client is configured WebClientOrchestrator webClientOrchestrator = new WebClientOrchestrator(serviceUri, client: httpClient, maxDownladingDegreeOfParallelism: 4);

On the server the SyncController uses the token to lookup which database to use for that user.

string databaseName = DatabaseHelper.GetDatabaseName(token);
SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(orchestrator.Provider.ConnectionString);
builder.InitialCatalog = databaseName;
orchestrator.Provider.ConnectionString = builder.ConnectionString;

We’ve rolled out this version of our app to less than 100 customers as beta testers. We’ve had one report of a customer seeing some data that was not theirs, mixed in with theirs. We immediately shut down the beta test.

Where is the flaw in our strategy for switching databases at runtime. Is there recommended practice?

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Comments: 18 (17 by maintainers)

Most upvoted comments

We’ve released this beta to 50+ beta testers and the issue does appear to be resolved. Thank you.

We’d like to incorporate this package into one of our commercial releases. When do you expect this fix to be out of beta?

I have changed the way the server is handling information for each client.

Breaking changes

  • The server code has changed.
  • The client code has not changed and is still compatible

I’ve removed the WebServerManager instance, and I’m injecting directly a WebServerOrchestrator in the pipeline. Be careful, the code on the controller is slightly different (see example below)

I’ve removed all the server cache usages, and replace them with the client session.

And for your particular need, I’ve changed a little bit the WebServerOrchestrator to let you create one dynamically in the controller.

So far, if you want to test it, just clone the last commit from the master branch and test it.

It’s not mandatory to migrate your clients, since the changes are only occurring on the server side (if your clients are already on version v0.8.0

Since we are using now the session, you need to add the session service and middleware. See here for more info : https://docs.microsoft.com/en-us/aspnet/core/fundamentals/app-state?view=aspnetcore-5.0#configure-session-state.

Sample

Startup.cs:

public void ConfigureServices(IServiceCollection services)
{
    services.AddControllers();

    services.AddDistributedMemoryCache();
    services.AddSession(options =>
    {
        options.IdleTimeout = TimeSpan.FromMinutes(30);
    });
}

Be careful, order is important here : UseSession is AFTER UseRouting and UseAuthorization and BEFORE UseEndpoints

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
{
    if (env.IsDevelopment())
    {
        app.UseDeveloperExceptionPage();
    }
    app.UseHttpsRedirection();
    app.UseRouting();
    app.UseAuthorization();
    app.UseSession();
    app.UseEndpoints(endpoints =>
    {
        endpoints.MapControllers();
    });
}

The SyncController class has changed too:

[ApiController]
[Route("api/[controller]")]
public class SyncController : ControllerBase
{
    public WebServerOrchestrator WebServerOrchestrator { get; }

    // Injected thanks to Dependency Injection
    public SyncController(WebServerOrchestrator webServerOrchestrator) 
          => this.WebServerOrchestrator = webServerOrchestrator;

    /// <summary>
    /// This POST handler is mandatory to handle all the sync process
    /// </summary>
    /// <returns></returns>
    [HttpPost]
    public Task Post() => WebServerOrchestrator.HandleRequestAsync(this.HttpContext);

    /// <summary>
    /// This GET handler is optional. It allows you to see the configuration hosted on the server
    /// </summary>
    [HttpGet]
    public Task Get() => WebServerManager.WriteHelloAsync(this.HttpContext, WebServerOrchestrator);

}

And for your particular use case, where you want to create a new WebServerOrchestrator for each request, with a different connection string:

SyncController.cs:

public SyncController(IConfiguration configuration)
{
    this.Configuration = configuration;
}

[HttpPost]
public async Task Post()
{
    var connectionString = Configuration.GetSection("ConnectionStrings")["SqlConnection"];

    var tables = new string[] {"ProductCategory", "ProductModel", "Product",
    "Address", "Customer", "CustomerAddress", "SalesOrderHeader", "SalesOrderDetail" };
    var provider = new SqlSyncChangeTrackingProvider(connectionString);

    var orchestrator = new WebServerOrchestrator(provider, tables);

    await orchestrator.HandleRequestAsync(this.HttpContext);
}

Let me know if it’s resolving your issue. It will be quite complicated for you, I know, since you have to redeploy and retests with 100 customers, but it’s complicated for me as well since I’m not able to reproduce your bug, and once again, you don’t share a lot of your code to help me figure out what can be the issue in your code…

With this fix, I’m almost 100% sure that nothing is shared between sessions.

@Mimetis i think that what @VagueGit is doing differently is they are using ONE WebAPI to handle request from different customers/companies/subscribers and then when the central WebAPI receives the sync request it tries to connect to a DIFFERENT central database depending on who the requesting customer/company/subscriber is, this is different from the idea of having a separate WebAPI for each customer/company

Can dotmimsync in principle cater for sharing ONE WebAPI between different customers/companies?

I’m trying to fix some weird bugs, and be sure all the tests are OK Once it’s done, I will probably make a new version in nuget I don’t know, maybe next week

@VagueGit Can you post more code please?

// it looks like you are re-using some ocrhestrator here?
orchestrator.Provider.ConnectionString = builder.ConnectionString;

Please make sure that you create one orchestrator per request. Also, ensure, that the orchestrator is registered as PER REQUEST instance (if you are using dependency injection)

We are still running a very old version of DMS, that had many more flaws, but this never occurred to us