umbraco / UmbracoIdentityExtensions

Code files & installation that enables easy extensibility points for ASP.Net Identity and the Umbraco back office
MIT License
38 stars 23 forks source link

Login loop - no errors or exceptions. #20

Closed clarkd closed 5 years ago

clarkd commented 7 years ago

Hi.

I'm having an issue with a new Umbraco 7.6 site. We've setup external auth with Azure Active Directory. That all appears to be working fine, but after a random period of time (seemingly) it stops working and our users cannot login. Clicking the "Login with Azure" does the redirection to Microsoft, but on return they just end up on back on the login page. I have to manually restart the app to get it working again. Sometimes it'll work for a few days, sometimes a few hours.

There doesn't appear to be any exceptions logged so I'm not sure where to start debugging. Any ideas?

aclave1 commented 7 years ago

@clarkd Have you solved this yet?

clarkd commented 7 years ago

No - although I'm not too sure where to start debugging the issue! Are you experiencing the same issue?

On 4 Aug 2017 23:10, "Alex Clavelle" notifications@github.com wrote:

@clarkd https://github.com/clarkd Have you solved this yet?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/umbraco/UmbracoIdentityExtensions/issues/20#issuecomment-320342268, or mute the thread https://github.com/notifications/unsubscribe-auth/ADng6_F-ra3I7TMVUC3Ll3K0Y2w2O6Inks5sU3q-gaJpZM4ODSTj .

aclave1 commented 7 years ago

Yes I was experiencing the same issue but I was able to figure it out using the code from this article. If you posted some of your code maybe I could take a stab at it.

clarkd commented 7 years ago

@aclave1 This is the code I have in UmbracoADAzureExtensions.cs... Would certainly appreciate you taking a look or sharing your version. I used the article you indicated but also hit a few problems along the way so the code does vary slightly...

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Security.Claims;
using System.Threading.Tasks;
using System.Web;
using Microsoft.AspNet.Identity;
using Microsoft.AspNet.Identity.Owin;
using Microsoft.IdentityModel.Protocols;
using Microsoft.Owin;
using Microsoft.Owin.Security;
using Microsoft.Owin.Security.Notifications;
using Owin;
using Umbraco.Core;
using Umbraco.Web.Security.Identity;
using Microsoft.Owin.Security.OpenIdConnect;
using Umbraco.Core.Models.Identity;
using Umbraco.Core.Security;
using Constants = Umbraco.Core.Constants;

namespace ASP
{
    public static class UmbracoADAuthExtensions
    {

        ///  <summary>
        ///  Configure ActiveDirectory sign-in
        ///  </summary>
        ///  <param name="app"></param>
        ///  <param name="tenant"></param>
        ///  <param name="clientId"></param>
        ///  <param name="postLoginRedirectUri">
        ///  The URL that will be redirected to after login is successful, example: http://mydomain.com/umbraco/;
        ///  </param>
        ///  <param name="issuerId">
        /// 
        ///  This is the "Issuer Id" for you Azure AD application. This a GUID value and can be found
        ///  in the Azure portal when viewing your configured application and clicking on 'View endpoints'
        ///  which will list all of the API endpoints. Each endpoint will contain a GUID value, this is
        ///  the Issuer Id which must be used for this value.        
        /// 
        ///  If this value is not set correctly then accounts won't be able to be detected 
        ///  for un-linking in the back office. 
        /// 
        ///  </param>
        /// <param name="caption"></param>
        /// <param name="style"></param>
        /// <param name="icon"></param>
        /// <remarks>
        ///  ActiveDirectory account documentation for ASP.Net Identity can be found:
        ///  https://github.com/AzureADSamples/WebApp-WebAPI-OpenIDConnect-DotNet
        ///  </remarks>
        public static void ConfigureBackOfficeAzureActiveDirectoryAuth(this IAppBuilder app, 
            string tenant, string clientId, string postLoginRedirectUri, Guid issuerId,
            string caption = "Active Directory", string style = "btn-microsoft", string icon = "fa-windows")
        {
            //var authority = string.Format(
            //    CultureInfo.InvariantCulture, 
            //    "https://login.windows.net/{0}", 
            //    tenant);

            var authority = string.Format(
                CultureInfo.InvariantCulture,
                "https://login.microsoftonline.com/{0}",
                tenant);

            var adOptions = new OpenIdConnectAuthenticationOptions
            {
                SignInAsAuthenticationType = Constants.Security.BackOfficeExternalAuthenticationType,
                ClientId = clientId,
                Authority = authority,
                RedirectUri = postLoginRedirectUri,
                AuthenticationMode = AuthenticationMode.Passive,
                Notifications = new OpenIdConnectAuthenticationNotifications()
                {
                    AuthorizationCodeReceived = AuthorizationCodeReceived,
                    SecurityTokenValidated = SecurityTokenValidated
                }
            };

            adOptions.ForUmbracoBackOffice(style, icon);            
            adOptions.Caption = caption;
            //Need to set the auth tyep as the issuer path
            adOptions.AuthenticationType = string.Format(
                CultureInfo.InvariantCulture,
                "https://sts.windows.net/{0}/",
                issuerId);

            adOptions.SetExternalSignInAutoLinkOptions(new ExternalSignInAutoLinkOptions(autoLinkExternalAccount: true));

            app.UseOpenIdConnectAuthentication(adOptions);            
        }

        private static async Task AuthorizationCodeReceived(AuthorizationCodeReceivedNotification context)
        {
            var userService = ApplicationContext.Current.Services.UserService;

            var email = context.JwtSecurityToken.Claims.First(x => x.Type == "upn").Value;
            var issuer = context.JwtSecurityToken.Claims.First(x => x.Type == "iss").Value;
            var providerKey = context.JwtSecurityToken.Claims.First(x => x.Type == "sub").Value;
            var name = context.JwtSecurityToken.Claims.First(x => x.Type == "name").Value;
            var userManager = context.OwinContext.GetUserManager<BackOfficeUserManager>();

            var user = userService.GetByEmail(email);

            if (user == null)
            {
                var writerUserType = userService.GetUserTypeByName("writer");
                user = userService.CreateUserWithIdentity(email, email, writerUserType);
            }

            var identity = await userManager.FindByEmailAsync(email);
            if (identity.Logins.All(x => x.ProviderKey != providerKey))
            {
                identity.Logins.Add(new IdentityUserLogin(issuer, providerKey, user.Id));
                identity.Name = name;
                var result = userManager.Update(identity);
            }
        }

        private static async Task SecurityTokenValidated(SecurityTokenValidatedNotification<OpenIdConnectMessage, OpenIdConnectAuthenticationOptions> n)
        {
            var identityUser = n.AuthenticationTicket.Identity;

            var newIdentityUser = new ClaimsIdentity(identityUser.AuthenticationType, ClaimTypes.GivenName, ClaimTypes.Role);

            newIdentityUser.AddClaim(new Claim(ClaimTypes.Email, identityUser.Name));
            newIdentityUser.AddClaim(new Claim(ClaimTypes.Upn, identityUser.Name));
            newIdentityUser.AddClaim(identityUser.FindFirst(ClaimTypes.NameIdentifier));
            newIdentityUser.AddClaim(identityUser.FindFirst(ClaimTypes.GivenName));

            n.AuthenticationTicket = new AuthenticationTicket(newIdentityUser, n.AuthenticationTicket.Properties);
        }
    }

}
arlan85 commented 6 years ago

@aclave1 and @clarkd I have the same issue on Umbraco 7.6.3. The code I use is the same in the post, but for some reason I keep having the same behavior, randomly the login sends me after I login with AAD to the Umbraco login page and in the Chrome console tells me: umbraco/backoffice/UmbracoApi/Authentication/IsAuthenticated is false. Did you get any feedback about this?

aclave1 commented 6 years ago

@arlan85 One of my main issues was that some of my settings were wrong. Make sure your reply url in azure matches the one you provide in umbraco and make sure the schemes match.

arlan85 commented 6 years ago

@aclave1 the login process works fine, but always when redirects after login, it comes to the umbraco login again over and over.

mark-data8 commented 6 years ago

I'm having the same issue - after logging in with AAD I get redirected back to the login page. All the code in AuthorizationCodeReceived executes as expected, and I don't see any exceptions thrown. Any ideas where I can start looking?

dnwhte commented 6 years ago

Anyone solve this yet?

Enoch-Ye commented 6 years ago

We are also facing the same issue on 7.8.1 that user gets redirected back to login page without any exception. Appreciate any help.

Shazwazza commented 6 years ago

Sounds like the required claims are missing. Not sure what everyone's code is doing but the above snippet is doing some claim transformations in SecurityTokenValidated for whatever reason. The original article mentioned above (https://www.jdibble.co.uk/blog/securing-umbraco-backoffice-with-azure-active-directory/) doesn't do this. I'd suggest keeping everything as simple as possible before modifying the whole login flow and if you've copy/pasted some code please make sure you know what it's doing.

The code to create an umbraco back office identity from a claims identity is here: https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Core/Security/UmbracoBackOfficeIdentity.cs#L26 ... which means you will need all required claims (which vary based on the umbraco version).

The original article doesn't do any claims transformations so that approach should still work ... but be sure you aren't using any Obsolete methods.

Shazwazza commented 6 years ago

Though since there's no errors, it may be something else. I had a similar problem when trying to debug Azure easy auth ... make sure this is NOT on! see here for full details https://shazwazza.com/post/getting-umbraco-to-work-with-azure-easy-auth/ If you are using easy auth then you shouldn't really be mixing the two technologies, but i suppose you can if you follow that guide.

If that's not the issue than if you have exact steps to replicate this from a vanilla Umbraco install please post those steps and i'll see what i can find.

Fjellvang commented 6 years ago

We have the same issue using AD FS instead of Azure for authorisation. It fails after a while in production, and requires a reset of the app pool to work again ..

arlan85 commented 6 years ago

All my apps have App Service Authentication OFF (Anonymous access is enabled on the App Service app. Users will not be prompted for login.). This issue is weird and randomly, even if you restart the app, the Azure Authentication redirects you to the login page, but in some cases making a swap between slots in the same app, it works. So, is a frequent, randomly but annoying issue. I'm using: Azure Umbraco 7.6.3 UmbracoCms.IdentityExtensions.AzureActiveDirectory 1.0.0. Microsoft.AspNet.Identity.Owin 2.2.1 Microsoft.Owin 3.0.1

Fjellvang commented 6 years ago

in our failing ADFS Solution we noticed that the redirect cookie is missing a lot of information from umbraco. Specifically UMB_EXTLOGIN and UMB_UCONTEXT, the user get authenticated through ADFS, but it makes sense that it loops if there is no Umbraco information the in cookie i guess.

Enoch-Ye commented 6 years ago

We've got some stack trace after modifying umbraco source code: System.InvalidOperationException: The user has no umbraco contextid - try logging in at Umbraco.Web.Security.WebSecurity.ValidateCurrentUser(Boolean throwExceptions, Boolean requiresApproval) in D:\Repo\Umbraco-CMS\src\Umbraco.Web\Security\WebSecurity.cs:line 257 at Umbraco.Web.Security.WebSecurity.AuthorizeRequest(Boolean throwExceptions) in D:\Repo\Umbraco-CMS\src\Umbraco.Web\Security\WebSecurity.cs:line 286 at Umbraco.Web.Editors.AuthenticationController.IsAuthenticated() in D:\Repo\Umbraco-CMS\src\Umbraco.Web\Editors\AuthenticationController.cs:line 140

This is pointing to AuthenticationExtensions.GetCurrentIdentity extension method. Not sure how the identity screw up in cookie from Azure Active Directory.

aclave1 commented 6 years ago

The issue occurs for me after a few days. I have deployed a potential fix to one of my client sites today. I will update you all with the fix if it works! edit: my fix did not fix it 😞

christ76 commented 6 years ago

We have the same issue here with Umbraco version 7.11.1 assembly: 1.0.6750.39767. Anyone manage to fix this? Thanks!

nul800sebastiaan commented 5 years ago

Tough one! While I recognize that a few people are battling this issue, we've not had any way to reproduce the issue yet. In cases like this I would start adding copious logging to our source code and deploy custom builds to at least get an idea of what might be going on and where.

I will close this issue for now until we can get some way to reproduce the problem. Fingers crossed 🤞 that we can get to the bottom of this one day!

clarkd commented 5 years ago

Seems absurd to close this issue...

On Thu, 15 Nov 2018, 10:36 Sebastiaan Janssen <notifications@github.com wrote:

Closed #20 https://github.com/umbraco/UmbracoIdentityExtensions/issues/20.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/umbraco/UmbracoIdentityExtensions/issues/20#event-1968264329, or mute the thread https://github.com/notifications/unsubscribe-auth/ADng65mGWsbkAMR5NSxbmam5njfMyDwsks5uvUObgaJpZM4ODSTj .

arlan85 commented 5 years ago

@clarkd I'm agree with you, we gave all information we have because it is a random issue, today I had again to redeploy my website because my editors can't login into the site. Is is so odd but it happens!

Tough one! While I recognize that a few people are battling this issue, we've not had any way to reproduce the issue yet. In cases like this I would start adding copious logging to our source code and deploy custom builds to at least get an idea of what might be going on and where.

I will close this issue for now until we can get some way to reproduce the problem. Fingers crossed 🤞 that we can get to the bottom of this one day!

Shazwazza commented 5 years ago

I'll add adfs login to my site and see if i can replicate, but i probably don't actively log into my site nearly as often as your editors so we'll see. Hopefully i can replicate at some point but until I have a way to do so i can't debug the issue. 🤞

@Enoch-Ye can you tell us what code you changed in core to try to debug the issue for the stack trace you posted?

clarkd commented 5 years ago

@Shazwazza The only pointer I might be able to add is that I think it might be related to some kind of inactivity - we used to experience this problem reasonably regularly but we've since grown in number of employees so our site (internal portal) is being accessed more regularly and I haven't seen the issue in a while - try leaving the site for a few days or something. FYI, we were using Azure AD auth.

CasperTDK commented 5 years ago

Just to chime in. We have the exact same code and issue. A recycle of app pool works. We have hundreds of editors on the site with the issue, and right now they are resorting to use a single umbraco username+password login, while this issue gets resolved.

See cookies set (no umbraco cookies set, but there is no log messages/errors): image

Shazwazza commented 5 years ago

Hi all, please try to not report bugs by saying

We have the exact same code and issue

Because I don't actually know what the 'exact code' you are actually using is. What is the actual code you are referring too? There's only one code block listed above https://github.com/umbraco/UmbracoIdentityExtensions/issues/20#issuecomment-323064200 and I've mentioned that to get to the bottom of this, we need to remove all complexity in the code https://github.com/umbraco/UmbracoIdentityExtensions/issues/20#issuecomment-393039094

So, for people having this problem what code are you actually using and why? please be specific. I've implemented this on my own site but i'm using the basic code provided with Identity Extensions, not with a lot of claims transformations and custom logic as posted above.

Shazwazza commented 5 years ago

I wrote a post yesterday on implementing Azure AD Auth https://shazwazza.com/post/configuring-azure-active-directory-login-with-umbraco/ so if you are doing things in code in vastly different ways i need to know why and if that is part of the problem (i.e. if you just implement it the simplest way, is it still a problem?)

CasperTDK commented 5 years ago

Alright I will share some more code and I'd test anything you would ask me to.

using System;
using System.Configuration;
using System.Globalization;
using System.Linq;
using System.Reflection;
using System.Security.Claims;
using System.Threading.Tasks;
using FrontendTemplate.MVC;
using log4net;
using Microsoft.AspNet.Identity.Owin;
using Microsoft.IdentityModel.Protocols;
using Microsoft.Owin.Security;
using Microsoft.Owin.Security.Notifications;
using Microsoft.Owin.Security.OpenIdConnect;
using Owin;
using Umbraco.Core;
using Umbraco.Core.Models.Identity;
using Umbraco.Core.Models.Membership;
using Umbraco.Core.Security;
using Umbraco.Web.Security.Identity;

namespace KLDK.Web
{
    public class KLAuth
    {
        protected static readonly ILog Log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);

        public static void ConfigureBackOfficeAzureActiveDirectoryAuth(IAppBuilder app)
        {
            string clientId = ConfigurationManager.AppSettings["ida:ClientId"];
            string tenantId = ConfigurationManager.AppSettings["ida:TenantId"];
            string caption = ConfigurationManager.AppSettings["ida:Caption"];
            var uri = string.IsNullOrEmpty(ConfigurationManager.AppSettings["customDomainName"]) ? ConfigurationManager.AppSettings["bindingurl"] : ConfigurationManager.AppSettings["customDomainName"];
            var loginRedirectUri = new UriBuilder(uri)
            {
                Port = -1,
                Path = "umbraco",
                Scheme = GeneralConfiguration.AppSettingValueOrDefault("MW.Umbraco.forceHttps", false) ? Uri.UriSchemeHttps : Uri.UriSchemeHttp
            };
            var postLoginRedirectUri = loginRedirectUri;
            var issuerId = new Guid(tenantId);

            string style = "btn-microsoft";
            string icon = "fa-windows";

            var authority = string.Format(
                CultureInfo.InvariantCulture,
                "https://login.microsoftonline.com/{0}",
                tenantId);

            var adOptions = new OpenIdConnectAuthenticationOptions
            {
                SignInAsAuthenticationType = Constants.Security.BackOfficeExternalAuthenticationType,
                ClientId = clientId,
                Authority = authority,
                RedirectUri = postLoginRedirectUri.ToString(),
                AuthenticationMode = AuthenticationMode.Passive,
                Notifications = new OpenIdConnectAuthenticationNotifications
                {
                    SecurityTokenValidated = SecurityTokenValidated,
                    AuthorizationCodeReceived = AuthorizationCodeReceived,
                    MessageReceived = (notification) =>
                    {
                        Log.Debug($"MessageReceived: ({notification.ProtocolMessage.PostTitle} {notification.State})");
                        return Task.FromResult(0);
                    },
                    AuthenticationFailed = (context) =>
                    {
                        Log.Error($"AuthenticationFailed: {context.Exception.Message} ({context.ProtocolMessage})");
                        if (context.Exception.Message.StartsWith("OICE_20004") || context.Exception.Message.Contains("IDX10311"))
                        {
                            context.SkipToNextMiddleware();
                            return Task.FromResult(0);
                        }
                        return Task.FromResult(0);
                    },
                    SecurityTokenReceived = (notification) =>
                    {
                        Log.Debug($"SecurityTokenReceived: ({notification.ProtocolMessage.PostTitle} {notification.State})");
                        return Task.FromResult(0);
                    },
                    RedirectToIdentityProvider = (notification) =>
                    {
                        Log.Debug($"RedirectToIdentityProvider: ({notification.ProtocolMessage.PostTitle} {notification.State})");
                        return Task.FromResult(0);
                    },
                },
                AuthenticationType = Umbraco.Core.Constants.Security.BackOfficeTokenAuthenticationType,
            };

            adOptions.ForUmbracoBackOffice(style, icon);
            adOptions.Caption = caption;
            //Need to set the auth tyep as the issuer path
            adOptions.AuthenticationType = string.Format(
                CultureInfo.InvariantCulture,
                "https://sts.windows.net/{0}/",
                issuerId);

            adOptions.SetExternalSignInAutoLinkOptions(new ExternalSignInAutoLinkOptions(true, defaultUserGroups: null, defaultCulture: ConfigurationManager.AppSettings["umbracoDefaultUILanguage"]));

            app.UseOpenIdConnectAuthentication(adOptions);
        }

        private static async Task AuthorizationCodeReceived(AuthorizationCodeReceivedNotification context)
        {
            var userService = ApplicationContext.Current.Services.UserService;

            var emailClaim = context.JwtSecurityToken.Claims.FirstOrDefault(x => x.Type == "email") ?? context.JwtSecurityToken.Claims.FirstOrDefault(x => x.Type == "upn");
            var email = emailClaim.Value.Replace("mediaworkers.dk", "kruso.dk");
            var issuer = context.JwtSecurityToken.Claims.First(x => x.Type == "iss").Value;
            var providerKey = context.JwtSecurityToken.Claims.First(x => x.Type == "sub").Value;
            var name = context.JwtSecurityToken.Claims.First(x => x.Type == "name").Value;
            var userManager = context.OwinContext.GetUserManager<BackOfficeUserManager>();

            var user = userService.GetByEmail(email);
            Log.Info($"Trying to authenticate user: {email}");

            if (user == null)
            {
                //todo
                IReadOnlyUserGroup userGroupByAlias;
                Log.Info($"User not found in umbraco. Creating user...");

                if (email.EndsWith("kruso.dk"))
                {
                    userGroupByAlias = userService.GetUserGroupByAlias("admin") as IReadOnlyUserGroup;
                }
                else
                {
                    userGroupByAlias = userService.GetUserGroupByAlias("employee") as IReadOnlyUserGroup;
                }
                user = userService.CreateUserWithIdentity(email, email);
                user.AddGroup(userGroupByAlias);
                userService.Save(user);
            }
            else
            {
                Log.Debug($"User exists in umbraco");
            }

            var identity = await userManager.FindByEmailAsync(email);
            if (identity.Logins.All(x => x.ProviderKey != providerKey))
            {
                identity.Logins.Add(new IdentityUserLogin(issuer, providerKey, user.Id));
                identity.Name = name;
                Log.Info($"Adding identification to user");
                await userManager.UpdateAsync(identity);
            }
        }

        private static async Task<int> SecurityTokenValidated(SecurityTokenValidatedNotification<OpenIdConnectMessage, OpenIdConnectAuthenticationOptions> n)
        {
            Log.Debug($"SecurityTokenValidated: ({n.ProtocolMessage.PostTitle} {n.State})");
            var identityUser = n.AuthenticationTicket.Identity;

            var newIdentityUser = new ClaimsIdentity(identityUser.AuthenticationType, ClaimTypes.GivenName, ClaimTypes.Role);

            newIdentityUser.AddClaim(new Claim(ClaimTypes.Email, identityUser.Name));
            newIdentityUser.AddClaim(new Claim(ClaimTypes.Upn, identityUser.Name));

            var nameIdentifier = identityUser.FindFirst(ClaimTypes.NameIdentifier);
            var givenName = identityUser.FindFirst(ClaimTypes.GivenName);

            if (nameIdentifier != null)
            {
                newIdentityUser.AddClaim(nameIdentifier);
            }
            if (givenName != null)
            {
                newIdentityUser.AddClaim(givenName);
            }

            n.AuthenticationTicket = new AuthenticationTicket(newIdentityUser, n.AuthenticationTicket.Properties);

            Log.Info($@"ADFS USER login. Name: {givenName?.Value ?? "NULL"} Email: {identityUser.Name}");

            return await Task.FromResult(0);
        }
    }
}

Packages:

  <package id="UmbracoCms" version="7.7.13" targetFramework="net452" />
  <package id="UmbracoCms.Core" version="7.7.13" targetFramework="net452" />
  <package id="UmbracoCms.IdentityExtensions" version="1.0.0" targetFramework="net452" />
  <package id="UmbracoCms.IdentityExtensions.AzureActiveDirectory" version="1.0.0" targetFramework="net452" />

I've tried disabling the SecurityTokenValidated event. I only see success statements in the log.

The strangest part is that the login works but periodically. An app-pool recycle seems to help, but again, only for a period. The reason for the 'complexity' is that the Azure AD provider does not provide email in the expected claim and we need to do some transformations. It turned out the Name claim was also the e-mail address, so I had to intercept the Notifications to pass in the Name claim as the Email claim

Shazwazza commented 5 years ago

Can you try updating these dependencies to these versions:

https://www.nuget.org/packages/Microsoft.Owin.Security/3.1.0 https://www.nuget.org/packages/Microsoft.Owin.Security.OpenIdConnect/3.1.0 https://www.nuget.org/packages/Microsoft.IdentityModel.Protocol.Extensions/1.0.4.403061554 https://www.nuget.org/packages/System.IdentityModel.Tokens.Jwt/4.0.4.403061554

Is there anything else i might need to know like is load balancing involved?

CasperTDK commented 5 years ago

Okay. Will do that now.

Hmm no, it is pretty straight forward setup. 1 server (Windows Server 2016 standard) and 1 instance of umbraco. The customer wants to only be authenticated through their Azure AD.

Shazwazza commented 5 years ago

ok thanks, at least we can rule out LB. It does seems odd that restarting the appdomain would fix it, i don't think there's a hidden cache within this setup. Let me know if you have any success with the updated libs. There's a 5.x (aspnet core compat) version of the Jwt one but i'm unsure if it's compatible, i'd need to test so not sure at this point.

CasperTDK commented 5 years ago

@Shazwazza It did not seem to help. I will try again on a different environment (update did not work on a different environment either). What i really need is insights into why the auth cookie is not being set. I can not make it produce any logs that provide helpful information. Do you have any good ideas on how we could test and get more information?

clarkd commented 5 years ago

@Shazwazza We're on Umbraco Cloud FYI so nothing atypical about our setup. Same workaround for us was to recycle the app pool (in our case, by toggling Debug mode on/off through Cloud UI). We're happy to add some additional tracing/logging if you point us in the right direction :)

CasperTDK commented 5 years ago

We've had a few days of stability and now it has stopped working again. I am very willing to help investigate, but I could use some pointers on how to gather the correct data. @Shazwazza

Shazwazza commented 5 years ago

Some of the code snippets above show that some logging is taking place at every possible corner of the OAuth process as far as OWIN goes, if none of those logs are yielding anything then I can only imagine that its something taking place within the BackOfficeController https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web/Editors/BackOfficeController.cs#L319

Perhaps this doesn't succeed but is returning no errors so you don't see anything? https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web/Editors/BackOfficeController.cs#L332

CasperTDK commented 5 years ago

@Shazwazza I can confirm none of the logs yield everything. When I test, the following statements are hit:

Log.Debug($"SecurityTokenValidated: ({n.ProtocolMessage.PostTitle} {n.State})"); Log.Info($"Trying to authenticate user: {email}"); Log.Debug($"User exists in umbraco");

I get the same logs when I successfully login and when it stops working (which is periodic )

So everything seems successful, but the user is redirected to login page and does not have an authenicated cookie.

CasperTDK commented 5 years ago

@Shazwazza I can't find a way to expose/log value of TempData[TokenExternalSignInError] = result.Errors; any ideas?

Shazwazza commented 5 years ago

You would see if there are errors in there in the UI when you login, that's what i'm saying above, perhaps that call is failing without returning any errors, only thing you can do is create a custom build of Umbraco and add logging throughout that method and/or step through the code to see what might be happening.

Another question: Do you guys have session timeouts enabled? i.e. umbracoTimeOutInMinutes in web.config and <keepUserLoggedIn>false</keepUserLoggedIn> in umbracoSettings.config ?

Shazwazza commented 5 years ago

On a side note - unfortunately i haven't been able to see this problem at all in my own implementation for Azure AD + Umbraco. It's been running for over a week and I log in each day without issue 🤔 I'm not doing any claims transforms, etc... just the simple/default setup

Back to this

The reason for the 'complexity' is that the Azure AD provider does not provide email in the expected claim and we need to do some transformations. It turned out the Name claim was also the e-mail address, so I had to intercept the Notifications to pass in the Name claim as the Email claim

I don't have this problem at all so that seems odd you have to add additional logic

I'd suggest reading this comment again https://github.com/umbraco/UmbracoIdentityExtensions/issues/20#issuecomment-393039094 to make sure that this isn't the underlying problem.

clarkd commented 5 years ago

Might be worth leaving that running for a few days without logging in and then try. See my comment RE inactivity above.

On Thu, 29 Nov 2018, 23:23 Shannon Deminick <notifications@github.com wrote:

On a side note - unfortunately i haven't been able to see this problem at all in my own implementation for Azure AD + Umbraco. It's been running for over a week and I log in each day without issue 🤔 I'm not doing any claims transforms, etc... just the simple/default setup

Back to this

The reason for the 'complexity' is that the Azure AD provider does not provide email in the expected claim and we need to do some transformations. It turned out the Name claim was also the e-mail address, so I had to intercept the Notifications to pass in the Name claim as the Email claim

I don't have this problem at all so that seems odd you have to add additional logic

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/umbraco/UmbracoIdentityExtensions/issues/20#issuecomment-443031494, or mute the thread https://github.com/notifications/unsubscribe-auth/ADng6415nU6juqY3ps9JvHiZ8K5Uspvjks5u0GxegaJpZM4ODSTj .

CasperTDK commented 5 years ago
    <add key="umbracoTimeOutInMinutes" value="20" />
    <keepUserLoggedIn>false</keepUserLoggedIn>
    <add key="umbracoConfigurationStatus" value="7.7.13" />

Yeah the biggest issue is the sporadic nature. Earlier this week, the AD login worked for the authors until around 10AM, then it just stopped working until a restart of the appPool. So it is not a question of inactivity for us.

I guess we will make a custom build of umbraco and put logging everywhere in BackOfficeController. Do you have any other services/controllers you think it would be good to place more logging in? We will need to make this build and push it to the customers and wait for days of testing, so i'd like to just add the logging once.

When you are testing, what umbraco version are you using and can you share the code snippet (just to make sure there are zero misunderstandings)? I could try and make the customer test it with their AD. @Shazwazza

CasperTDK commented 5 years ago

@Shazwazza I've added logging and can confirm that

BackOfficeController.ExternalSignInAsync
BackOfficeController.AutoLinkAndSignInExternalAccount
BackOfficeController.ExternalLinkLoginCallback

is not being invoked when I get stuck in the login page redirect issue. Where should I then place more logging?

CasperTDK commented 5 years ago

@Shazwazza I am able to send you network log or anything else you need as well

Shazwazza commented 5 years ago

I've had the AD provider on my site for a bit now without any issues and have left it for quite a while and still seems ok, the configuration I'm using is bare minimum, i've blogged about it https://shazwazza.com/post/configuring-azure-active-directory-login-with-umbraco/

AD should return the required claims without additional config but there might be an option you need to select as mentioned by Darren http://disq.us/p/1xjvv1i though this wasn't required for my setup

My site is running 7.12.4

When logging in with an External provider, the request will come in at BackOfficeController.Default -> RenderDefaultOrProcessExternalLoginAsync -> ExternalSignInAsync

Do you have any custom HttpModules running at all? or other OWIN based code that could intercept these calls?

CasperTDK commented 5 years ago

@Shazwazza I've tried ruling them out and still no clues in the logs. I am running the same setup as your blog but with these additions:

I also have a few adjustments I still need to make. 1) The users should be created THROUGH the AD (see: AuthorizationCodeReceived), as we do not want to store password/logins in umbraco. My pastebin above also contains this logic and it works 2) We can not update umbraco from 7.7.13 -> 7.12.4 at this phase of the project. There are too many risks right now. There is also no release notes that notes changes/fixes to the OAUTH authentication process 3) This line of code: adOptions.SetExternalSignInAutoLinkOptions(new ExternalSignInAutoLinkOptions(true, defaultUserGroups: null, defaultCulture: ConfigurationManager.AppSettings["umbracoDefaultUILanguage"]));

CasperTDK commented 5 years ago

The reason why I am using AuthorizationCode instead of autoLinkOptions.OnAutoLinking is because of the e-mail bug i mentioned: image

Shazwazza commented 5 years ago

@CasperTDK out of curiosity, when you log into the back office via AD does it tell you that your account is linked?

image

Can i suggest you try with the latest Umbraco version on a test environment just to see if you can replicate it? Could even be just a plain old simple umbraco install with this enabled.

CasperTDK commented 5 years ago

@Shazwazza Yes, it does say unlink when I am able to login: image

Yeah I can definitely try and do this. The biggest issue here is that the error is sporadic and there be several days without hiccups. It seems that when multiple editors are using the AD login, the issue will occur faster, but I have no actual evidence here. So if you have any other suggestions, that would be great

gjelhus commented 5 years ago

Hi

We are experiencing the same issue with version 7.12.3 of umbraco, using ADFS on windows server 2016 as our authentication provider. It works for a few hours at best a day then it fails. Restarting the application pool fixes the issue. We have so far been unable to reproduce the issue in our Test and QA environment, it only happens in production. We have a pretty quick release cycle so if there is any logging you need done we should be able to do that, but any hints on where to look would be appreciated. We could even build a custom version of the sourcecode.

CasperTDK commented 5 years ago

After being live for a while. the problem has worsened, and I am being told that the feature is only functioning 2-10 minutes before the login stops working. Effectively making the solution unusable. I am hesitant to do the upgrade suggestion as @gjelhus is running the newest version.

So please give any guidance on how to resolve this matter, we are ready to spend time and effort on it. @Shazwazza

We are running a custom build of umbraco, where we've inserted the extra log statements, but it has not given us relevant feedback.

This is the one that causes the most concern:

@Shazwazza I've added logging and can confirm that

BackOfficeController.ExternalSignInAsync BackOfficeController.AutoLinkAndSignInExternalAccount BackOfficeController.ExternalLinkLoginCallback is not being invoked when I get stuck in the login page redirect issue.

I am fairly certain of this, but I can do more testing

CasperTDK commented 5 years ago

Hi guys. I have solved the issue with the following hacks:

 public override void Configuration(IAppBuilder app)
        {
            app.UseKentorOwinCookieSaver();
            app.UseCookieAuthentication(new CookieAuthenticationOptions()
            {
                CookieManager = new SystemWebCookieManager()
            });
...

Sources: https://github.com/Sustainsys/owin-cookie-saver https://dotnetcodetips.com/Tip/91/Azure-OWIN-website-login-gets-stuck-on-a-never-ending-redirect-loop

I call them a hack, but I do not completely understand the problem nor the fix... But we've had 100% stability for over a week now! Before it was a daily issue