We have moved to GitHub Issues
Created by Shannon Deminick 31 Aug 2017, 04:32:35 Updated by Sebastiaan Janssen 17 Oct 2017, 13:20:05Tags: Up For Grabs Unscheduled PR Community Contrib
Relates to: U4-9081
Currently there are 2 overridable methods on UmbracoDefaultOwinStartup:
which mimic how ASP.NET Core does startup and that is fine, however in some cases it requires that a developer re-implement much of what is going on inside of these methods and it would be nicer for better cross cutting concerns than to override a whole block.
PR for external identities: https://github.com/umbraco/Umbraco-CMS/pull/2162/files
Quote from previous issue:
After trying to figure out why Facebook/Google OAuth providers would appear and the OAuth calls occurred but Umbraco would never link or allow logins, I realized that the configuration for the OAuth providers needs to happen after ConfigureMiddleware runs. It doesn't seem to me that ConfigureMiddleware is a logical place for users to be overriding for this purpose, so I've put together a PR at https://github.com/umbraco/Umbraco-CMS/pull/2162 that adds a new ConfigureExternalLogins method that runs after ConfigureMiddleware . Users can configure OAuth providers by overriding that method.
The question becomes what to do with UmbracoIdentityExtensions. In order to get the benefit of the restructuring to make things more sensible, we'd need to jump the dependency requirements all the way forward to now. What are your thoughts?
The base methods of
ConfigureMiddleware does quite a lot of things:
//Ensure owin is configured for Umbraco back office authentication. If you have any front-end OWIN // cookie configuration, this must be declared after it. app .UseUmbracoBackOfficeCookieAuthentication(ApplicationContext, PipelineStage.Authenticate) .UseUmbracoBackOfficeExternalCookieAuthentication(ApplicationContext, PipelineStage.Authenticate) .UseUmbracoPreviewAuthentication(ApplicationContext, PipelineStage.Authorize) .UseSignalR() .FinalizeMiddlewareConfiguration();
These are all important and need to be done in that order - and the last part
FinalizeMiddlewareConfiguration is a bit of undocumented stuff ... this actually emits an event that you could subscribe to instead of swapping out your own Owin startup:
UmbracoDefaultOwinStartup.MiddlewareConfigured (can't remember when this was added but definitely after 7.3). You have the option to use this event too if you wanted.
I'd prefer to not have a method called
ConfigureExternalLogins which is very specific. If you really want to have a method to override that is not
ConfigureMiddleware then it should be more like
ConfigureAuthentication and you could either:
ConfigureCustomAuthentication) which executes before
...I'd prefer option 1 if we go down this path
@Shandem I'm sorry about the confusion with
ConfigureServices. I've been poking around with ASP.NET Core 2.0 and it uses
ConfigureServices for OAuth config.
Today was the first time I configured OAuth with Umbraco. I banged my head against the wall getting things to work because I had the config out of order, so I tried to look at dummyproofing the OAuth config process to make it easy for users. If things are so sensitive that they have to be in just the right order for it all to work, it seems to me that it's worth considering a placeholder method like I proposed in my pull request. It eliminates the chance that a user will call methods out of order, like I did, or fail to call something, like some commenters in U4-9081 did.
I can understand your concern about adding another method to the codebase. You're the core developer. It's your call.
I like the option #1 I listed above. This means Core's
ConfigureMiddleware methods would look like:
ConfigureUmbracoAuthentication(app, ApplicationContext); app.UseSignalR() .FinalizeMiddlewareConfiguration();
ConfigureUmbracoAuthentication would be:
app .UseUmbracoBackOfficeCookieAuthentication(ApplicationContext, PipelineStage.Authenticate) .UseUmbracoBackOfficeExternalCookieAuthentication(ApplicationContext, PipelineStage.Authenticate) .UseUmbracoPreviewAuthentication(ApplicationContext, PipelineStage.Authorize);
Then you can override
ConfigureUmbracoAuthentication (and ensure you call the
Does that work for you (and make sense)? If so can you update your PR that way?
@Shandem The changes make sense. I've been traveling this weekend (it's Labor Day weekend here in the US). I'll submit an updated PR tomorrow.
@Shandem I've updated the pull request. https://github.com/umbraco/Umbraco-CMS/pull/2162
If this looks good, I'll work on updating UmbracoIdentity.
Looks good, have merged in :)
Backwards Compatible: True
Due in version: 7.7.3, 7.6.10
Sprint: Sprint 69
Story Points: 0.5