Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpaProxy should not inject middleware with ASPNETCORE_HOSTINGSTARTUPASSEMBLIES #45130

Closed
1 task done
jez9999 opened this issue Nov 16, 2022 · 17 comments
Closed
1 task done
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-spa Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. Status: No Recent Activity

Comments

@jez9999
Copy link

jez9999 commented Nov 16, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

No response

Describe the solution you'd like

Injecting middleware with ASPNETCORE_HOSTINGSTARTUPASSEMBLIES prevents me from controlling the order middleware gets added to my pipeline. Frankly, I don't even know why the feature exists, it seems to me like there would never be a good reason to use it. SpaProxy gets added to ASP.NET SPA projects through this variable right now and I hate it. Why is it so bad to add the middleware from Program.cs instead so the developer can choose the order in which it gets added? What if they want to run some middleware before the SpaProxy middleware?

Additional context

No response

@javiercn javiercn added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-spa labels Nov 16, 2022
@javiercn
Copy link
Member

@jez9999 thanks for contacting us.

Many of our development experiences work this way. Like hot reload and dotnet-watch.

What if they want to run some middleware before the SpaProxy middleware?

Could you give a concrete scenario where you would like to do this?

The SpaProxy is only meant to launch the proxy automatically after the .NET project starts. You can choose to remove it and launch the proxy manually or when your app starts.

@javiercn javiercn added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Nov 16, 2022
@ghost
Copy link

ghost commented Nov 16, 2022

Hi @jez9999. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@jez9999
Copy link
Author

jez9999 commented Nov 16, 2022

Could you give a concrete scenario where you would like to do this?

The SpaProxy is only meant to launch the proxy automatically after the .NET project starts. You can choose to remove it and launch the proxy manually or when your app starts.

I'd like to add some middleware where, on each call, it checks to see whether they're authenticated and if not, it assigns a cookie on-the-fly (probably with SignInAsync) with a Guid as the user name identifier so that I can have anonymous users tracked to some extent across a session. Presumably if that has to run after the SpaProxy middleware it won't get to have the opportunity to examine headers and set the cookie on each request.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Nov 16, 2022
@javiercn
Copy link
Member

I'd like to add some middleware where, on each call, it checks to see whether they're authenticated and if not, it assigns a cookie on-the-fly (probably with SignInAsync) with a Guid as the user name identifier so that I can have anonymous users tracked to some extent across a session. Presumably if that has to run after the SpaProxy middleware it won't get to have the opportunity to examine headers and set the cookie on each request.

The middleware only interacts with the app at the beginning, where it launches the proxy and waits to redirect the user to the dev server proxy. after that, it is passthrough.

That said, you can do this by registering your own IStartupFilter before the SPA middleware with services.Insert(0,ServiceDescriptor.Singleton<IStartupFilter,YourFilter>())

@javiercn javiercn added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Nov 16, 2022
@ghost
Copy link

ghost commented Nov 16, 2022

Hi @jez9999. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@richstokoe
Copy link
Contributor

I've covered some of these use cases in another separate issue: #44187

I'd like to add some middleware where, on each call, it checks to see whether they're authenticated and if not

This is exactly how we'd architected our SPA because the old SpaServices proxy led you toward this approach.

The worst part is that in production you don't have SpaProxy, you have UseSpaStaticFiles / endpoints.MapFallbackToFile in the middleware pipeline so things actually work differently in development than in production and can lead to issues not being found until it hits real users. (see my comment on the other issue for a concrete example).

@jez9999
Copy link
Author

jez9999 commented Nov 20, 2022

@richstokoe Yes, I think the right approach would be for Microsoft to actively support both approaches going forward, even if the new approach is default, the old one should remain an option for those of us who prefer it.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Nov 20, 2022
@javiercn
Copy link
Member

The worst part is that in production you don't have SpaProxy, you have UseSpaStaticFiles / endpoints.MapFallbackToFile in the middleware pipeline so things actually work differently in development than in production and can lead to issues not being found until it hits real users. (see my comment on the other issue for a concrete example).

The SPA proxy is a development feature, it has always been there in the old and the new version, the only thing we changed is the order in which it gets applied during development (behind the asp.net core process vs in front of it).

In production the only thing we use is MapFallbackToFile which takes care of serving the default doc for unknown routes.

Note that you had the opposite problem in the previous implementation, where your server routes might take over routes in your SPA, so the change we made in .NET 6.0 only changed the "direction" of the problem. (A SPA route might take precedence over a server route during development as opposed to otherwise). With that said, we are in a better position here, because we know how to gather all the server routes, and we might be able in the future to actually way more accurately identify the server routes we have to proxy, which was impossible with the old implementation.

@javiercn javiercn added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Nov 24, 2022
@ghost
Copy link

ghost commented Nov 24, 2022

Hi @jez9999. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@jez9999
Copy link
Author

jez9999 commented Nov 24, 2022

@javiercn If the asp.net core process is going to be the front-facing web server on production, doesn't it make more sense for it to be in development too? If an asp.net route will take precedence over a JS one, it ought to do so just the same on dev and prod. Both options should be supported.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Nov 24, 2022
@javiercn
Copy link
Member

@javiercn If the asp.net core process is going to be the front-facing web server on production, doesn't it make more sense for it to be in development too? If an asp.net route will take precedence over a JS one, it ought to do so just the same on dev and prod. Both options should be supported.

That's not the development experience that SPA frameworks support. The problem with what we were doing before was that it was not something blessed by the existing SPA frameworks and that resulted in all sorts of integration issues that we had to be accommodating for. Things like hot reload not working on the SPA because of our custom proxy during development, etc.

Having the SPA proxy in front of the backend is what the SPA frameworks recommend and support, and it puts us in a better position for the future. We can't know the routes the SPA targets because they are specific to each framework, but we know the ones that the Server targets, so we can actually very accurately configure the SPA proxy to automatically redirect the requests that would target the server.

We do not do this today, but it's something that we are investigating for the future.

@javiercn javiercn added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Nov 24, 2022
@ghost
Copy link

ghost commented Nov 24, 2022

Hi @jez9999. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@jez9999
Copy link
Author

jez9999 commented Nov 24, 2022

I just think that's exaggerated. I'm developing a Vue application using the old ASP.NET dev proxy and hot reloads work fine, as do WebSockets. Basically you could keep supporting that way of doing things if you wanted, whether SPA frameworks "bless" it or not.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Nov 24, 2022
@richstokoe
Copy link
Contributor

richstokoe commented Nov 24, 2022

That's not the development experience that SPA frameworks support.

aspnetcore has supported this for several years, provided templates that lead us all to design solutions like this, demonstrated at conferences exactly this, and we've all built applications around this pattern. Now we need to perform non-trivial rearchitecture of our apps to support this new approach.

I'm fine with breaking changes, with enough notice, but the documentation is only now being written. Some of us are using LTS versions only because we're in domains or projects that require it and 3.1 is EOL next month. We can't move to 6 without redesigning our suite of services to match this approach.

Jez has followed the same pattern as we have:

I'd like to add some middleware where, on each call, it checks to see whether they're authenticated and if not, it assigns a cookie on-the-fly

We host static Razor landing pages for speed of first load, and redirect to the SPA only once authentication (Cookie auth) has been completed.

This is a perfectly reasonable approach that maximises user experience.

We also like to ensure our development experience is close to production to spot issues early. That is also a reasonable expectation but one that is not possible with this new approach.

SPA frameworks will of course say to put the back end behind the SPA because they're not building back end frameworks. Their incentives and responsibilities are not the same as the .NET team's, nor should they be. ASP.NET allows hybrid applications with front and back ends in the same project, so we've all built them, and now this is unexpected TOIL with high risk (such as having to redesign, test, revalidate, and in some domains recertify with regulators, authentication mechanisms).

I still haven't seen a valid reason why the aspnetcore team won't support both ways even for one more LTS release. If this is how Microsoft will be treating its community going forward then we'll just switch to something else.

We can't know the routes the SPA targets because they are specific to each framework, but we know the ones that the Server targets, so we can actually very accurately configure the SPA proxy to automatically redirect the requests that would target the server.

You're thinking about this too abstractly and making this choice for us - you might not know but we DO know the routes the SPA targets because we're building the software. By putting the SPA proxy into the startup pipeline we can choose whether to redirect to auth, load the SPA, or load a Razor page. The solution provided now is to use Regex in JS files for routes, but just to work around the fact that the Dev inner loop is now different from the Production running behaviour.

Folks, this is not right.

@javiercn
Copy link
Member

@richstokoe thanks for the additional details.

Its late in my timezone and I can't give you a more detailed answer right now, but I want to at least touch on a few points.

aspnetcore has supported this for several years, provided templates that lead us all to design solutions like this, demonstrated at conferences exactly this, and we've all built applications around this pattern. Now we need to perform non-trivial rearchitecture of our apps to support this new approach.

I'm fine with breaking changes, with enough notice, but the documentation is only now being written. Some of us are using LTS versions only because we're in domains or projects that require it and 3.1 is EOL next month. We can't move to 6 without redesigning our suite of services to match this approach.

I want to make sure it is clear that the existing Spa.Extensions is supported in .NET 6.0, which will last until November 2024 (almost 2 years from now).
We have not even decided to obsolete Spa.Extensions in .NET 7.0.
If we make a decision in .NET 8.0, that will give you at least until November 2026, which is 4 years from now.

We are not forcing you to change your project in any way for the time being.

I still haven't seen a valid reason why the aspnetcore team won't support both ways even for one more LTS release. If this is how Microsoft will be treating its community going forward then we'll just switch to something else.

Again, let me reiterate that Spa.Extensions is still supported, just not our default experience any longer.

Jez has followed the same pattern as we have:

I'd like to add some middleware where, on each call, it checks to see whether they're authenticated and if not, it assigns a cookie on-the-fly

We host static Razor landing pages for speed of first load, and redirect to the SPA only once authentication (Cookie auth) has been completed.

This is a perfectly reasonable approach that maximises user experience.

We also like to ensure our development experience is close to production to spot issues early. That is also a reasonable expectation but one that is not possible with this new approach.

SPA frameworks will of course say to put the back end behind the SPA because they're not building back end frameworks. Their incentives and responsibilities are not the same as the .NET team's, nor should they be. ASP.NET allows hybrid applications with front and back ends in the same project, so we've all built them, and now this is unexpected TOIL with high risk (such as having to redesign, test, revalidate, and in some domains recertify with regulators, authentication mechanisms).

It would be helpful if you can provide concrete examples of the issues you are running into. I think this will help understand the gaps that you are facing with the new experience and give us a list of things that we can do to improve it.

We understand that there are different trade-offs with the current experience and the previous experience, and that in some cases, some scenarios can be harder. We hope you can understand that we decided to go in a different direction because there was a non-trivial cost and risk for us to support the existing approach that we realized over time. Specifically:

  • We continued having a long trail of bugs that were caused by our proxy.
  • We had to support all SPA framework versions for a given release simultaneously.
  • In more than one case, when a SPA framework released a new version, it broke our Spa.Extensions support.
  • When there were big changes in how frameworks operated, people were blocked from updating to newer versions without us updating our support too, which was not always possible when were in the middle of a release.

So, to summarize a bit:

  • Spa.Extensions is still supported up until (at the very least) November 2026.
  • We have implemented a new approach that we use as the default for new projects.
  • We are looking for concrete scenarios in which you are having issues with the new experience so that we can work together on addressing those gaps.
    • If you have repro projects or samples, that will help us understand the problems you are running into.
  • We appreciate the time and effort you take to provide feedback, and we hope that at least we can agree that we also need to factor other aspects in our decisions.

@javiercn javiercn added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Nov 24, 2022
@ghost
Copy link

ghost commented Nov 24, 2022

Hi @jez9999. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@ghost
Copy link

ghost commented Nov 28, 2022

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Dec 1, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 31, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-spa Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. Status: No Recent Activity
Projects
None yet
Development

No branches or pull requests

3 participants