Skip to content
This repository was archived by the owner on Feb 5, 2025. It is now read-only.

Ability to use complex segments for ManagementBasePath #13

Closed
michaeldimoudis opened this issue Mar 25, 2022 · 4 comments · Fixed by DuendeArchive/BFF#109
Closed

Ability to use complex segments for ManagementBasePath #13

michaeldimoudis opened this issue Mar 25, 2022 · 4 comments · Fixed by DuendeArchive/BFF#109

Comments

@michaeldimoudis
Copy link

Which version of Duende BFF are you using?
None yet, still spiking with latest versions

Which version of .NET are you using?
.NET 6.0

Describe the bug

We would like to overwrite the ManagementBasePath to a complex segment. So instead of it being /bff we want regex, ie: /{path:regex(^[a-zA-Z\\d-]+$)}/bff.

To Reproduce

You can test this out by changing the ManagementBasePath to the below code:

// Add BFF services to DI - also add server-side session management
services.AddBff(options =>
    {
        options.ManagementBasePath = new PathString("/{path:regex(^[a-zA-Z\\d-]+$)}/bff");
    })
    .AddRemoteApis();

After this change calling /admin/bff endpoints do not work.

Expected behavior

That calling /admin/bff endpoints work as per the regex.

The change to make this work is simply updating code in BffEndpointRouteBuilderExtensions.cs from options.SomePath to options.SomePath.Value.

ie:

endpoints.MapGet(options.LoginPath.Value, ProcessWith<ILoginService>);
endpoints.MapGet(options.SilentLoginPath.Value, ProcessWith<ISilentLoginService>);
endpoints.MapGet(options.SilentLoginCallbackPath.Value, ProcessWith<ISilentLoginCallbackService>);
endpoints.MapGet(options.LogoutPath.Value, ProcessWith<ILogoutService>);
etc...

Additional context

I'm wondering if you'd consider this change.

Thanks.

@leastprivilege
Copy link
Member

Why?

@michaeldimoudis
Copy link
Author

Why?

Our tenancy model is a bit unique, this will open up opportunities for us to simplify our deployment and configurations. It's not something we need tomorrow, our spike is done and this is what we found. I just wanted to start the conversation, we'll be deciding on which approach to go in a few months depending if this request is implemented.

@leastprivilege
Copy link
Member

OK - thanks!

We will discuss it.

@brockallen
Copy link
Member

PR submitted. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants