From e495faf83c2e927578092f64560d0f6179474bae Mon Sep 17 00:00:00 2001 From: Benjamin Carleski Date: Wed, 7 Aug 2019 10:31:08 -0700 Subject: [PATCH] Fix FIPS compliance, add FIPS tests (#5978) --- .../ControllerTesting/TestRunner.cs | 10 +- src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + .../AuthenticationControllerTests.cs | 139 ++++++++++++++++++ .../Web/Controllers/UsersControllerTests.cs | 97 +++++++++++- .../Models/Mapping/UserMapDefinition.cs | 2 +- 5 files changed, 243 insertions(+), 6 deletions(-) create mode 100644 src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs diff --git a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestRunner.cs b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestRunner.cs index 8c598281dd..9f9f933d72 100644 --- a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestRunner.cs +++ b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestRunner.cs @@ -25,19 +25,23 @@ namespace Umbraco.Tests.TestHelpers.ControllerTesting public async Task> Execute(string controllerName, string actionName, HttpMethod method, HttpContent content = null, MediaTypeWithQualityHeaderValue mediaTypeHeader = null, - bool assertOkResponse = true) + bool assertOkResponse = true, object routeDefaults = null, string url = null) { if (mediaTypeHeader == null) { mediaTypeHeader = new MediaTypeWithQualityHeaderValue("application/json"); } + if (routeDefaults == null) + { + routeDefaults = new { controller = controllerName, action = actionName, id = RouteParameter.Optional }; + } var startup = new TestStartup( configuration => { configuration.Routes.MapHttpRoute("Default", routeTemplate: "{controller}/{action}/{id}", - defaults: new { controller = controllerName, action = actionName, id = RouteParameter.Optional }); + defaults: routeDefaults); }, _controllerFactory); @@ -45,7 +49,7 @@ namespace Umbraco.Tests.TestHelpers.ControllerTesting { var request = new HttpRequestMessage { - RequestUri = new Uri("https://testserver/"), + RequestUri = new Uri("https://testserver/" + (url ?? "")), Method = method }; diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 717006b702..f788168ddc 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -243,6 +243,7 @@ + diff --git a/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs new file mode 100644 index 0000000000..3d264663b5 --- /dev/null +++ b/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs @@ -0,0 +1,139 @@ +using System; +using System.Collections.Concurrent; +using System.IO; +using System.Linq; +using System.Net.Http; +using System.Reflection; +using System.Security.Cryptography; +using System.Threading; +using System.Web; +using System.Web.Hosting; +using System.Web.Http; +using Moq; +using Newtonsoft.Json; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Cache; +using Umbraco.Core.Composing; +using Umbraco.Core.Configuration; +using Umbraco.Core.IO; +using Umbraco.Core.Logging; +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.Mappers; +using Umbraco.Core.Persistence.Querying; +using Umbraco.Core.Persistence.SqlSyntax; +using Umbraco.Core.Services; +using Umbraco.Tests.TestHelpers; +using Umbraco.Tests.TestHelpers.ControllerTesting; +using Umbraco.Tests.Testing; +using Umbraco.Web; +using Umbraco.Web.Editors; +using Umbraco.Web.Features; +using Umbraco.Web.Models.ContentEditing; +using IUser = Umbraco.Core.Models.Membership.IUser; + +namespace Umbraco.Tests.Web.Controllers +{ + [TestFixture] + [UmbracoTest(Database = UmbracoTestOptions.Database.None)] + public class AuthenticationControllerTests : TestWithDatabaseBase + { + protected override void ComposeApplication(bool withApplication) + { + base.ComposeApplication(withApplication); + //if (!withApplication) return; + + // replace the true IUserService implementation with a mock + // so that each test can configure the service to their liking + Composition.RegisterUnique(f => Mock.Of()); + + // kill the true IEntityService too + Composition.RegisterUnique(f => Mock.Of()); + + Composition.RegisterUnique(); + } + + + [Test] + public async System.Threading.Tasks.Task GetCurrentUser_Fips() + { + ApiController CtrlFactory(HttpRequestMessage message, IUmbracoContextAccessor umbracoContextAccessor, UmbracoHelper helper) + { + //setup some mocks + var userServiceMock = Mock.Get(Current.Services.UserService); + userServiceMock.Setup(service => service.GetUserById(It.IsAny())) + .Returns(() => null); + + if (Thread.GetDomain().GetData(".appPath") != null) + { + HttpContext.Current = new HttpContext(new SimpleWorkerRequest("", "", new StringWriter())); + } + else + { + var baseDir = IOHelper.MapPath("", false).TrimEnd(IOHelper.DirSepChar); + HttpContext.Current = new HttpContext(new SimpleWorkerRequest("/", baseDir, "", "", new StringWriter())); + } + IOHelper.ForceNotHosted = true; + var usersController = new AuthenticationController( + Factory.GetInstance(), + umbracoContextAccessor, + Factory.GetInstance(), + Factory.GetInstance(), + Factory.GetInstance(), + Factory.GetInstance(), + Factory.GetInstance(), + helper); + return usersController; + } + + Mock.Get(Current.SqlContext) + .Setup(x => x.Query()) + .Returns(new Query(Current.SqlContext)); + + var syntax = new SqlCeSyntaxProvider(); + + Mock.Get(Current.SqlContext) + .Setup(x => x.SqlSyntax) + .Returns(syntax); + + var mappers = new MapperCollection(new[] + { + new UserMapper(new Lazy(() => Current.SqlContext), new ConcurrentDictionary>()) + }); + + Mock.Get(Current.SqlContext) + .Setup(x => x.Mappers) + .Returns(mappers); + + // Testing what happens if the system were configured to only use FIPS-compliant algorithms + var typ = typeof(CryptoConfig); + var flds = typ.GetFields(BindingFlags.Static | BindingFlags.NonPublic); + var haveFld = flds.FirstOrDefault(f => f.Name == "s_haveFipsAlgorithmPolicy"); + var isFld = flds.FirstOrDefault(f => f.Name == "s_fipsAlgorithmPolicy"); + var originalFipsValue = CryptoConfig.AllowOnlyFipsAlgorithms; + + try + { + if (!originalFipsValue) + { + haveFld.SetValue(null, true); + isFld.SetValue(null, true); + } + + var runner = new TestRunner(CtrlFactory); + var response = await runner.Execute("Authentication", "GetCurrentUser", HttpMethod.Get); + + var obj = JsonConvert.DeserializeObject(response.Item2); + Assert.AreEqual(-1, obj.UserId); + } + finally + { + if (!originalFipsValue) + { + haveFld.SetValue(null, false); + isFld.SetValue(null, false); + } + } + } + } +} diff --git a/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs index a4c3078b8f..85dd303432 100644 --- a/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs @@ -4,6 +4,8 @@ using System.Collections.Generic; using System.Linq; using System.Net.Http; using System.Net.Http.Formatting; +using System.Reflection; +using System.Security.Cryptography; using System.Web.Http; using Moq; using Newtonsoft.Json; @@ -155,7 +157,7 @@ namespace Umbraco.Tests.Web.Controllers var runner = new TestRunner(CtrlFactory); var response = await runner.Execute("Users", "GetPagedUsers", HttpMethod.Get); - var obj = JsonConvert.DeserializeObject>(response.Item2); + var obj = JsonConvert.DeserializeObject>(response.Item2); Assert.AreEqual(0, obj.TotalItems); } @@ -190,9 +192,100 @@ namespace Umbraco.Tests.Web.Controllers var runner = new TestRunner(CtrlFactory); var response = await runner.Execute("Users", "GetPagedUsers", HttpMethod.Get); - var obj = JsonConvert.DeserializeObject>(response.Item2); + var obj = JsonConvert.DeserializeObject>(response.Item2); Assert.AreEqual(10, obj.TotalItems); Assert.AreEqual(10, obj.Items.Count()); } + + [Test] + public async System.Threading.Tasks.Task GetPagedUsers_Fips() + { + await RunFipsTest("GetPagedUsers", mock => + { + var users = MockedUser.CreateMulipleUsers(10); + long outVal = 10; + mock.Setup(service => service.GetAll( + It.IsAny(), It.IsAny(), out outVal, It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())) + .Returns(() => users); + }, response => + { + var obj = JsonConvert.DeserializeObject>(response.Item2); + Assert.AreEqual(10, obj.TotalItems); + Assert.AreEqual(10, obj.Items.Count()); + }); + } + + [Test] + public async System.Threading.Tasks.Task GetById_Fips() + { + const int mockUserId = 1234; + var user = MockedUser.CreateUser(); + + await RunFipsTest("GetById", mock => + { + mock.Setup(service => service.GetUserById(1234)) + .Returns((int i) => i == mockUserId ? user : null); + }, response => + { + var obj = JsonConvert.DeserializeObject(response.Item2); + Assert.AreEqual(user.Username, obj.Username); + Assert.AreEqual(user.Email, obj.Email); + }, new { controller = "Users", action = "GetById" }, $"Users/GetById/{mockUserId}"); + } + + + private async System.Threading.Tasks.Task RunFipsTest(string action, Action> userServiceSetup, + Action> verification, + object routeDefaults = null, string url = null) + { + ApiController CtrlFactory(HttpRequestMessage message, IUmbracoContextAccessor umbracoContextAccessor, UmbracoHelper helper) + { + //setup some mocks + var userServiceMock = Mock.Get(Current.Services.UserService); + userServiceSetup(userServiceMock); + + var usersController = new UsersController( + Factory.GetInstance(), + umbracoContextAccessor, + Factory.GetInstance(), + Factory.GetInstance(), + Factory.GetInstance(), + Factory.GetInstance(), + Factory.GetInstance(), + helper); + return usersController; + } + + // Testing what happens if the system were configured to only use FIPS-compliant algorithms + var typ = typeof(CryptoConfig); + var flds = typ.GetFields(BindingFlags.Static | BindingFlags.NonPublic); + var haveFld = flds.FirstOrDefault(f => f.Name == "s_haveFipsAlgorithmPolicy"); + var isFld = flds.FirstOrDefault(f => f.Name == "s_fipsAlgorithmPolicy"); + var originalFipsValue = CryptoConfig.AllowOnlyFipsAlgorithms; + + try + { + if (!originalFipsValue) + { + haveFld.SetValue(null, true); + isFld.SetValue(null, true); + } + + MockForGetPagedUsers(); + + var runner = new TestRunner(CtrlFactory); + var response = await runner.Execute("Users", action, HttpMethod.Get, routeDefaults: routeDefaults, url: url); + verification(response); + } + finally + { + if (!originalFipsValue) + { + haveFld.SetValue(null, false); + isFld.SetValue(null, false); + } + } + } } } diff --git a/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs b/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs index 3860d5d525..4acda1e552 100644 --- a/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs +++ b/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs @@ -301,7 +301,7 @@ namespace Umbraco.Web.Models.Mapping target.Avatars = source.GetUserAvatarUrls(_appCaches.RuntimeCache); target.Culture = source.GetUserCulture(_textService, _globalSettings).ToString(); target.Email = source.Email; - target.EmailHash = source.Email.ToLowerInvariant().Trim().ToMd5(); + target.EmailHash = source.Email.ToLowerInvariant().Trim().GenerateHash(); target.Id = source.Id; target.Key = source.Key; target.LastLoginDate = source.LastLoginDate == default ? null : (DateTime?) source.LastLoginDate;