Fixed bug whereby the presence of the Basic authorizer contributor wo…

…uld cause security negotiation even for resources not decorated with the required attribute.

The AuthorizerContributor now just attempts to set the User.Identity - but will never return Unauthorized. There's now an explict RequiresBasicAuthenticationAttribute(realm) attribute that will trigger the WWW-Authenticate negotation if an unauthorized request is received for a secured resource.

src/OpenRasta.Tests.Unit/OpenRasta.Tests.Unit.csproj

@@ -127,6 +127,7 @@
     <Compile Include="Authentication\DigestCredentialsReader_Specification.cs" />
     <Compile Include="Pipeline\PipelineRunner_Specification.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
+    <Compile Include="Security\RequiresBasicAuthenticationInterceptor_Specification.cs" />
     <Compile Include="Security\RequiresAuthenticationInterceptor_Specification.cs" />
     <Compile Include="Security\RequiresRoleInterceptor_Specification.cs" />
     <Compile Include="Text\Rfc2047Encoding_Specification.cs" />

src/OpenRasta.Tests.Unit/Pipeline/Contributors/BasicAuthorizer_Specification.cs

@@ -36,61 +36,5 @@ public void BasicAuthorizer_Succeeds_For_Valid_Credentials(string username, stri
             result.ShouldBe(PipelineContinuation.Continue);
             Context.User.Identity.Name.ShouldBe(username);
         }
-
-        [Test]
-        public void BasicAuthorizer_Returns_Authenticate_Header_For_Invalid_Credentials()
-        {
-            var mockAuthenticationProvider = new Mock<IAuthenticationProvider>();
-            mockAuthenticationProvider.Setup(ap => ap.GetByUsername(It.IsAny<string>())).Returns(new Credentials());
-            mockAuthenticationProvider.Setup(ap => ap.ValidatePassword(It.IsAny<Credentials>(), It.IsAny<string>())).Returns(false);
-            given_dependency(mockAuthenticationProvider.Object);
-            given_pipeline_contributor<BasicAuthorizerContributor>();
-            Context.Request.Headers.Add("Authorization", "Basic " + Convert.ToBase64String(Encoding.ASCII.GetBytes("username:password")));
-            var result = when_sending_notification<KnownStages.IHandlerSelection>();
-            result.ShouldBe(PipelineContinuation.RenderNow);
-            Context.OperationResult.ShouldBeOfType<OperationResult.Unauthorized>();
-            Context.User.ShouldBe(null);
-        }
-
-        [Test]
-        [TestCase("Basic THIS_IS_NOT_A_BASE64_ENCODED_CREDENTIAL")]
-        [TestCase("Invalid Header")]
-        public void BasicAuthorizer_Returns_Unauthorized_When_Header_Is_Invalid(string header)
-        {
-            var mockAuthenticationProvider = new Mock<IAuthenticationProvider>();
-            given_dependency(mockAuthenticationProvider.Object);
-            given_pipeline_contributor<BasicAuthorizerContributor>();
-
-            Context.Request.Headers.Add("Authorization", header);
-
-            var result = when_sending_notification<KnownStages.IHandlerSelection>();
-            result.ShouldBe(PipelineContinuation.RenderNow);
-            Context.OperationResult.ShouldBeOfType<OperationResult.Unauthorized>();
-        }
-
-        [Test]
-        public void BasicAuthorizer_Sends_WwwAuthenticate_For_Missing_Credentials()
-        {
-            var mockAuthenticationProvider = new Mock<IAuthenticationProvider>();
-            given_dependency(mockAuthenticationProvider.Object);
-            given_pipeline_contributor<BasicAuthorizerContributor>();
-
-            when_sending_notification<KnownStages.IHandlerSelection>();
-            when_sending_notification<KnownStages.IOperationResultInvocation>();
-            var header = String.Format("Basic realm=\"{0}\"", BasicAuthenticationRequiredHeader.DEFAULT_REALM);
-            Context.Response.Headers["WWW-Authenticate"].ShouldBe(header);
-        }
-
-        [Test]
-        public void BasicAuthorizer_Returns_NotAuthorized_For_Missing_Credentials()
-        {
-            var mockAuthenticationProvider = new Mock<IAuthenticationProvider>();
-            given_dependency(mockAuthenticationProvider.Object);
-            given_pipeline_contributor<BasicAuthorizerContributor>();
-
-            var result = when_sending_notification<KnownStages.IHandlerSelection>();
-            result.ShouldBe(PipelineContinuation.RenderNow);
-            Context.OperationResult.ShouldBeOfType<OperationResult.Unauthorized>();
-        }
     }
 }

src/OpenRasta.Tests.Unit/Security/RequiresAuthenticationInterceptor_Specification.cs

@@ -38,4 +38,5 @@ public void execution_is_allowed()
                 .ShouldBeTrue();
         }
     }
-}
+}
+

src/OpenRasta.Tests.Unit/Security/RequiresBasicAuthenticationInterceptor_Specification.cs

@@ -0,0 +1,28 @@
+using System;
+using System.Diagnostics;
+using System.Security.Principal;
+using Moq;
+using NUnit.Framework;
+using OpenRasta.Hosting.InMemory;
+using OpenRasta.OperationModel;
+using OpenRasta.Security;
+using OpenRasta.Testing;
+using OpenRasta.Web;
+
+namespace RequiresBasicAuthenticationInterceptor_Specification
+{
+    public class when_the_user_is_not_authenticated : context
+    {
+        [Test]
+        public void execution_is_denied()
+        {
+            var context = new InMemoryCommunicationContext();
+            const string REALM = "Test Realm";
+            var authenticationInterceptor = new RequiresBasicAuthenticationInterceptor(context, REALM);
+            authenticationInterceptor.BeforeExecute(new Mock<IOperation>().Object).ShouldBeFalse();
+            context.OperationResult.ShouldBeOfType<OperationResult.Unauthorized>();
+            var expectedHeader = String.Format("Basic realm=\"{0}\"", REALM);
+            context.Response.Headers["WWW-Authenticate"].ShouldBe(expectedHeader);
+        }
+    }
+}

src/OpenRasta/OpenRasta.csproj

@@ -329,6 +329,8 @@
     <Compile Include="Security\DigestHeader.cs" />
     <Compile Include="Security\IAuthenticationProvider.cs" />
     <Compile Include="Security\PrincipalAuthorizationAttribute.cs" />
+    <Compile Include="Security\RequiresBasicAuthenticationInterceptor.cs" />
+    <Compile Include="Security\RequiresBasicAuthenticationAttribute.cs" />
     <Compile Include="Security\RequiresAuthenticationAttribute.cs" />
     <Compile Include="Security\RequiresAuthenticationInterceptor.cs" />
     <Compile Include="Security\RequiresRoleAttribute.cs" />

src/OpenRasta/Pipeline/Contributors/BasicAuthorizerContributor.cs

@@ -26,8 +26,6 @@ namespace OpenRasta.Pipeline.Contributors
 {
     public class BasicAuthorizerContributor : IPipelineContributor
     {
-        public const string REALM = "Basic Authentication";
-
         private readonly IDependencyResolver _resolver;
         private IAuthenticationProvider _authentication;
 
@@ -43,72 +41,39 @@ public void Initialize(IPipeline pipelineRunner)
                 .After<KnownStages.IBegin>()
                 .And
                 .Before<KnownStages.IHandlerSelection>();
-
-            pipelineRunner.Notify(WriteCredentialRequest)
-                .After<KnownStages.IOperationResultInvocation>()
-                .And
-                .Before<KnownStages.IResponseCoding>();
         }
 
         public PipelineContinuation ReadCredentials(ICommunicationContext context)
         {
-            if (!_resolver.HasDependency(typeof(IAuthenticationProvider)))
-            {
-                return NotAuthorized(context);
-            }
-
-            _authentication = _resolver.Resolve<IAuthenticationProvider>();
-
-            try
+            if (_resolver.HasDependency(typeof(IAuthenticationProvider)))
             {
+                _authentication = _resolver.Resolve<IAuthenticationProvider>();
                 var header = ReadBasicAuthHeader(context);
-
-                if (header == null)
-                {
-                    return NotAuthorized(context);
-                }
-
-                var credentials = _authentication.GetByUsername(header.Username);
-
-                if (credentials == null)
+                if (header != null)
                 {
-                    return NotAuthorized(context);
-                }
+                    var credentials = _authentication.GetByUsername(header.Username);
 
-                if (!_authentication.ValidatePassword(credentials, header.Password))
-                {
-                    return NotAuthorized(context);
+                    if (_authentication.ValidatePassword(credentials, header.Password))
+                    {
+                        IIdentity id = new GenericIdentity(credentials.Username, "Basic");
+                        context.User = new GenericPrincipal(id, credentials.Roles);
+                    }
                 }
-                IIdentity id = new GenericIdentity(credentials.Username, "Basic");
-                context.User = new GenericPrincipal(id, credentials.Roles);
-                return PipelineContinuation.Continue;
-            } 
-            catch (ArgumentException ex)
-            {
-                return NotAuthorized(context);
             }
-        }
-
-        private static BasicAuthorizationHeader ReadBasicAuthHeader(ICommunicationContext context)
-        {
-            var header = context.Request.Headers["Authorization"];
-            return string.IsNullOrEmpty(header) ? null : BasicAuthorizationHeader.Parse(header);
-        }
 
-        private static PipelineContinuation NotAuthorized(ICommunicationContext context)
-        {
-            context.OperationResult = new OperationResult.Unauthorized();
-            return PipelineContinuation.RenderNow;
+            return PipelineContinuation.Continue;
         }
 
-        private static PipelineContinuation WriteCredentialRequest(ICommunicationContext context)
+        private static BasicAuthorizationHeader ReadBasicAuthHeader(ICommunicationContext context)
         {
-            if (context.OperationResult is OperationResult.Unauthorized)
+            try
+            {
+                var header = context.Request.Headers["Authorization"];
+                return string.IsNullOrEmpty(header) ? null : BasicAuthorizationHeader.Parse(header);
+            } catch (ArgumentException ex)
             {
-                var header = new BasicAuthenticationRequiredHeader().ServerResponseHeader;
-                context.Response.Headers["WWW-Authenticate"] = header;
+                return (null);
             }
-            return PipelineContinuation.Continue;
         }
     }
 }

src/OpenRasta/Security/BasicAuthorizationHeader.cs

@@ -30,13 +30,9 @@ private BasicAuthorizationHeader(string username, string password)
         public static BasicAuthorizationHeader Parse(string header)
         {
             var tokens = header.Split(' ');
-            if (tokens.Length != 2)
+            if (tokens.Length != 2 || tokens[0] != "Basic")
             {
-                throw (new ArgumentException("Supplied header is not in the format Basic {base64-encoded credential pair}", "header"));
-            }
-            if (tokens[0] != "Basic")
-            {
-                throw (new ArgumentException("Supplied header is not an HTTP Basic authorization header", header));
+                return (null);
             }
             try
             {
@@ -46,7 +42,7 @@ public static BasicAuthorizationHeader Parse(string header)
                 return (new BasicAuthorizationHeader(credentials[0], credentials[1]));
             } catch (FormatException ex)
             {
-                throw (new ArgumentException("Supplied header doesn't contain valid base64 encoded credentials", ex));
+                return (null);
             }
         }
     }

src/OpenRasta/Security/RequiresAuthenticationInterceptor.cs

@@ -17,11 +17,18 @@ public override bool BeforeExecute(IOperation operation)
         {
             if (_context.User == null || _context.User.Identity == null || !_context.User.Identity.IsAuthenticated)
             {
-                _context.OperationResult = new OperationResult.Unauthorized();
+                DenyAuthorization(_context);
                 return false;
             }
 
             return true;
         }
+
+        protected virtual void DenyAuthorization(ICommunicationContext context)
+        {
+            _context.OperationResult = new OperationResult.Unauthorized();
+        }
     }
+
+
 }

src/OpenRasta/Security/RequiresBasicAuthenticationAttribute.cs

@@ -0,0 +1,57 @@
+#region License
+/* Authors:
+ *      Sebastien Lambla (seb@serialseb.com)
+ * Copyright:
+ *      (C) 2007-2009 Caffeine IT & naughtyProd Ltd (http://www.caffeine-it.com)
+ * License:
+ *      This file is distributed under the terms of the MIT License found at the end of this file.
+ */
+#endregion
+
+using System;
+using System.Collections.Generic;
+using OpenRasta.DI;
+using OpenRasta.OperationModel;
+using OpenRasta.OperationModel.Interceptors;
+using OpenRasta.Web;
+
+namespace OpenRasta.Security
+{
+    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
+    public class RequiresBasicAuthenticationAttribute : InterceptorProviderAttribute
+    {
+        private readonly string realm;
+
+        public RequiresBasicAuthenticationAttribute(string realm)
+        {
+            this.realm = realm;
+        }
+
+        public override IEnumerable<IOperationInterceptor> GetInterceptors(IOperation operation)
+        {
+            return new[]
+            {
+                new RequiresBasicAuthenticationInterceptor(DependencyManager.GetService<ICommunicationContext>(),realm)
+            };
+        }
+    }
+}
+
+#region Full license
+// Permission is hereby granted, free of charge, to any person obtaining
+// a copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to
+// permit persons to whom the Software is furnished to do so, subject to
+// the following conditions:
+// The above copyright notice and this permission notice shall be
+// included in all copies or substantial portions of the Software.
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+#endregion

src/OpenRasta/Security/RequiresBasicAuthenticationInterceptor.cs

@@ -0,0 +1,25 @@
+using OpenRasta.OperationModel;
+using OpenRasta.OperationModel.Interceptors;
+using OpenRasta.Pipeline;
+using OpenRasta.Web;
+
+namespace OpenRasta.Security
+{
+    public class RequiresBasicAuthenticationInterceptor : RequiresAuthenticationInterceptor
+    {
+        private readonly string realm;
+
+        public RequiresBasicAuthenticationInterceptor(ICommunicationContext context, string realm)
+            : base(context)
+        {
+            this.realm = realm;
+        }
+
+        protected override void DenyAuthorization(ICommunicationContext context)
+        {
+            base.DenyAuthorization(context);
+            var header = new BasicAuthenticationRequiredHeader(realm).ServerResponseHeader;
+            context.Response.Headers["WWW-Authenticate"] = header;
+        }
+    }
+}