Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ControllerTypeExtensions should look for ControllerBase type #16187

Merged
merged 4 commits into from
May 30, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace OrchardCore.Demo.Controllers
[Route("api/demo")]
[Authorize(AuthenticationSchemes = "Api"), IgnoreAntiforgeryToken, AllowAnonymous]
[ApiController]
public class ContentApiController : Controller
public class ContentApiController : ControllerBase
{
private readonly IAuthorizationService _authorizationService;
private readonly IContentManager _contentManager;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ namespace OrchardCore.Queries.Controllers
[Route("api/queries")]
[ApiController]
[Authorize(AuthenticationSchemes = "Api"), IgnoreAntiforgeryToken, AllowAnonymous]
public class ApiController : Controller
public class QueryApiController : ControllerBase
{
private readonly IAuthorizationService _authorizationService;
private readonly IQueryManager _queryManager;

public ApiController(
public QueryApiController(
IAuthorizationService authorizationService,
IQueryManager queryManager
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ namespace OrchardCore.Search.Elasticsearch
[Route("api/elasticsearch")]
[ApiController]
[Authorize(AuthenticationSchemes = "Api"), IgnoreAntiforgeryToken, AllowAnonymous]
public class ApiController : Controller
public class ElasticsearchApiController : ControllerBase
{
private readonly IAuthorizationService _authorizationService;
private readonly ElasticQuerySource _elasticQuerySource;

public ApiController(
public ElasticsearchApiController(
IAuthorizationService authorizationService,
ElasticQuerySource elasticQuerySource)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ namespace OrchardCore.Search.Lucene.Controllers
[Route("api/lucene")]
[ApiController]
[Authorize(AuthenticationSchemes = "Api"), IgnoreAntiforgeryToken, AllowAnonymous]
public class ApiController : Controller
public class LuceneApiController : ControllerBase
{
private readonly IAuthorizationService _authorizationService;
private readonly LuceneQuerySource _luceneQuerySource;

public ApiController(
public LuceneApiController(
IAuthorizationService authorizationService,
LuceneQuerySource luceneQuerySource)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace OrchardCore.Tenants.Controllers
[Route("api/tenants")]
[ApiController]
[Authorize(AuthenticationSchemes = "Api"), IgnoreAntiforgeryToken, AllowAnonymous]
public class ApiController : Controller
public class TenantApiController : ControllerBase
{
private readonly IShellHost _shellHost;
private readonly ShellSettings _currentShellSettings;
Expand All @@ -50,7 +50,7 @@ public class ApiController : Controller
protected readonly IStringLocalizer S;
private readonly ILogger _logger;

public ApiController(
public TenantApiController(
IShellHost shellHost,
ShellSettings currentShellSettings,
IShellRemovalManager shellRemovalManager,
Expand All @@ -64,8 +64,8 @@ public ApiController(
IOptions<TenantsOptions> tenantsOptions,
IEnumerable<DatabaseProvider> databaseProviders,
ITenantValidator tenantValidator,
IStringLocalizer<ApiController> stringLocalizer,
ILogger<ApiController> logger)
IStringLocalizer<TenantApiController> stringLocalizer,
ILogger<TenantApiController> logger)
{
_shellHost = shellHost;
_currentShellSettings = currentShellSettings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace Microsoft.AspNetCore.Mvc
{
public static class ControllerExtensions
public static class ControllerBaseExtensions
{
/// <summary>
/// Returns the proper actionresult for unauthorized or unauthenticated users.
Expand All @@ -12,7 +12,7 @@ public static class ControllerExtensions
/// </summary>
/// <param name="controller"></param>
/// <returns>The proper actionresult based upon if the user is authenticated.</returns>
public static ActionResult ChallengeOrForbid(this Controller controller)
public static ActionResult ChallengeOrForbid(this ControllerBase controller)
=> controller.User?.Identity?.IsAuthenticated ?? false ? (ActionResult)controller.Forbid() : controller.Challenge();

/// <summary>
Expand All @@ -25,15 +25,15 @@ public static ActionResult ChallengeOrForbid(this Controller controller)
/// <param name="controller"></param>
/// <param name="authenticationSchemes">The authentication schemes to challenge.</param>
/// <returns>The proper actionresult based upon if the user is authenticated.</returns>
public static ActionResult ChallengeOrForbid(this Controller controller, params string[] authenticationSchemes)
public static ActionResult ChallengeOrForbid(this ControllerBase controller, params string[] authenticationSchemes)
Copy link
Contributor Author

@Skrypt Skrypt Jun 5, 2024

Choose a reason for hiding this comment

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

@sebastienros There is also this which needed to also be extended on top of the ControllerBase just because we need it for Api Controllers too.

=> controller.User?.Identity?.IsAuthenticated ?? false ? (ActionResult)controller.Forbid(authenticationSchemes) : controller.Challenge(authenticationSchemes);

/// <summary>
/// Creates <see cref="ObjectResult"/> that produces a <see cref="HttpStatusCode.InternalServerError"/> response.
/// </summary>
/// <param name="controller">The <see cref="Controller"/>.</param>
/// <param name="value">An optional value to set on <see cref="ObjectResult"/>.</param>
public static ActionResult InternalServerError(this Controller controller, object value = null)
public static ActionResult InternalServerError(this ControllerBase controller, object value = null)
=> controller.StatusCode((int)HttpStatusCode.InternalServerError, value);

/// <summary>
Expand All @@ -42,7 +42,7 @@ public static ActionResult InternalServerError(this Controller controller, objec
/// <param name="controller"></param>
/// <param name="localUrl">The local URL to redirect to.</param>
/// <param name="escapeUrl">Whether to escape the url.</param>
public static ActionResult LocalRedirect(this Controller controller, string localUrl, bool escapeUrl)
public static ActionResult LocalRedirect(this ControllerBase controller, string localUrl, bool escapeUrl)
{
if (!escapeUrl)
{
Expand All @@ -59,7 +59,7 @@ public static ActionResult LocalRedirect(this Controller controller, string loca
/// <param name="controller"></param>
/// <param name="url">The URL to redirect to.</param>
/// <param name="escapeUrl">Whether to escape the url.</param>
public static ActionResult Redirect(this Controller controller, string url, bool escapeUrl)
public static ActionResult Redirect(this ControllerBase controller, string url, bool escapeUrl)
{
if (!escapeUrl)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ public static class ControllerTypeExtensions
{
public static string ControllerName(this Type controllerType)
{
if (!typeof(Controller).IsAssignableFrom(controllerType))
if (!typeof(ControllerBase).IsAssignableFrom(controllerType))
{
throw new ArgumentException($"The specified type must inherit from '{nameof(Controller)}'", nameof(controllerType));
throw new ArgumentException($"The specified type must inherit from '{nameof(ControllerBase)}'", nameof(controllerType));
}

return controllerType.Name.EndsWith(nameof(Controller), StringComparison.OrdinalIgnoreCase)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public async Task ShouldGetNewTenantSetupToken_WhenThePreviousTokenExpired()
Assert.NotEqual(token1, token2);
}

private ApiController CreateController()
private TenantApiController CreateController()
{
var defaultShellSettings = new ShellSettings()
.AsDefaultShell()
Expand Down Expand Up @@ -159,7 +159,7 @@ private ApiController CreateController()
Mock.Of<IDbConnectionValidator>(),
stringLocalizerMock.Object);

return new ApiController(
return new TenantApiController(
shellHostMock.Object,
defaultShellSettings,
Mock.Of<IShellRemovalManager>(),
Expand All @@ -173,8 +173,8 @@ private ApiController CreateController()
Options.Create(new TenantsOptions()),
[],
tenantValidator,
Mock.Of<IStringLocalizer<ApiController>>(),
Mock.Of<ILogger<ApiController>>())
Mock.Of<IStringLocalizer<TenantApiController>>(),
Mock.Of<ILogger<TenantApiController>>())
{
ControllerContext = new ControllerContext { HttpContext = CreateHttpContext() }
};
Expand Down