Finbuckle.MultiTenant icon indicating copy to clipboard operation
Finbuckle.MultiTenant copied to clipboard

Should WithBasePathStrategy clear OriginalPathBase?

Open Vizual-Dev opened this issue 2 years ago • 5 comments

Should WithBasePathStrategy clear the OriginalPathBase authentication feature when RebaseAspNetCorePathBase is enabled?

I am attempting to implement external logins using Duende Identity Server with ASP.NET Identity Integration.

I am using the base path strategy with RebaseAspNetCorePathBase enabled. Everything works fine however each time I add a new tenant I have to log into my Google/Facebook/Azure developer account and add a new redirect URI e.g. https://localhost:5001/tenant2/signin-google.

To work around this I am clearing the OriginalPathBase in Challenge.cshtml.

public IActionResult OnGet(string scheme, string returnUrl)
{
  if (string.IsNullOrEmpty(returnUrl)) returnUrl = "~/";

  // validate returnUrl - either it is a valid OIDC URL or back to a local page
  if (Url.IsLocalUrl(returnUrl) == false && _interactionService.IsValidReturnUrl(returnUrl) == false)
  {
    // user might have clicked on a malicious link - should be logged
    throw new Exception("invalid return URL");
  }

  // start challenge and roundtrip the return URL and scheme 
  var props = new AuthenticationProperties
  {
    RedirectUri = Url.Page("/externallogin/callback"),

    Items =
    {
      {"returnUrl", returnUrl},
      {"scheme", scheme}
    }
  };

  // START NEW CODE
  var feature = HttpContext.Features.Get<IAuthenticationFeature>();
  if (feature != null)
  {
    feature.OriginalPathBase = null;
  }
  // END NEW CODE

  return Challenge(props, scheme);
}

This prevents the AuthenticationHandler.BuildRedirectUri method including the tenant in the redirect uri and means that I only need a single https://localhost:5001/signin-google entry in my Google developer console.

Vizual-Dev avatar Jul 05 '23 14:07 Vizual-Dev

Hi, I'm glad you have a good workaround. In your app pipeline does the multitenant or authentication middleware run first?

If multitenant runs first then in theory authentication never sees the base path, but I could see a conflict as you described if authentication runs first.

AndrewTriesToCode avatar Jul 13 '23 00:07 AndrewTriesToCode

Hi, the multitenant middleware appears before authentication in my pipeline:

public static WebApplication ConfigurePipeline(this WebApplication app)
{
  app.UseSerilogRequestLogging();

  if (app.Environment.IsDevelopment())
  {
    app.UseDeveloperExceptionPage();
  }

  app.UseMultiTenant();
  app.UseStaticFiles();
  app.UseRouting();
  app.UseIdentityServer();
  app.UseAuthorization();

  app.MapRazorPages()
    .RequireAuthorization();

  return app;
}

However the ASP.NET Core AuthenticationHandler uses the OriginalPathBase when building the redirect uri as shown here. The outcome is that the redirect uri still contains the tenant identifier.

Vizual-Dev avatar Jul 17 '23 07:07 Vizual-Dev

Thanks or the detail. In thinking this through I think you are doing the right thing and I should probably think of a better way to handle this. The tenant info is embedded in the authentication properties that are passed back to the .../signin-google redirect so the base path shouldn't be needed there at all and yeah you definitely don't want to have to register a different redirect URL for each tenant.

Your workaround is good. Any ideas?

AndrewTriesToCode avatar Jul 19 '23 03:07 AndrewTriesToCode

In the RedirectToIdentityProvider handler you get an opportunity to change the redirect url after the default one was created. You can set this on the OpenIdConnectOptions.

RedirectToIdentityProviderForSignOut too. I don't see any other spots where it would be needed in the OpenIdConnect handler.

AndrewTriesToCode avatar Jul 19 '23 03:07 AndrewTriesToCode