Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

MapMiddleware Map function adds filter to base path, breaking compat with previous version #354

Closed
wizarrc opened this issue Jul 24, 2015 · 11 comments
Labels

Comments

@wizarrc
Copy link

wizarrc commented Jul 24, 2015

Looks like the initial port from issue #20 is not backwards compatible. It adds the PathMatch to the PathBase. This seems to break everything MVC based from my experience. I think it would make sense to add the PathMatch to path instead.

context.Request.PathBase = pathBase + _options.PathMatch;
context.Request.Path = remainingPath;

Was this intended? Is there another way to setup MVC with this pathbase containing the match? Anyways, I was able to get around it with mapwhen and check the request path manually each time.

@muratg
Copy link

muratg commented Jul 24, 2015

@wizarrc you mean not backwards compatible with MVC ASP.NET 4? (MVC 5)

@wizarrc
Copy link
Author

wizarrc commented Jul 24, 2015

@muratg no, I was referring to it not working with MVC 6 where it worked in MVC 5. I had logic in place to apply app.UseMvc inside an app.Map call where it causes all sorts of issues with routing. When a UseMVC call is inside a Map function, all my routing is changed to be a base of the matching path. This behavior never happened in MVC 5. If I apply UseMvc outside of the Map function, routing works fine, except it breaks my goal of trying to apply a different authentication method to different map paths. i.e. Windows Integrated vs Identity

For each map call, I setup different authentication, but I ran into problems setting up Identity after the UseMVC routes are setup.

For example:

public IServiceProvider ConfigureServices(IServiceCollection services)
{
             services.AddMvc();
             services.AddIdentity<ApplicationUser, IdentityRole>(configureOptions =>
            {
...
            }
}
public void Configure(IApplicationBuilder app)
{
...
            app.Map("/Training/Admin", mapApp => MappedMVCRegistration(mapApp));
            app.Map("/Training", mapApp => MappedTrainingCookieConfiguration(mapApp));
            MappedMVCRegistration(app);
}
private void MappedTrainingCookieConfiguration(IApplicationBuilder app)
{
            app.UseIdentity();
            app.UseCookieAuthentication(options =>
            {
...
            }
...
            MappedMVCRegistration(app);
}

private void MappedMVCRegistration(IApplicationBuilder app)
{
            app.UseMvc(routes =>
            {
...
            }
}

The first one would have Windows Integrated Authentication and the second one would use Identity.

I got everything working by replacing my app.Map commands with:

app.MapWhen(y => y.Request.Path.StartsWithSegments("/Training/Admin"), mapApp => MappedMVCRegistration(mapApp));
app.MapWhen(y => y.Request.Path.StartsWithSegments("/Training"), mapApp => MappedTrainingCookieConfiguration(mapApp));

@wizarrc
Copy link
Author

wizarrc commented Jul 24, 2015

I forgot to point out that in MVC 5, the flow was to register the mvc routes first, then you could configure authentication based on per route using map. It appears that if you try to do that in AspNet 5, MVC6, it will present you with the error upon signing in using Identity: "The following authentication scheme was not accepted: Microsoft.AspNet.Identity.Application".

I believe if I could add UseMVC in the parent IApplicationBuilder, but from within the map function, that might solve the ordering issue. See #351

@Tratcher
Copy link
Member

You can have multiple authentication types in the same pipeline, you don't need to use map the separate them. It's better to use the Authorize attribute and authorization policies in MVC to mark your controllers and routs.

@wizarrc
Copy link
Author

wizarrc commented Jul 28, 2015

@Tratcher If I remember correctly, back in MVC5 days, Identity Framework and Windows Authentication would not play nice together with Authentication. If Windows Authentication was used in the pipeline, it wouldn't let Identity later authenticate without cleaver hacks. The path of least resistance appeared to be to use the map function. There was also a problem that would arise due to an old IE perf optimization bug with submitting a form while using Integrated Windows and cookie based Authentication where IE would submit only headers with no body for the first request after being idle 30+ seconds while making an XMLHttpRequest in JavaScript (which still is a bug today). IE would assume that the first request would always be denied and optimized the request. When allowing both authentication for a given route, it made for very unpredictable results using IE.

@Tratcher
Copy link
Member

We've done quite a bit of work to make them play better together. Try it with the beta6 bits. If you still have trouble I recommend opening an issue in the Security repo so we can get it straitened out.

@wizarrc
Copy link
Author

wizarrc commented Jul 28, 2015

@Tratcher Good to hear. Will have to try it out. But that still doesn't answer the original post. What is the reasoning behind changing the base path with map? What benefit does it provide? By definition, the base path is the application root path, not the new branch root path. I think at the very least it should be well documented on what it is doing and how it can affect all the requests going though the pipeline. I think the average developer would think that Map and MapWhen provide the same capabilities overall, but they don't due to that subtle change to the BasePath.

@Tratcher
Copy link
Member

Map is considered to break your application into sub apps, each with their own PathBase and request processing pipeline. Inside of those sub-apps you no longer need to consider that matched portion of the path in any of your operations. Hence if I configure MVC inside of a Map("/MVC",...) I do not include "/MVC" in the definition of every route. The StaticFiles and Authentication middleware get similar benefits in their configurations.

Note you can also nest Maps:

app.Map("/level1", level1App =>
{
 level1App.Map("/level2a", level2AApp =>
 {
  // "/level1/level2a"
  //...
 });
 level1App.Map("/level2b", level2BApp =>
 {
  // "/level1/level2b"
  //...
 });
});

@davidfowl
Copy link
Member

@wizarrc did you figure it out?

@wizarrc
Copy link
Author

wizarrc commented Aug 21, 2015

@davidfowl I was able to work around my issue by using MapWhen. I understand the usage of each function's intentions now and that it is the intended feature, but I'm still puzzled why an API design would have Map automatically change the base path and not for MapWhen. Seems like an odd design choice to me. I do still rely on MapWhen not changing my base path though.

As for using the new beta, I have not had time to test and see if I can add all the needed Authentication filters without it blowing up in my face. I still feel that it's good to not use Windows Authentication in a section that accepts cookies and I still like to use content references that are anonymous, like css and scripts. MapWhen allows me to easily switch between authentication zones without having a sub app. When breaking dependencies from IIS, reverse proxy filtering, and upgrading to AspNet 5, I am relying a lot more on the core functionality of AspNet.

@davidfowl
Copy link
Member

Great! Closing out this issue.

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

No branches or pull requests

4 participants