From 4a59c27793c86d6defb3f4825dce7986ad50a5bb Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 27 Aug 2015 18:33:38 +0200 Subject: [PATCH] Refactors new IPublicAccessService to return Attempt+Result so we know what happened, also ensures events are firing with msgs (events were not firing at all before). --- .../Services/IPublicAccessService.cs | 10 +- .../Services/OperationStatusType.cs | 2 +- .../Services/PublicAccessService.cs | 72 ++++- .../Services/PublicAccessServiceTests.cs | 126 ++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + src/Umbraco.Web.UI/App_Start/Test.cs | 272 ------------------ src/umbraco.cms/businesslogic/web/Access.cs | 73 +++-- 7 files changed, 235 insertions(+), 321 deletions(-) create mode 100644 src/Umbraco.Tests/Services/PublicAccessServiceTests.cs delete mode 100644 src/Umbraco.Web.UI/App_Start/Test.cs diff --git a/src/Umbraco.Core/Services/IPublicAccessService.cs b/src/Umbraco.Core/Services/IPublicAccessService.cs index 29bdb42f8b..617a8f220d 100644 --- a/src/Umbraco.Core/Services/IPublicAccessService.cs +++ b/src/Umbraco.Core/Services/IPublicAccessService.cs @@ -47,9 +47,9 @@ namespace Umbraco.Core.Services /// /// /// - /// + /// /// - PublicAccessEntry AddOrUpdateRule(IContent content, string ruleType, string ruleValue); + Attempt> AddOrUpdateRule(IContent content, string ruleType, string newRuleValue); /// /// Removes a rule @@ -57,19 +57,19 @@ namespace Umbraco.Core.Services /// /// /// - void RemoveRule(IContent content, string ruleType, string ruleValue); + Attempt RemoveRule(IContent content, string ruleType, string ruleValue); /// /// Saves the entry /// /// - void Save(PublicAccessEntry entry); + Attempt Save(PublicAccessEntry entry); /// /// Deletes the entry and all associated rules /// /// - void Delete(PublicAccessEntry entry); + Attempt Delete(PublicAccessEntry entry); } } \ No newline at end of file diff --git a/src/Umbraco.Core/Services/OperationStatusType.cs b/src/Umbraco.Core/Services/OperationStatusType.cs index 7398572810..14f24c5c4e 100644 --- a/src/Umbraco.Core/Services/OperationStatusType.cs +++ b/src/Umbraco.Core/Services/OperationStatusType.cs @@ -1,7 +1,7 @@ namespace Umbraco.Core.Services { /// - /// A status type of the result of publishing a content item + /// A status type of the result of saving an item /// /// /// Anything less than 10 = Success! diff --git a/src/Umbraco.Core/Services/PublicAccessService.cs b/src/Umbraco.Core/Services/PublicAccessService.cs index 6ca6660895..7dd35142d2 100644 --- a/src/Umbraco.Core/Services/PublicAccessService.cs +++ b/src/Umbraco.Core/Services/PublicAccessService.cs @@ -68,7 +68,7 @@ namespace Umbraco.Core.Services ids.Reverse(); using (var repo = RepositoryFactory.CreatePublicAccessRepository(UowProvider.GetUnitOfWork())) - { + { //This will retrieve from cache! var entries = repo.GetAll().ToArray(); @@ -109,32 +109,44 @@ namespace Umbraco.Core.Services /// /// /// - /// + /// /// - public PublicAccessEntry AddOrUpdateRule(IContent content, string ruleType, string ruleValue) + public Attempt> AddOrUpdateRule(IContent content, string ruleType, string newRuleValue) { + var evtMsgs = EventMessagesFactory.Get(); var uow = UowProvider.GetUnitOfWork(); using (var repo = RepositoryFactory.CreatePublicAccessRepository(uow)) { var entry = repo.GetAll().FirstOrDefault(x => x.ProtectedNodeId == content.Id); - if (entry == null) return null; + if (entry == null) + return Attempt>.Fail(); - var existingRule = entry.Rules.FirstOrDefault(x => x.RuleType == ruleType && x.RuleValue == ruleValue); + var existingRule = entry.Rules.FirstOrDefault(x => x.RuleType == ruleType); if (existingRule == null) { - entry.AddRule(ruleValue, ruleType); + entry.AddRule(newRuleValue, ruleType); } else { existingRule.RuleType = ruleType; - existingRule.RuleValue = ruleValue; + existingRule.RuleValue = newRuleValue; + } + + if (Saving.IsRaisedEventCancelled( + new SaveEventArgs(entry, evtMsgs), + this)) + { + return Attempt>.Fail( + new OperationStatus(entry, OperationStatusType.FailedCancelledByEvent, evtMsgs)); } repo.AddOrUpdate(entry); uow.Commit(); - return entry; + Saved.RaiseEvent(new SaveEventArgs(entry, false, evtMsgs), this); + return Attempt>.Succeed( + new OperationStatus(entry, OperationStatusType.Success, evtMsgs)); } } @@ -144,51 +156,85 @@ namespace Umbraco.Core.Services /// /// /// - public void RemoveRule(IContent content, string ruleType, string ruleValue) + public Attempt RemoveRule(IContent content, string ruleType, string ruleValue) { + var evtMsgs = EventMessagesFactory.Get(); var uow = UowProvider.GetUnitOfWork(); using (var repo = RepositoryFactory.CreatePublicAccessRepository(uow)) { var entry = repo.GetAll().FirstOrDefault(x => x.ProtectedNodeId == content.Id); - if (entry == null) return; + if (entry == null) return Attempt.Fail(); var existingRule = entry.Rules.FirstOrDefault(x => x.RuleType == ruleType && x.RuleValue == ruleValue); - if (existingRule == null) return; + if (existingRule == null) return Attempt.Fail(); entry.RemoveRule(existingRule); + if (Saving.IsRaisedEventCancelled( + new SaveEventArgs(entry, evtMsgs), + this)) + { + return Attempt.Fail(OperationStatus.Cancelled(evtMsgs)); + } + repo.AddOrUpdate(entry); uow.Commit(); + + Saved.RaiseEvent(new SaveEventArgs(entry, false, evtMsgs), this); + return Attempt.Succeed(OperationStatus.Success(evtMsgs)); } + } /// /// Saves the entry /// /// - public void Save(PublicAccessEntry entry) + public Attempt Save(PublicAccessEntry entry) { + var evtMsgs = EventMessagesFactory.Get(); + if (Saving.IsRaisedEventCancelled( + new SaveEventArgs(entry, evtMsgs), + this)) + { + return Attempt.Fail(OperationStatus.Cancelled(evtMsgs)); + } + var uow = UowProvider.GetUnitOfWork(); using (var repo = RepositoryFactory.CreatePublicAccessRepository(uow)) { repo.AddOrUpdate(entry); uow.Commit(); } + + Saved.RaiseEvent(new SaveEventArgs(entry, false, evtMsgs), this); + return Attempt.Succeed(OperationStatus.Success(evtMsgs)); } /// /// Deletes the entry and all associated rules /// /// - public void Delete(PublicAccessEntry entry) + public Attempt Delete(PublicAccessEntry entry) { + var evtMsgs = EventMessagesFactory.Get(); + if (Deleting.IsRaisedEventCancelled( + new DeleteEventArgs(entry, evtMsgs), + this)) + { + return Attempt.Fail(OperationStatus.Cancelled(evtMsgs)); + } + var uow = UowProvider.GetUnitOfWork(); using (var repo = RepositoryFactory.CreatePublicAccessRepository(uow)) { repo.Delete(entry); uow.Commit(); } + + Deleted.RaiseEvent(new DeleteEventArgs(entry, false, evtMsgs), this); + return Attempt.Succeed(OperationStatus.Success(evtMsgs)); } /// diff --git a/src/Umbraco.Tests/Services/PublicAccessServiceTests.cs b/src/Umbraco.Tests/Services/PublicAccessServiceTests.cs new file mode 100644 index 0000000000..c1bdaabe3a --- /dev/null +++ b/src/Umbraco.Tests/Services/PublicAccessServiceTests.cs @@ -0,0 +1,126 @@ +using System; +using System.Linq; +using NUnit.Framework; +using Umbraco.Core.Models; +using Umbraco.Core.Services; +using Umbraco.Tests.TestHelpers; +using Umbraco.Tests.TestHelpers.Entities; + +namespace Umbraco.Tests.Services +{ + [DatabaseTestBehavior(DatabaseBehavior.NewDbFileAndSchemaPerTest)] + [TestFixture, RequiresSTA] + public class PublicAccessServiceTests : BaseServiceTest + { + [SetUp] + public override void Initialize() + { + base.Initialize(); + } + + [TearDown] + public override void TearDown() + { + base.TearDown(); + } + + [Test] + public void Can_Add_New_Entry() + { + // Arrange + var contentService = ServiceContext.ContentService; + var contentTypeService = ServiceContext.ContentTypeService; + var ct = MockedContentTypes.CreateSimpleContentType("blah", "Blah"); + contentTypeService.Save(ct); + var c = MockedContent.CreateSimpleContent(ct, "Test", -1); + contentService.Save(c); + var publicAccessService = ServiceContext.PublicAccessService; + + + // Act + var entry = new PublicAccessEntry(c, c, c, new[] + { + new PublicAccessRule() + { + RuleType = "TestType", + RuleValue = "TestVal" + }, + }); + var result = publicAccessService.Save(entry); + + // Assert + Assert.IsTrue(result.Success); + Assert.AreEqual(OperationStatusType.Success, result.Result.StatusType); + Assert.IsTrue(entry.HasIdentity); + Assert.AreNotEqual(entry.Key, Guid.Empty); + Assert.AreEqual(c.Id, entry.LoginNodeId); + Assert.AreEqual(c.Id, entry.NoAccessNodeId); + Assert.AreEqual(c.Id, entry.ProtectedNodeId); + } + + [Test] + public void Can_Add_Rule() + { + // Arrange + var contentService = ServiceContext.ContentService; + var contentTypeService = ServiceContext.ContentTypeService; + var ct = MockedContentTypes.CreateSimpleContentType("blah", "Blah"); + contentTypeService.Save(ct); + var c = MockedContent.CreateSimpleContent(ct, "Test", -1); + contentService.Save(c); + var publicAccessService = ServiceContext.PublicAccessService; + var entry = new PublicAccessEntry(c, c, c, new[] + { + new PublicAccessRule() + { + RuleType = "TestType", + RuleValue = "TestVal" + }, + }); + publicAccessService.Save(entry); + + // Act + var updated = publicAccessService.AddOrUpdateRule(c, "TestType", "AnotherVal"); + //re-get + entry = publicAccessService.GetEntryForContent(c); + + // Assert + Assert.IsTrue(updated.Success); + Assert.AreEqual(OperationStatusType.Success, updated.Result.StatusType); + Assert.AreEqual(2, entry.Rules.Count()); + } + + [Test] + public void Can_Update_Rule() + { + // Arrange + var contentService = ServiceContext.ContentService; + var contentTypeService = ServiceContext.ContentTypeService; + var ct = MockedContentTypes.CreateSimpleContentType("blah", "Blah"); + contentTypeService.Save(ct); + var c = MockedContent.CreateSimpleContent(ct, "Test", -1); + contentService.Save(c); + var publicAccessService = ServiceContext.PublicAccessService; + var entry = new PublicAccessEntry(c, c, c, new[] + { + new PublicAccessRule() + { + RuleType = "TestType", + RuleValue = "TestValue" + }, + }); + publicAccessService.Save(entry); + + // Act + var updated = publicAccessService.AddOrUpdateRule(c, "TestType", "AnotherVal"); + //re-get + entry = publicAccessService.GetEntryForContent(c); + + // Assert + Assert.IsTrue(updated.Success); + Assert.AreEqual(OperationStatusType.Success, updated.Result.StatusType); + Assert.AreEqual(1, entry.Rules.Count()); + Assert.AreEqual("AnotherVal", entry.Rules.ElementAt(0).RuleValue); + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 05a57928e0..6dbf92a7f4 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -188,6 +188,7 @@ + diff --git a/src/Umbraco.Web.UI/App_Start/Test.cs b/src/Umbraco.Web.UI/App_Start/Test.cs deleted file mode 100644 index d70f18eb9b..0000000000 --- a/src/Umbraco.Web.UI/App_Start/Test.cs +++ /dev/null @@ -1,272 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Globalization; -using System.Linq; -using System.Threading.Tasks; -using System.Web; -using Microsoft.Owin; -using Microsoft.Owin.Security.Google; -using Microsoft.Owin.Security.OpenIdConnect; -using Owin; -using Umbraco.Core; -using Umbraco.Core.Security; -using Umbraco.Web.Security.Identity; -using Umbraco.Web.UI.App_Start; - -[assembly: OwinStartup("UmbracoStandardOwinStartup2", typeof(UmbracoStandardOwinStartup2))] - -namespace Umbraco.Web.UI.App_Start -{ - /// - /// A custom way to configure OWIN for Umbraco - /// - /// - /// The startup type is specified in appSettings under owin:appStartup - change it to "CustomUmbracoStartup" to use this class - /// - /// This startup class would allow you to customize the Identity IUserStore and/or IUserManager for the Umbraco Backoffice - /// - public class UmbracoCustomOwinStartup - { - public void Configuration(IAppBuilder app) - { - //Configure the Identity user manager and user store for use with Umbraco Back office - - // *** EXPERT: overloads of this method allow you to specify a custom UserStore or even a custom UserManager! - // *** If you plan to implement your own custom 2 factor auth, you will need a custom implementation of: - // *** BackOfficeUserManager & BackOfficeUserStore. Your custom BackOfficeUserManager will need to override/implement: - // *** - SupportsUserTwoFactor to return true - // *** - Umbraco.Web.Security.Identity.IUmbracoBackOfficeTwoFactorOptions - // *** The result view returned from IUmbracoBackOfficeTwoFactorOptions.GetTwoFactorView will be the angular - // *** view displayed to the user to enter the 2 factor authentication code. You will need to create/implement - // *** the custom angular view and all logic to handle the REST call to verify the code and log the user in - // *** based on the username provided on the $scope of your view. - // *** Your custom BackOfficeUserStore will need to override/implement: - // *** - Microsoft.AspNet.Identity.IUserTwoFactorStore - - app.ConfigureUserManagerForUmbracoBackOffice( - ApplicationContext.Current, - global::Umbraco.Core.Security.MembershipProviderExtensions.GetUsersMembershipProvider().AsUmbracoMembershipProvider()); - - //Ensure owin is configured for Umbraco back office authentication - app - .UseUmbracoBackOfficeCookieAuthentication(ApplicationContext.Current) - .UseUmbracoBackOfficeExternalCookieAuthentication(ApplicationContext.Current); - /* - * Configure external logins for the back office: - * - * Depending on the authentication sources you would like to enable, you will need to install - * certain Nuget packages. - * - * For Google auth: Install-Package UmbracoCms.IdentityExtensions.Google - * For Facebook auth: Install-Package UmbracoCms.IdentityExtensions.Facebook - * For Microsoft auth: Install-Package UmbracoCms.IdentityExtensions.Microsoft - * For Azure ActiveDirectory auth: Install-Package UmbracoCms.IdentityExtensions.AzureActiveDirectory - * - * There are many more providers such as Twitter, Yahoo, ActiveDirectory, etc... most information can - * be found here: http://www.asp.net/web-api/overview/security/external-authentication-services - * - * For sample code on using external providers with the Umbraco back office, install one of the - * packages listed above to review it's code samples - * - */ - /* - * To configure a simple auth token server for the back office: - * - * By default the CORS policy is to allow all requests - * - * app.UseUmbracoBackOfficeTokenAuth(new BackOfficeAuthServerProviderOptions()); - * - * If you want to have a custom CORS policy for the token server you can provide - * a custom CORS policy, example: - * - * app.UseUmbracoBackOfficeTokenAuth( - * new BackOfficeAuthServerProviderOptions() - * { - * //Modify the CorsPolicy as required - * CorsPolicy = new CorsPolicy() - * { - * AllowAnyHeader = true, - * AllowAnyMethod = true, - * Origins = { "http://mywebsite.com" } - * } - * }); - */ - } - } - - /// - /// The standard way to configure OWIN for Umbraco - /// - /// - /// The startup type is specified in appSettings under owin:appStartup - change it to "StandardUmbracoStartup" to use this class - /// - public class UmbracoStandardOwinStartup2 : UmbracoDefaultOwinStartup - { - public override void Configuration(IAppBuilder app) - { - //ensure the default options are configured - base.Configuration(app); - - // - app.ConfigureBackOfficeGoogleAuth( - "1072120697051-p41pro11srud3o3n90j7m00geq426jqt.apps.googleusercontent.com", - "cs_LJTXh2rtI01C5OIt9WFkt"); - - /* - * Configure external logins for the back office: - * - * Depending on the authentication sources you would like to enable, you will need to install - * certain Nuget packages. - * - * For Google auth: Install-Package UmbracoCms.IdentityExtensions.Google - * For Facebook auth: Install-Package UmbracoCms.IdentityExtensions.Facebook - * For Microsoft auth: Install-Package UmbracoCms.IdentityExtensions.Microsoft - * For Azure ActiveDirectory auth: Install-Package UmbracoCms.IdentityExtensions.AzureActiveDirectory - * - * There are many more providers such as Twitter, Yahoo, ActiveDirectory, etc... most information can - * be found here: http://www.asp.net/web-api/overview/security/external-authentication-services - * - * For sample code on using external providers with the Umbraco back office, install one of the - * packages listed above to review it's code samples - * - */ - - /* - * To configure a simple auth token server for the back office: - * - * By default the CORS policy is to allow all requests - * - * app.UseUmbracoBackOfficeTokenAuth(new BackOfficeAuthServerProviderOptions()); - * - * If you want to have a custom CORS policy for the token server you can provide - * a custom CORS policy, example: - * - * app.UseUmbracoBackOfficeTokenAuth( - * new BackOfficeAuthServerProviderOptions() - * { - * //Modify the CorsPolicy as required - * CorsPolicy = new CorsPolicy() - * { - * AllowAnyHeader = true, - * AllowAnyMethod = true, - * Origins = { "http://mywebsite.com" } - * } - * }); - */ - - } - } - - public static class UmbracoGoogleAuthExtensions - { - /// - /// Configure google sign-in - /// - /// - /// - /// - /// - /// - /// - /// - /// - /// Nuget installation: - /// Microsoft.Owin.Security.Google - /// - /// Google account documentation for ASP.Net Identity can be found: - /// - /// http://www.asp.net/web-api/overview/security/external-authentication-services#GOOGLE - /// - /// Google apps can be created here: - /// - /// https://developers.google.com/accounts/docs/OpenIDConnect#getcredentials - /// - /// - public static void ConfigureBackOfficeGoogleAuth(this IAppBuilder app, string clientId, string clientSecret, - string caption = "Google", string style = "btn-google-plus", string icon = "fa-google-plus") - { - var googleOptions = new GoogleOAuth2AuthenticationOptions() - { - ClientId = clientId, - ClientSecret = clientSecret, - //In order to allow using different google providers on the front-end vs the back office, - // these settings are very important to make them distinguished from one another. - SignInAsAuthenticationType = global::Umbraco.Core.Constants.Security.BackOfficeExternalAuthenticationType, - // By default this is '/signin-google', you will need to change that default value in your - // Google developer settings for your web-app in the "REDIRECT URIS" setting - CallbackPath = new PathString("/umbraco-google-signin") - }; - - googleOptions.SetExternalSignInAutoLinkOptions( - new ExternalSignInAutoLinkOptions( - autoLinkExternalAccount: true)); - - googleOptions.ForUmbracoBackOffice(style, icon); - googleOptions.Caption = caption; - app.UseGoogleAuthentication(googleOptions); - } - - /// - /// Configure ActiveDirectory sign-in - /// - /// - /// - /// - /// - /// The URL that will be redirected to after login is successful, example: http://mydomain.com/umbraco/; - /// - /// - /// - /// This is the "Issuer Id" for you Azure AD application. This a GUID value and can be found - /// in the Azure portal when viewing your configured application and clicking on 'View endpoints' - /// which will list all of the API endpoints. Each endpoint will contain a GUID value, this is - /// the Issuer Id which must be used for this value. - /// - /// If this value is not set correctly then accounts won't be able to be detected - /// for un-linking in the back office. - /// - /// - /// - /// - /// - /// - /// ActiveDirectory account documentation for ASP.Net Identity can be found: - /// https://github.com/AzureADSamples/WebApp-WebAPI-OpenIDConnect-DotNet - /// - public static void ConfigureBackOfficeAzureActiveDirectoryAuth(this IAppBuilder app, - string tenant, string clientId, string postLoginRedirectUri, Guid issuerId, - string caption = "Active Directory", string style = "btn-microsoft", string icon = "fa-windows") - { - var authority = string.Format( - CultureInfo.InvariantCulture, - "https://login.windows.net/{0}", - tenant); - - var adOptions = new OpenIdConnectAuthenticationOptions - { - SignInAsAuthenticationType = global::Umbraco.Core.Constants.Security.BackOfficeExternalAuthenticationType, - ClientId = clientId, - Authority = authority, - Notifications = new OpenIdConnectAuthenticationNotifications - { - RedirectToIdentityProvider = notification => - { - return Task.FromResult(0); - } - } - }; - - adOptions.SetExternalSignInAutoLinkOptions(new ExternalSignInAutoLinkOptions()); - - adOptions.ForUmbracoBackOffice(style, icon); - adOptions.Caption = caption; - //Need to set the auth tyep as the issuer path - adOptions.AuthenticationType = string.Format( - CultureInfo.InvariantCulture, - "https://sts.windows.net/{0}/", - issuerId); - app.UseOpenIdConnectAuthentication(adOptions); - } - - } -} \ No newline at end of file diff --git a/src/umbraco.cms/businesslogic/web/Access.cs b/src/umbraco.cms/businesslogic/web/Access.cs index 8271fa581f..b2d4b4c103 100644 --- a/src/umbraco.cms/businesslogic/web/Access.cs +++ b/src/umbraco.cms/businesslogic/web/Access.cs @@ -76,7 +76,7 @@ namespace umbraco.cms.businesslogic.web Constants.Conventions.PublicAccess.MemberRoleRuleType, role); - if (entry == null) + if (entry.Success == false && entry.Result.Entity == null) { throw new Exception("Document is not protected!"); } @@ -95,12 +95,14 @@ namespace umbraco.cms.businesslogic.web if (content == null) throw new Exception("No content found with document id " + DocumentId); - var entry = ApplicationContext.Current.Services.PublicAccessService.AddOrUpdateRule( - content, - Constants.Conventions.PublicAccess.MemberGroupIdRuleType, - MemberGroupId.ToString(CultureInfo.InvariantCulture)); - - Save(); + if (ApplicationContext.Current.Services.PublicAccessService.AddOrUpdateRule( + content, + Constants.Conventions.PublicAccess.MemberGroupIdRuleType, + MemberGroupId.ToString(CultureInfo.InvariantCulture))) + { + Save(); + } + } [Obsolete("This method is no longer supported. Use the ASP.NET MemberShip methods instead", true)] @@ -111,12 +113,14 @@ namespace umbraco.cms.businesslogic.web if (content == null) throw new Exception("No content found with document id " + DocumentId); - ApplicationContext.Current.Services.PublicAccessService.AddOrUpdateRule( - content, - Constants.Conventions.PublicAccess.MemberIdRuleType, - MemberId.ToString(CultureInfo.InvariantCulture)); - - Save(); + if (ApplicationContext.Current.Services.PublicAccessService.AddOrUpdateRule( + content, + Constants.Conventions.PublicAccess.MemberIdRuleType, + MemberId.ToString(CultureInfo.InvariantCulture))) + { + Save(); + } + } public static void AddMembershipUserToDocument(int documentId, string membershipUserName) @@ -133,14 +137,17 @@ namespace umbraco.cms.businesslogic.web Constants.Conventions.PublicAccess.MemberUsernameRuleType, membershipUserName); - if (entry == null) + if (entry.Success == false && entry.Result.Entity == null) { throw new Exception("Document is not protected!"); } - Save(); - - new Access().FireAfterAddMembershipUserToDocument(doc, membershipUserName, e); + if (entry) + { + Save(); + new Access().FireAfterAddMembershipUserToDocument(doc, membershipUserName, e); + } + } [Obsolete("This method is no longer supported. Use the ASP.NET MemberShip methods instead", true)] @@ -153,11 +160,15 @@ namespace umbraco.cms.businesslogic.web Constants.Conventions.PublicAccess.MemberGroupIdRuleType, MemberGroupId.ToString(CultureInfo.InvariantCulture)); - if (entry == null) + if (entry.Success == false && entry.Result.Entity == null) { throw new Exception("Document is not protected!"); } - Save(); + + if (entry) + { + Save(); + } } public static void RemoveMembershipRoleFromDocument(int documentId, string role) @@ -168,14 +179,15 @@ namespace umbraco.cms.businesslogic.web if (e.Cancel) return; - ApplicationContext.Current.Services.PublicAccessService.RemoveRule( + if (ApplicationContext.Current.Services.PublicAccessService.RemoveRule( doc.ContentEntity, Constants.Conventions.PublicAccess.MemberRoleRuleType, - role); - - Save(); - - new Access().FireAfterRemoveMemberShipRoleFromDocument(doc, role, e); + role)) + { + Save(); + new Access().FireAfterRemoveMemberShipRoleFromDocument(doc, role, e); + }; + } public static bool RenameMemberShipRole(string oldRolename, string newRolename) @@ -222,11 +234,12 @@ namespace umbraco.cms.businesslogic.web new List()); } - ApplicationContext.Current.Services.PublicAccessService.Save(entry); - - Save(); - - new Access().FireAfterAddProtection(new Document(DocumentId), e); + if (ApplicationContext.Current.Services.PublicAccessService.Save(entry)) + { + Save(); + new Access().FireAfterAddProtection(new Document(DocumentId), e); + } + } public static void RemoveProtection(int DocumentId)