From 688065a02bf3966987a2ef79976976c69051046c Mon Sep 17 00:00:00 2001 From: Dmitry Harnitski Date: Thu, 7 May 2015 10:59:10 -0400 Subject: [PATCH 1/6] upgrade xUnit to 2.0.0 in order to support [Theory] add HTTP header Authentication for secure token stored in config file --- .../AccessTokenAuthoriser.cs | 74 ++++++++++ .../Contracts/ICheckRequests.cs | 6 +- .../Domain/PackageInstallationSettings.cs | 3 + .../HttpRequestAuthoriser.cs | 8 ++ .../Sitecore.Ship.Core.csproj | 1 + .../PackageInstallationConfiguration.cs | 4 + ...ackageInstallationConfigurationProvider.cs | 3 +- .../Web/HttpRequestChecker.cs | 8 +- ...itecore.Ship.AspNet.Web.Accept.Test.csproj | 6 +- .../packages.config | 2 +- .../App.config | 2 +- .../PackageInstallationConfigurationTests.cs | 6 + ...ecore.Ship.Infrastructure.Intg.Test.csproj | 19 ++- .../packages.config | 8 +- .../Sitecore.Ship.AspNet.Test.csproj | 19 ++- .../Sitecore.Ship.AspNet.Test/packages.config | 8 +- .../AccessTokenAuthoriserBehaviour.cs | 128 ++++++++++++++++++ ...s.cs => HttpRequestAuthoriserBehaviour.cs} | 0 .../AccessTokenAuthoriserLoggingBehaviour.cs | 80 +++++++++++ .../HttpRequestAuthoriserLoggingBehaviour.cs | 14 -- .../Sitecore.Ship.Core.Test.csproj | 24 ++-- .../Sitecore.Ship.Core.Test/packages.config | 8 +- .../Sitecore.Ship.Test.csproj | 19 ++- .../Sitecore.Ship.Test/packages.config | 8 +- 24 files changed, 402 insertions(+), 56 deletions(-) create mode 100644 src/Sitecore.Ship.Core/AccessTokenAuthoriser.cs create mode 100644 tests/unit-test/Sitecore.Ship.Core.Test/HttpRequestAuthoriser/AccessTokenAuthoriserBehaviour.cs rename tests/unit-test/Sitecore.Ship.Core.Test/HttpRequestAuthoriser/{HttpRequestAuthoriserTests.cs => HttpRequestAuthoriserBehaviour.cs} (100%) create mode 100644 tests/unit-test/Sitecore.Ship.Core.Test/HttpRequestAuthoriser/Logging/AccessTokenAuthoriserLoggingBehaviour.cs diff --git a/src/Sitecore.Ship.Core/AccessTokenAuthoriser.cs b/src/Sitecore.Ship.Core/AccessTokenAuthoriser.cs new file mode 100644 index 0000000..0882d14 --- /dev/null +++ b/src/Sitecore.Ship.Core/AccessTokenAuthoriser.cs @@ -0,0 +1,74 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Text; +using Sitecore.Ship.Core.Contracts; +using Sitecore.Ship.Core.Domain; + +namespace Sitecore.Ship.Core +{ + public class AccessTokenAuthoriser : IAuthoriser + { + private readonly ICheckRequests _checkRequests; + private readonly PackageInstallationSettings _packageInstallationSettings; + private readonly ILog _logger; + + private const string AuthorizationHeaderKey = "Authorization"; + + public AccessTokenAuthoriser(ICheckRequests checkRequests, + PackageInstallationSettings packageInstallationSettings, ILog logger) + { + if (checkRequests == null) + throw new ArgumentNullException("checkRequests"); + if (packageInstallationSettings == null) + throw new ArgumentNullException("packageInstallationSettings"); + + _checkRequests = checkRequests; + _packageInstallationSettings = packageInstallationSettings; + _logger = logger; + } + + public bool IsAllowed() + { + if (!_packageInstallationSettings.TokenRequired) + return true; + + if (_checkRequests.Headers == null || !_checkRequests.Headers.AllKeys.Contains(AuthorizationHeaderKey)) + { + LogAccessDenial(AuthorizationHeaderKey + " header missing"); + return false; + } + + var allAuthorizationHeadervalues = _checkRequests.Headers.GetValues(AuthorizationHeaderKey); + +// ReSharper disable once AssignNullToNotNullAttribute + string bearerHeader = allAuthorizationHeadervalues.FirstOrDefault(authorizationHeader => authorizationHeader.StartsWith("bearer", StringComparison.InvariantCultureIgnoreCase)); + + if (bearerHeader == null) + { + LogAccessDenial("Bearer authentication scheme required"); + return false; + } + + var token = bearerHeader.Substring(6).Trim(); + + var success = token == _packageInstallationSettings.AccessToken; + + if (!success) + { + LogAccessDenial("Wrong Authentication Token"); + } + + return success; + } + + private void LogAccessDenial(string diagnostic) + { + if (!_packageInstallationSettings.MuteAuthorisationFailureLogging) + { + _logger.Write(string.Format("Sitecore.Ship access denied: {0}", diagnostic)); + } + } + } +} diff --git a/src/Sitecore.Ship.Core/Contracts/ICheckRequests.cs b/src/Sitecore.Ship.Core/Contracts/ICheckRequests.cs index e5742f1..b338f88 100644 --- a/src/Sitecore.Ship.Core/Contracts/ICheckRequests.cs +++ b/src/Sitecore.Ship.Core/Contracts/ICheckRequests.cs @@ -1,9 +1,13 @@ -namespace Sitecore.Ship.Core.Contracts +using System.Collections.Specialized; + +namespace Sitecore.Ship.Core.Contracts { public interface ICheckRequests { bool IsLocal { get; } string UserHostAddress { get; } + + NameValueCollection Headers { get; } } } \ No newline at end of file diff --git a/src/Sitecore.Ship.Core/Domain/PackageInstallationSettings.cs b/src/Sitecore.Ship.Core/Domain/PackageInstallationSettings.cs index f2bcc33..e774d12 100644 --- a/src/Sitecore.Ship.Core/Domain/PackageInstallationSettings.cs +++ b/src/Sitecore.Ship.Core/Domain/PackageInstallationSettings.cs @@ -22,5 +22,8 @@ public PackageInstallationSettings() public bool MuteAuthorisationFailureLogging { get; set; } public bool HasAddressWhitelist { get { return AddressWhitelist.Count > 0; } } + + public string AccessToken { get; set; } + public bool TokenRequired { get { return !string.IsNullOrWhiteSpace(AccessToken); } } } } \ No newline at end of file diff --git a/src/Sitecore.Ship.Core/HttpRequestAuthoriser.cs b/src/Sitecore.Ship.Core/HttpRequestAuthoriser.cs index 44121a6..81cea3b 100644 --- a/src/Sitecore.Ship.Core/HttpRequestAuthoriser.cs +++ b/src/Sitecore.Ship.Core/HttpRequestAuthoriser.cs @@ -48,6 +48,14 @@ public bool IsAllowed() } } + if (_packageInstallationSettings.TokenRequired) + { + var tokenAuthorizer = new AccessTokenAuthoriser(_checkRequests, _packageInstallationSettings, _logger); + var authorized = tokenAuthorizer.IsAllowed(); + if (!authorized) + return false; + } + return true; } diff --git a/src/Sitecore.Ship.Core/Sitecore.Ship.Core.csproj b/src/Sitecore.Ship.Core/Sitecore.Ship.Core.csproj index 6b86010..39fdc68 100644 --- a/src/Sitecore.Ship.Core/Sitecore.Ship.Core.csproj +++ b/src/Sitecore.Ship.Core/Sitecore.Ship.Core.csproj @@ -48,6 +48,7 @@ Properties\CommonVersionInfo.cs + diff --git a/src/Sitecore.Ship.Infrastructure/Configuration/PackageInstallationConfiguration.cs b/src/Sitecore.Ship.Infrastructure/Configuration/PackageInstallationConfiguration.cs index ffb5f01..352624d 100644 --- a/src/Sitecore.Ship.Infrastructure/Configuration/PackageInstallationConfiguration.cs +++ b/src/Sitecore.Ship.Infrastructure/Configuration/PackageInstallationConfiguration.cs @@ -11,6 +11,7 @@ public class PackageInstallationConfiguration : ConfigurationSection const string RecordInstallationHistoryKey = "recordInstallationHistory"; const string WhitelistElementName = "Whitelist"; const string MuteAuthorisationFailureLoggingKey = "muteAuthorisationFailureLogging"; + const string AccessTokenKey = "accessToken"; public static PackageInstallationConfiguration GetConfiguration() { @@ -38,6 +39,9 @@ public WhitelistCollection Whitelist [ConfigurationProperty(MuteAuthorisationFailureLoggingKey, IsRequired = false, DefaultValue = false)] public bool MuteAuthorisationFailureLogging { get { return (bool)this[MuteAuthorisationFailureLoggingKey]; } } + + [ConfigurationProperty(AccessTokenKey, IsRequired = false, DefaultValue = "")] + public string AccessToken { get { return (string)this[AccessTokenKey]; } } } [ConfigurationCollection(typeof(WhitelistElement))] diff --git a/src/Sitecore.Ship.Infrastructure/Configuration/PackageInstallationConfigurationProvider.cs b/src/Sitecore.Ship.Infrastructure/Configuration/PackageInstallationConfigurationProvider.cs index ccd94a8..c58a57b 100644 --- a/src/Sitecore.Ship.Infrastructure/Configuration/PackageInstallationConfigurationProvider.cs +++ b/src/Sitecore.Ship.Infrastructure/Configuration/PackageInstallationConfigurationProvider.cs @@ -15,7 +15,8 @@ public PackageInstallationConfigurationProvider() AllowRemoteAccess = config.AllowRemoteAccess, AllowPackageStreaming = config.AllowPackageStreaming, RecordInstallationHistory = config.RecordInstallationHistory, - MuteAuthorisationFailureLogging = config.MuteAuthorisationFailureLogging + MuteAuthorisationFailureLogging = config.MuteAuthorisationFailureLogging, + AccessToken = config.AccessToken }; if (config.Whitelist.Count > 0) diff --git a/src/Sitecore.Ship.Infrastructure/Web/HttpRequestChecker.cs b/src/Sitecore.Ship.Infrastructure/Web/HttpRequestChecker.cs index 87937fc..eea5f1f 100644 --- a/src/Sitecore.Ship.Infrastructure/Web/HttpRequestChecker.cs +++ b/src/Sitecore.Ship.Infrastructure/Web/HttpRequestChecker.cs @@ -1,4 +1,5 @@ -using System.Web; +using System.Collections.Specialized; +using System.Web; using Sitecore.Ship.Core.Contracts; namespace Sitecore.Ship.Infrastructure.Web @@ -14,5 +15,10 @@ public string UserHostAddress { get { return HttpContext.Current.Request.UserHostAddress; } } + + public NameValueCollection Headers + { + get { return HttpContext.Current.Request.Headers; } + } } } \ No newline at end of file diff --git a/tests/acceptance-test/Sitecore.Ship.AspNet.Web.Accept.Test/Sitecore.Ship.AspNet.Web.Accept.Test.csproj b/tests/acceptance-test/Sitecore.Ship.AspNet.Web.Accept.Test/Sitecore.Ship.AspNet.Web.Accept.Test.csproj index 0061534..c3b5b2e 100644 --- a/tests/acceptance-test/Sitecore.Ship.AspNet.Web.Accept.Test/Sitecore.Ship.AspNet.Web.Accept.Test.csproj +++ b/tests/acceptance-test/Sitecore.Ship.AspNet.Web.Accept.Test/Sitecore.Ship.AspNet.Web.Accept.Test.csproj @@ -1,6 +1,6 @@  - + Debug @@ -22,7 +22,7 @@ ..\..\..\ true - e34a2b6a + be5dc727 @@ -115,7 +115,7 @@ This project references NuGet package(s) that are missing on this computer. Enable NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. - + + + + diff --git a/src/Sitecore.Ship.AspNet/BaseHttpHandler.cs b/src/Sitecore.Ship.AspNet/BaseHttpHandler.cs index 0401913..06d553c 100644 --- a/src/Sitecore.Ship.AspNet/BaseHttpHandler.cs +++ b/src/Sitecore.Ship.AspNet/BaseHttpHandler.cs @@ -37,6 +37,7 @@ public void ProcessRequest(HttpContext context) if (!_authoriser.IsAllowed()) { context.Response.StatusCode = (int) HttpStatusCode.Unauthorized; + return; } context.Items.Add(StartTime, DateTime.UtcNow); diff --git a/src/Sitecore.Ship.Core/AccessTokenAuthoriser.cs b/src/Sitecore.Ship.Core/AccessTokenAuthoriser.cs index 0882d14..bd873f5 100644 --- a/src/Sitecore.Ship.Core/AccessTokenAuthoriser.cs +++ b/src/Sitecore.Ship.Core/AccessTokenAuthoriser.cs @@ -8,7 +8,7 @@ namespace Sitecore.Ship.Core { - public class AccessTokenAuthoriser : IAuthoriser + public class AccessTokenAuthoriser// : IAuthoriser - do not inherit from Interface to make sure that IoC dependencies resolution is not affected { private readonly ICheckRequests _checkRequests; private readonly PackageInstallationSettings _packageInstallationSettings; diff --git a/tests/unit-test/Sitecore.Ship.Test/AuthorizationTests.cs b/tests/unit-test/Sitecore.Ship.Test/AuthorizationTests.cs new file mode 100644 index 0000000..25c003b --- /dev/null +++ b/tests/unit-test/Sitecore.Ship.Test/AuthorizationTests.cs @@ -0,0 +1,57 @@ +using Moq; +using Nancy.Testing; +using Sitecore.Ship.Core.Contracts; +using Xunit; + +namespace Sitecore.Ship.Test +{ + public class AuthorizationTests + { + private readonly Mock _mockAuthoriser; + private readonly Browser _browser; + private readonly Mock _mockDependency; + + public AuthorizationTests() + { + _mockAuthoriser = new Mock(); + _mockDependency = new Mock(); + + var bootstrapper = new ConfigurableBootstrapper(with => + { + //todo: mock module and Verify if EmptyMethod() called instead of using Dependency + with.Module(); + //dependency is used for verification of method execution + with.Dependency(_mockAuthoriser.Object); + with.Dependency(_mockDependency.Object); + }); + + _browser = new Browser(bootstrapper); + } + + [Fact] + public void method_executed_when_request_authorized() + { + //Arrange + _mockAuthoriser.Setup(x => x.IsAllowed()).Returns(true); + + //Act + _browser.Get("/services/test/empty"); + + //Assert + _mockDependency.Verify(t => t.DoSomething(), Times.Once); + } + + [Fact] + public void method_not_executed_when_request_not_authorized() + { + //Arrange + _mockAuthoriser.Setup(x => x.IsAllowed()).Returns(false); + + //Act + _browser.Get("/services/test/empty"); + + //Assert + _mockDependency.Verify(t => t.DoSomething(), Times.Never); + } + } +} \ No newline at end of file diff --git a/tests/unit-test/Sitecore.Ship.Test/Sitecore.Ship.Test.csproj b/tests/unit-test/Sitecore.Ship.Test/Sitecore.Ship.Test.csproj index 0bc4c10..3b4040d 100644 --- a/tests/unit-test/Sitecore.Ship.Test/Sitecore.Ship.Test.csproj +++ b/tests/unit-test/Sitecore.Ship.Test/Sitecore.Ship.Test.csproj @@ -79,6 +79,7 @@ + @@ -86,6 +87,7 @@ + diff --git a/tests/unit-test/Sitecore.Ship.Test/TestModule.cs b/tests/unit-test/Sitecore.Ship.Test/TestModule.cs new file mode 100644 index 0000000..e7e34bb --- /dev/null +++ b/tests/unit-test/Sitecore.Ship.Test/TestModule.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Nancy; +using Sitecore.Ship.Core.Contracts; + +namespace Sitecore.Ship.Test +{ + public interface IDepencency + { + void DoSomething(); + } + + public class TestModule : ShipBaseModule + { + private readonly IDepencency _dependency; + + public TestModule(IAuthoriser authoriser, IDepencency dependency) + : base(authoriser, "/services/test") + { + _dependency = dependency; + Get["/empty"] = EmptyMethod; + } + + public virtual dynamic EmptyMethod(dynamic o) + { + _dependency.DoSomething(); + + return Response.AsJson(DateTime.Now); + } + } +} From 1093ba3f1ad0d20cd497af73e6a44e6c96e04a96 Mon Sep 17 00:00:00 2001 From: Dmitry Harnitski Date: Fri, 8 May 2015 13:31:55 -0400 Subject: [PATCH 4/6] update documentation for token Authorization --- README.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6232d26..ff4cde4 100644 --- a/README.md +++ b/README.md @@ -185,9 +185,9 @@ Issue a GET request to `/services/about` Shown below is a fully specified configuration section for Sitecore.Ship: - + - + @@ -201,6 +201,7 @@ Default configuration: * allowPackageStreaming = false * recordInstallationHistory = false * IP address whitelisting is disabled if no elements are specified below the `` element or if the element is omited. +* access token is not specified and service can be used without `Authorization` HTTP header When `recordInstallationHistory` has been set to true packages should follow the naming conventions set out below: @@ -219,6 +220,15 @@ For example: 02-HomePage.zip +### Tocken based security + +Service can be protected by secure access token. Every request without correct access token in its header will get `401 Unauthorized` HTTP error code. + +Token transmition: + + `curl -i -H "Authorization: Bearer some-key" https://mysite/services/about` + + ### Tools POSTMAN is a powerful HTTP client that runs as a Chrome browser extension and greatly helps with testing test REST web services. Find out more From 32e7464f2e76a53ce0038962e8083e62f6c1ef22 Mon Sep 17 00:00:00 2001 From: Dmitry Harnitski Date: Fri, 8 May 2015 13:40:17 -0400 Subject: [PATCH 5/6] fix for documentation markup --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ff4cde4..595f4da 100644 --- a/README.md +++ b/README.md @@ -226,7 +226,7 @@ Service can be protected by secure access token. Every request without correct a Token transmition: - `curl -i -H "Authorization: Bearer some-key" https://mysite/services/about` + curl -i -H "Authorization: Bearer some-key" https://mysite/services/about ### Tools From 7967676c823f58f285572f2c91faaf94e15b3cd8 Mon Sep 17 00:00:00 2001 From: Dmitry Harnitski Date: Fri, 8 May 2015 14:01:15 -0400 Subject: [PATCH 6/6] documentation - add message regarding to importance of HTTPS --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 595f4da..3ecdfd6 100644 --- a/README.md +++ b/README.md @@ -220,10 +220,12 @@ For example: 02-HomePage.zip -### Tocken based security +### Token based security Service can be protected by secure access token. Every request without correct access token in its header will get `401 Unauthorized` HTTP error code. +**Important:** Token cannot protect your service without transport level security. Do not forget to use `https` when you call APIs. + Token transmition: curl -i -H "Authorization: Bearer some-key" https://mysite/services/about