diff --git a/src/Umbraco.Web.UI.Client/src/common/security/interceptor.js b/src/Umbraco.Web.UI.Client/src/common/security/interceptor.js index b4ea11a6af..6374b789d9 100644 --- a/src/Umbraco.Web.UI.Client/src/common/security/interceptor.js +++ b/src/Umbraco.Web.UI.Client/src/common/security/interceptor.js @@ -1,23 +1,35 @@ angular.module('umbraco.security.interceptor', ['umbraco.security.retryQueue']) // This http interceptor listens for authentication failures -.factory('securityInterceptor', ['$injector', 'securityRetryQueue', function($injector, queue) { - return function(promise) { - // Intercept failed requests - return promise.then(null, function(originalResponse) { - if(originalResponse.status === 401) { - // The request bounced because it was not authorized - add a new request to the retry queue - promise = queue.pushRetryFn('unauthorized-server', function retryRequest() { - // We must use $injector to get the $http service to prevent circular dependency - return $injector.get('$http')(originalResponse.config); +.factory('securityInterceptor', ['$injector', 'securityRetryQueue', function ($injector, queue) { + return function (promise) { + // Intercept failed requests + return promise.then(null, function (originalResponse) { + + if (originalResponse.status === 401) { + // The request bounced because it was not authorized - add a new request to the retry queue + promise = queue.pushRetryFn('unauthorized-server', function retryRequest() { + // We must use $injector to get the $http service to prevent circular dependency + return $injector.get('$http')(originalResponse.config); + }); + } + else if (originalResponse.status === 401) { + //if the status was a 403 it means the user didn't have permission to do what the request was trying to do. + //How do we deal with this now, need to tell the user somehow that they don't have permission to do the thing that was + //requested. We can either deal with this globally here, or we can deal with it globally for individual requests on the umbRequestHelper, + // or completely custom for services calling resources. + + //http://issues.umbraco.org/issue/U4-2749 + + //For now, I'm just going to do an alert! + alert("Unauthorized access to URL \r\n" + originalResponse.config.url + "\r\n with data \r\n" + angular.toJson(originalResponse.config.data)); + } + return promise; }); - } - return promise; - }); - }; + }; }]) // We have to add the interceptor to the queue as a string because the interceptor depends upon service instances that are not available in the config block. -.config(['$httpProvider', function($httpProvider) { - $httpProvider.responseInterceptors.push('securityInterceptor'); +.config(['$httpProvider', function ($httpProvider) { + $httpProvider.responseInterceptors.push('securityInterceptor'); }]); \ No newline at end of file diff --git a/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js index 3a40169710..60fbbefcf2 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js @@ -74,8 +74,8 @@ function contentEditingHelper($location, $routeParams, notificationsService, ser * @function * * @description - * A function to handle the validation (modelState) errors collection which will happen on a 403 error indicating validation errors - * It's worth noting that when a 403 occurs, the data is still saved just never published, though this depends on if the entity is a new + * A function to handle the validation (modelState) errors collection which will happen on a 400 error indicating validation errors + * It's worth noting that when a 400 occurs, the data is still saved just never published, though this depends on if the entity is a new * entity and whether or not the data fulfils the absolute basic requirements like having a mandatory Name. */ handleValidationErrors: function (allProps, modelState) { @@ -131,7 +131,7 @@ function contentEditingHelper($location, $routeParams, notificationsService, ser * A function to handle what happens when we have validation issues from the server side */ handleSaveError: function (args) { - + if (!args.err) { throw "args.err cannot be null"; } @@ -139,10 +139,10 @@ function contentEditingHelper($location, $routeParams, notificationsService, ser throw "args.allNewProps must be a valid array"; } - //When the status is a 403 status, we have validation errors. + //When the status is a 400 status with a custom header: X-Status-Reason: Validation failed, we have validation errors. //Otherwise the error is probably due to invalid data (i.e. someone mucking around with the ids or something). //Or, some strange server error - if (args.err.status === 403) { + if (args.err.status === 400) { //now we need to look through all the validation errors if (args.err.data && (args.err.data.ModelState)) { diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/content-editing-helper.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/content-editing-helper.spec.js index 140e37fb10..2c73044d3e 100644 --- a/src/Umbraco.Web.UI.Client/test/unit/common/services/content-editing-helper.spec.js +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/content-editing-helper.spec.js @@ -18,13 +18,13 @@ describe('contentEditingHelper tests', function () { describe('handles http validation errors', function () { - it('handles validation errors for 403 results', function () { + it('handles validation errors for 400 results', function () { //arrange var content = mocksUtils.getMockContent(1234); var err = { data: content, - status: 403 + status: 400 }; err.data.ModelState = {}; @@ -39,11 +39,11 @@ describe('contentEditingHelper tests', function () { expect(handled).toBe(true); }); - it('does not handle validation errors that are not 403', function () { + it('does not handle validation errors that are not 400', function () { //arrange var err = { - status: 400 + status: 410 }; //act @@ -57,13 +57,13 @@ describe('contentEditingHelper tests', function () { expect(handled).toBe(false); }); - it('does not handle validation errors that are 403 without model state', function () { + it('does not handle validation errors that are 400 without model state', function () { //arrange var content = mocksUtils.getMockContent(1234); var err = { data: content, - status: 403 + status: 400 }; //act diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index efe2e678a8..fb57c8078c 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -215,10 +215,10 @@ namespace Umbraco.Web.Editors && (contentItem.Action == ContentSaveAction.SaveNew || contentItem.Action == ContentSaveAction.PublishNew)) { //ok, so the absolute mandatory data is invalid and it's new, we cannot actually continue! - // add the modelstate to the outgoing object and throw a 403 + // add the modelstate to the outgoing object and throw a validation message var forDisplay = Mapper.Map(contentItem.PersistedContent); forDisplay.Errors = ModelState.ToErrorDictionary(); - throw new HttpResponseException(Request.CreateResponse(HttpStatusCode.Forbidden, forDisplay)); + throw new HttpResponseException(Request.CreateValidationErrorResponse(forDisplay)); } @@ -352,7 +352,7 @@ namespace Umbraco.Web.Editors if (contentService.Sort(sortedContent) == false) { LogHelper.Warn("Content sorting failed, this was probably caused by an event being cancelled"); - return Request.CreateErrorResponse(HttpStatusCode.Forbidden, "Content sorting failed, this was probably caused by an event being cancelled"); + return Request.CreateValidationErrorResponse("Content sorting failed, this was probably caused by an event being cancelled"); } return Request.CreateResponse(HttpStatusCode.OK); } diff --git a/src/Umbraco.Web/Editors/ContentControllerBase.cs b/src/Umbraco.Web/Editors/ContentControllerBase.cs index 0bb45625f7..a7d603c562 100644 --- a/src/Umbraco.Web/Editors/ContentControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentControllerBase.cs @@ -11,6 +11,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; using Umbraco.Web.Models.ContentEditing; +using Umbraco.Web.WebApi; using Umbraco.Web.WebApi.Filters; namespace Umbraco.Web.Editors @@ -122,7 +123,7 @@ namespace Umbraco.Web.Editors if (!ModelState.IsValid) { display.Errors = ModelState.ToErrorDictionary(); - throw new HttpResponseException(Request.CreateResponse(HttpStatusCode.Forbidden, display)); + throw new HttpResponseException(Request.CreateValidationErrorResponse(display)); } } diff --git a/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs b/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs index 0ea0b1ff7f..b4d9be726a 100644 --- a/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs +++ b/src/Umbraco.Web/Editors/ContentPostValidateAttribute.cs @@ -8,6 +8,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Security; +using Umbraco.Web.WebApi; using umbraco.BusinessLogic.Actions; namespace Umbraco.Web.Editors @@ -86,7 +87,7 @@ namespace Umbraco.Web.Editors permissionToCheck, contentToCheck) == false) { - actionContext.Response = actionContext.Request.CreateResponse(HttpStatusCode.Unauthorized); + actionContext.Response = actionContext.Request.CreateUserNoAccessResponse(); return; } } diff --git a/src/Umbraco.Web/Editors/DataTypeValidateAttribute.cs b/src/Umbraco.Web/Editors/DataTypeValidateAttribute.cs index c97d5828fd..26688ff5fe 100644 --- a/src/Umbraco.Web/Editors/DataTypeValidateAttribute.cs +++ b/src/Umbraco.Web/Editors/DataTypeValidateAttribute.cs @@ -12,6 +12,7 @@ using Umbraco.Core.Models.EntityBase; using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; +using Umbraco.Web.WebApi; namespace Umbraco.Web.Editors { @@ -113,7 +114,7 @@ namespace Umbraco.Web.Editors if (actionContext.ModelState.IsValid == false) { //if it is not valid, do not continue and return the model state - actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.Forbidden, actionContext.ModelState); + actionContext.Response = actionContext.Request.CreateValidationErrorResponse(actionContext.ModelState); return; } diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs index c2e33a3a9f..bd7e89feca 100644 --- a/src/Umbraco.Web/Editors/MediaController.cs +++ b/src/Umbraco.Web/Editors/MediaController.cs @@ -159,10 +159,10 @@ namespace Umbraco.Web.Editors && (contentItem.Action == ContentSaveAction.SaveNew)) { //ok, so the absolute mandatory data is invalid and it's new, we cannot actually continue! - // add the modelstate to the outgoing object and throw a 403 + // add the modelstate to the outgoing object and throw validation response var forDisplay = Mapper.Map(contentItem.PersistedContent); forDisplay.Errors = ModelState.ToErrorDictionary(); - throw new HttpResponseException(Request.CreateResponse(HttpStatusCode.Forbidden, forDisplay)); + throw new HttpResponseException(Request.CreateValidationErrorResponse(forDisplay)); } } @@ -216,7 +216,7 @@ namespace Umbraco.Web.Editors if (mediaService.Sort(sortedMedia) == false) { LogHelper.Warn("Media sorting failed, this was probably caused by an event being cancelled"); - return Request.CreateErrorResponse(HttpStatusCode.Forbidden, "Media sorting failed, this was probably caused by an event being cancelled"); + return Request.CreateValidationErrorResponse("Media sorting failed, this was probably caused by an event being cancelled"); } return Request.CreateResponse(HttpStatusCode.OK); } diff --git a/src/Umbraco.Web/Editors/MediaPostValidateAttribute.cs b/src/Umbraco.Web/Editors/MediaPostValidateAttribute.cs index 933fda20fe..fdb70f1d39 100644 --- a/src/Umbraco.Web/Editors/MediaPostValidateAttribute.cs +++ b/src/Umbraco.Web/Editors/MediaPostValidateAttribute.cs @@ -1,6 +1,7 @@ using System; using System.Net; using System.Net.Http; +using System.Web.Http; using System.Web.Http.Controllers; using System.Web.Http.Filters; using Umbraco.Core; @@ -8,6 +9,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Security; +using Umbraco.Web.WebApi; namespace Umbraco.Web.Editors { @@ -60,7 +62,7 @@ namespace Umbraco.Web.Editors case ContentSaveAction.PublishNew: default: //we don't support this for media - actionContext.Response = actionContext.Request.CreateResponse(HttpStatusCode.Unauthorized); + actionContext.Response = actionContext.Request.CreateResponse(HttpStatusCode.NotFound); return; } @@ -71,8 +73,7 @@ namespace Umbraco.Web.Editors contentToCheck.Id, contentToCheck) == false) { - actionContext.Response = actionContext.Request.CreateResponse(HttpStatusCode.Unauthorized); - return; + throw new HttpResponseException(actionContext.Request.CreateUserNoAccessResponse()); } } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index a6ffcefb0b..2d76024a3b 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -555,6 +555,7 @@ + diff --git a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs index 98bf7823b5..a968deef2e 100644 --- a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs @@ -61,6 +61,7 @@ namespace Umbraco.Web.WebApi.Filters { if (UmbracoContext.Current.Security.CurrentUser == null) { + //not logged in throw new HttpResponseException(System.Net.HttpStatusCode.Unauthorized); } @@ -105,7 +106,7 @@ namespace Umbraco.Web.WebApi.Filters } else { - throw new HttpResponseException(System.Net.HttpStatusCode.Unauthorized); + throw new HttpResponseException(actionContext.Request.CreateUserNoAccessResponse()); } } diff --git a/src/Umbraco.Web/WebApi/HttpRequestMessageExtensions.cs b/src/Umbraco.Web/WebApi/HttpRequestMessageExtensions.cs new file mode 100644 index 0000000000..44cc8793fe --- /dev/null +++ b/src/Umbraco.Web/WebApi/HttpRequestMessageExtensions.cs @@ -0,0 +1,83 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Text; +using System.Threading.Tasks; +using System.Web.Http; +using System.Web.Http.ModelBinding; + +namespace Umbraco.Web.WebApi +{ + + public static class HttpRequestMessageExtensions + { + /// + /// Create a 403 (Forbidden) response indicating that hte current user doesn't have access to the resource + /// requested or the action it needs to take. + /// + /// + /// + /// + /// This is different from a 401 which indicates that the user is not logged in. + /// + public static HttpResponseMessage CreateUserNoAccessResponse(this HttpRequestMessage request) + { + return request.CreateResponse(HttpStatusCode.Forbidden); + } + + /// + /// Create a 400 response message indicating that a validation error occurred + /// + /// + /// + /// + /// + public static HttpResponseMessage CreateValidationErrorResponse(this HttpRequestMessage request, T value) + { + var msg = request.CreateResponse(HttpStatusCode.BadRequest, value); + msg.Headers.Add("X-Status-Reason", "Validation failed"); + return msg; + } + + /// + /// Create a 400 response message indicating that a validation error occurred + /// + /// + /// + public static HttpResponseMessage CreateValidationErrorResponse(this HttpRequestMessage request) + { + var msg = request.CreateResponse(HttpStatusCode.BadRequest); + msg.Headers.Add("X-Status-Reason", "Validation failed"); + return msg; + } + + /// + /// Create a 400 response message indicating that a validation error occurred + /// + /// + /// + /// + public static HttpResponseMessage CreateValidationErrorResponse(this HttpRequestMessage request, string errorMessage) + { + var msg = request.CreateErrorResponse(HttpStatusCode.BadRequest, errorMessage); + msg.Headers.Add("X-Status-Reason", "Validation failed"); + return msg; + } + + /// + /// Create a 400 response message indicating that a validation error occurred + /// + /// + /// + /// + public static HttpResponseMessage CreateValidationErrorResponse(this HttpRequestMessage request, ModelStateDictionary modelState) + { + var msg = request.CreateErrorResponse(HttpStatusCode.BadRequest, modelState); + msg.Headers.Add("X-Status-Reason", "Validation failed"); + return msg; + } + } + +} diff --git a/src/Umbraco.Web/WebServices/DomainsApiController.cs b/src/Umbraco.Web/WebServices/DomainsApiController.cs index fd53e7ad95..43bf9bf092 100644 --- a/src/Umbraco.Web/WebServices/DomainsApiController.cs +++ b/src/Umbraco.Web/WebServices/DomainsApiController.cs @@ -7,6 +7,7 @@ using System.Web.Http; using Umbraco.Core; using Umbraco.Web.WebApi; //using umbraco.cms.businesslogic.language; +using umbraco.BusinessLogic.Actions; using umbraco.cms.businesslogic.web; namespace Umbraco.Web.WebServices @@ -30,7 +31,7 @@ namespace Umbraco.Web.WebServices ReasonPhrase = "Node Not Found." }); - if (!UmbracoUser.GetPermissions(node.Path).Contains(global::umbraco.BusinessLogic.Actions.ActionAssignDomain.Instance.Letter)) + if (UmbracoUser.GetPermissions(node.Path).Contains(ActionAssignDomain.Instance.Letter) == false) throw new HttpResponseException(new HttpResponseMessage(HttpStatusCode.Unauthorized) { Content = new StringContent("You do not have permission to assign domains on that node."),