diff --git a/src/Umbraco.Core/Constants-Security.cs b/src/Umbraco.Core/Constants-Security.cs index bd2e1c5acf..d5e3b9242f 100644 --- a/src/Umbraco.Core/Constants-Security.cs +++ b/src/Umbraco.Core/Constants-Security.cs @@ -8,6 +8,8 @@ namespace Umbraco.Core public static class Security { + public const string AdminGroupAlias = "admin"; + public const string BackOfficeAuthenticationType = "UmbracoBackOffice"; public const string BackOfficeExternalAuthenticationType = "UmbracoExternalCookie"; public const string BackOfficeExternalCookieName = "UMB_EXTLOGIN"; diff --git a/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs b/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs index 56e3c995be..8295ae566d 100644 --- a/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs +++ b/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs @@ -24,7 +24,7 @@ namespace Umbraco.Core.Models.Identity .ForMember(user => user.StartMediaId, expression => expression.MapFrom(user => user.StartMediaId)) .ForMember(user => user.StartContentId, expression => expression.MapFrom(user => user.StartContentId)) .ForMember(user => user.AccessFailedCount, expression => expression.MapFrom(user => user.FailedPasswordAttempts)) - .ForMember(user => user.Groups, expression => expression.MapFrom(user => user.Groups.Select(x => x.Name).ToArray())) + .ForMember(user => user.Groups, expression => expression.MapFrom(user => user.Groups.ToArray())) .ForMember(user => user.AllowedSections, expression => expression.MapFrom(user => user.AllowedSections.ToArray())); config.CreateMap() diff --git a/src/Umbraco.Core/Models/Membership/EntityPermission.cs b/src/Umbraco.Core/Models/Membership/EntityPermission.cs index 90430e26a8..15bdb4cbe9 100644 --- a/src/Umbraco.Core/Models/Membership/EntityPermission.cs +++ b/src/Umbraco.Core/Models/Membership/EntityPermission.cs @@ -26,7 +26,9 @@ namespace Umbraco.Core.Models.Membership /// /// public void AddAdditionalPermissions(string[] additionalPermissions) - { + { + //TODO: Fix the performance of this, we need to use things like HashSet and equality checkers, we are iterating too much + var newPermissions = AssignedPermissions.ToList(); newPermissions.AddRange(additionalPermissions); AssignedPermissions = newPermissions diff --git a/src/Umbraco.Core/Models/Membership/IUser.cs b/src/Umbraco.Core/Models/Membership/IUser.cs index 171a9f0c99..c4ccb6328b 100644 --- a/src/Umbraco.Core/Models/Membership/IUser.cs +++ b/src/Umbraco.Core/Models/Membership/IUser.cs @@ -18,19 +18,12 @@ namespace Umbraco.Core.Models.Membership /// /// Gets the groups that user is part of /// - IEnumerable Groups { get; } + IEnumerable Groups { get; } - /// - /// Indicates if the groups for a user have been loaded - /// - bool GroupsLoaded { get; } - - void RemoveGroup(IUserGroup group); - - void AddGroup(IUserGroup group); - - void SetGroupsLoaded(); + void RemoveGroup(string group); + void AddGroup(string group); + IEnumerable AllowedSections { get; } /// diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 8dd1f31654..0d30164b69 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -16,11 +16,14 @@ namespace Umbraco.Core.Models.Membership [Serializable] [DataContract(IsReference = true)] public class User : Entity, IUser - { + { + /// + /// Constructor for creating a new/empty user + /// public User() { SessionTimeout = 60; - _groupCollection = new List(); + _userGroups = new List(); _language = GlobalSettings.DefaultUILanguage; _isApproved = true; _isLockedOut = false; @@ -29,14 +32,60 @@ namespace Umbraco.Core.Models.Membership //cannot be null _rawPasswordValue = ""; } - + + /// + /// Constructor for creating a new/empty user + /// + /// + /// + /// + /// public User(string name, string email, string username, string rawPasswordValue) : this() { + if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value cannot be null or whitespace.", "name"); + if (string.IsNullOrWhiteSpace(email)) throw new ArgumentException("Value cannot be null or whitespace.", "email"); + if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", "username"); + if (string.IsNullOrWhiteSpace(rawPasswordValue)) throw new ArgumentException("Value cannot be null or whitespace.", "rawPasswordValue"); + _name = name; _email = email; _username = username; _rawPasswordValue = rawPasswordValue; + _allowedSections = new List(); + _userGroups = new List(); + _isApproved = true; + _isLockedOut = false; + _startContentId = -1; + _startMediaId = -1; + } + + /// + /// Constructor for creating a new User instance for an existing user + /// + /// + /// + /// + /// + /// + /// + /// + public User(int id, string name, string email, string username, string rawPasswordValue, IEnumerable allowedSections, IEnumerable userGroups) + : this() + { + if (allowedSections == null) throw new ArgumentNullException("allowedSections"); + if (userGroups == null) throw new ArgumentNullException("userGroups"); + if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value cannot be null or whitespace.", "name"); + if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", "username"); + if (string.IsNullOrWhiteSpace(rawPasswordValue)) throw new ArgumentException("Value cannot be null or whitespace.", "rawPasswordValue"); + + Id = id; + _name = name; + _email = email; + _username = username; + _rawPasswordValue = rawPasswordValue; + _allowedSections = allowedSections; + _userGroups = new List(userGroups); _isApproved = true; _isLockedOut = false; _startContentId = -1; @@ -45,8 +94,6 @@ namespace Umbraco.Core.Models.Membership private string _name; private string _securityStamp; - private List _groupCollection; - private bool _groupsLoaded; private int _sessionTimeout; private int _startContentId; private int _startMediaId; @@ -55,6 +102,8 @@ namespace Umbraco.Core.Models.Membership private string _username; private string _email; private string _rawPasswordValue; + private IEnumerable _allowedSections; + private List _userGroups; private bool _isApproved; private bool _isLockedOut; private string _language; @@ -87,6 +136,8 @@ namespace Umbraco.Core.Models.Membership public readonly PropertyInfo LanguageSelector = ExpressionHelper.GetPropertyInfo(x => x.Language); public readonly PropertyInfo DefaultToLiveEditingSelector = ExpressionHelper.GetPropertyInfo(x => x.DefaultToLiveEditing); + + public readonly PropertyInfo UserGroupsSelector = ExpressionHelper.GetPropertyInfo>(x => x.Groups); } #region Implementation of IMembershipUser @@ -183,17 +234,7 @@ namespace Umbraco.Core.Models.Membership public IEnumerable AllowedSections { - get - { - if (GroupsLoaded == false) - { - return Enumerable.Empty(); - } - - return Groups - .SelectMany(x => x.AllowedSections) - .Distinct(); - } + get { return _allowedSections; } } public IProfile ProfileData @@ -268,37 +309,29 @@ namespace Umbraco.Core.Models.Membership /// Gets or sets the groups that user is part of /// [DataMember] - public IEnumerable Groups + public IEnumerable Groups { - get { return _groupCollection; } + get { return _userGroups; } } - - /// - /// Indicates if the groups for a user have been loaded - /// - public bool GroupsLoaded { get { return _groupsLoaded; } } - - public void RemoveGroup(IUserGroup group) + + public void RemoveGroup(string group) { - if (_groupCollection.Select(x => x.Id).Contains(group.Id)) + if (_userGroups.Contains(group)) { - _groupCollection.Remove(group); + _userGroups.Remove(group); + OnPropertyChanged(Ps.Value.UserGroupsSelector); } } - public void AddGroup(IUserGroup group) + public void AddGroup(string group) { - if (_groupCollection.Select(x => x.Id).Contains(group.Id) == false) + if (_userGroups.Contains(group) == false) { - _groupCollection.Add(group); + _userGroups.Add(group); + OnPropertyChanged(Ps.Value.UserGroupsSelector); } } - - public void SetGroupsLoaded() - { - _groupsLoaded = true; - } - + #endregion public override object DeepClone() @@ -307,7 +340,8 @@ namespace Umbraco.Core.Models.Membership //turn off change tracking clone.DisableChangeTracking(); //need to create new collections otherwise they'll get copied by ref - clone._groupCollection = new List(_groupCollection.ToList()); + clone._userGroups = new List(_userGroups); + clone._allowedSections = new List(_allowedSections); //re-create the event handler //this shouldn't really be needed since we're not tracking clone.ResetDirtyProperties(false); diff --git a/src/Umbraco.Core/Models/Rdbms/UserDto.cs b/src/Umbraco.Core/Models/Rdbms/UserDto.cs index 73434ef9f3..ca308bbfee 100644 --- a/src/Umbraco.Core/Models/Rdbms/UserDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/UserDto.cs @@ -69,5 +69,8 @@ namespace Umbraco.Core.Models.Rdbms [Column("lastLoginDate")] [NullSetting(NullSetting = NullSettings.Null)] public DateTime? LastLoginDate { get; set; } + + [ResultColumn] + public List UserGroupDtos { get; set; } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Rdbms/UserGroupDto.cs b/src/Umbraco.Core/Models/Rdbms/UserGroupDto.cs index dd1023b68a..11444a5a23 100644 --- a/src/Umbraco.Core/Models/Rdbms/UserGroupDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/UserGroupDto.cs @@ -15,10 +15,12 @@ namespace Umbraco.Core.Models.Rdbms [Column("userGroupAlias")] [Length(200)] + [Index(IndexTypes.UniqueNonClustered, Name = "IX_umbracoUserGroup_userGroupAlias")] public string Alias { get; set; } [Column("userGroupName")] [Length(200)] + [Index(IndexTypes.UniqueNonClustered, Name = "IX_umbracoUserGroup_userGroupName")] public string Name { get; set; } [Column("userGroupDefaultPermissions")] diff --git a/src/Umbraco.Core/Models/UserExtensions.cs b/src/Umbraco.Core/Models/UserExtensions.cs index 7cadc1c892..87b97df602 100644 --- a/src/Umbraco.Core/Models/UserExtensions.cs +++ b/src/Umbraco.Core/Models/UserExtensions.cs @@ -92,7 +92,7 @@ namespace Umbraco.Core.Models public static bool IsAdmin(this IUser user) { if (user == null) throw new ArgumentNullException("user"); - return user.Groups != null && user.Groups.Any(x => x.Alias == "admin"); + return user.Groups != null && user.Groups.Any(x => x == Constants.Security.AdminGroupAlias); } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Factories/UserFactory.cs b/src/Umbraco.Core/Persistence/Factories/UserFactory.cs index 3d4b02f804..ea30773757 100644 --- a/src/Umbraco.Core/Persistence/Factories/UserFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/UserFactory.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Globalization; using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.Rdbms; @@ -9,25 +10,33 @@ namespace Umbraco.Core.Persistence.Factories { #region Implementation of IEntityFactory - public IUser BuildEntity(UserDto dto) + public static IUser BuildEntity(UserDto dto) { var guidId = dto.Id.ToGuid(); - var user = new User(); + + var groupAliases = new HashSet(); + var allowedSections = new HashSet(); + var userGroups = dto.UserGroupDtos; + foreach (var userGroup in userGroups) + { + groupAliases.Add(userGroup.Alias); + foreach (var section in userGroup.UserGroup2AppDtos) + { + allowedSections.Add(section.AppAlias); + } + } + + var user = new User(dto.Id, dto.UserName, dto.Email, dto.Login,dto.Password, allowedSections, groupAliases); try { user.DisableChangeTracking(); - - user.Id = dto.Id; + user.Key = guidId; user.StartContentId = dto.ContentStartId; - user.StartMediaId = dto.MediaStartId.HasValue ? dto.MediaStartId.Value : -1; - user.RawPasswordValue = dto.Password; - user.Username = dto.Login; - user.Name = dto.UserName; + user.StartMediaId = dto.MediaStartId ?? -1; user.IsLockedOut = dto.NoConsole; user.IsApproved = dto.Disabled == false; - user.Email = dto.Email; user.Language = dto.UserLanguage; user.SecurityStamp = dto.SecurityStampToken; user.FailedPasswordAttempts = dto.FailedLoginAttempts ?? 0; diff --git a/src/Umbraco.Core/Persistence/Migrations/Initial/BaseDataCreation.cs b/src/Umbraco.Core/Persistence/Migrations/Initial/BaseDataCreation.cs index dbf66cebdd..90768d2153 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Initial/BaseDataCreation.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Initial/BaseDataCreation.cs @@ -175,7 +175,7 @@ namespace Umbraco.Core.Persistence.Migrations.Initial private void CreateUmbracoUserGroupData() { - _database.Insert("umbracoUserGroup", "id", false, new UserGroupDto { Id = 1, Alias = "admin", Name = "Administrators", DefaultPermissions = "CADMOSKTPIURZ:5F7" }); + _database.Insert("umbracoUserGroup", "id", false, new UserGroupDto { Id = 1, Alias = Constants.Security.AdminGroupAlias, Name = "Administrators", DefaultPermissions = "CADMOSKTPIURZ:5F7" }); _database.Insert("umbracoUserGroup", "id", false, new UserGroupDto { Id = 2, Alias = "writer", Name = "Writers", DefaultPermissions = "CAH:F" }); _database.Insert("umbracoUserGroup", "id", false, new UserGroupDto { Id = 3, Alias = "editor", Name = "Editors", DefaultPermissions = "CADMOSKTPUZ:5F" }); _database.Insert("umbracoUserGroup", "id", false, new UserGroupDto { Id = 4, Alias = "translator", Name = "Translators", DefaultPermissions = "AF" }); diff --git a/src/Umbraco.Core/Persistence/Relators/UserGroupRelator.cs b/src/Umbraco.Core/Persistence/Relators/UserGroupRelator.cs new file mode 100644 index 0000000000..7a52f710a4 --- /dev/null +++ b/src/Umbraco.Core/Persistence/Relators/UserGroupRelator.cs @@ -0,0 +1,88 @@ +using System; +using System.Collections.Generic; +using Umbraco.Core.Models.Rdbms; + +namespace Umbraco.Core.Persistence.Relators +{ + internal class UserGroupRelator + { + private UserDto _currentUser; + + internal UserDto Map(UserDto user, UserGroupDto group, UserGroup2AppDto section) + { + // Terminating call. Since we can return null from this function + // we need to be ready for PetaPoco to callback later with null + // parameters + if (user == null) + return _currentUser; + + // Is this the same User as the current one we're processing + if (_currentUser != null && _currentUser.Id == user.Id) + { + AddOrUpdateGroup(group, section); + + // Return null to indicate we're not done with this object yet + return null; + } + + // This is a different user to the current one, or this is the + // first time through and we don't have one yet + + // Save the current user + var prev = _currentUser; + + // Setup the new current user + _currentUser = user; + _currentUser.UserGroupDtos = new List(); + + AddOrUpdateGroup(group, section); + + // Return the now populated previous user (or null if first time through) + return prev; + } + + private void AddOrUpdateGroup(UserGroupDto group, UserGroup2AppDto section) + { + //I don't even think this situation can happen but if it could, we'd want the section added to the latest group check if this is a new group + if (group == null && section != null) + { + AddSection(section); + } + + //this can be null since we are doing a left join + if (group != null && group.Alias.IsNullOrWhiteSpace() == false) + { + //check if this is a new group + var latestGroup = _currentUser.UserGroupDtos.Count > 0 + ? _currentUser.UserGroupDtos[_currentUser.UserGroupDtos.Count - 1] + : null; + + if (latestGroup == null || latestGroup.Id != group.Id) + { + //add the current (new) group + _currentUser.UserGroupDtos.Add(group); + } + + AddSection(section); + } + } + + private void AddSection(UserGroup2AppDto section) + { + var latestGroup = _currentUser.UserGroupDtos.Count > 0 + ? _currentUser.UserGroupDtos[_currentUser.UserGroupDtos.Count - 1] + : null; + + if (latestGroup == null || latestGroup.Id != section.UserGroupId) + throw new InvalidOperationException("The user group and section info don't match"); + + if (latestGroup.UserGroup2AppDtos == null) + latestGroup.UserGroup2AppDtos = new List(); + + //add it if it doesn't exist + if (latestGroup.UserGroup2AppDtos.TrueForAll(dto => dto.AppAlias != section.AppAlias)) + latestGroup.UserGroup2AppDtos.Add(section); + } + + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs index 45abdc70b4..a26833a4ce 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs @@ -5,6 +5,13 @@ namespace Umbraco.Core.Persistence.Repositories { public interface IUserGroupRepository : IRepositoryQueryable { + /// + /// Gets a group by it's alias + /// + /// + /// + IUserGroup Get(string alias); + /// /// This is useful when an entire section is removed from config /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserRepository.cs index 620350f563..64d770e64f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserRepository.cs @@ -21,14 +21,7 @@ namespace Umbraco.Core.Persistence.Repositories /// /// bool Exists(string username); - - /// - /// Gets all groups for a given user - /// - /// Id of user - /// An enumerable list of - IEnumerable GetGroupsForUser(int userId); - + /// /// Gets a list of objects associated with a given group /// diff --git a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs index f87561e32f..f81725cc9b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using Umbraco.Core.Logging; using Umbraco.Core.Models; +using Umbraco.Core.Cache; using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.Rdbms; using Umbraco.Core.Persistence.Factories; @@ -26,6 +27,34 @@ namespace Umbraco.Core.Persistence.Repositories _cacheHelper = cacheHelper; } + public const string GetByAliasCacheKeyPrefix = "UserGroupRepository_GetByAlias_"; + public static string GetByAliasCacheKey(string alias) + { + return GetByAliasCacheKeyPrefix + alias; + } + + public IUserGroup Get(string alias) + { + try + { + //need to do a simple query to get the id - put this cache + var id = IsolatedCache.GetCacheItem(GetByAliasCacheKey(alias), () => + { + var groupId = Database.ExecuteScalar("SELECT id FROM umbracoUserGroup WHERE userGroupAlias=@alias", new { alias = alias }); + if (groupId.HasValue == false) throw new InvalidOperationException("No group found with alias " + alias); + return groupId.Value; + }); + + //return from the normal method which will cache + return Get(id); + } + catch (InvalidOperationException) + { + //if this is caught it's because we threw this in the caching method + return null; + } + } + public IEnumerable GetGroupsAssignedToSection(string sectionAlias) { //Here we're building up a query that looks like this, a sub query is required because the resulting structure @@ -75,7 +104,7 @@ namespace Umbraco.Core.Persistence.Repositories Database.Insert(dto); } } - + /// /// Gets the group permissions for the specified entities /// diff --git a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs index 7456bfc88c..321f9e1057 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs @@ -32,78 +32,54 @@ namespace Umbraco.Core.Persistence.Repositories protected override IUser PerformGet(int id) { - var sql = GetBaseQuery(false); + var sql = GetQueryWithGroups(); sql.Where(GetBaseWhereClause(), new { Id = id }); - //must be sorted this way for the relator to work - sql.OrderBy(x => x.Id, SqlSyntax); + sql //must be included for relator to work + .OrderBy(d => d.Id, SqlSyntax) + .OrderBy(d => d.Id, SqlSyntax); - var dto = Database.Fetch(sql).FirstOrDefault(); + var dto = Database.Fetch(new UserGroupRelator().Map, sql) + .FirstOrDefault(); if (dto == null) - return null; + return null; - var userFactory = new UserFactory(); - var user = userFactory.BuildEntity(dto); - AssociateGroupsWithUser(user); + var user = UserFactory.BuildEntity(dto); return user; } - - private void AssociateGroupsWithUser(IUser user) - { - if (user != null) - { - foreach (var group in GetGroupsForUser(user.Id)) - { - user.AddGroup(group); - } - - user.SetGroupsLoaded(); - } - } - + protected override IEnumerable PerformGetAll(params int[] ids) { - var sql = GetBaseQuery(false); - + var sql = GetQueryWithGroups(); if (ids.Any()) { sql.Where("umbracoUser.id in (@ids)", new {ids = ids}); } - - //must be sorted this way for the relator to work - sql.OrderBy(x => x.Id, SqlSyntax); - - var users = ConvertFromDtos(Database.Fetch(sql)) + sql //must be included for relator to work + .OrderBy(d => d.Id, SqlSyntax) + .OrderBy(d => d.Id, SqlSyntax); + + var users = ConvertFromDtos(Database.Fetch(new UserGroupRelator().Map, sql)) .ToArray(); // important so we don't iterate twice, if we don't do this we can end up with null values in cache if we were caching. - - foreach (var user in users) - { - AssociateGroupsWithUser(user); - } - + return users; } protected override IEnumerable PerformGetByQuery(IQuery query) { - var sqlClause = GetBaseQuery(false); + var sqlClause = GetQueryWithGroups(); var translator = new SqlTranslator(sqlClause, query); var sql = translator.Translate(); + sql //must be included for relator to work + .OrderBy(d => d.Id, SqlSyntax) + .OrderBy(d => d.Id, SqlSyntax); - //must be sorted this way for the relator to work - sql.OrderBy(x => x.Id, SqlSyntax); - - var dtos = Database.Fetch(sql) + var dtos = Database.Fetch(new UserGroupRelator().Map, sql) .DistinctBy(x => x.Id); var users = ConvertFromDtos(dtos) .ToArray(); // important so we don't iterate twice, if we don't do this we can end up with null values in cache if we were caching. - - foreach (var user in users) - { - AssociateGroupsWithUser(user); - } - + return users; } @@ -124,6 +100,23 @@ namespace Umbraco.Core.Persistence.Repositories } return sql; } + + /// + /// A query to return a user with it's groups and with it's groups sections + /// + /// + private Sql GetQueryWithGroups() + { + //base query includes user groups + var sql = GetBaseQuery("umbracoUser.*, umbracoUserGroup.*, umbracoUserGroup2App.*"); + sql.LeftJoin(SqlSyntax) + .On(SqlSyntax, dto => dto.UserId, dto => dto.Id) + .LeftJoin(SqlSyntax) + .On(SqlSyntax, dto => dto.Id, dto => dto.UserGroupId) + .LeftJoin(SqlSyntax) + .On(SqlSyntax, dto => dto.UserGroupId, dto => dto.Id); + return sql; + } private Sql GetBaseQuery(string columns) { @@ -140,16 +133,16 @@ namespace Umbraco.Core.Persistence.Repositories } protected override IEnumerable GetDeleteClauses() - { + { var list = new List - { - "DELETE FROM cmsTask WHERE userId = @Id", - "DELETE FROM cmsTask WHERE parentUserId = @Id", - "DELETE FROM umbracoUser2UserGroup WHERE userId = @Id", - "DELETE FROM umbracoUser2NodeNotify WHERE userId = @Id", - "DELETE FROM umbracoUser WHERE id = @Id", - "DELETE FROM umbracoExternalLogin WHERE id = @Id" - }; + { + "DELETE FROM cmsTask WHERE userId = @Id", + "DELETE FROM cmsTask WHERE parentUserId = @Id", + "DELETE FROM umbracoUser2UserGroup WHERE userId = @Id", + "DELETE FROM umbracoUser2NodeNotify WHERE userId = @Id", + "DELETE FROM umbracoUser WHERE id = @Id", + "DELETE FROM umbracoExternalLogin WHERE id = @Id" + }; return list; } @@ -236,21 +229,20 @@ namespace Umbraco.Core.Persistence.Repositories { Database.Update(userDto, changedCols); } + + //lookup all assigned + var assigned = Database.Fetch("WHERE userGroupAlias IN (@aliases)", new {aliases = entity.Groups}); - //update the groups - var user = (User)entity; + //first delete all + //TODO: We could do this a nicer way instead of "Nuke and Pave" + Database.Delete("WHERE UserId = @UserId", new { UserId = entity.Id }); - //first delete all - Database.Delete("WHERE UserId = @UserId", - new { UserId = user.Id }); - - //then re-add any associated with the user - foreach (var group in user.Groups) + foreach (var groupDto in assigned) { var dto = new User2UserGroupDto { - UserGroupId = group.Id, - UserId = user.Id + UserGroupId = groupDto.Id, + UserId = entity.Id }; Database.Insert(dto); } @@ -284,36 +276,7 @@ namespace Umbraco.Core.Persistence.Repositories return Database.ExecuteScalar(sql) > 0; } - - /// - /// Gets all groups for a given user - /// - /// Id of user - /// An enumerable list of - public IEnumerable GetGroupsForUser(int userId) - { - var tables = SqlSyntax.GetTablesInSchema(ApplicationContext.Current.DatabaseContext.Database).ToArray(); - if (tables.InvariantContains("umbracoUserGroup") == false) - return new List(); - - var sql = new Sql(); - sql.Select("*") - .From() - .LeftJoin() - .On(left => left.Id, right => right.UserGroupId); - - var innerSql = new Sql(); - innerSql.Select("umbracoUserGroup.id") - .From() - .LeftJoin() - .On(left => left.Id, right => right.UserGroupId) - .Where("umbracoUser2UserGroup.userId = " + userId); - - sql.Where(string.Format("umbracoUserGroup.id IN ({0})", innerSql.SQL)); - var dtos = Database.Fetch(new UserGroupSectionRelator().Map, sql); - return ConvertFromDtos(dtos); - } - + /// /// Gets a list of objects associated with a given group /// @@ -416,20 +379,16 @@ namespace Umbraco.Core.Persistence.Repositories private IEnumerable ConvertFromDtos(IEnumerable dtos) { - return dtos.Select(dto => - { - var userFactory = new UserFactory(); - return userFactory.BuildEntity(dto); - }); + return dtos.Select(UserFactory.BuildEntity); } - private IEnumerable ConvertFromDtos(IEnumerable dtos) - { - return dtos.Select(dto => - { - var userGroupFactory = new UserGroupFactory(); - return userGroupFactory.BuildEntity(dto); - }); - } + //private IEnumerable ConvertFromDtos(IEnumerable dtos) + //{ + // return dtos.Select(dto => + // { + // var userGroupFactory = new UserGroupFactory(); + // return userGroupFactory.BuildEntity(dto); + // }); + //} } } \ No newline at end of file diff --git a/src/Umbraco.Core/Security/BackOfficeUserStore.cs b/src/Umbraco.Core/Security/BackOfficeUserStore.cs index 1f73998526..4206cc731e 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserStore.cs @@ -404,11 +404,11 @@ namespace Umbraco.Core.Security } var foundUser = _userService.GetUserById(asInt.Result); - var foundGroup = _userService.GetUserGroupByName(roleName); + var foundGroup = _userService.GetUserGroupByAlias(roleName); if (foundUser != null && foundGroup != null) { - foundUser.AddGroup(foundGroup); + foundUser.AddGroup(foundGroup.Alias); } return Task.FromResult(0); @@ -433,11 +433,11 @@ namespace Umbraco.Core.Security } var foundUser = _userService.GetUserById(asInt.Result); - var foundGroup = _userService.GetUserGroupByName(roleName); + var foundGroup = _userService.GetUserGroupByAlias(roleName); if (foundUser != null && foundGroup != null) { - foundUser.RemoveGroup(foundGroup); + foundUser.RemoveGroup(foundGroup.Alias); } return Task.FromResult(0); @@ -689,21 +689,21 @@ namespace Umbraco.Core.Security user.SecurityStamp = identityUser.SecurityStamp; } - if (user.Groups.Select(x => x.Name).ContainsAll(identityUser.Groups) == false - || identityUser.Groups.ContainsAll(user.Groups.Select(x => x.Name)) == false) + if (user.Groups.ContainsAll(identityUser.Groups) == false + || identityUser.Groups.ContainsAll(user.Groups) == false) { anythingChanged = true; - foreach (var group in user.Groups) + + //clear out the current groups (need to ToArray since we are modifying the iterator) + foreach (var group in user.Groups.ToArray()) { user.RemoveGroup(group); } - foreach (var group in identityUser.Groups) + //get all of the ones found by alias and add them + var foundGroups = _userService.GetUserGroupsByAlias(identityUser.Groups); + foreach (var group in foundGroups) { - var foundGroup = _userService.GetUserGroupByName(group); - if (foundGroup != null) - { - user.AddGroup(foundGroup); - } + user.AddGroup(group.Alias); } } diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index dafe746fc8..bba95792ee 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -71,6 +71,18 @@ namespace Umbraco.Core.Services /// An enumerable list of IEnumerable GetPermissions(IUser user, params int[] nodeIds); + /// + /// Get permissions set for a group and optional node Ids + /// + /// Group to retrieve permissions for + /// + /// Flag indicating if we want to get just the permissions directly assigned for the group and path, + /// or fall back to the group's default permissions when nothing is directly assigned + /// + /// Specifiying nothing will return all permissions for all nodes + /// An enumerable list of + IEnumerable GetPermissions(string groupAlias, bool directlyAssignedOnly, params int[] nodeIds); + /// /// Get permissions set for a group and optional node Ids /// @@ -94,7 +106,19 @@ namespace Umbraco.Core.Services /// /// Gets the permissions for the provided group and path /// - /// User to check permissions for + /// Group alias to check permissions for + /// Path to check permissions for + /// + /// Flag indicating if we want to get just the permissions directly assigned for the group and path, + /// or fall back to the group's default permissions when nothing is directly assigned + /// + /// String indicating permissions for provided user and path + string GetPermissionsForPath(string groupAlias, string path, bool directlyAssignedOnly = true); + + /// + /// Gets the permissions for the provided group and path + /// + /// Group to check permissions for /// Path to check permissions for /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, @@ -145,34 +169,27 @@ namespace Umbraco.Core.Services /// Optional Ids of UserGroups to retrieve /// An enumerable list of IEnumerable GetAllUserGroups(params int[] ids); - - /// - /// Gets all UserGroups for a given user - /// - /// Id of user - /// An enumerable list of - IEnumerable GetGroupsForUser(int userId); - + /// /// Gets a UserGroup by its Alias /// /// Alias of the UserGroup to retrieve /// - IUserGroup GetUserGroupByAlias(string alias); + IEnumerable GetUserGroupsByAlias(params string[] alias); + + /// + /// Gets a UserGroup by its Alias + /// + /// Name of the UserGroup to retrieve + /// + IUserGroup GetUserGroupByAlias(string name); /// /// Gets a UserGroup by its Id /// /// Id of the UserGroup to retrieve /// - IUserGroup GetUserGroupById(int id); - - /// - /// Gets a UserGroup by its Name - /// - /// Name of the UserGroup to retrieve - /// - IUserGroup GetUserGroupByName(string name); + IUserGroup GetUserGroupById(int id); /// /// Saves a UserGroup diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 0c1e1fa63f..07c0ad4e5b 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -10,7 +10,8 @@ using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.UnitOfWork; -using Umbraco.Core.Security; +using Umbraco.Core.Security; +using Umbraco.Core.Persistence.Querying; namespace Umbraco.Core.Services { @@ -631,20 +632,19 @@ namespace Umbraco.Core.Services return repository.GetAll(ids).OrderBy(x => x.Name); } } - - /// - /// Gets all UserGroups for a given user - /// - /// Id of user - /// An enumerable list of - public IEnumerable GetGroupsForUser(int userId) + + public IEnumerable GetUserGroupsByAlias(params string[] aliases) { + if (aliases.Length == 0) return Enumerable.Empty(); + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { - var repository = RepositoryFactory.CreateUserRepository(uow); - return repository.GetGroupsForUser(userId); + var repository = RepositoryFactory.CreateUserGroupRepository(uow); + var query = Query.Builder.Where(x => aliases.SqlIn(x.Alias)); + var contents = repository.GetByQuery(query); + return contents.ToArray(); } - } + } /// /// Gets a UserGroup by its Alias @@ -653,12 +653,14 @@ namespace Umbraco.Core.Services /// public IUserGroup GetUserGroupByAlias(string alias) { + if (string.IsNullOrWhiteSpace(alias)) throw new ArgumentException("Value cannot be null or whitespace.", "alias"); + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); var query = Query.Builder.Where(x => x.Alias == alias); var contents = repository.GetByQuery(query); - return contents.SingleOrDefault(); + return contents.FirstOrDefault(); } } @@ -676,21 +678,6 @@ namespace Umbraco.Core.Services } } - /// - /// Gets a UserGroup by its Name - /// - /// Name of the UserGroup to retrieve - /// - public IUserGroup GetUserGroupByName(string name) - { - using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) - { - var repository = RepositoryFactory.CreateUserGroupRepository(uow); - var query = Query.Builder.Where(x => x.Name == name); - return repository.GetByQuery(query).SingleOrDefault(); - } - } - /// /// Saves a UserGroup /// @@ -808,14 +795,10 @@ namespace Umbraco.Core.Services /// An enumerable list of public IEnumerable GetPermissions(IUser user, params int[] nodeIds) { - if (user.GroupsLoaded == false) - { - throw new InvalidOperationException("Cannot determine permissions for user as their associated groups are not loaded"); - } - var result = new List(); foreach (var group in user.Groups) - { + { + //TODO: This may perform horribly :/ foreach (var permission in GetPermissions(group, false, nodeIds)) { AddOrAmendPermissionList(result, permission); @@ -828,6 +811,26 @@ namespace Umbraco.Core.Services /// /// Get permissions set for a group and node Id /// + /// Group to retrieve permissions for + /// + /// Flag indicating if we want to get just the permissions directly assigned for the group and path, + /// or fall back to the group's default permissions when nothing is directly assigned + /// + /// Specifiying nothing will return all permissions for all nodes + /// An enumerable list of + public IEnumerable GetPermissions(string groupAlias, bool directlyAssignedOnly, params int[] nodeIds) + { + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) + { + var repository = RepositoryFactory.CreateUserGroupRepository(uow); + var group = repository.Get(groupAlias); + return GetPermissionsInternal(repository, group, directlyAssignedOnly, nodeIds); + } + } + + /// + /// Get permissions set for a group and optional node Ids + /// /// Group to retrieve permissions for /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, @@ -837,28 +840,32 @@ namespace Umbraco.Core.Services /// An enumerable list of public IEnumerable GetPermissions(IUserGroup group, bool directlyAssignedOnly, params int[] nodeIds) { - + if (group == null) throw new ArgumentNullException("group"); using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); - var explicitPermissions = repository.GetPermissionsForEntities(group.Id, nodeIds); - var result = new List(explicitPermissions); - - // If requested, and no permissions are assigned to a particular node, then we will fill in those permissions with the group's defaults - if (directlyAssignedOnly == false) - { - var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToList(); - if (missingIds.Any()) - { - result.AddRange(missingIds - .Select(i => new EntityPermission(i, group.Permissions.ToArray()))); - } - } - - return result; + return GetPermissionsInternal(repository, group, directlyAssignedOnly, nodeIds); } } + private IEnumerable GetPermissionsInternal(IUserGroupRepository repository, IUserGroup group, bool directlyAssignedOnly, params int[] nodeIds) + { + var explicitPermissions = repository.GetPermissionsForEntities(group.Id, nodeIds); + var result = new List(explicitPermissions); + + // If requested, and no permissions are assigned to a particular node, then we will fill in those permissions with the group's defaults + if (directlyAssignedOnly == false) + { + var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToList(); + if (missingIds.Any()) + { + result.AddRange(missingIds + .Select(i => new EntityPermission(i, group.Permissions.ToArray()))); + } + } + return result; + } + /// /// For an existing list of , takes a new and aggregates it. /// If a permission for the entity associated with the new permission already exists, it's updated with those permissions to create a distinct, most permissive set. @@ -868,8 +875,9 @@ namespace Umbraco.Core.Services /// New permission to aggregate private void AddOrAmendPermissionList(IList permissions, EntityPermission groupPermission) { - var existingPermission = permissions - .SingleOrDefault(x => x.EntityId == groupPermission.EntityId); + //TODO: Fix the performance of this, we need to use things like HashSet and equality checkers, we are iterating too much + + var existingPermission = permissions.FirstOrDefault(x => x.EntityId == groupPermission.EntityId); if (existingPermission != null) { existingPermission.AddAdditionalPermissions(groupPermission.AssignedPermissions); @@ -887,12 +895,7 @@ namespace Umbraco.Core.Services /// Path to check permissions for /// String indicating permissions for provided user and path public string GetPermissionsForPath(IUser user, string path) - { - if (user.GroupsLoaded == false) - { - throw new InvalidOperationException("Cannot determine permissions for user as their associated groups are not loaded"); - } - + { var assignedPermissions = GetPermissionsForGroupsAndPath(user.Groups, path); return GetAggregatePermissions(assignedPermissions); } @@ -903,7 +906,7 @@ namespace Umbraco.Core.Services /// List of groups associated with the user /// Path to check permissions for /// List of strings indicating permissions for each groups - private IEnumerable GetPermissionsForGroupsAndPath(IEnumerable groups, string path) + private IEnumerable GetPermissionsForGroupsAndPath(IEnumerable groups, string path) { return groups .Select(g => GetPermissionsForPath(g, path, directlyAssignedOnly: false)) @@ -925,7 +928,27 @@ namespace Umbraco.Core.Services /// /// Gets the permissions for the provided group and path /// - /// User to check permissions for + /// User to check permissions for + /// Path to check permissions for + /// + /// Flag indicating if we want to get just the permissions directly assigned for the group and path, + /// or fall back to the group's default permissions when nothing is directly assigned + /// + /// String indicating permissions for provided user and path + public string GetPermissionsForPath(string groupAlias, string path, bool directlyAssignedOnly = true) + { + var nodeId = GetNodeIdFromPath(path); + var permission = GetPermissions(groupAlias, directlyAssignedOnly, nodeId) + .FirstOrDefault(); + return permission != null + ? string.Join(string.Empty, permission.AssignedPermissions) + : string.Empty; + } + + /// + /// Gets the permissions for the provided group and path + /// + /// Group to check permissions for /// Path to check permissions for /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, @@ -936,11 +959,12 @@ namespace Umbraco.Core.Services { var nodeId = GetNodeIdFromPath(path); var permission = GetPermissions(group, directlyAssignedOnly, nodeId) - .SingleOrDefault(); - return permission != null + .FirstOrDefault(); + return permission != null ? string.Join(string.Empty, permission.AssignedPermissions) : string.Empty; - } + } + /// /// Parses a path to find the lowermost node id @@ -953,13 +977,7 @@ namespace Umbraco.Core.Services ? int.Parse(path.Substring(path.LastIndexOf(",", StringComparison.Ordinal) + 1)) : int.Parse(path); } - - //private static bool IsNotNullActionPermission(EntityPermission x) - //{ - // const string NullActionChar = "-"; - // return string.Join(string.Empty, x.AssignedPermissions) != NullActionChar; - //} - + /// /// Checks in a set of permissions associated with a user for those related to a given nodeId /// diff --git a/src/Umbraco.Core/Services/UserServiceExtensions.cs b/src/Umbraco.Core/Services/UserServiceExtensions.cs index 6130077771..0505500055 100644 --- a/src/Umbraco.Core/Services/UserServiceExtensions.cs +++ b/src/Umbraco.Core/Services/UserServiceExtensions.cs @@ -27,40 +27,40 @@ namespace Umbraco.Core.Services userService.ReplaceUserGroupPermissions(groupId, new char[] { }); } - /// - /// Maps a custom provider's information to an umbraco user account - /// - /// - /// - /// - /// To maintain compatibility we have to check the login name if the provider key lookup fails but otherwise - /// we'll store the provider user key in the login column. - /// - public static IUser CreateUserMappingForCustomProvider(this IUserService userService, MembershipUser member) - { - if (member == null) throw new ArgumentNullException("member"); + ///// + ///// Maps a custom provider's information to an umbraco user account + ///// + ///// + ///// + ///// + ///// To maintain compatibility we have to check the login name if the provider key lookup fails but otherwise + ///// we'll store the provider user key in the login column. + ///// + //public static IUser CreateUserMappingForCustomProvider(this IUserService userService, MembershipUser member) + //{ + // if (member == null) throw new ArgumentNullException("member"); - var valToLookup = member.ProviderUserKey == null ? member.UserName : member.ProviderUserKey.ToString(); - var found = userService.GetByUsername(valToLookup); - if (found == null && member.ProviderUserKey != null) - { - //try by username - found = userService.GetByUsername(member.UserName); - } + // var valToLookup = member.ProviderUserKey == null ? member.UserName : member.ProviderUserKey.ToString(); + // var found = userService.GetByUsername(valToLookup); + // if (found == null && member.ProviderUserKey != null) + // { + // //try by username + // found = userService.GetByUsername(member.UserName); + // } - if (found == null) - { - var user = new User( - member.UserName, - member.Email ?? Guid.NewGuid().ToString("N") + "@example.com", //email cannot be empty - member.ProviderUserKey == null ? member.UserName : member.ProviderUserKey.ToString(), - Guid.NewGuid().ToString("N")); //pass cannot be empty - userService.Save(user); - return user; - } + // if (found == null) + // { + // var user = new User( + // member.UserName, + // member.Email ?? Guid.NewGuid().ToString("N") + "@example.com", //email cannot be empty + // member.ProviderUserKey == null ? member.UserName : member.ProviderUserKey.ToString(), + // Guid.NewGuid().ToString("N")); //pass cannot be empty + // userService.Save(user); + // return user; + // } - return found; - } + // return found; + //} } } \ No newline at end of file diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 1f3f352292..12d142b7b0 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -571,6 +571,7 @@ + diff --git a/src/Umbraco.Tests/Services/UserServiceTests.cs b/src/Umbraco.Tests/Services/UserServiceTests.cs index 6c513e0956..cc54f085b6 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -56,9 +56,9 @@ namespace Umbraco.Tests.Services //assert Assert.AreEqual(3, permissions.Count()); - Assert.AreEqual(17, permissions.ElementAt(0).AssignedPermissions.Count()); - Assert.AreEqual(17, permissions.ElementAt(1).AssignedPermissions.Count()); - Assert.AreEqual(17, permissions.ElementAt(2).AssignedPermissions.Count()); + Assert.AreEqual(17, permissions.ElementAt(0).AssignedPermissions.Length); + Assert.AreEqual(17, permissions.ElementAt(1).AssignedPermissions.Length); + Assert.AreEqual(17, permissions.ElementAt(2).AssignedPermissions.Length); } [Test] @@ -89,9 +89,9 @@ namespace Umbraco.Tests.Services //assert Assert.AreEqual(3, permissions.Count()); - Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Count()); - Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Count()); - Assert.AreEqual(1, permissions.ElementAt(2).AssignedPermissions.Count()); + Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Length); + Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Length); + Assert.AreEqual(1, permissions.ElementAt(2).AssignedPermissions.Length); } [Test] @@ -122,9 +122,9 @@ namespace Umbraco.Tests.Services //assert Assert.AreEqual(3, permissions.Count()); - Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Count()); - Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Count()); - Assert.AreEqual(1, permissions.ElementAt(2).AssignedPermissions.Count()); + Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Length); + Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Length); + Assert.AreEqual(1, permissions.ElementAt(2).AssignedPermissions.Length); } [Test] @@ -154,9 +154,9 @@ namespace Umbraco.Tests.Services //assert Assert.AreEqual(3, permissions.Count()); - Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Count()); - Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Count()); - Assert.AreEqual(17, permissions.ElementAt(2).AssignedPermissions.Count()); + Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Length); + Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Length); + Assert.AreEqual(17, permissions.ElementAt(2).AssignedPermissions.Length); } [Test] @@ -627,8 +627,7 @@ namespace Umbraco.Tests.Services var userGroup = CreateTestUserGroup(); var user = ServiceContext.UserService.CreateUserWithIdentity("test1", "test1@test.com"); - user.AddGroup(userGroup); - user.SetGroupsLoaded(); + user.AddGroup(userGroup.Alias); ServiceContext.UserService.Save(user); return user; } diff --git a/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs b/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs index 5f36f60bab..06a59815b1 100644 --- a/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs @@ -29,6 +29,11 @@ namespace Umbraco.Web.Cache public override void RefreshAll() { ClearAllIsolatedCacheByEntityType(); + var userGroupCache = ApplicationContext.Current.ApplicationCache.IsolatedRuntimeCache.GetCache(); + if (userGroupCache) + { + userGroupCache.Result.ClearCacheByKeySearch(UserGroupRepository.GetByAliasCacheKeyPrefix); + } if (UserGroupPermissionsCache) { UserGroupPermissionsCache.Result.ClearCacheByKeySearch(CacheKeys.UserGroupPermissionsCacheKey); @@ -47,7 +52,10 @@ namespace Umbraco.Web.Cache { var userGroupCache = ApplicationContext.Current.ApplicationCache.IsolatedRuntimeCache.GetCache(); if (userGroupCache) + { userGroupCache.Result.ClearCacheItem(RepositoryBase.GetCacheIdKey(id)); + userGroupCache.Result.ClearCacheByKeySearch(UserGroupRepository.GetByAliasCacheKeyPrefix); + } if (UserGroupPermissionsCache) { diff --git a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs index 213abc5afc..b43dd1b5a2 100644 --- a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs @@ -45,7 +45,7 @@ namespace Umbraco.Web.Models.Mapping .ForMember(detail => detail.Id, opt => opt.MapFrom(user => user.Id)) .ForMember(detail => detail.AllowedApplications, opt => opt.MapFrom(user => user.AllowedSections)) .ForMember(detail => detail.RealName, opt => opt.MapFrom(user => user.Name)) - .ForMember(detail => detail.Roles, opt => opt.MapFrom(user => user.Groups.Select(x => x.Name))) + .ForMember(detail => detail.Roles, opt => opt.MapFrom(user => user.Groups.ToArray())) .ForMember(detail => detail.StartContentNode, opt => opt.MapFrom(user => user.StartContentId)) .ForMember(detail => detail.StartMediaNode, opt => opt.MapFrom(user => user.StartMediaId)) .ForMember(detail => detail.Username, opt => opt.MapFrom(user => user.Username)) diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index 9309d27d1c..e459bb9eb4 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -67,7 +67,7 @@ namespace Umbraco.Web.Security get { //only load it once per instance! (but make sure groups are loaded) - if (_currentUser == null || _currentUser.GroupsLoaded == false) + if (_currentUser == null) { var id = GetUserId(); if (id == -1) diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs index de60960e5a..08aff1b605 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs @@ -312,16 +312,16 @@ namespace umbraco.cms.presentation.user { var userService = ApplicationContext.Current.Services.UserService; var allGroups = userService.GetAllUserGroups(); - var groupsForUser = userService.GetGroupsForUser(u.Id); - + var groupsForUser = userService.GetUserGroupsByAlias(u.GetGroups()); + lstInGroups.DataSource = groupsForUser; - lstInGroups.DataValueField = "Id"; + lstInGroups.DataValueField = "Alias"; lstInGroups.DataTextField = "Name"; lstInGroups.DataBind(); lstNotInGroups.DataSource = allGroups .Where(x => groupsForUser.Select(y => y.Id).Contains(x.Id) == false); - lstNotInGroups.DataValueField = "Id"; + lstNotInGroups.DataValueField = "Alias"; lstNotInGroups.DataTextField = "Name"; lstNotInGroups.DataBind(); } @@ -558,7 +558,7 @@ namespace umbraco.cms.presentation.user u.ClearGroups(); foreach (ListItem li in lstInGroups.Items) { - u.AddGroup(int.Parse(li.Value), li.Text); + u.AddGroup(li.Value); } u.Save(); diff --git a/src/umbraco.businesslogic/User.cs b/src/umbraco.businesslogic/User.cs index 0d7eaa9b1f..3a425ddef3 100644 --- a/src/umbraco.businesslogic/User.cs +++ b/src/umbraco.businesslogic/User.cs @@ -657,20 +657,20 @@ namespace umbraco.BusinessLogic /// /// Adds a group to the list of groups for the user, ensure to call Save() afterwords /// - public void AddGroup(int groupId, string groupName) + public void AddGroup(string groupAlias) { if (_lazyId.HasValue) SetupUser(_lazyId.Value); - UserEntity.AddGroup(new Umbraco.Core.Models.Membership.UserGroup - { - Id = groupId, - Name = groupName, - }); + UserEntity.AddGroup(groupAlias); } - + + /// + /// Returns the assigned user group aliases for the user + /// + /// public string[] GetGroups() { if (_lazyId.HasValue) SetupUser(_lazyId.Value); - return UserEntity.Groups.Select(x => x.Name).ToArray(); + return UserEntity.Groups.ToArray(); }