diff --git a/src/Umbraco.Core/Models/Identity/ExternalLogin.cs b/src/Umbraco.Core/Models/Identity/ExternalLogin.cs index 485ec66df4..c599dcab96 100644 --- a/src/Umbraco.Core/Models/Identity/ExternalLogin.cs +++ b/src/Umbraco.Core/Models/Identity/ExternalLogin.cs @@ -2,6 +2,7 @@ using System; namespace Umbraco.Cms.Core.Models.Identity { + /// public class ExternalLogin : IExternalLogin { diff --git a/src/Umbraco.Core/Models/Identity/ExternalLoginToken.cs b/src/Umbraco.Core/Models/Identity/ExternalLoginToken.cs new file mode 100644 index 0000000000..23f532fdfa --- /dev/null +++ b/src/Umbraco.Core/Models/Identity/ExternalLoginToken.cs @@ -0,0 +1,27 @@ +using System; + +namespace Umbraco.Cms.Core.Models.Identity +{ + /// + public class ExternalLoginToken : IExternalLoginToken + { + /// + /// Initializes a new instance of the class. + /// + public ExternalLoginToken(string loginProvider, string name, string value) + { + LoginProvider = loginProvider ?? throw new ArgumentNullException(nameof(loginProvider)); + Name = name ?? throw new ArgumentNullException(nameof(name)); + Value = value ?? throw new ArgumentNullException(nameof(value)); + } + + /// + public string LoginProvider { get; } + + /// + public string Name { get; } + + /// + public string Value { get; } + } +} diff --git a/src/Umbraco.Core/Models/Identity/IExternalLoginToken.cs b/src/Umbraco.Core/Models/Identity/IExternalLoginToken.cs new file mode 100644 index 0000000000..7cc3065e35 --- /dev/null +++ b/src/Umbraco.Core/Models/Identity/IExternalLoginToken.cs @@ -0,0 +1,23 @@ +namespace Umbraco.Cms.Core.Models.Identity +{ + /// + /// Used to persist an external login token for a user + /// + public interface IExternalLoginToken + { + /// + /// Gets the login provider + /// + string LoginProvider { get; } + + /// + /// Gets the name of the token + /// + string Name { get; } + + /// + /// Gets the value of the token + /// + string Value { get; } + } +} diff --git a/src/Umbraco.Core/Models/Identity/IIdentityUserLogin.cs b/src/Umbraco.Core/Models/Identity/IIdentityUserLogin.cs index 89ec823875..21d8f842af 100644 --- a/src/Umbraco.Core/Models/Identity/IIdentityUserLogin.cs +++ b/src/Umbraco.Core/Models/Identity/IIdentityUserLogin.cs @@ -24,7 +24,7 @@ namespace Umbraco.Cms.Core.Models.Identity string UserId { get; set; } // TODO: This should be able to be used by both users and members /// - /// Gets or sets any arbitrary data for the user and external provider - like user tokens returned from the provider + /// Gets or sets any arbitrary data for the user and external provider /// string UserData { get; set; } } diff --git a/src/Umbraco.Core/Models/Identity/IIdentityUserToken.cs b/src/Umbraco.Core/Models/Identity/IIdentityUserToken.cs new file mode 100644 index 0000000000..c3a451f31d --- /dev/null +++ b/src/Umbraco.Core/Models/Identity/IIdentityUserToken.cs @@ -0,0 +1,30 @@ +using Umbraco.Cms.Core.Models.Entities; + +namespace Umbraco.Cms.Core.Models.Identity +{ + /// + /// An external login provider token + /// + public interface IIdentityUserToken : IEntity + { + /// + /// Gets or sets user Id for the user who owns this token + /// + string UserId { get; set; } + + /// + /// Gets or sets the login provider for the login (i.e. Facebook, Google) + /// + string LoginProvider { get; set; } + + /// + /// Gets or sets the token name + /// + string Name { get; set; } + + /// + /// Gets or set the token value + /// + string Value { get; set; } + } +} diff --git a/src/Umbraco.Core/Models/Identity/IdentityUserToken.cs b/src/Umbraco.Core/Models/Identity/IdentityUserToken.cs new file mode 100644 index 0000000000..42e79a89dd --- /dev/null +++ b/src/Umbraco.Core/Models/Identity/IdentityUserToken.cs @@ -0,0 +1,44 @@ +using System; +using Umbraco.Cms.Core.Models.Entities; + +namespace Umbraco.Cms.Core.Models.Identity +{ + public class IdentityUserToken : EntityBase, IIdentityUserToken + { + /// + /// Initializes a new instance of the class. + /// + public IdentityUserToken(string loginProvider, string name, string value, string userId) + { + LoginProvider = loginProvider ?? throw new ArgumentNullException(nameof(loginProvider)); + Name = name ?? throw new ArgumentNullException(nameof(name)); + Value = value ?? throw new ArgumentNullException(nameof(value)); + UserId = userId; + } + + /// + /// Initializes a new instance of the class. + /// + public IdentityUserToken(int id, string loginProvider, string name, string value, string userId, DateTime createDate) + { + Id = id; + LoginProvider = loginProvider ?? throw new ArgumentNullException(nameof(loginProvider)); + Name = name ?? throw new ArgumentNullException(nameof(name)); + Value = value ?? throw new ArgumentNullException(nameof(value)); + UserId = userId; + CreateDate = createDate; + } + + /// + public string LoginProvider { get; set; } + + /// + public string Name { get; set; } + + /// + public string Value { get; set; } + + /// + public string UserId { get; set; } + } +} diff --git a/src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs b/src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs index b09991ae20..f0f891d48e 100644 --- a/src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs +++ b/src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs @@ -1,4 +1,4 @@ -// ReSharper disable once CheckNamespace +// ReSharper disable once CheckNamespace namespace Umbraco.Cms.Core { public static partial class Constants @@ -52,6 +52,7 @@ namespace Umbraco.Cms.Core public const string UserGroup2App = TableNamePrefix + "UserGroup2App"; public const string UserGroup2NodePermission = TableNamePrefix + "UserGroup2NodePermission"; public const string ExternalLogin = TableNamePrefix + "ExternalLogin"; + public const string ExternalLoginToken = TableNamePrefix + "ExternalLoginToken"; public const string Macro = /*TableNamePrefix*/ "cms" + "Macro"; public const string MacroProperty = /*TableNamePrefix*/ "cms" + "MacroProperty"; diff --git a/src/Umbraco.Core/Persistence/Repositories/IExternalLoginRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IExternalLoginRepository.cs index 8821c332c3..9f755c2256 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IExternalLoginRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IExternalLoginRepository.cs @@ -3,9 +3,22 @@ using Umbraco.Cms.Core.Models.Identity; namespace Umbraco.Cms.Core.Persistence.Repositories { - public interface IExternalLoginRepository : IReadWriteQueryRepository + public interface IExternalLoginRepository : IReadWriteQueryRepository, IQueryRepository { + /// + /// Replaces all external login providers for the user + /// + /// + /// void Save(int userId, IEnumerable logins); + + /// + /// Replaces all external login provider tokens for the providers specified for the user + /// + /// + /// + void Save(int userId, IEnumerable tokens); + void DeleteUserLogins(int memberId); } } diff --git a/src/Umbraco.Core/Services/IExternalLoginService.cs b/src/Umbraco.Core/Services/IExternalLoginService.cs index e19d30e293..8834a4b33f 100644 --- a/src/Umbraco.Core/Services/IExternalLoginService.cs +++ b/src/Umbraco.Core/Services/IExternalLoginService.cs @@ -4,7 +4,7 @@ using Umbraco.Cms.Core.Models.Identity; namespace Umbraco.Cms.Core.Services { /// - /// Used to store the external login info, this can be replaced with your own implementation + /// Used to store the external login info /// public interface IExternalLoginService : IService { @@ -13,7 +13,14 @@ namespace Umbraco.Cms.Core.Services /// /// /// - IEnumerable GetAll(int userId); + IEnumerable GetExternalLogins(int userId); + + /// + /// Returns all user login tokens assigned + /// + /// + /// + IEnumerable GetExternalLoginTokens(int userId); /// /// Returns all logins matching the login info - generally there should only be one but in some cases @@ -37,10 +44,16 @@ namespace Umbraco.Cms.Core.Services void Save(int userId, IEnumerable logins); /// - /// Save a single external login record + /// Saves the external login tokens associated with the user /// - /// - void Save(IIdentityUserLogin login); + /// + /// The user associated with the tokens + /// + /// + /// + /// This will replace all external login tokens for the user + /// + void Save(int userId, IEnumerable tokens); /// /// Deletes all user logins - normally used when a member is deleted diff --git a/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs b/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs index da83a7c913..78bb73f301 100644 --- a/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs +++ b/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs @@ -1,22 +1,20 @@ -using System; +using System; using System.IO; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.Json; using Microsoft.Extensions.FileProviders; using Newtonsoft.Json; using Newtonsoft.Json.Linq; +using Umbraco.Cms.Core.Configuration; namespace Umbraco.Cms.Core.Configuration { public class JsonConfigManipulator : IConfigManipulator { - private static readonly object s_locker = new object(); private readonly IConfiguration _configuration; + private readonly object _locker = new object(); - public JsonConfigManipulator(IConfiguration configuration) - { - _configuration = configuration; - } + public JsonConfigManipulator(IConfiguration configuration) => _configuration = configuration; public string UmbracoConnectionPath { get; } = $"ConnectionStrings:{ Cms.Core.Constants.System.UmbracoConnectionName}"; public void RemoveConnectionString() @@ -53,11 +51,13 @@ namespace Umbraco.Cms.Core.Configuration JToken token = json; foreach (var propertyName in key.Split(new[] { ':' })) { - if (token is null) break; + if (token is null) + break; token = CaseSelectPropertyValues(token, propertyName); } - if (token is null) return; + if (token is null) + return; var writer = new JTokenWriter(); writer.WriteValue(value); @@ -162,11 +162,9 @@ namespace Umbraco.Cms.Core.Configuration token?.Parent?.Remove(); } - - - private static void SaveJson(JsonConfigurationProvider provider, JObject json) + private void SaveJson(JsonConfigurationProvider provider, JObject json) { - lock (s_locker) + lock (_locker) { if (provider.Source.FileProvider is PhysicalFileProvider physicalFileProvider) { @@ -184,26 +182,26 @@ namespace Umbraco.Cms.Core.Configuration } } - private static JObject GetJson(JsonConfigurationProvider provider) + private JObject GetJson(JsonConfigurationProvider provider) { - if (provider.Source.FileProvider is PhysicalFileProvider physicalFileProvider) + lock (_locker) { - var jsonFilePath = Path.Combine(physicalFileProvider.Root, provider.Source.Path); - - var serializer = new JsonSerializer(); - using (var sr = new StreamReader(jsonFilePath)) - using (var jsonTextReader = new JsonTextReader(sr)) + if (provider.Source.FileProvider is PhysicalFileProvider physicalFileProvider) { - return serializer.Deserialize(jsonTextReader); + var jsonFilePath = Path.Combine(physicalFileProvider.Root, provider.Source.Path); + + var serializer = new JsonSerializer(); + using (var sr = new StreamReader(jsonFilePath)) + using (var jsonTextReader = new JsonTextReader(sr)) + { + return serializer.Deserialize(jsonTextReader); + } } + + return null; } - - return null; } - - - private JsonConfigurationProvider GetJsonConfigurationProvider(string requiredKey = null) { if (_configuration is IConfigurationRoot configurationRoot) diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs index e06818a589..0da54785f2 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs @@ -156,7 +156,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install { _configManipulator.SaveConnectionString(EmbeddedDatabaseConnectionString, Constants.DbProviderNames.SqlCe); - var path = _hostingEnvironment.MapPathContentRoot(Path.Combine("App_Data", "Umbraco.sdf")); + var path = _hostingEnvironment.MapPathContentRoot(Path.Combine(Constants.SystemDirectories.Data, "Umbraco.sdf")); if (File.Exists(path) == false) { // this should probably be in a "using (new SqlCeEngine)" clause but not sure diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseSchemaCreator.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseSchemaCreator.cs index d4ce35aebc..0225a60162 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseSchemaCreator.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseSchemaCreator.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -76,6 +76,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install typeof (AccessRuleDto), typeof (CacheInstructionDto), typeof (ExternalLoginDto), + typeof (ExternalLoginTokenDto), typeof (RedirectUrlDto), typeof (LockDto), typeof (UserGroupDto), diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs index 64d704bb11..3879549e43 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs @@ -196,11 +196,15 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade // to 8.9.0 To("{B5838FF5-1D22-4F6C-BCEB-F83ACB14B575}"); -// to 8.10.0 - To("{D6A8D863-38EC-44FB-91EC-ACD6A668BD18}"); // to 8.10.0 + To("{D6A8D863-38EC-44FB-91EC-ACD6A668BD18}"); + + // to 9.0.0 To("{22D801BA-A1FF-4539-BFCC-2139B55594F8}"); + To("{50A43237-A6F4-49E2-A7A6-5DAD65C84669}"); + To("{3D8DADEF-0FDA-4377-A5F0-B52C2110E8F2}"); + //FINAL } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/ExternalLoginTableIndexes.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/ExternalLoginTableIndexes.cs new file mode 100644 index 0000000000..cf1124c4d9 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/ExternalLoginTableIndexes.cs @@ -0,0 +1,72 @@ +using System.Collections.Generic; +using System.Linq; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; + +namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_0_0 +{ + public class ExternalLoginTableIndexes : MigrationBase + { + public ExternalLoginTableIndexes(IMigrationContext context) + : base(context) + { + } + + /// + /// Adds new indexes to the External Login table + /// + public override void Migrate() + { + // Before adding these indexes we need to remove duplicate data. + // Get all logins by latest + var logins = Database.Fetch() + .OrderByDescending(x => x.CreateDate) + .ToList(); + + var toDelete = new List(); + // used to track duplicates so they can be removed + var keys = new HashSet<(string, string)>(); + foreach(ExternalLoginDto login in logins) + { + if (!keys.Add((login.ProviderKey, login.LoginProvider))) + { + // if it already exists we need to remove this one + toDelete.Add(login.Id); + } + } + if (toDelete.Count > 0) + { + Database.DeleteMany().Where(x => toDelete.Contains(x.Id)).Execute(); + } + + var indexName1 = "IX_" + ExternalLoginDto.TableName + "_LoginProvider"; + + if (!IndexExists(indexName1)) + { + Create + .Index(indexName1) + .OnTable(ExternalLoginDto.TableName) + .OnColumn("loginProvider") + .Ascending() + .WithOptions() + .Unique() + .WithOptions() + .NonClustered() + .Do(); + } + + var indexName2 = "IX_" + ExternalLoginDto.TableName + "_ProviderKey"; + + if (!IndexExists(indexName2)) + { + Create + .Index(indexName2) + .OnTable(ExternalLoginDto.TableName) + .OnColumn("loginProvider").Ascending() + .OnColumn("providerKey").Ascending() + .WithOptions() + .NonClustered() + .Do(); + } + } + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/ExternalLoginTokenTable.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/ExternalLoginTokenTable.cs new file mode 100644 index 0000000000..e098bbd0b9 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/ExternalLoginTokenTable.cs @@ -0,0 +1,28 @@ +using System.Collections.Generic; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_0_0 +{ + public class ExternalLoginTokenTable : MigrationBase + { + public ExternalLoginTokenTable(IMigrationContext context) + : base(context) + { + } + + /// + /// Adds new External Login token table + /// + public override void Migrate() + { + IEnumerable tables = SqlSyntax.GetTablesInSchema(Context.Database); + if (tables.InvariantContains(ExternalLoginTokenDto.TableName)) + { + return; + } + + Create.Table().Do(); + } + } +} diff --git a/src/Umbraco.Infrastructure/Persistence/Dtos/ExternalLoginDto.cs b/src/Umbraco.Infrastructure/Persistence/Dtos/ExternalLoginDto.cs index d7aa4ac173..3fc65b28a5 100644 --- a/src/Umbraco.Infrastructure/Persistence/Dtos/ExternalLoginDto.cs +++ b/src/Umbraco.Infrastructure/Persistence/Dtos/ExternalLoginDto.cs @@ -1,34 +1,39 @@ -using System; +using System; using NPoco; using Umbraco.Cms.Infrastructure.Persistence.DatabaseAnnotations; using Umbraco.Cms.Infrastructure.Persistence.DatabaseModelDefinitions; namespace Umbraco.Cms.Infrastructure.Persistence.Dtos { - [TableName(Cms.Core.Constants.DatabaseSchema.Tables.ExternalLogin)] + [TableName(TableName)] [ExplicitColumns] [PrimaryKey("Id")] internal class ExternalLoginDto { + public const string TableName = Cms.Core.Constants.DatabaseSchema.Tables.ExternalLogin; + [Column("id")] - [PrimaryKeyColumn(Name = "PK_umbracoExternalLogin")] + [PrimaryKeyColumn] public int Id { get; set; } - // TODO: This is completely missing a FK!!? + // TODO: This is completely missing a FK!!? ... IIRC that is because we want to change this to a GUID + // to support both members and users for external logins and that will not have any referential integrity + // This should be part of the members task for enabling external logins. [Column("userId")] + [Index(IndexTypes.NonClustered)] public int UserId { get; set; } - // TODO: There should be an index on both LoginProvider and ProviderKey - [Column("loginProvider")] - [Length(4000)] + [Length(4000)] // TODO: This value seems WAY too high, this is just a name [NullSetting(NullSetting = NullSettings.NotNull)] + [Index(IndexTypes.UniqueNonClustered, Name = "IX_" + TableName + "_LoginProvider")] public string LoginProvider { get; set; } [Column("providerKey")] [Length(4000)] [NullSetting(NullSetting = NullSettings.NotNull)] + [Index(IndexTypes.NonClustered, ForColumns = "loginProvider,providerKey", Name = "IX_" + TableName + "_ProviderKey")] public string ProviderKey { get; set; } [Column("createDate")] diff --git a/src/Umbraco.Infrastructure/Persistence/Dtos/ExternalLoginTokenDto.cs b/src/Umbraco.Infrastructure/Persistence/Dtos/ExternalLoginTokenDto.cs new file mode 100644 index 0000000000..c5f3b7c8ac --- /dev/null +++ b/src/Umbraco.Infrastructure/Persistence/Dtos/ExternalLoginTokenDto.cs @@ -0,0 +1,42 @@ +using System; +using NPoco; +using Umbraco.Cms.Infrastructure.Persistence.DatabaseAnnotations; +using Umbraco.Cms.Infrastructure.Persistence.DatabaseModelDefinitions; + +namespace Umbraco.Cms.Infrastructure.Persistence.Dtos +{ + [TableName(TableName)] + [ExplicitColumns] + [PrimaryKey("Id")] + internal class ExternalLoginTokenDto + { + public const string TableName = Cms.Core.Constants.DatabaseSchema.Tables.ExternalLoginToken; + + [Column("id")] + [PrimaryKeyColumn] + public int Id { get; set; } + + [Column("externalLoginId")] + [ForeignKey(typeof(ExternalLoginDto), Column = "id")] + public int ExternalLoginId { get; set; } + + [Column("name")] + [Length(255)] + [NullSetting(NullSetting = NullSettings.NotNull)] + [Index(IndexTypes.UniqueNonClustered, ForColumns = "externalLoginId,name", Name = "IX_" + TableName + "_Name")] + public string Name { get; set; } + + [Column("value")] + [SpecialDbType(SpecialDbTypes.NVARCHARMAX)] + [NullSetting(NullSetting = NullSettings.NotNull)] + public string Value { get; set; } + + [Column("createDate")] + [Constraint(Default = SystemMethods.CurrentDateTime)] + public DateTime CreateDate { get; set; } + + [ResultColumn] + [Reference(ReferenceType.OneToOne, ColumnName = "ExternalLoginId")] + public ExternalLoginDto ExternalLoginDto { get; set; } + } +} diff --git a/src/Umbraco.Infrastructure/Persistence/Factories/ExternalLoginFactory.cs b/src/Umbraco.Infrastructure/Persistence/Factories/ExternalLoginFactory.cs index f356274d04..772545e2a3 100644 --- a/src/Umbraco.Infrastructure/Persistence/Factories/ExternalLoginFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/Factories/ExternalLoginFactory.cs @@ -6,6 +6,15 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Factories { internal static class ExternalLoginFactory { + public static IIdentityUserToken BuildEntity(ExternalLoginTokenDto dto) + { + var entity = new IdentityUserToken(dto.Id, dto.ExternalLoginDto.LoginProvider, dto.Name, dto.Value, dto.ExternalLoginDto.UserId.ToString(), dto.CreateDate); + + // reset dirty initial properties (U4-1946) + entity.ResetDirtyProperties(false); + return entity; + } + public static IIdentityUserLogin BuildEntity(ExternalLoginDto dto) { var entity = new IdentityUserLogin(dto.Id, dto.LoginProvider, dto.ProviderKey, dto.UserId.ToString(), dto.CreateDate) @@ -47,5 +56,19 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Factories return dto; } + + public static ExternalLoginTokenDto BuildDto(int externalLoginId, IExternalLoginToken token, int? id = null) + { + var dto = new ExternalLoginTokenDto + { + Id = id ?? default, + ExternalLoginId = externalLoginId, + Name = token.Name, + Value = token.Value, + CreateDate = DateTime.Now + }; + + return dto; + } } } diff --git a/src/Umbraco.Infrastructure/Persistence/Mappers/ExternalLoginMapper.cs b/src/Umbraco.Infrastructure/Persistence/Mappers/ExternalLoginMapper.cs index 78f321fe4b..198554be0c 100644 --- a/src/Umbraco.Infrastructure/Persistence/Mappers/ExternalLoginMapper.cs +++ b/src/Umbraco.Infrastructure/Persistence/Mappers/ExternalLoginMapper.cs @@ -1,4 +1,4 @@ -using System; +using System; using Umbraco.Cms.Core.Models.Identity; using Umbraco.Cms.Infrastructure.Persistence.Dtos; diff --git a/src/Umbraco.Infrastructure/Persistence/Mappers/ExternalLoginTokenMapper.cs b/src/Umbraco.Infrastructure/Persistence/Mappers/ExternalLoginTokenMapper.cs new file mode 100644 index 0000000000..11cb60dc83 --- /dev/null +++ b/src/Umbraco.Infrastructure/Persistence/Mappers/ExternalLoginTokenMapper.cs @@ -0,0 +1,25 @@ +using System; +using Umbraco.Cms.Core.Models.Identity; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; + +namespace Umbraco.Cms.Infrastructure.Persistence.Mappers +{ + [MapperFor(typeof(IIdentityUserToken))] + [MapperFor(typeof(IdentityUserToken))] + public sealed class ExternalLoginTokenMapper : BaseMapper + { + public ExternalLoginTokenMapper(Lazy sqlContext, MapperConfigurationStore maps) + : base(sqlContext, maps) + { } + + protected override void DefineMaps() + { + DefineMap(nameof(IdentityUserToken.Id), nameof(ExternalLoginTokenDto.Id)); + DefineMap(nameof(IdentityUserToken.CreateDate), nameof(ExternalLoginTokenDto.CreateDate)); + DefineMap(nameof(IdentityUserToken.Name), nameof(ExternalLoginTokenDto.Name)); + DefineMap(nameof(IdentityUserToken.Value), nameof(ExternalLoginTokenDto.Value)); + // separate table + DefineMap(nameof(IdentityUserToken.UserId), nameof(ExternalLoginDto.UserId)); + } + } +} diff --git a/src/Umbraco.Infrastructure/Persistence/Mappers/MapperCollectionBuilder.cs b/src/Umbraco.Infrastructure/Persistence/Mappers/MapperCollectionBuilder.cs index 38f1176b75..e797319810 100644 --- a/src/Umbraco.Infrastructure/Persistence/Mappers/MapperCollectionBuilder.cs +++ b/src/Umbraco.Infrastructure/Persistence/Mappers/MapperCollectionBuilder.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.Composing; namespace Umbraco.Cms.Infrastructure.Persistence.Mappers @@ -50,6 +50,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Mappers Add(); Add(); Add(); + Add(); Add(); Add(); Add(); diff --git a/src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs b/src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs index efe52b0271..05f15f7372 100644 --- a/src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs +++ b/src/Umbraco.Infrastructure/Persistence/NPocoSqlExtensions.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections; using System.Collections.Generic; using System.Linq; @@ -1015,6 +1015,7 @@ namespace Umbraco.Extensions /// /// The Sql statement. /// The Sql statement. + /// NOTE: This method will not work for all queries, only simple ones! public static Sql ForUpdate(this Sql sql) { // go find the first FROM clause, and append the lock hint diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs index 91408d0687..16c411c772 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs @@ -27,9 +27,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement : base(scopeAccessor, cache, logger) { } - /// - protected override Guid NodeObjectTypeId => throw new NotSupportedException(); - /// protected override IAuditEntry PerformGet(int id) { diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditRepository.cs index 1558266d55..d528f69dce 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditRepository.cs @@ -117,11 +117,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement throw new NotImplementedException(); } - protected override Guid NodeObjectTypeId - { - get { throw new NotImplementedException(); } - } - public void CleanLogs(int maximumAgeOfLogsInMinutes) { var oldestPermittedLogEntry = DateTime.Now.Subtract(new TimeSpan(0, maximumAgeOfLogsInMinutes, 0)); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ConsentRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ConsentRepository.cs index 0c1cc7376d..a64828ee54 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ConsentRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ConsentRepository.cs @@ -26,9 +26,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement : base(scopeAccessor, cache, logger) { } - /// - protected override Guid NodeObjectTypeId => throw new NotSupportedException(); - /// protected override IConsent PerformGet(int id) { diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs index da96e6dfc8..c6cf516283 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -27,10 +27,9 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement internal sealed class ContentRepositoryBase { /// - /// /// This is used for unit tests ONLY /// - public static bool ThrowOnWarning = false; + public static bool ThrowOnWarning { get; set; } = false; } public abstract class ContentRepositoryBase : EntityRepositoryBase, IContentRepository @@ -72,6 +71,11 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement protected abstract TRepository This { get; } + /// + /// Gets the node object type for the repository's entity + /// + protected abstract Guid NodeObjectTypeId { get; } + protected ILanguageRepository LanguageRepository { get; } protected IDataTypeService DataTypeService { get; } protected IRelationRepository RelationRepository { get; } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 7c8d816ec8..e5f618878d 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Data; using System.Globalization; @@ -44,6 +44,11 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement protected ILanguageRepository LanguageRepository { get; } protected abstract bool SupportsPublishing { get; } + /// + /// Gets the node object type for the repository's entity + /// + protected abstract Guid NodeObjectTypeId { get; } + public IEnumerable> Move(TEntity moving, EntityContainer container) { var parentId = Cms.Core.Constants.System.Root; diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DataTypeRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DataTypeRepository.cs index 5647ae4d72..001f2db000 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DataTypeRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DataTypeRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Data; using System.Globalization; @@ -112,7 +112,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return Array.Empty(); } - protected override Guid NodeObjectTypeId => Cms.Core.Constants.ObjectTypes.DataType; + protected Guid NodeObjectTypeId => Cms.Core.Constants.ObjectTypes.DataType; #endregion diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DictionaryRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DictionaryRepository.cs index 3cb9662a89..b953bc5b55 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DictionaryRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DictionaryRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -120,11 +120,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return new List(); } - protected override Guid NodeObjectTypeId - { - get { throw new NotImplementedException(); } - } - #endregion #region Unit of Work Implementation diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs index 3500f0458d..a0e651f1dd 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -956,6 +956,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement // reading repository purely for looking up by GUID // TODO: ugly and to fix we need to decouple the IRepositoryQueryable -> IRepository -> IReadRepository which should all be separate things! + // This sub-repository pattern is super old and totally unecessary anymore, caching can be handled in much nicer ways without this private class ContentByGuidReadRepository : EntityRepositoryBase { private readonly DocumentRepository _outerRepo; @@ -966,8 +967,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement _outerRepo = outerRepo; } - protected override Guid NodeObjectTypeId => _outerRepo.NodeObjectTypeId; - protected override IContent PerformGet(Guid id) { var sql = _outerRepo.GetBaseQuery(QueryType.Single) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DomainRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DomainRepository.cs index fe9f74cd4a..4db9a3b0cc 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DomainRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DomainRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Data; using System.Linq; @@ -81,11 +81,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId - { - get { throw new NotImplementedException(); } - } - protected override void PersistNewItem(IDomain entity) { var exists = Database.ExecuteScalar("SELECT COUNT(*) FROM umbracoDomain WHERE domainName = @domainName", new { domainName = entity.DomainName }); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityContainerRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityContainerRepository.cs index 9f6912c0fe..bc1b1b1881 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityContainerRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityContainerRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -113,20 +113,11 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return sql; } - protected override string GetBaseWhereClause() - { - return "umbracoNode.id = @id and nodeObjectType = @NodeObjectType"; - } + protected override string GetBaseWhereClause() => "umbracoNode.id = @id and nodeObjectType = @NodeObjectType"; - protected override IEnumerable GetDeleteClauses() - { - throw new NotImplementedException(); - } + protected override IEnumerable GetDeleteClauses() => throw new NotImplementedException(); - protected override Guid NodeObjectTypeId - { - get { return _containerObjectType; } - } + protected Guid NodeObjectTypeId => _containerObjectType; protected override void PersistDeletedItem(EntityContainer entity) { diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepositoryBase.cs index 3b93e09867..25927213e5 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepositoryBase.cs @@ -78,11 +78,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return PerformCount(query); })); - /// - /// Gets the node object type for the repository's entity - /// - protected abstract Guid NodeObjectTypeId { get; } - /// /// Gets the repository cache policy /// diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ExternalLoginRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ExternalLoginRepository.cs index ff64fb0d49..a262636bfc 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ExternalLoginRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ExternalLoginRepository.cs @@ -6,6 +6,7 @@ using NPoco; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models.Entities; using Umbraco.Cms.Core.Models.Identity; +using Umbraco.Cms.Core.Persistence; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Persistence.Repositories; using Umbraco.Cms.Core.Scoping; @@ -24,10 +25,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement : base(scopeAccessor, cache, logger) { } - public void DeleteUserLogins(int memberId) - { - Database.Delete("WHERE userId=@userId", new { userId = memberId }); - } + public void DeleteUserLogins(int memberId) => Database.Delete("WHERE userId=@userId", new { userId = memberId }); public void Save(int userId, IEnumerable logins) { @@ -44,41 +42,37 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement var toDelete = new List(); var toInsert = new List(logins); - var existingLogins = Database.Query(sql).OrderByDescending(x => x.CreateDate).ToList(); - // used to track duplicates so they can be removed - var keys = new HashSet<(string, string)>(); - + var existingLogins = Database.Fetch(sql); + foreach (var existing in existingLogins) { - if (!keys.Add((existing.ProviderKey, existing.LoginProvider))) - { - // if it already exists we need to remove this one - toDelete.Add(existing.Id); - } - else - { - var found = logins.FirstOrDefault(x => + var found = logins.FirstOrDefault(x => x.LoginProvider.Equals(existing.LoginProvider, StringComparison.InvariantCultureIgnoreCase) && x.ProviderKey.Equals(existing.ProviderKey, StringComparison.InvariantCultureIgnoreCase)); - if (found != null) - { - toUpdate.Add(existing.Id, found); - // if it's an update then it's not an insert - toInsert.RemoveAll(x => x.ProviderKey == found.ProviderKey && x.LoginProvider == found.LoginProvider); - } - else - { - toDelete.Add(existing.Id); - } + if (found != null) + { + toUpdate.Add(existing.Id, found); + // if it's an update then it's not an insert + toInsert.RemoveAll(x => x.ProviderKey == found.ProviderKey && x.LoginProvider == found.LoginProvider); + } + else + { + toDelete.Add(existing.Id); } } // do the deletes, updates and inserts if (toDelete.Count > 0) + { Database.DeleteMany().Where(x => toDelete.Contains(x.Id)).Execute(); + } + foreach (var u in toUpdate) + { Database.Update(ExternalLoginFactory.BuildDto(userId, u.Value, u.Key)); + } + Database.InsertBulk(toInsert.Select(i => ExternalLoginFactory.BuildDto(userId, i))); } @@ -157,10 +151,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return sql; } - protected override string GetBaseWhereClause() - { - return "umbracoExternalLogin.id = @id"; - } + protected override string GetBaseWhereClause() => "umbracoExternalLogin.id = @id"; protected override IEnumerable GetDeleteClauses() { @@ -171,11 +162,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId - { - get { throw new NotImplementedException(); } - } - protected override void PersistNewItem(IIdentityUserLogin entity) { entity.AddingEntity(); @@ -198,5 +184,113 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement entity.ResetDirtyProperties(); } + + /// + /// Query for user tokens + /// + /// + /// + public IEnumerable Get(IQuery query) + { + Sql sqlClause = GetBaseTokenQuery(false); + + var translator = new SqlTranslator(sqlClause, query); + Sql sql = translator.Translate(); + + List dtos = Database.Fetch(sql); + + foreach (ExternalLoginTokenDto dto in dtos) + { + yield return ExternalLoginFactory.BuildEntity(dto); + } + } + + /// + /// Count for user tokens + /// + /// + /// + public int Count(IQuery query) + { + Sql sql = Sql().SelectCount().From(); + return Database.ExecuteScalar(sql); + } + + public void Save(int userId, IEnumerable tokens) + { + // get the existing logins (provider + id) + var existingUserLogins = Database + .Fetch(GetBaseQuery(false).Where(x => x.UserId == userId)) + .ToDictionary(x => x.LoginProvider, x => x.Id); + + // deduplicate the tokens + tokens = tokens.DistinctBy(x => x.LoginProvider + x.Name).ToList(); + + var providers = tokens.Select(x => x.LoginProvider).Distinct().ToList(); + + Sql sql = GetBaseTokenQuery(true) + .WhereIn(x => x.LoginProvider, providers) + .Where(x => x.UserId == userId); + + var toUpdate = new Dictionary(); + var toDelete = new List(); + var toInsert = new List(tokens); + + var existingTokens = Database.Fetch(sql); + + foreach (ExternalLoginTokenDto existing in existingTokens) + { + IExternalLoginToken found = tokens.FirstOrDefault(x => + x.LoginProvider.InvariantEquals(existing.ExternalLoginDto.LoginProvider) + && x.Name.InvariantEquals(existing.Name)); + + if (found != null) + { + toUpdate.Add(existing.Id, (found, existing.ExternalLoginId)); + // if it's an update then it's not an insert + toInsert.RemoveAll(x => x.LoginProvider.InvariantEquals(found.LoginProvider) && x.Name.InvariantEquals(found.Name)); + } + else + { + toDelete.Add(existing.Id); + } + } + + // do the deletes, updates and inserts + if (toDelete.Count > 0) + { + Database.DeleteMany().Where(x => toDelete.Contains(x.Id)).Execute(); + } + + foreach (KeyValuePair u in toUpdate) + { + Database.Update(ExternalLoginFactory.BuildDto(u.Value.externalLoginId, u.Value.externalLoginToken, u.Key)); + } + + var insertDtos = new List(); + foreach(IExternalLoginToken t in toInsert) + { + if (!existingUserLogins.TryGetValue(t.LoginProvider, out int externalLoginId)) + { + throw new InvalidOperationException($"A token was attempted to be saved for login provider {t.LoginProvider} which is not assigned to this user"); + } + insertDtos.Add(ExternalLoginFactory.BuildDto(externalLoginId, t)); + } + Database.InsertBulk(insertDtos); + } + + private Sql GetBaseTokenQuery(bool forUpdate) + => forUpdate ? Sql() + .Select(r => r.Select(x => x.ExternalLoginDto)) + .From() + .Append(" WITH (UPDLOCK)") // ensure these table values are locked for updates, the ForUpdate ext method does not work here + .InnerJoin() + .On(x => x.ExternalLoginId, x => x.Id) + : Sql() + .Select() + .AndSelect(x => x.LoginProvider, x => x.UserId) + .From() + .InnerJoin() + .On(x => x.ExternalLoginId, x => x.Id); } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs index 40dfb4cecf..74988d2026 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -33,8 +33,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement #region Overrides of EntityRepositoryBase - protected override Guid NodeObjectTypeId => throw new NotSupportedException(); - protected override Sql GetBaseQuery(bool isCount) { var sql = SqlContext.Sql(); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LanguageRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LanguageRepository.cs index 73c77e9e05..90d53df9ab 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LanguageRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LanguageRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -124,8 +124,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId => throw new NotImplementedException(); - #endregion #region Unit of Work Implementation diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LogViewerQueryRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LogViewerQueryRepository.cs index 35f0b8fdab..556aba8e22 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LogViewerQueryRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LogViewerQueryRepository.cs @@ -63,11 +63,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId - { - get { throw new NotImplementedException(); } - } - protected override void PersistNewItem(ILogViewerQuery entity) { var exists = Database.ExecuteScalar($"SELECT COUNT(*) FROM {Core.Constants.DatabaseSchema.Tables.LogViewerQuery} WHERE name = @name", diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MacroRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MacroRepository.cs index d4ee347c98..0a9eb7b0ef 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MacroRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MacroRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -136,8 +136,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId => throw new NotImplementedException(); - protected override void PersistNewItem(IMacro entity) { entity.AddingEntity(); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs index ba1fe05d0c..3efe35e4c4 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Text.RegularExpressions; @@ -407,13 +407,10 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return _mediaByGuidReadRepository.Exists(id); } - /// - /// A reading repository purely for looking up by GUID - /// - /// - /// TODO: This is ugly and to fix we need to decouple the IRepositoryQueryable -> IRepository -> IReadRepository which should all be separate things! - /// Then we can do the same thing with repository instances and we wouldn't need to leave all these methods as not implemented because we wouldn't need to implement them - /// + + // A reading repository purely for looking up by GUID + // TODO: This is ugly and to fix we need to decouple the IRepositoryQueryable -> IRepository -> IReadRepository which should all be separate things! + // This sub-repository pattern is super old and totally unecessary anymore, caching can be handled in much nicer ways without this private class MediaByGuidReadRepository : EntityRepositoryBase { private readonly MediaRepository _outerRepo; @@ -424,8 +421,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement _outerRepo = outerRepo; } - protected override Guid NodeObjectTypeId => _outerRepo.NodeObjectTypeId; - protected override IMedia PerformGet(Guid id) { var sql = _outerRepo.GetBaseQuery(QueryType.Single) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberGroupRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberGroupRepository.cs index 84523777ee..abce17a331 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberGroupRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberGroupRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -89,7 +89,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId => Cms.Core.Constants.ObjectTypes.MemberGroup; + protected Guid NodeObjectTypeId => Cms.Core.Constants.ObjectTypes.MemberGroup; protected override void PersistNewItem(IMemberGroup entity) { diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PermissionRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PermissionRepository.cs index 34fe764203..582120992b 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PermissionRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PermissionRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -280,8 +280,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return new List(); } - protected override Guid NodeObjectTypeId => throw new InvalidOperationException("This property won't be implemented."); - protected override void PersistDeletedItem(ContentPermissionSet entity) { throw new InvalidOperationException("This method won't be implemented."); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PublicAccessRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PublicAccessRepository.cs index c8bb10e7d9..a618e0e49f 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PublicAccessRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PublicAccessRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -81,8 +81,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId => throw new NotImplementedException(); - protected override void PersistNewItem(PublicAccessEntry entity) { entity.AddingEntity(); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RedirectUrlRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RedirectUrlRepository.cs index 5048812a3a..1536768bae 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RedirectUrlRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RedirectUrlRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Security.Cryptography; @@ -79,11 +79,6 @@ JOIN umbracoNode ON umbracoRedirectUrl.contentKey=umbracoNode.uniqueID"); return list; } - protected override Guid NodeObjectTypeId - { - get { throw new NotImplementedException(); } - } - protected override void PersistNewItem(IRedirectUrl entity) { var dto = Map(entity); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RelationRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RelationRepository.cs index 0927fe66af..f531c61037 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RelationRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RelationRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -126,11 +126,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId - { - get { throw new NotImplementedException(); } - } - #endregion #region Unit of Work Implementation diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RelationTypeRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RelationTypeRepository.cs index 7ae651cc9e..f6f3454560 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RelationTypeRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RelationTypeRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -121,11 +121,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId - { - get { throw new NotImplementedException(); } - } - #endregion #region Unit of Work Implementation diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ServerRegistrationRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ServerRegistrationRepository.cs index 44cdd445d1..66498b3f67 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ServerRegistrationRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ServerRegistrationRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -94,8 +94,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId => throw new NotImplementedException(); - protected override void PersistNewItem(IServerRegistration entity) { entity.AddingEntity(); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/SimpleGetRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/SimpleGetRepository.cs index 7fe013290b..6053d35085 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/SimpleGetRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/SimpleGetRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -81,8 +81,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement throw new InvalidOperationException("This method won't be implemented."); } - protected sealed override Guid NodeObjectTypeId => throw new InvalidOperationException("This property won't be implemented."); - protected sealed override void PersistNewItem(TEntity entity) { throw new InvalidOperationException("This method won't be implemented."); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TagRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TagRepository.cs index 9557383347..8fbc784576 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TagRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TagRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Text; @@ -25,9 +25,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement #region Manage Tag Entities - /// - protected override Guid NodeObjectTypeId => throw new NotSupportedException(); - /// protected override ITag PerformGet(int id) { diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs index e39a5eb7b7..ec54a6bef8 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -140,7 +140,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId => Cms.Core.Constants.ObjectTypes.Template; + protected Guid NodeObjectTypeId => Cms.Core.Constants.ObjectTypes.Template; protected override void PersistNewItem(ITemplate entity) { diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs index d396fdcff7..ad5e36c0ab 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -291,8 +291,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return list; } - protected override Guid NodeObjectTypeId => throw new InvalidOperationException("This property won't be implemented."); - protected override void PersistNewItem(IUserGroup entity) { entity.AddingEntity(); @@ -403,8 +401,6 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement throw new InvalidOperationException("This method won't be implemented."); } - protected override Guid NodeObjectTypeId => throw new InvalidOperationException("This property won't be implemented."); - #endregion protected override void PersistNewItem(UserGroupWithUsers entity) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs index 4e4f34bf54..93700549ac 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; @@ -431,8 +431,6 @@ ORDER BY colName"; return list; } - protected override Guid NodeObjectTypeId => throw new NotImplementedException(); - protected override void PersistNewItem(IUser entity) { // the use may have no identity, ie ID is zero, and be v7 super diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs index bd05ce0461..8205a380ef 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs @@ -102,6 +102,7 @@ namespace Umbraco.Cms.Core.Security // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(BackOfficeIdentityUser.Logins)); + var isTokensPropertyDirty = user.IsPropertyDirty(nameof(BackOfficeIdentityUser.LoginTokens)); UpdateMemberProperties(userEntity, user); @@ -125,6 +126,16 @@ namespace Umbraco.Cms.Core.Security x.UserData))); } + if (isTokensPropertyDirty) + { + _externalLoginService.Save( + userEntity.Id, + user.LoginTokens.Select(x => new ExternalLoginToken( + x.LoginProvider, + x.Name, + x.Value))); + } + return Task.FromResult(IdentityResult.Success); } @@ -151,6 +162,7 @@ namespace Umbraco.Cms.Core.Security { // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(BackOfficeIdentityUser.Logins)); + var isTokensPropertyDirty = user.IsPropertyDirty(nameof(BackOfficeIdentityUser.LoginTokens)); if (UpdateMemberProperties(found, user)) { @@ -166,6 +178,16 @@ namespace Umbraco.Cms.Core.Security x.ProviderKey, x.UserData))); } + + if (isTokensPropertyDirty) + { + _externalLoginService.Save( + found.Id, + user.LoginTokens.Select(x => new ExternalLoginToken( + x.LoginProvider, + x.Name, + x.Value))); + } } scope.Complete(); @@ -543,7 +565,9 @@ namespace Umbraco.Cms.Core.Security { if (user != null) { - user.SetLoginsCallback(new Lazy>(() => _externalLoginService.GetAll(UserIdToInt(user.Id)))); + var userId = UserIdToInt(user.Id); + user.SetLoginsCallback(new Lazy>(() => _externalLoginService.GetExternalLogins(userId))); + user.SetTokensCallback(new Lazy>(() => _externalLoginService.GetExternalLoginTokens(userId))); } return user; @@ -757,24 +781,102 @@ namespace Umbraco.Cms.Core.Security [EditorBrowsable(EditorBrowsableState.Never)] public override Task> GetUsersForClaimAsync(Claim claim, CancellationToken cancellationToken = default) => throw new NotImplementedException(); - // TODO: We should support these /// - /// Not supported in Umbraco + /// Overridden to support Umbraco's own data storage requirements + /// + /// + /// The base class's implementation of this calls into FindTokenAsync and AddUserTokenAsync, both methods will only work with ORMs that are change + /// tracking ORMs like EFCore. + /// + /// + public override Task SetTokenAsync(BackOfficeIdentityUser user, string loginProvider, string name, string value, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + ThrowIfDisposed(); + + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } + + IIdentityUserToken token = user.LoginTokens.FirstOrDefault(x => x.LoginProvider.InvariantEquals(loginProvider) && x.Name.InvariantEquals(name)); + if (token == null) + { + user.LoginTokens.Add(new IdentityUserToken(loginProvider, name, value, user.Id)); + } + else + { + token.Value = value; + } + + return Task.CompletedTask; + } + + /// + /// Overridden to support Umbraco's own data storage requirements + /// + /// + /// The base class's implementation of this calls into FindTokenAsync, RemoveUserTokenAsync and AddUserTokenAsync, both methods will only work with ORMs that are change + /// tracking ORMs like EFCore. + /// + /// + public override Task RemoveTokenAsync(BackOfficeIdentityUser user, string loginProvider, string name, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + ThrowIfDisposed(); + + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } + + IIdentityUserToken token = user.LoginTokens.FirstOrDefault(x => x.LoginProvider.InvariantEquals(loginProvider) && x.Name.InvariantEquals(name)); + if (token != null) + { + user.LoginTokens.Remove(token); + } + + return Task.CompletedTask; + } + + /// + /// Overridden to support Umbraco's own data storage requirements + /// + /// + /// The base class's implementation of this calls into FindTokenAsync, RemoveUserTokenAsync and AddUserTokenAsync, both methods will only work with ORMs that are change + /// tracking ORMs like EFCore. + /// + /// + public override Task GetTokenAsync(BackOfficeIdentityUser user, string loginProvider, string name, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + ThrowIfDisposed(); + + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } + IIdentityUserToken token = user.LoginTokens.FirstOrDefault(x => x.LoginProvider.InvariantEquals(loginProvider) && x.Name.InvariantEquals(name)); + + return Task.FromResult(token?.Value); + } + + /// + /// Not supported in Umbraco, see comments above on GetTokenAsync, RemoveTokenAsync, SetTokenAsync /// /// - [EditorBrowsable(EditorBrowsableState.Never)] protected override Task> FindTokenAsync(BackOfficeIdentityUser user, string loginProvider, string name, CancellationToken cancellationToken) => throw new NotImplementedException(); /// - /// Not supported in Umbraco + /// Not supported in Umbraco, see comments above on GetTokenAsync, RemoveTokenAsync, SetTokenAsync /// /// [EditorBrowsable(EditorBrowsableState.Never)] protected override Task AddUserTokenAsync(IdentityUserToken token) => throw new NotImplementedException(); /// - /// Not supported in Umbraco + /// Not supported in Umbraco, see comments above on GetTokenAsync, RemoveTokenAsync, SetTokenAsync /// /// [EditorBrowsable(EditorBrowsableState.Never)] diff --git a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs index 6be981b3d1..2f2127201d 100644 --- a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs @@ -689,6 +689,12 @@ namespace Umbraco.Cms.Core.Security } } + if (member.IsApproved != identityUserMember.IsApproved) + { + anythingChanged = true; + member.IsApproved = identityUserMember.IsApproved; + } + if (identityUserMember.IsPropertyDirty(nameof(MemberIdentityUser.UserName)) && member.Username != identityUserMember.UserName && identityUserMember.UserName.IsNullOrWhiteSpace() == false) { diff --git a/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs b/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs index 9d3c1ad7c9..525e7f839a 100644 --- a/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs +++ b/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs @@ -37,7 +37,9 @@ namespace Umbraco.Cms.Core.Models.Identity private string _passwordHash; private DateTime? _lastPasswordChangeDateUtc; private ObservableCollection _logins; + private ObservableCollection _tokens; private Lazy> _getLogins; + private Lazy> _getTokens; private ObservableCollection> _roles; /// @@ -180,6 +182,38 @@ namespace Umbraco.Cms.Core.Models.Identity } } + /// + /// Gets the external login tokens collection + /// + public ICollection LoginTokens + { + get + { + // return if it exists + if (_tokens is not null) + { + return _tokens; + } + + _tokens = new ObservableCollection(); + + // if the callback is there and hasn't been created yet then execute it and populate the logins + // if (_getTokens != null && !_getTokens.IsValueCreated) + if (_getTokens?.IsValueCreated != true) + { + foreach (IIdentityUserToken l in _getTokens.Value) + { + _tokens.Add(l); + } + } + + // now assign events + _tokens.CollectionChanged += LoginTokens_CollectionChanged; + + return _tokens; + } + } + /// /// Gets or sets user ID (Primary Key) /// @@ -266,10 +300,18 @@ namespace Umbraco.Cms.Core.Models.Identity /// Used to set a lazy call back to populate the user's Login list /// /// The lazy value - public void SetLoginsCallback(Lazy> callback) => _getLogins = callback ?? throw new ArgumentNullException(nameof(callback)); + internal void SetLoginsCallback(Lazy> callback) => _getLogins = callback ?? throw new ArgumentNullException(nameof(callback)); + + /// + /// Used to set a lazy call back to populate the user's token list + /// + /// The lazy value + internal void SetTokensCallback(Lazy> callback) => _getTokens = callback ?? throw new ArgumentNullException(nameof(callback)); private void Logins_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) => BeingDirty.OnPropertyChanged(nameof(Logins)); + private void LoginTokens_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) => BeingDirty.OnPropertyChanged(nameof(LoginTokens)); + private void Roles_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) => BeingDirty.OnPropertyChanged(nameof(Roles)); } } diff --git a/src/Umbraco.Infrastructure/Services/Implement/ExternalLoginService.cs b/src/Umbraco.Infrastructure/Services/Implement/ExternalLoginService.cs index bbec75338b..79c827f641 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/ExternalLoginService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/ExternalLoginService.cs @@ -20,16 +20,28 @@ namespace Umbraco.Cms.Core.Services.Implement } /// - public IEnumerable GetAll(int userId) + public IEnumerable GetExternalLogins(int userId) { using (var scope = ScopeProvider.CreateScope(autoComplete: true)) { - var asString = userId.ToString(); // TODO: This is temp until we update the external service to support guids for both users and members + // TODO: This is temp until we update the external service to support guids for both users and members + var asString = userId.ToString(); return _externalLoginRepository.Get(Query().Where(x => x.UserId == asString)) .ToList(); } } + public IEnumerable GetExternalLoginTokens(int userId) + { + using (var scope = ScopeProvider.CreateScope(autoComplete: true)) + { + // TODO: This is temp until we update the external service to support guids for both users and members + var asString = userId.ToString(); + return _externalLoginRepository.Get(Query().Where(x => x.UserId == asString)) + .ToList(); + } + } + /// public IEnumerable Find(string loginProvider, string providerKey) { @@ -51,12 +63,11 @@ namespace Umbraco.Cms.Core.Services.Implement } } - /// - public void Save(IIdentityUserLogin login) + public void Save(int userId, IEnumerable tokens) { using (var scope = ScopeProvider.CreateScope()) { - _externalLoginRepository.Save(login); + _externalLoginRepository.Save(userId, tokens); scope.Complete(); } } @@ -70,7 +81,5 @@ namespace Umbraco.Cms.Core.Services.Implement scope.Complete(); } } - - } } diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ExternalLoginServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ExternalLoginServiceTests.cs index 64eb16ff8a..e56891601c 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ExternalLoginServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ExternalLoginServiceTests.cs @@ -2,14 +2,13 @@ // See LICENSE for more details. using System; -using System.Collections.Generic; using System.Linq; using System.Threading; using NUnit.Framework; using Umbraco.Cms.Core.Models.Identity; -using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; @@ -17,17 +16,18 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services { [TestFixture] [Apartment(ApartmentState.STA)] - [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerFixture)] + [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] public class ExternalLoginServiceTests : UmbracoIntegrationTest { private IUserService UserService => GetRequiredService(); - private IExternalLoginService ExternalLoginService => GetRequiredService(); + private IExternalLoginService ExternalLoginService => (IExternalLoginService)GetRequiredService(); [Test] + [Ignore("We don't support duplicates anymore, this removing on save was a breaking change work around, this needs to be ported to a migration")] public void Removes_Existing_Duplicates_On_Save() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); string providerKey = Guid.NewGuid().ToString("N"); @@ -63,7 +63,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services ExternalLoginService.Save(user.Id, externalLogins); - var logins = ExternalLoginService.GetAll(user.Id).ToList(); + var logins = ExternalLoginService.GetExternalLogins(user.Id).ToList(); // duplicates will be removed, keeping the latest entries Assert.AreEqual(2, logins.Count); @@ -75,7 +75,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Does_Not_Persist_Duplicates() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); string providerKey = Guid.NewGuid().ToString("N"); @@ -87,53 +87,14 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services ExternalLoginService.Save(user.Id, externalLogins); - var logins = ExternalLoginService.GetAll(user.Id).ToList(); + var logins = ExternalLoginService.GetExternalLogins(user.Id).ToList(); Assert.AreEqual(1, logins.Count); } - [Test] - public void Single_Create() - { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); - UserService.Save(user); - - var extLogin = new IdentityUserLogin("test1", Guid.NewGuid().ToString("N"), user.Id.ToString()) - { - UserData = "hello" - }; - ExternalLoginService.Save(extLogin); - - IEnumerable found = ExternalLoginService.GetAll(user.Id); - - Assert.AreEqual(1, found.Count()); - Assert.IsTrue(extLogin.HasIdentity); - Assert.IsTrue(extLogin.Id > 0); - } - - [Test] - public void Single_Update() - { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); - UserService.Save(user); - - var extLogin = new IdentityUserLogin("test1", Guid.NewGuid().ToString("N"), user.Id.ToString()) - { - UserData = "hello" - }; - ExternalLoginService.Save(extLogin); - - extLogin.UserData = "world"; - ExternalLoginService.Save(extLogin); - - var found = ExternalLoginService.GetAll(user.Id).ToList(); - Assert.AreEqual(1, found.Count); - Assert.AreEqual("world", found[0].UserData); - } - [Test] public void Multiple_Update() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); string providerKey1 = Guid.NewGuid().ToString("N"); @@ -152,7 +113,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services }; ExternalLoginService.Save(user.Id, extLogins); - var found = ExternalLoginService.GetAll(user.Id).OrderBy(x => x.LoginProvider).ToList(); + var found = ExternalLoginService.GetExternalLogins(user.Id).OrderBy(x => x.LoginProvider).ToList(); Assert.AreEqual(2, found.Count); Assert.AreEqual("123456", found[0].UserData); Assert.AreEqual("987654", found[1].UserData); @@ -161,7 +122,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Can_Find_As_Extended_Type() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); string providerKey1 = Guid.NewGuid().ToString("N"); @@ -182,7 +143,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Add_Logins() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); ExternalLogin[] externalLogins = new[] @@ -193,7 +154,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services ExternalLoginService.Save(user.Id, externalLogins); - var logins = ExternalLoginService.GetAll(user.Id).OrderBy(x => x.LoginProvider).ToList(); + var logins = ExternalLoginService.GetExternalLogins(user.Id).OrderBy(x => x.LoginProvider).ToList(); Assert.AreEqual(2, logins.Count); for (int i = 0; i < logins.Count; i++) { @@ -202,10 +163,35 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services } } + [Test] + public void Add_Tokens() + { + var user = new UserBuilder().Build(); + UserService.Save(user); + + ExternalLogin[] externalLogins = new[] + { + new ExternalLogin("test1", Guid.NewGuid().ToString("N")) + }; + + ExternalLoginService.Save(user.Id, externalLogins); + + ExternalLoginToken[] externalTokens = new[] + { + new ExternalLoginToken(externalLogins[0].LoginProvider, "hello1", "world1"), + new ExternalLoginToken(externalLogins[0].LoginProvider, "hello2", "world2") + }; + + ExternalLoginService.Save(user.Id, externalTokens); + + var tokens = ExternalLoginService.GetExternalLoginTokens(user.Id).ToList(); + Assert.AreEqual(2, tokens.Count); + } + [Test] public void Add_Update_Delete_Logins() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); ExternalLogin[] externalLogins = new[] @@ -218,26 +204,71 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services ExternalLoginService.Save(user.Id, externalLogins); - var logins = ExternalLoginService.GetAll(user.Id).OrderBy(x => x.LoginProvider).ToList(); + var logins = ExternalLoginService.GetExternalLogins(user.Id).OrderBy(x => x.LoginProvider).ToList(); logins.RemoveAt(0); // remove the first one logins.Add(new IdentityUserLogin("test5", Guid.NewGuid().ToString("N"), user.Id.ToString())); // add a new one + logins[0].ProviderKey = "abcd123"; // update // save new list ExternalLoginService.Save(user.Id, logins.Select(x => new ExternalLogin(x.LoginProvider, x.ProviderKey))); - var updatedLogins = ExternalLoginService.GetAll(user.Id).OrderBy(x => x.LoginProvider).ToList(); + var updatedLogins = ExternalLoginService.GetExternalLogins(user.Id).OrderBy(x => x.LoginProvider).ToList(); Assert.AreEqual(4, updatedLogins.Count); for (int i = 0; i < updatedLogins.Count; i++) { Assert.AreEqual(logins[i].LoginProvider, updatedLogins[i].LoginProvider); + Assert.AreEqual(logins[i].ProviderKey, updatedLogins[i].ProviderKey); + } + } + + [Test] + public void Add_Update_Delete_Tokens() + { + var user = new UserBuilder().Build(); + UserService.Save(user); + + ExternalLogin[] externalLogins = new[] + { + new ExternalLogin("test1", Guid.NewGuid().ToString("N")), + new ExternalLogin("test2", Guid.NewGuid().ToString("N")) + }; + + ExternalLoginService.Save(user.Id, externalLogins); + + ExternalLoginToken[] externalTokens = new[] + { + new ExternalLoginToken(externalLogins[0].LoginProvider, "hello1", "world1"), + new ExternalLoginToken(externalLogins[0].LoginProvider, "hello1a", "world1a"), + new ExternalLoginToken(externalLogins[1].LoginProvider, "hello2", "world2"), + new ExternalLoginToken(externalLogins[1].LoginProvider, "hello2a", "world2a") + }; + + ExternalLoginService.Save(user.Id, externalTokens); + + var tokens = ExternalLoginService.GetExternalLoginTokens(user.Id).OrderBy(x => x.LoginProvider).ToList(); + + tokens.RemoveAt(0); // remove the first one + tokens.Add(new IdentityUserToken(externalLogins[1].LoginProvider, "hello2b", "world2b", user.Id.ToString())); // add a new one + tokens[0].Value = "abcd123"; // update + + // save new list + ExternalLoginService.Save(user.Id, tokens.Select(x => new ExternalLoginToken(x.LoginProvider, x.Name, x.Value))); + + var updatedTokens = ExternalLoginService.GetExternalLoginTokens(user.Id).OrderBy(x => x.LoginProvider).ToList(); + Assert.AreEqual(4, updatedTokens.Count); + for (int i = 0; i < updatedTokens.Count; i++) + { + Assert.AreEqual(tokens[i].LoginProvider, updatedTokens[i].LoginProvider); + Assert.AreEqual(tokens[i].Name, updatedTokens[i].Name); + Assert.AreEqual(tokens[i].Value, updatedTokens[i].Value); } } [Test] public void Add_Retrieve_User_Data() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); ExternalLogin[] externalLogins = new[] @@ -247,7 +278,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services ExternalLoginService.Save(user.Id, externalLogins); - var logins = ExternalLoginService.GetAll(user.Id).ToList(); + var logins = ExternalLoginService.GetExternalLogins(user.Id).ToList(); Assert.AreEqual("hello world", logins[0].UserData); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs index aa6677c994..8c8b6b0504 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Data; using System.Linq; using System.Threading.Tasks; using AngleSharp.Common; @@ -24,6 +25,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Mapping; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.PropertyEditors; +using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; @@ -85,7 +87,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers .ReturnsAsync(() => IdentityResult.Success); var value = new MemberDisplay(); - string reason = "Validation failed"; // act ActionResult result = sut.PostSave(fakeMemberData).Result; @@ -204,7 +205,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers .Setup(x => x.ValidatePasswordAsync(It.IsAny())) .ReturnsAsync(() => IdentityResult.Success); - string password = "fakepassword9aw89rnyco3938cyr^%&*()i8Y"; Mock.Get(umbracoMembersUserManager) .Setup(x => x.UpdateAsync(It.IsAny())) .ReturnsAsync(() => IdentityResult.Success); @@ -215,6 +215,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers SetupPasswordSuccess(umbracoMembersUserManager, passwordChanger); Mock.Get(memberService).Setup(x => x.GetByUsername(It.IsAny())).Returns(() => member); + Mock.Get(memberService).Setup(x => x.GetById(It.IsAny())).Returns(() => member); Mock.Get(memberService).SetupSequence( x => x.GetByEmail(It.IsAny())) .Returns(() => null) @@ -336,13 +337,15 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers Mock.Get(umbracoMembersUserManager) .Setup(x => x.ValidatePasswordAsync(It.IsAny())) .ReturnsAsync(() => IdentityResult.Success); + Mock.Get(umbracoMembersUserManager) + .Setup(x => x.AddToRolesAsync(It.IsAny(), It.IsAny>())) + .ReturnsAsync(() => IdentityResult.Success); Mock.Get(memberService).SetupSequence( x => x.GetByEmail(It.IsAny())) .Returns(() => member); MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor, passwordChanger, globalSettings, user); - string reason = "Validation failed"; // act ActionResult result = sut.PostSave(fakeMemberData).Result; @@ -357,19 +360,18 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers [Test] [AutoMoqData] public async Task PostSaveMember_SaveExistingMember_WithNoRoles_Add1Role_ExpectSuccessResponse( - [Frozen] IMemberManager umbracoMembersUserManager, - IMemberService memberService, - IMemberTypeService memberTypeService, - IMemberGroupService memberGroupService, - IDataTypeService dataTypeService, - IBackOfficeSecurityAccessor backOfficeSecurityAccessor, - IBackOfficeSecurity backOfficeSecurity, - IPasswordChanger passwordChanger, - IOptions globalSettings, - IUser user) + [Frozen] IMemberManager umbracoMembersUserManager, + IMemberService memberService, + IMemberTypeService memberTypeService, + IMemberGroupService memberGroupService, + IDataTypeService dataTypeService, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IBackOfficeSecurity backOfficeSecurity, + IPasswordChanger passwordChanger, + IOptions globalSettings, + IUser user) { // arrange - string password = "fakepassword9aw89rnyco3938cyr^%&*()i8Y"; var roleName = "anyrole"; IMember member = SetupMemberTestData(out MemberSave fakeMemberData, out MemberDisplay memberDisplay, ContentSaveAction.Save); fakeMemberData.Groups = new List() @@ -386,9 +388,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers Mock.Get(umbracoMembersUserManager) .Setup(x => x.UpdateAsync(It.IsAny())) .ReturnsAsync(() => IdentityResult.Success); + Mock.Get(umbracoMembersUserManager) + .Setup(x => x.AddToRolesAsync(It.IsAny(), It.IsAny>())) + .ReturnsAsync(() => IdentityResult.Success); Mock.Get(memberTypeService).Setup(x => x.GetDefault()).Returns("fakeAlias"); Mock.Get(backOfficeSecurityAccessor).Setup(x => x.BackOfficeSecurity).Returns(backOfficeSecurity); Mock.Get(memberService).Setup(x => x.GetByUsername(It.IsAny())).Returns(() => member); + Mock.Get(memberService).Setup(x => x.GetById(It.IsAny())).Returns(() => member); SetupUserAccess(backOfficeSecurityAccessor, backOfficeSecurity, user); SetupPasswordSuccess(umbracoMembersUserManager, passwordChanger); @@ -465,7 +471,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers Mock.Get(dataEditor).Setup(x => x.GetValueEditor()).Returns(new TextOnlyValueEditor(Mock.Of(), Mock.Of(), new DataEditorAttribute(Constants.PropertyEditors.Aliases.TextBox, "Test Textbox", "textbox"), textService.Object, Mock.Of(), Mock.Of())); var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(new[] { dataEditor })); - + IMapDefinition memberMapDefinition = new MemberMapDefinition( commonMapper, new CommonTreeNodeMapper(Mock.Of()), @@ -517,7 +523,14 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers dataTypeService, backOfficeSecurityAccessor, new ConfigurationEditorJsonSerializer(), - passwordChanger); + passwordChanger, + Mock.Of(x => x.CreateScope( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny()) == Mock.Of())); } /// diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index ff95ea1ec0..f53a4cab6b 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -32,6 +32,7 @@ using Umbraco.Cms.Web.Common.Controllers; using Umbraco.Cms.Web.Common.Filters; using Umbraco.Extensions; using Constants = Umbraco.Cms.Core.Constants; +using SignInResult = Microsoft.AspNetCore.Identity.SignInResult; namespace Umbraco.Cms.Web.BackOffice.Controllers { @@ -314,7 +315,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers { // Sign the user in with username/password, this also gives a chance for developers to // custom verify the credentials and auto-link user accounts with a custom IBackOfficePasswordChecker - var result = await _signInManager.PasswordSignInAsync( + SignInResult result = await _signInManager.PasswordSignInAsync( loginModel.Username, loginModel.Password, isPersistent: true, lockoutOnFailure: true); if (result.Succeeded) @@ -331,7 +332,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return new ValidationErrorResult($"The registered {typeof(IBackOfficeTwoFactorOptions)} of type {_backOfficeTwoFactorOptions.GetType()} did not return a view for two factor auth "); } - var attemptedUser = _userService.GetByUsername(loginModel.Username); + IUser attemptedUser = _userService.GetByUsername(loginModel.Username); // create a with information to display a custom two factor send code view var verifyResponse = new ObjectResult(new diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 115c782af7..d97ae3e8ae 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -42,6 +42,9 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers [IsBackOffice] public class BackOfficeController : UmbracoController { + // See here for examples of what a lot of this is doing: https://github.com/dotnet/aspnetcore/blob/main/src/Identity/samples/IdentitySample.Mvc/Controllers/AccountController.cs + // along with our AuthenticationController + // NOTE: Each action must either be explicitly authorized or explicitly [AllowAnonymous], the latter is optional because // this controller itself doesn't require authz but it's more clear what the intention is. @@ -339,11 +342,15 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers /// /// Callback path when the user initiates a link login request from the back office to the external provider from the action /// + /// + /// An example of this is here https://github.com/dotnet/aspnetcore/blob/main/src/Identity/samples/IdentitySample.Mvc/Controllers/AccountController.cs#L155 + /// which this is based on + /// [Authorize(Policy = AuthorizationPolicies.BackOfficeAccess)] [HttpGet] public async Task ExternalLinkLoginCallback() { - var user = await _userManager.GetUserAsync(User); + BackOfficeIdentityUser user = await _userManager.GetUserAsync(User); if (user == null) { // ... this should really not happen @@ -351,22 +358,20 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return RedirectToLocal(Url.Action(nameof(Default), this.GetControllerName())); } - var loginInfo = await _signInManager.GetExternalLoginInfoAsync(await _userManager.GetUserIdAsync(user)); + ExternalLoginInfo info = await _signInManager.GetExternalLoginInfoAsync(await _userManager.GetUserIdAsync(user)); - if (loginInfo == null) + if (info == null) { //Add error and redirect for it to be displayed TempData[ViewDataExtensions.TokenExternalSignInError] = new[] { "An error occurred, could not get external login info" }; return RedirectToLocal(Url.Action(nameof(Default), this.GetControllerName())); } - var addLoginResult = await _userManager.AddLoginAsync(user, loginInfo); + IdentityResult addLoginResult = await _userManager.AddLoginAsync(user, info); if (addLoginResult.Succeeded) { - // Update any authentication tokens if login succeeded - // TODO: This is a new thing that we need to implement and because we can store data with the external login now, this is exactly - // what this is for but we'll need to peek under the code here to figure out exactly what goes on. - //await _signInManager.UpdateExternalAuthenticationTokensAsync(loginInfo); + // Update any authentication tokens if succeeded + await _signInManager.UpdateExternalAuthenticationTokensAsync(info); return RedirectToLocal(Url.Action(nameof(Default), this.GetControllerName())); } @@ -426,12 +431,13 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers // Sign in the user with this external login provider (which auto links, etc...) var result = await _signInManager.ExternalLoginSignInAsync(loginInfo, isPersistent: false); - + var errors = new List(); if (result == Microsoft.AspNetCore.Identity.SignInResult.Success) { - + // Update any authentication tokens if succeeded + await _signInManager.UpdateExternalAuthenticationTokensAsync(loginInfo); } else if (result == Microsoft.AspNetCore.Identity.SignInResult.TwoFactorRequired) { diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index 4a24cf1377..05e4cc7486 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -20,6 +20,7 @@ using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.PropertyEditors; +using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; @@ -58,6 +59,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers private readonly IJsonSerializer _jsonSerializer; private readonly IShortStringHelper _shortStringHelper; private readonly IPasswordChanger _passwordChanger; + private readonly IScopeProvider _scopeProvider; /// /// Initializes a new instance of the class. @@ -90,7 +92,8 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IJsonSerializer jsonSerializer, - IPasswordChanger passwordChanger) + IPasswordChanger passwordChanger, + IScopeProvider scopeProvider) : base(cultureDictionary, loggerFactory, shortStringHelper, eventMessages, localizedTextService, jsonSerializer) { _propertyEditors = propertyEditors; @@ -104,6 +107,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers _jsonSerializer = jsonSerializer; _shortStringHelper = shortStringHelper; _passwordChanger = passwordChanger; + _scopeProvider = scopeProvider; } /// @@ -261,9 +265,13 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return new ValidationErrorResult(forDisplay); } + // Create a scope here which will wrap all child data operations in a single transaction. + // We'll complete this at the end of this method if everything succeeeds, else + // all data operations will roll back. + using IScope scope = _scopeProvider.CreateScope(); + // Depending on the action we need to first do a create or update using the membership manager // this ensures that passwords are formatted correctly and also performs the validation on the provider itself. - switch (contentItem.Action) { case ContentSaveAction.Save: @@ -287,9 +295,6 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return NotFound(); } - // TODO: There's 3 things saved here and we should do this all in one transaction, which we can do here by wrapping in a scope - // but it would be nicer to have this taken care of within the Save method itself - // return the updated model MemberDisplay display = _umbracoMapper.Map(contentItem.PersistedContent); @@ -300,19 +305,20 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return new ValidationErrorResult(display, StatusCodes.Status403Forbidden); } - ILocalizedTextService localizedTextService = _localizedTextService; - // put the correct messages in switch (contentItem.Action) { case ContentSaveAction.Save: case ContentSaveAction.SaveNew: display.AddSuccessNotification( - localizedTextService.Localize("speechBubbles/editMemberSaved"), - localizedTextService.Localize("speechBubbles/editMemberSaved")); + _localizedTextService.Localize("speechBubbles/editMemberSaved"), + _localizedTextService.Localize("speechBubbles/editMemberSaved")); break; } + // Mark transaction to commit all changes + scope.Complete(); + return display; } @@ -329,7 +335,8 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers } // map the custom properties - this will already be set for new entities in our member binder - contentItem.PersistedContent.Email = contentItem.Email; + contentItem.PersistedContent.IsApproved = contentItem.IsApproved; + contentItem.PersistedContent.Email = contentItem.Email.Trim(); contentItem.PersistedContent.Username = contentItem.Username; // use the base method to map the rest of the properties @@ -395,15 +402,29 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers _memberService.Save(member); contentItem.PersistedContent = member; - await AddOrUpdateRoles(contentItem, identityMember); + ActionResult rolesChanged = await AddOrUpdateRoles(contentItem.Groups, identityMember); + if (!rolesChanged.Value && rolesChanged.Result != null) + { + return rolesChanged.Result; + } + return true; } /// - /// Update the member security data - /// If the password has been reset then this method will return the reset/generated password, otherwise will return null. + /// Update existing member data /// /// The member to save + /// + /// We need to use both IMemberService and ASP.NET Identity to do our updates because Identity is responsible for passwords/security. + /// When this method is called, the IMember will already have updated/mapped values from the http POST. + /// So then we do this in order: + /// 1. Deal with sensitive property values on IMember + /// 2. Use IMemberService to persist all changes + /// 3. Use ASP.NET and MemberUserManager to deal with lockouts + /// 4. Use ASP.NET, MemberUserManager and password changer to deal with passwords + /// 5. Deal with groups/roles + /// private async Task> UpdateMemberAsync(MemberSave contentItem) { contentItem.PersistedContent.WriterId = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; @@ -421,6 +442,9 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers foreach (IPropertyType sensitiveProperty in sensitiveProperties) { + // TODO: This logic seems to deviate from the logic that is in v8 where we are explitly checking + // against 3 properties: Comments, IsApproved, IsLockedOut, is the v8 version incorrect? + ContentPropertyBasic destProp = contentItem.Properties.FirstOrDefault(x => x.Alias == sensitiveProperty.Alias); if (destProp != null) { @@ -431,27 +455,37 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers } } - bool isLockedOut = contentItem.IsLockedOut; + // First save the IMember with mapped values before we start updating data with aspnet identity + _memberService.Save(contentItem.PersistedContent); - // if they were locked but now they are trying to be unlocked - if (contentItem.PersistedContent.IsLockedOut && isLockedOut == false) - { - contentItem.PersistedContent.IsLockedOut = false; - contentItem.PersistedContent.FailedPasswordAttempts = 0; - } - else if (!contentItem.PersistedContent.IsLockedOut && isLockedOut) - { - // NOTE: This should not ever happen unless someone is mucking around with the request data. - // An admin cannot simply lock a user, they get locked out by password attempts, but an admin can un-approve them - ModelState.AddModelError("custom", "An admin cannot lock a user"); - } + bool needsResync = false; MemberIdentityUser identityMember = await _memberManager.FindByIdAsync(contentItem.Id.ToString()); if (identityMember == null) { - return new ValidationErrorResult("Identity member was not found"); + return new ValidationErrorResult("Member was not found"); } + // Handle unlocking with the member manager (takes care of other nuances) + if (identityMember.IsLockedOut && contentItem.IsLockedOut == false) + { + IdentityResult unlockResult = await _memberManager.SetLockoutEndDateAsync(identityMember, DateTimeOffset.Now.AddMinutes(-1)); + if (unlockResult.Succeeded == false) + { + return new ValidationErrorResult( + $"Could not unlock for member {contentItem.Id} - error {unlockResult.Errors.ToErrorMessage()}"); + } + needsResync = true; + } + else if (identityMember.IsLockedOut == false && contentItem.IsLockedOut) + { + // NOTE: This should not ever happen unless someone is mucking around with the request data. + // An admin cannot simply lock a user, they get locked out by password attempts, but an admin can unlock them + return new ValidationErrorResult("An admin cannot lock a member"); + } + + // If we're changing the password... + // Handle changing with the member manager & password changer (takes care of other nuances) if (contentItem.Password != null) { IdentityResult validatePassword = await _memberManager.ValidatePasswordAsync(contentItem.Password.NewPassword); @@ -466,18 +500,6 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return new ValidationErrorResult("Member ID was not valid"); } - IMember foundMember = _memberService.GetById(intId.Result); - if (foundMember == null) - { - return new ValidationErrorResult("Member was not found"); - } - - IUser currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser; - // if the current user has access to reset/manually change the password - if (currentUser.HasSectionAccess(Constants.Applications.Members) == false) - { - return new ValidationErrorResult("The current user is not authorized"); - } var changingPasswordModel = new ChangingPasswordModel { Id = intId.Result, @@ -485,44 +507,39 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers NewPassword = contentItem.Password.NewPassword, }; + // Change and persist the password Attempt passwordChangeResult = await _passwordChanger.ChangePasswordWithIdentityAsync(changingPasswordModel, _memberManager); - if (passwordChangeResult.Result.ChangeError?.MemberNames != null) + if (!passwordChangeResult.Success) { - foreach (string memberName in passwordChangeResult.Result.ChangeError?.MemberNames) + foreach (string memberName in passwordChangeResult.Result?.ChangeError?.MemberNames ?? Enumerable.Empty()) { ModelState.AddModelError(memberName, passwordChangeResult.Result.ChangeError?.ErrorMessage ?? string.Empty); } return new ValidationErrorResult(new SimpleValidationModel(ModelState.ToErrorDictionary())); } - if (!passwordChangeResult.Success) - { - return new ValidationErrorResult("The password could not be changed"); - } - - // get the identity member now the password and dates have changed - identityMember = await _memberManager.FindByIdAsync(contentItem.Id.ToString()); - - //TODO: confirm this is correct - contentItem.PersistedContent.RawPasswordValue = identityMember.PasswordHash; - - if (identityMember.LastPasswordChangeDateUtc != null) - { - contentItem.PersistedContent.LastPasswordChangeDate = (DateTime)identityMember.LastPasswordChangeDateUtc; - } + needsResync = true; } - IdentityResult updatedResult = await _memberManager.UpdateAsync(identityMember); - - if (updatedResult.Succeeded == false) + // Update the roles and check for changes + ActionResult rolesChanged = await AddOrUpdateRoles(contentItem.Groups, identityMember); + if (!rolesChanged.Value && rolesChanged.Result != null) { - return new ValidationErrorResult(updatedResult.Errors.ToErrorMessage()); + return rolesChanged.Result; + } + else + { + needsResync = true; } - _memberService.Save(contentItem.PersistedContent); + // If there have been underlying changes made by ASP.NET Identity, then we need to resync the + // IMember on the PersistedContent with what is stored since it will be mapped to display. + if (needsResync) + { + contentItem.PersistedContent = _memberService.GetById(contentItem.PersistedContent.Id); + } - await AddOrUpdateRoles(contentItem, identityMember); return true; } @@ -585,10 +602,12 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers /// /// Add or update the identity roles /// - /// The member content item + /// The groups to updates /// The member as an identity user - private async Task AddOrUpdateRoles(MemberSave contentItem, MemberIdentityUser identityMember) + private async Task> AddOrUpdateRoles(IEnumerable groups, MemberIdentityUser identityMember) { + var hasChanges = false; + // We're gonna look up the current roles now because the below code can cause // events to be raised and developers could be manually adding roles to members in // their handlers. If we don't look this up now there's a chance we'll just end up @@ -597,22 +616,34 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers // find the ones to remove and remove them IEnumerable roles = currentRoles.ToList(); - string[] rolesToRemove = roles.Except(contentItem.Groups).ToArray(); + string[] rolesToRemove = roles.Except(groups).ToArray(); // Now let's do the role provider stuff - now that we've saved the content item (that is important since // if we are changing the username, it must be persisted before looking up the member roles). if (rolesToRemove.Any()) { - IdentityResult rolesIdentityResult = await _memberManager.RemoveFromRolesAsync(identityMember, rolesToRemove); + IdentityResult identityResult = await _memberManager.RemoveFromRolesAsync(identityMember, rolesToRemove); + if (!identityResult.Succeeded) + { + return ValidationErrorResult.CreateNotificationValidationErrorResult(identityResult.Errors.ToErrorMessage()); + } + hasChanges = true; } // find the ones to add and add them - string[] toAdd = contentItem.Groups.Except(roles).ToArray(); + string[] toAdd = groups.Except(roles).ToArray(); if (toAdd.Any()) { // add the ones submitted IdentityResult identityResult = await _memberManager.AddToRolesAsync(identityMember, toAdd); + if (!identityResult.Succeeded) + { + return ValidationErrorResult.CreateNotificationValidationErrorResult(identityResult.Errors.ToErrorMessage()); + } + hasChanges = true; } + + return hasChanges; } /// diff --git a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs index 77bb026d69..5cd20d9f9a 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.Globalization; using System.IO; using System.Linq; @@ -685,10 +686,9 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return new ValidationErrorResult("Password reset is not allowed"); } - // if the current user has access to reset/manually change the password - if (currentUser.HasSectionAccess(Constants.Applications.Users) == false) + if (!currentUser.IsAdmin() && found.IsAdmin()) { - return new ValidationErrorResult("The current user is not authorized"); + return new ValidationErrorResult("The current user cannot change the password for the specified user"); } Attempt passwordChangeResult = await _passwordChanger.ChangePasswordWithIdentityAsync(changingPasswordModel, _userManager); @@ -783,7 +783,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers continue; } - var unlockResult = await _userManager.SetLockoutEndDateAsync(user, DateTimeOffset.Now); + var unlockResult = await _userManager.SetLockoutEndDateAsync(user, DateTimeOffset.Now.AddMinutes(-1)); if (unlockResult.Succeeded == false) { return new ValidationErrorResult( diff --git a/src/Umbraco.Web.BackOffice/Security/IBackOfficeSignInManager.cs b/src/Umbraco.Web.BackOffice/Security/IBackOfficeSignInManager.cs index 18e749bddb..38ed6ba74a 100644 --- a/src/Umbraco.Web.BackOffice/Security/IBackOfficeSignInManager.cs +++ b/src/Umbraco.Web.BackOffice/Security/IBackOfficeSignInManager.cs @@ -22,5 +22,6 @@ namespace Umbraco.Cms.Web.BackOffice.Security Task SignInAsync(BackOfficeIdentityUser user, bool isPersistent, string authenticationMethod = null); Task CreateUserPrincipalAsync(BackOfficeIdentityUser user); Task TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberClient); + Task UpdateExternalAuthenticationTokensAsync(ExternalLoginInfo externalLogin); } } diff --git a/src/Umbraco.Web.UI.NetCore/appsettings.json b/src/Umbraco.Web.UI.NetCore/appsettings.json index 53a30cbcea..0d0330b07b 100644 --- a/src/Umbraco.Web.UI.NetCore/appsettings.json +++ b/src/Umbraco.Web.UI.NetCore/appsettings.json @@ -39,7 +39,7 @@ }, "RuntimeMinification": { "dataFolder": "umbraco/Data/TEMP/Smidge", - "version": "1" + "version": "637510451273675926" }, "Security": { "KeepUserLoggedIn": false, diff --git a/src/Umbraco.Web.UI.NetCore/umbraco/UmbracoInstall/Index.cshtml b/src/Umbraco.Web.UI.NetCore/umbraco/UmbracoInstall/Index.cshtml index 79cbe8305a..605e355894 100644 --- a/src/Umbraco.Web.UI.NetCore/umbraco/UmbracoInstall/Index.cshtml +++ b/src/Umbraco.Web.UI.NetCore/umbraco/UmbracoInstall/Index.cshtml @@ -1,4 +1,4 @@ -@using Umbraco.Extensions +@using Umbraco.Extensions @{ Layout = null; } @@ -74,6 +74,6 @@ }; - +