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

Collate issues for new template at Core5 #70

Open
JimKerslake opened this issue Nov 1, 2021 · 14 comments
Open

Collate issues for new template at Core5 #70

JimKerslake opened this issue Nov 1, 2021 · 14 comments

Comments

@JimKerslake
Copy link
Contributor

Collation of various template changes awaiting implementation as part of more wide-ranging CS upgrade

@JimKerslake
Copy link
Contributor Author

Support for Linux under a reverse proxy scenario requires addition of header forwarding in startup:

cloudscribe/cloudscribe#753

https://github.com/GreatHouseBarn/ProductPlatformCore3/commit/8e041bad7e2a46ddebd827f2f4f27709f15b0f22

quote:
// This code allows dotnet core to run behind a proxy server where it is the proxy
// that serves content over http and https, while the proxy itself talks to us via http only, but
// we want our httpContext to contain the correct client IP address (rather than 127.0.0.1)
// and the correct protocol (http or https).
// See: https://docs.microsoft.com/en-us/aspnet/core/host-and-deploy/linux-apache?view=aspnetcore-3.1#configure-apache

@JimKerslake
Copy link
Contributor Author

Add new default placeholder config lines to appsettings for new authentication mode (root users can login... = false)

@JimKerslake
Copy link
Contributor Author

(not directly a template thing but...) Note there are two typos in appsettings json names that will constitute potential breaking changes when fixed, so warrant an announcement:
cloudscribe/cloudscribe.SimpleContent#510
cloudscribe/cloudscribe#732

@JimKerslake
Copy link
Contributor Author

Review this, but we may always want the final 'always' line here:

services.Configure<CookiePolicyOptions>(options =>
            {
                // This lambda determines whether user consent for non-essential cookies is needed for a given request.
                options.CheckConsentNeeded = cloudscribe.Core.Identity.SiteCookieConsent.NeedsConsent;
                options.MinimumSameSitePolicy = Microsoft.AspNetCore.Http.SameSiteMode.None;
                options.ConsentCookie.Name = "cookieconsent_status";
                options.Secure = CookieSecurePolicy.Always;
            });

@JimKerslake
Copy link
Contributor Author

Previous work on Talkabout contains a comment that the template no longer needs to deliver the view
TalkAboutForumImageModalContent.cshtml (Note Forum in that name)
Or alternatively does it maybe need an updated copy of the one packaged in the talkabout Nuget?

https://github.com/exeGesIS-SDM/cloudscribe.TalkAbout/issues/23

@JimKerslake
Copy link
Contributor Author

Need to re-investigate whether endpoint routing is still impossible for cs, due to the bug in URL culture route segments

@JimKerslake
Copy link
Contributor Author

Review all cs code for any lurking deprecated .AddEntityFrameworkSqlServer() calls (and similar for Pg etc.)

@StewartBellamy
Copy link

Update package.json references

@JimKerslake
Copy link
Contributor Author

Add call to services.AddMemoryCache();
(some issue to do with it getting constructed with a memory limit if done later in EFCore setup)

@StewartBellamy
Copy link

@JimKerslake - I think perhaps that the same site mode for cookies should be lax and not none, to help prevent CSRF attacks. Secure policy should be always too I think.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite

@JimKerslake
Copy link
Contributor Author

@StewartBellamy Sure can set it to lax...
this is template-delivered startup code, so is easily changed by the template user, they're not stuck with it.

@joeaudette
Copy link
Collaborator

joeaudette commented Nov 22, 2021 via email

@JimKerslake
Copy link
Contributor Author

OK thanks @joeaudette - one more thing to test :)

FYI - looks like MS still didn't fix that endpoint routing bug.
Will post more about that here shortly.

@JimKerslake
Copy link
Contributor Author

JimKerslake commented Nov 22, 2021

Endpoint routing issue (still)

Refs:
cloudscribe/cloudscribe.SimpleContent#466
dotnet/aspnetcore#14877

My understanding is that the main problem in CS (but maybe not the only one?)
relates to the ability to persist a culture segment in the routing.

So for example if you set the site up to support the culture fr-FR, then the user interface will use the French ResX translations when you visit a page with this culture in the route e.g. /fr-FR/siteadmin

Where this breaks - is when you try to persist that culture by auto-generating links to other pages using URL helpers that are supposed to respect and persist that culture.

So the intention is that when viewing a simpleContent page at /fr-FR/myPageSlug
then the page editing control button should take you to /fr-fr/editpage/myPageSlug

In the underlying view this is generated by the Razor incantation:
Url.RouteUrl("pageedit-culture", new { slug = "myPageSlug" });

where the underlying route definition for pageedit-culture looks like this:

 routes.MapControllerRoute(
               name: ProjectConstants.CulturePageEditRouteName,
               pattern: "{culture}/editpage/{slug?}"
               , defaults: new { controller = "Page", action = "Edit" }
               , constraints: new { culture = cultureConstraint }
               );

To replicate this:
If you set up a minimal 'test' view (and have a 'test' controller returning that view)
and then in your view include
var debug = Url.RouteUrl("pageedit-culture", new { slug = "myPageSlug" });

you can see that in old MVC routing,
when you view the page /fr-FR/test this value is returned as /fr-fr/editpage/myPageSlug

With endpoint routing used instead of MVC, this just returns null... which would be a broken link.
This still seems to be true running at Net5.0

This is because
"Endpoint routing doesn't preserve ambient values when generating links to other actions. We'll look into solving this somehow during 5.0."
dotnet/aspnetcore#14877
(the latter has not happened, I guess. And by 'ambient values' they mean the fr-FR culture segment.)

What I don't fully understand:
I think that perhaps some changes might have been made to these links in Simplecontent a while ago, such that at the moment
(and ever since Core3.x) the simplecontent edit links (and a couple of others) don't retain the culture segment anyway, even when running in MVC.

So if you're viewing a content page at /fr-FR/myPageSlug then the edit link takes you back to default language at /editpage/myPageSlug
and so dumps your editing experience back out of French, regardless of MVC or endpoint routing.

I guess that's preferable to having a broken link if the above RouteUrl answers null in endpoint routing... but seems incorrect in MVC routing setup.

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

No branches or pull requests

3 participants