Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Add ForbiddenResult #3443

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 122 additions & 0 deletions src/Microsoft.AspNet.Mvc.Core/ForbiddenResult.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.AspNet.Http.Authentication;
using Microsoft.AspNet.Mvc.Logging;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNet.Mvc
{
/// <summary>
/// An <see cref="ActionResult"/> that on execution issues a 403 forbidden response
/// if the authentication challenge is unacceptable.
/// </summary>
public class ForbiddenResult : ActionResult
{
/// <summary>
/// Initializes a new instance of <see cref="ForbiddenResult"/>.
/// </summary>
public ForbiddenResult()
: this(new string[] { })
{
}

/// <summary>
/// Initializes a new instance of <see cref="ForbiddenResult"/> with the
/// specified authentication scheme.
/// </summary>
/// <param name="authenticationScheme">The authentication scheme to challenge.</param>
public ForbiddenResult(string authenticationScheme)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add param docs

: this(new[] { authenticationScheme })
{
}

/// <summary>
/// Initializes a new instance of <see cref="ForbiddenResult"/> with the
/// specified authentication schemes.
/// </summary>
/// <param name="authenticationScheme">The authentication schemes to challenge.</param>
public ForbiddenResult(IList<string> authenticationSchemes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add param docs

: this(authenticationSchemes, properties: null)
{
}

/// <summary>
/// Initializes a new instance of <see cref="ForbiddenResult"/> with the
/// specified <paramref name="properties"/>.
/// </summary>
/// <param name="properties"><see cref="AuthenticationProperties"/> used to perform the authentication
/// challenge.</param>
public ForbiddenResult(AuthenticationProperties properties)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add param docs

: this(new string[] { }, properties)
{
}

/// <summary>
/// Initializes a new instance of <see cref="ForbiddenResult"/> with the
/// specified authentication scheme and <paramref name="properties"/>.
/// </summary>
/// <param name="authenticationScheme">The authentication schemes to challenge.</param>
/// <param name="properties"><see cref="AuthenticationProperties"/> used to perform the authentication
/// challenge.</param>
public ForbiddenResult(string authenticationScheme, AuthenticationProperties properties)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add param docs

: this(new[] { authenticationScheme }, properties)
{
}

/// <summary>
/// Initializes a new instance of <see cref="ForbiddenResult"/> with the
/// specified authentication schemes and <paramref name="properties"/>.
/// </summary>
/// <param name="authenticationScheme">The authentication scheme to challenge.</param>
/// <param name="properties"><see cref="AuthenticationProperties"/> used to perform the authentication
/// challenge.</param>
public ForbiddenResult(IList<string> authenticationSchemes, AuthenticationProperties properties)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add param docs

{
AuthenticationSchemes = authenticationSchemes;
Properties = properties;
}

/// <summary>
/// Gets or sets the authentication schemes that are challenged.
/// </summary>
public IList<string> AuthenticationSchemes { get; set; }

/// <summary>
/// Gets or sets the <see cref="AuthenticationProperties"/> used to perform the authentication challenge.
/// </summary>
public AuthenticationProperties Properties { get; set; }

/// <inheritdoc />
public override async Task ExecuteResultAsync(ActionContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}

var loggerFactory = context.HttpContext.RequestServices.GetRequiredService<ILoggerFactory>();
var logger = loggerFactory.CreateLogger<ForbiddenResult>();

var authentication = context.HttpContext.Authentication;

if (AuthenticationSchemes != null && AuthenticationSchemes.Count > 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AuthenticationSchemes?.Count > 0 #PranavPoints ®️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I care about unnecessary boxing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👊 - there's some unnecessary boxing for you.

{
for (var i = 0; i < AuthenticationSchemes.Count; i++)
{
await authentication.ForbidAsync(AuthenticationSchemes[i], Properties);
}
}
else
{
await authentication.ForbidAsync(Properties);
}

logger.ForbiddenResultExecuting(AuthenticationSchemes);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNet.Mvc.Logging
{
internal static class ForbiddenResultLoggerExtensions
{
private static readonly Action<ILogger, string[], Exception> _resultExecuting =
LoggerMessage.Define<string[]>(
LogLevel.Information,
eventId: 1,
formatString: $"Executing {nameof(ForbiddenResult)} with authentication schemes ({{Schemes}}).");

public static void ForbiddenResultExecuting(this ILogger logger, IList<string> authenticationSchemes)
{
_resultExecuting(logger, authenticationSchemes.ToArray(), null);
}
}
}
154 changes: 154 additions & 0 deletions test/Microsoft.AspNet.Mvc.Core.Test/ForbiddenResultTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Threading.Tasks;
using Microsoft.AspNet.Http;
using Microsoft.AspNet.Http.Authentication;
using Microsoft.AspNet.Mvc.Abstractions;
using Microsoft.AspNet.Mvc.Internal;
using Microsoft.AspNet.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Testing;
using Moq;
using Xunit;

namespace Microsoft.AspNet.Mvc
{
public class ForbiddenResultTest
{
[Fact]
public async Task ExecuteResultAsync_InvokesForbiddenAsyncOnAuthenticationManager()
{
// Arrange
var authenticationManager = new Mock<AuthenticationManager>();
authenticationManager
.Setup(c => c.ForbidAsync("", null))
.Returns(TaskCache.CompletedTask)
.Verifiable();
var httpContext = new Mock<HttpContext>();
httpContext.Setup(c => c.RequestServices).Returns(CreateServices());
httpContext.Setup(c => c.Authentication).Returns(authenticationManager.Object);
var result = new ForbiddenResult("", null);
var routeData = new RouteData();

var actionContext = new ActionContext(
httpContext.Object,
routeData,
new ActionDescriptor());

// Act
await result.ExecuteResultAsync(actionContext);

// Assert
authenticationManager.Verify();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to see a case with an empty list of schemes. I almost gave the feedback to oversimplify that loop, so it seems worth having.

[Fact]
public async Task ExecuteResultAsync_InvokesForbiddenAsyncOnAllConfiguredSchemes()
{
// Arrange
var authProperties = new AuthenticationProperties();
var authenticationManager = new Mock<AuthenticationManager>();
authenticationManager
.Setup(c => c.ForbidAsync("Scheme1", authProperties))
.Returns(TaskCache.CompletedTask)
.Verifiable();
authenticationManager
.Setup(c => c.ForbidAsync("Scheme2", authProperties))
.Returns(TaskCache.CompletedTask)
.Verifiable();
var httpContext = new Mock<HttpContext>();
httpContext.Setup(c => c.RequestServices).Returns(CreateServices());
httpContext.Setup(c => c.Authentication).Returns(authenticationManager.Object);
var result = new ForbiddenResult(new[] { "Scheme1", "Scheme2" }, authProperties);
var routeData = new RouteData();

var actionContext = new ActionContext(
httpContext.Object,
routeData,
new ActionDescriptor());

// Act
await result.ExecuteResultAsync(actionContext);

// Assert
authenticationManager.Verify();
}

public static TheoryData ExecuteResultAsync_InvokesForbiddenAsyncWithAuthPropertiesData =>
new TheoryData<AuthenticationProperties>
{
null,
new AuthenticationProperties()
};

[Theory]
[MemberData(nameof(ExecuteResultAsync_InvokesForbiddenAsyncWithAuthPropertiesData))]
public async Task ExecuteResultAsync_InvokesForbiddenAsyncWithAuthProperties(AuthenticationProperties expected)
{
// Arrange
var authenticationManager = new Mock<AuthenticationManager>();
authenticationManager
.Setup(c => c.ForbidAsync(expected))
.Returns(TaskCache.CompletedTask)
.Verifiable();
var httpContext = new Mock<HttpContext>();
httpContext.Setup(c => c.RequestServices).Returns(CreateServices());
httpContext.Setup(c => c.Authentication).Returns(authenticationManager.Object);
var result = new ForbiddenResult(expected);
var routeData = new RouteData();

var actionContext = new ActionContext(
httpContext.Object,
routeData,
new ActionDescriptor());

// Act
await result.ExecuteResultAsync(actionContext);

// Assert
authenticationManager.Verify();
}

[Theory]
[MemberData(nameof(ExecuteResultAsync_InvokesForbiddenAsyncWithAuthPropertiesData))]
public async Task ExecuteResultAsync_InvokesForbiddenAsyncWithAuthProperties_WhenAuthenticationSchemesIsEmpty(
AuthenticationProperties expected)
{
// Arrange
var authenticationManager = new Mock<AuthenticationManager>();
authenticationManager
.Setup(c => c.ForbidAsync(expected))
.Returns(TaskCache.CompletedTask)
.Verifiable();
var httpContext = new Mock<HttpContext>();
httpContext.Setup(c => c.RequestServices).Returns(CreateServices());
httpContext.Setup(c => c.Authentication).Returns(authenticationManager.Object);
var result = new ForbiddenResult(expected)
{
AuthenticationSchemes = new string[0]
};
var routeData = new RouteData();

var actionContext = new ActionContext(
httpContext.Object,
routeData,
new ActionDescriptor());

// Act
await result.ExecuteResultAsync(actionContext);

// Assert
authenticationManager.Verify();
}

private static IServiceProvider CreateServices()
{
return new ServiceCollection()
.AddInstance<ILoggerFactory>(NullLoggerFactory.Instance)
.BuildServiceProvider();
}
}
}