Merge pull request #2071 from sbwalker/dev

OIDC improvements

Oqtane.Client/Modules/Admin/Users/Index.razor

@@ -170,8 +170,8 @@ else
 						<Label Class="col-sm-3" For="allowsitelogin" HelpText="Do You Want To Allow Users To Sign In Using A Username And Password That Is Managed Locally On This Site? Note That You Should Only Disable This Option If You Have Already Sucessfully Configured An External Login Provider, Or Else You May Lock Yourself Out Of This Site." ResourceKey="AllowSiteLogin">Allow Site Login? </Label>
 						<div class="col-sm-9">
 							<select id="allowsitelogin" class="form-select" @bind="@_allowsitelogin" required>
-								<option value="True">@SharedLocalizer["Yes"]</option>
+								<option value="true">@SharedLocalizer["Yes"]</option>
-								<option value="False">@SharedLocalizer["No"]</option>
+								<option value="false">@SharedLocalizer["No"]</option>
 							</select>
 						</div>
 					</div>
@@ -229,7 +229,7 @@ else
 		_clientsecret = SettingService.GetSetting(settings, "OpenIdConnectOptions:ClientSecret", "");
 		_metadata = SettingService.GetSetting(settings, "OpenIdConnectOptions:MetadataAddress", "");
 		_logouturl = SettingService.GetSetting(settings, "OpenIdConnectOptions:LogoutUrl", "");
-		_allowsitelogin = SettingService.GetSetting(settings, "AllowSiteLogin", "True");
+		_allowsitelogin = SettingService.GetSetting(settings, "AllowSiteLogin", "true");
 	}
 
 	private List<UserRole> Search(string search)

Oqtane.Server/Extensions/OqtaneSiteAuthenticationBuilderExtensions.cs

@@ -2,7 +2,6 @@ using System;
 using System.Linq;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.AspNetCore.Authentication.OpenIdConnect;
 using Oqtane.Infrastructure;
 using Oqtane.Models;
@@ -15,7 +14,6 @@ using Oqtane.Repository;
 using System.IO;
 using System.Collections.Generic;
 using Oqtane.Security;
-using System.Net;
 using Microsoft.AspNetCore.Http;
 
 namespace Oqtane.Extensions
@@ -26,22 +24,11 @@ namespace Oqtane.Extensions
             this OqtaneSiteOptionsBuilder<TAlias> builder)
             where TAlias : class, IAlias, new()
         {
-            builder.WithSiteAuthenticationCore();
             builder.WithSiteAuthenticationOptions();
 
             return builder;
         }
 
-        public static OqtaneSiteOptionsBuilder<TAlias> WithSiteAuthenticationCore<TAlias>(
-            this OqtaneSiteOptionsBuilder<TAlias> builder)
-            where TAlias : class, IAlias, new()
-        {
-            builder.Services.DecorateService<IAuthenticationService, SiteAuthenticationService<TAlias>>();
-            builder.Services.Replace(ServiceDescriptor.Singleton<IAuthenticationSchemeProvider, SiteAuthenticationSchemeProvider>());
-
-            return builder;
-        }
-
         public static OqtaneSiteOptionsBuilder<TAlias> WithSiteAuthenticationOptions<TAlias>(
             this OqtaneSiteOptionsBuilder<TAlias> builder)
             where TAlias : class, IAlias, new()
@@ -75,8 +62,8 @@ namespace Oqtane.Extensions
 
                 // openid connect events
                 options.Events.OnTokenValidated = OnTokenValidated;
-                options.Events.OnRedirectToIdentityProvider = OnRedirectToIdentityProvider;
                 options.Events.OnRedirectToIdentityProviderForSignOut = OnRedirectToIdentityProviderForSignOut;
+                options.Events.OnAccessDenied = OnAccessDenied;
                 options.Events.OnRemoteFailure = OnRemoteFailure;
             });
 
@@ -97,6 +84,7 @@ namespace Oqtane.Extensions
             var email = context.Principal.FindFirstValue(ClaimTypes.Email);
             var providerKey = context.Principal.FindFirstValue(ClaimTypes.NameIdentifier);
             var loginProvider = context.HttpContext.GetAlias().SiteSettings["OpenIdConnectOptions:Authority"];
+            var alias = context.HttpContext.GetAlias();
             var _logger = context.HttpContext.RequestServices.GetRequiredService<ILogManager>();
 
             if (email != null)
@@ -117,10 +105,10 @@ namespace Oqtane.Extensions
                     if (result.Succeeded)
                     {
                         // add user login
-                        await _identityUserManager.AddLoginAsync(identityuser, new UserLoginInfo(loginProvider, providerKey, ""));
+                        await _identityUserManager.AddLoginAsync(identityuser, new UserLoginInfo(loginProvider, providerKey, email));
 
                         user = new User();
-                        user.SiteId = context.HttpContext.GetAlias().SiteId;
+                        user.SiteId = alias.SiteId;
                         user.Username = email;
                         user.DisplayName = email;
                         user.Email = email;
@@ -145,11 +133,11 @@ namespace Oqtane.Extensions
                                 Capacity = Constants.UserFolderCapacity,
                                 IsSystem = true,
                                 Permissions = new List<Permission>
-                            {
+                                {
-                                new Permission(PermissionNames.Browse, user.UserId, true),
+                                    new Permission(PermissionNames.Browse, user.UserId, true),
-                                new Permission(PermissionNames.View, RoleNames.Everyone, true),
+                                    new Permission(PermissionNames.View, RoleNames.Everyone, true),
-                                new Permission(PermissionNames.Edit, user.UserId, true)
+                                    new Permission(PermissionNames.Edit, user.UserId, true)
-                            }.EncodePermissions()
+                                }.EncodePermissions()
                             });
                         }
 
@@ -210,8 +198,8 @@ namespace Oqtane.Extensions
                     }
 
                     // add Oqtane claims
-                    List<UserRole> userroles = _userRoles.GetUserRoles(user.UserId, context.HttpContext.GetAlias().SiteId).ToList();
+                    List<UserRole> userroles = _userRoles.GetUserRoles(user.UserId, user.SiteId).ToList();
-                    var identity = UserSecurity.CreateClaimsIdentity(context.HttpContext.GetAlias(), user, userroles);
+                    var identity = UserSecurity.CreateClaimsIdentity(alias, user, userroles);
                     principal.AddClaims(identity.Claims);
 
                     // add provider
@@ -224,12 +212,6 @@ namespace Oqtane.Extensions
             }
         }
 
-        private static Task OnRedirectToIdentityProvider(RedirectContext context)
-        {
-            //context.ProtocolMessage.SetParameter("key", "value");
-            return Task.CompletedTask;
-        }
-
         private static Task OnRedirectToIdentityProviderForSignOut(RedirectContext context)
         {
             var logoutUrl = context.HttpContext.GetAlias().SiteSettings.GetValue("OpenIdConnectOptions:LogoutUrl", "");
@@ -251,59 +233,25 @@ namespace Oqtane.Extensions
             return Task.CompletedTask;
         }
 
-        private static Task OnRemoteFailure(RemoteFailureContext context)
+        private static Task OnAccessDenied(AccessDeniedContext context)
         {
             var _logger = context.HttpContext.RequestServices.GetRequiredService<ILogManager>();
-            _logger.Log(LogLevel.Error, nameof(OqtaneSiteAuthenticationBuilderExtensions), Enums.LogFunction.Security, "OpenId Connect Remote Failure {Error}", context.Failure.Message);
+            _logger.Log(LogLevel.Information, nameof(OqtaneSiteAuthenticationBuilderExtensions), Enums.LogFunction.Security, "OpenId Connect Access Denied - User May Have Cancelled Their External Login Attempt");
-            context.Response.Redirect(context.Properties.RedirectUri);
+            // redirect to login page
+            var alias = context.HttpContext.GetAlias();
+            context.Response.Redirect(alias.Path + "/login?returnurl=" + context.Properties.RedirectUri);
             context.HandleResponse();
             return Task.CompletedTask;
         }
 
-        public static bool DecorateService<TService, TImpl>(this IServiceCollection services, params object[] parameters)
+        private static Task OnRemoteFailure(RemoteFailureContext context)
         {
-            var existingService = services.SingleOrDefault(s => s.ServiceType == typeof(TService));
+            var _logger = context.HttpContext.RequestServices.GetRequiredService<ILogManager>();
-            if (existingService == null)
+            _logger.Log(LogLevel.Error, nameof(OqtaneSiteAuthenticationBuilderExtensions), Enums.LogFunction.Security, "OpenId Connect Remote Failure - {Error}", context.Failure.Message);
-                return false;
+            // redirect to original page
-
+            context.Response.Redirect(context.Properties.RedirectUri);
-            var newService = new ServiceDescriptor(existingService.ServiceType,
+            context.HandleResponse();
-                sp =>
+            return Task.CompletedTask;
-                {
-                    TService inner = (TService)ActivatorUtilities.CreateInstance(sp, existingService.ImplementationType!);
-
-                    var parameters2 = new object[parameters.Length + 1];
-                    Array.Copy(parameters, 0, parameters2, 1, parameters.Length);
-                    parameters2[0] = inner;
-
-                    return ActivatorUtilities.CreateInstance<TImpl>(sp, parameters2)!;
-                },
-                existingService.Lifetime);
-
-            if (existingService.ImplementationInstance != null)
-            {
-                newService = new ServiceDescriptor(existingService.ServiceType,
-                    sp =>
-                    {
-                        TService inner = (TService)existingService.ImplementationInstance;
-                        return ActivatorUtilities.CreateInstance<TImpl>(sp, inner, parameters)!;
-                    },
-                    existingService.Lifetime);
-            }
-            else if (existingService.ImplementationFactory != null)
-            {
-                newService = new ServiceDescriptor(existingService.ServiceType,
-                    sp =>
-                    {
-                        TService inner = (TService)existingService.ImplementationFactory(sp);
-                        return ActivatorUtilities.CreateInstance<TImpl>(sp, inner, parameters)!;
-                    },
-                    existingService.Lifetime);
-            }
-
-            services.Remove(existingService);
-            services.Add(newService);
-
-            return true;
         }
     }
 }

Oqtane.Server/Infrastructure/Internal/SiteAuthenticationSchemeProvider.cs

@@ -1,112 +0,0 @@
-using System;
-using System.Collections.Generic;
-using System.Threading.Tasks;
-using Microsoft.AspNetCore.Authentication;
-using Microsoft.Extensions.Options;
-
-namespace Oqtane.Infrastructure
-{
-    internal class SiteAuthenticationSchemeProvider : IAuthenticationSchemeProvider
-    {
-        public SiteAuthenticationSchemeProvider(IOptions<AuthenticationOptions> options)
-            : this(options, new Dictionary<string, AuthenticationScheme>(StringComparer.Ordinal))
-        {
-        }
-
-        public SiteAuthenticationSchemeProvider(IOptions<AuthenticationOptions> options, IDictionary<string, AuthenticationScheme> schemes)
-        {
-            _optionsProvider = options;
-
-            _schemes = schemes ?? throw new ArgumentNullException(nameof(schemes));
-            _requestHandlers = new List<AuthenticationScheme>();
-
-            foreach (var builder in _optionsProvider.Value.Schemes)
-            {
-                var scheme = builder.Build();
-                AddScheme(scheme);
-            }
-        }
-
-        private readonly IOptions<AuthenticationOptions> _optionsProvider;
-        private readonly object _lock = new object();
-
-        private readonly IDictionary<string, AuthenticationScheme> _schemes;
-        private readonly List<AuthenticationScheme> _requestHandlers;
-
-        private Task<AuthenticationScheme> GetDefaultSchemeAsync()
-            => _optionsProvider.Value.DefaultScheme != null
-            ? GetSchemeAsync(_optionsProvider.Value.DefaultScheme)
-            : Task.FromResult<AuthenticationScheme>(null);
-
-        public virtual Task<AuthenticationScheme> GetDefaultAuthenticateSchemeAsync()
-            => _optionsProvider.Value.DefaultAuthenticateScheme != null
-            ? GetSchemeAsync(_optionsProvider.Value.DefaultAuthenticateScheme)
-            : GetDefaultSchemeAsync();
-
-        public virtual Task<AuthenticationScheme> GetDefaultChallengeSchemeAsync()
-            => _optionsProvider.Value.DefaultChallengeScheme != null
-            ? GetSchemeAsync(_optionsProvider.Value.DefaultChallengeScheme)
-            : GetDefaultSchemeAsync();
-
-        public virtual Task<AuthenticationScheme> GetDefaultForbidSchemeAsync()
-            => _optionsProvider.Value.DefaultForbidScheme != null
-            ? GetSchemeAsync(_optionsProvider.Value.DefaultForbidScheme)
-            : GetDefaultChallengeSchemeAsync();
-
-        public virtual Task<AuthenticationScheme> GetDefaultSignInSchemeAsync()
-            => _optionsProvider.Value.DefaultSignInScheme != null
-            ? GetSchemeAsync(_optionsProvider.Value.DefaultSignInScheme)
-            : GetDefaultSchemeAsync();
-
-        public virtual Task<AuthenticationScheme> GetDefaultSignOutSchemeAsync()
-            => _optionsProvider.Value.DefaultSignOutScheme != null
-            ? GetSchemeAsync(_optionsProvider.Value.DefaultSignOutScheme)
-            : GetDefaultSignInSchemeAsync();
-
-        public virtual Task<AuthenticationScheme> GetSchemeAsync(string name)
-            => Task.FromResult(_schemes.ContainsKey(name) ? _schemes[name] : null);
-
-        public virtual Task<IEnumerable<AuthenticationScheme>> GetRequestHandlerSchemesAsync()
-            => Task.FromResult<IEnumerable<AuthenticationScheme>>(_requestHandlers);
-
-        public virtual void AddScheme(AuthenticationScheme scheme)
-        {
-            if (_schemes.ContainsKey(scheme.Name))
-            {
-                throw new InvalidOperationException("Scheme already exists: " + scheme.Name);
-            }
-            lock (_lock)
-            {
-                if (_schemes.ContainsKey(scheme.Name))
-                {
-                    throw new InvalidOperationException("Scheme already exists: " + scheme.Name);
-                }
-                if (typeof(IAuthenticationRequestHandler).IsAssignableFrom(scheme.HandlerType))
-                {
-                    _requestHandlers.Add(scheme);
-                }
-                _schemes[scheme.Name] = scheme;
-            }
-        }
-
-        public virtual void RemoveScheme(string name)
-        {
-            if (!_schemes.ContainsKey(name))
-            {
-                return;
-            }
-            lock (_lock)
-            {
-                if (_schemes.ContainsKey(name))
-                {
-                    var scheme = _schemes[name];
-                    _requestHandlers.Remove(scheme);
-                    _schemes.Remove(name);
-                }
-            }
-        }
-
-        public virtual Task<IEnumerable<AuthenticationScheme>> GetAllSchemesAsync()
-            => Task.FromResult<IEnumerable<AuthenticationScheme>>(_schemes.Values);
-    }
-}

Oqtane.Server/Infrastructure/Internal/SiteAuthenticationService.cs

@@ -1,62 +0,0 @@
-using System.Security.Claims;
-using System.Threading.Tasks;
-using Microsoft.AspNetCore.Authentication;
-using Microsoft.AspNetCore.Http;
-using Oqtane.Extensions;
-using Oqtane.Models;
-using Oqtane.Shared;
-
-namespace Oqtane.Infrastructure
-{
-    internal class SiteAuthenticationService<TAlias> : IAuthenticationService
-        where TAlias : class, IAlias, new()
-    {
-        private readonly IAuthenticationService _inner;
-
-        public SiteAuthenticationService(IAuthenticationService inner)
-        {
-            _inner = inner ?? throw new System.ArgumentNullException(nameof(inner));
-        }
-
-        private static void AddTenantIdentifierToProperties(HttpContext context, ref AuthenticationProperties properties)
-        {
-            // add site identifier to the authentication properties so on the callback we can use it to set context
-            var alias = context.GetAlias();
-            if (alias != null)
-            {
-                properties ??= new AuthenticationProperties();
-                if (!properties.Items.Keys.Contains(Constants.SiteToken))
-                {
-                    properties.Items.Add(Constants.SiteToken, alias.SiteKey);
-                }
-            }
-        }
-
-        public Task<AuthenticateResult> AuthenticateAsync(HttpContext context, string scheme)
-            => _inner.AuthenticateAsync(context, scheme);
-
-        public async Task ChallengeAsync(HttpContext context, string scheme, AuthenticationProperties properties)
-        {
-            AddTenantIdentifierToProperties(context, ref properties);
-            await _inner.ChallengeAsync(context, scheme, properties);
-        }
-
-        public async Task ForbidAsync(HttpContext context, string scheme, AuthenticationProperties properties)
-        {
-            AddTenantIdentifierToProperties(context, ref properties);
-            await _inner.ForbidAsync(context, scheme, properties);
-        }
-
-        public async Task SignInAsync(HttpContext context, string scheme, ClaimsPrincipal principal, AuthenticationProperties properties)
-        {
-            AddTenantIdentifierToProperties(context, ref properties);
-            await _inner.SignInAsync(context, scheme, principal, properties);
-        }
-
-        public async Task SignOutAsync(HttpContext context, string scheme, AuthenticationProperties properties)
-        {
-            AddTenantIdentifierToProperties(context, ref properties);
-            await _inner.SignOutAsync(context, scheme, properties);
-        }
-    }
-}

Oqtane.Server/Infrastructure/Middleware/TenantMiddleware.cs

@@ -33,7 +33,7 @@ namespace Oqtane.Infrastructure
                     alias.SiteSettings = cache.GetOrCreate("sitesettings:" + alias.SiteKey, entry =>
                     {
                         var settingRepository = context.RequestServices.GetService(typeof(ISettingRepository)) as ISettingRepository;
-                        return settingRepository.GetSettings(EntityNames.Site)
+                        return settingRepository.GetSettings(EntityNames.Site, alias.SiteId)
                             .ToDictionary(setting => setting.SettingName, setting => setting.SettingValue);
                     });
                     // save alias in HttpContext

Oqtane.Server/Security/PrincipalValidator.cs

@@ -24,7 +24,7 @@ namespace Oqtane.Security
                     if (alias != null)
                     {
                         // verify principal was authenticated for current tenant
-                        if (context.Principal.Claims.FirstOrDefault(item => item.Type == ClaimTypes.GroupSid)?.Value != alias.AliasId.ToString())
+                        if (context.Principal.Claims.FirstOrDefault(item => item.Type == ClaimTypes.GroupSid)?.Value != alias.SiteKey)
                         {
                             // tenant agnostic requests must be ignored 
                             string path = context.Request.Path.ToString().ToLower();

Oqtane.Shared/Security/UserSecurity.cs

@@ -134,7 +134,7 @@ namespace Oqtane.Security
             {
                 identity.AddClaim(new Claim(ClaimTypes.Name, user.Username));
                 identity.AddClaim(new Claim(ClaimTypes.PrimarySid, user.UserId.ToString()));
-                identity.AddClaim(new Claim(ClaimTypes.GroupSid, alias.AliasId.ToString()));
+                identity.AddClaim(new Claim(ClaimTypes.GroupSid, alias.SiteKey));
                 if (user.Roles.Contains(RoleNames.Host))
                 {
                     // host users are site admins by default