Skip to content

Commit

Permalink
Fix Anon Endpoints (#215)
Browse files Browse the repository at this point in the history
* Remove fallback policy.

* Add authorization headers.

* Return 401 when authentication fails.

* Correctly annotate endpoints that return 403.

* Require authentication to create a run.

* Update openapi.json.

* Require mod status to create a category.

* Don't use moderator policy.

* Remove old comments.

* Fix faulty LB test.

* Don't explicitly fail auth.
  • Loading branch information
TheTedder authored Jul 17, 2024
1 parent 8977552 commit be5d650
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 80 deletions.
8 changes: 2 additions & 6 deletions LeaderboardBackend.Test/Leaderboards.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,8 @@ await _apiClient.Post<LeaderboardViewModel>(
}

CreateLeaderboardRequest reqForInexistentBoard = _createBoardReqFaker.Generate();

RequestFailureException e = Assert.ThrowsAsync<RequestFailureException>(
() => _apiClient.Get<string>($"/api/leaderboards/{reqForInexistentBoard.Slug}", new())
)!;

e.Response.StatusCode.Should().Be(HttpStatusCode.NotFound);
Func<Task<string>> act = async () => await _apiClient.Get<string>($"/api/leaderboards/{reqForInexistentBoard.Slug}", new());
await act.Should().ThrowAsync<RequestFailureException>().Where(e => e.Response.StatusCode == HttpStatusCode.NotFound);
}

private static string ListToQueryString<T>(IEnumerable<T> list, string key)
Expand Down
4 changes: 2 additions & 2 deletions LeaderboardBackend/Authorization/MiddlewareResultHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ public async Task HandleAsync(
PolicyAuthorizationResult policyAuthorizationResult
)
{
if (policyAuthorizationResult.Forbidden)
if (policyAuthorizationResult.AuthorizationFailure?.FailureReasons.Any(fr => fr.Handler is UserTypeAuthorizationHandler) ?? false)
{
httpContext.Response.StatusCode = (int)HttpStatusCode.NotFound;
httpContext.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
return;
}

Expand Down
60 changes: 19 additions & 41 deletions LeaderboardBackend/Authorization/UserTypeAuthorizationHandler.cs
Original file line number Diff line number Diff line change
@@ -1,28 +1,20 @@
using System.Diagnostics.CodeAnalysis;
using LeaderboardBackend.Models.Entities;
using LeaderboardBackend.Result;
using LeaderboardBackend.Services;
using Microsoft.AspNetCore.Authorization;
using Microsoft.Extensions.Options;
using Microsoft.IdentityModel.Tokens;

namespace LeaderboardBackend.Authorization;

public class UserTypeAuthorizationHandler : AuthorizationHandler<UserTypeRequirement>
public class UserTypeAuthorizationHandler(
IOptions<JwtConfig> config,
IUserService userService
) : AuthorizationHandler<UserTypeRequirement>
{
private readonly IAuthService _authService;
private readonly TokenValidationParameters _jwtValidationParams;
private readonly IUserService _userService;

public UserTypeAuthorizationHandler(
IAuthService authService,
IOptions<JwtConfig> config,
IUserService userService
)
{
_authService = authService;
_jwtValidationParams = Jwt.ValidationParameters.GetInstance(config.Value);
_userService = userService;
}
private readonly TokenValidationParameters _jwtValidationParams = Jwt.ValidationParameters.GetInstance(config.Value);
private readonly IUserService _userService = userService;

protected override async Task HandleRequirementAsync(
AuthorizationHandlerContext context,
Expand All @@ -34,37 +26,23 @@ UserTypeRequirement requirement
return;
}

Guid? userId = _authService.GetUserIdFromClaims(context.User);
GetUserResult res = await _userService.GetUserFromClaims(context.User);

if (userId is null)
res.Switch(user =>
{
context.Fail();
return;
}

User? user = _userService.GetUserById(userId.Value).Result;

if (user is null || !Handle(user, requirement))
{
// FIXME: Work out how to fail as a ForbiddenResult.
context.Fail();
return;
}

context.Succeed(requirement);

return;
if (Handle(user, requirement))
{
context.Succeed(requirement);
}
}, badCredentials => context.Fail(new(this, "Bad Credentials")), notFound => context.Fail(new(this, "User Not Found")));
}

private bool Handle(User user, UserTypeRequirement requirement)
private static bool Handle(User user, UserTypeRequirement requirement) => requirement.Type switch
{
return requirement.Type switch
{
UserTypes.ADMINISTRATOR => user.IsAdmin,
UserTypes.USER => true,
_ => false,
};
}
UserTypes.ADMINISTRATOR => user.IsAdmin,
UserTypes.USER => true,
_ => false,
};
private static bool TryGetJwtFromHttpContext(
AuthorizationHandlerContext context,
[NotNullWhen(true)] out string? token
Expand Down
1 change: 1 addition & 0 deletions LeaderboardBackend/Controllers/AccountController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ public async Task<ActionResult<LoginResponse>> Login(
);
}

[Authorize]
[HttpPost("confirm")]
[SwaggerOperation("Resends the account confirmation link.")]
[SwaggerResponse(200, "A new confirmation link was generated.")]
Expand Down
9 changes: 3 additions & 6 deletions LeaderboardBackend/Controllers/CategoriesController.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using LeaderboardBackend.Authorization;
using LeaderboardBackend.Models.Entities;
using LeaderboardBackend.Models.Requests;
using LeaderboardBackend.Models.ViewModels;
Expand All @@ -24,8 +25,6 @@ public CategoriesController(ICategoryService categoryService)
[SwaggerResponse(404)]
public async Task<ActionResult<CategoryViewModel>> GetCategory(long id)
{
// NOTE: Should this use [AllowAnonymous]? - Ero

Category? category = await _categoryService.GetCategory(id);

if (category == null)
Expand All @@ -36,18 +35,16 @@ public async Task<ActionResult<CategoryViewModel>> GetCategory(long id)
return Ok(CategoryViewModel.MapFrom(category));
}

[Authorize(Policy = UserTypes.ADMINISTRATOR)]
[HttpPost]
[SwaggerOperation("Creates a new Category. This request is restricted to Moderators.")]
[SwaggerResponse(201)]
[SwaggerResponse(404)]
[SwaggerResponse(403)]
[SwaggerResponse(422, Type = typeof(ValidationProblemDetails))]
public async Task<ActionResult<CategoryViewModel>> CreateCategory(
[FromBody] CreateCategoryRequest request
)
{
// FIXME: Allow only moderators to call this! - Ero
// NOTE: Allow administrators to call this as well? - Ero

// NOTE: Check that body.PlayersMax > body.PlayersMin? - Ero
Category category =
new()
Expand Down
2 changes: 1 addition & 1 deletion LeaderboardBackend/Controllers/LeaderboardsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ [FromQuery] long[] ids
[SwaggerOperation("Creates a new leaderboard. This request is restricted to Administrators.")]
[SwaggerResponse(201)]
[SwaggerResponse(401)]
[SwaggerResponse(404, "The requesting `User` is unauthorized to create `Leaderboard`s.")]
[SwaggerResponse(403, "The requesting `User` is unauthorized to create `Leaderboard`s.")]
[SwaggerResponse(422, Type = typeof(ValidationProblemDetails))]
public async Task<ActionResult<LeaderboardViewModel>> CreateLeaderboard(
[FromBody] CreateLeaderboardRequest request
Expand Down
1 change: 1 addition & 0 deletions LeaderboardBackend/Controllers/RunsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public async Task<ActionResult<RunViewModel>> GetRun(Guid id)
return Ok(RunViewModel.MapFrom(run));
}

[Authorize]
[HttpPost]
[SwaggerOperation("Creates a new Run.")]
[SwaggerResponse(201)]
Expand Down
1 change: 1 addition & 0 deletions LeaderboardBackend/Controllers/UsersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public async Task<ActionResult<UserViewModel>> GetUserById(
return Ok(UserViewModel.MapFrom(user));
}

[Authorize]
[HttpGet("me")]
[SwaggerOperation(
"Gets the currently logged-in User.",
Expand Down
28 changes: 7 additions & 21 deletions LeaderboardBackend/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,40 +221,26 @@
JsonWebTokenHandler.DefaultInboundClaimTypeMap.Clear();
builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme).AddJwtBearer();

// Configure authorisation.
builder.Services.AddAuthorization(options =>
{
options.AddPolicy(
UserTypes.ADMINISTRATOR,
policy =>
builder.Services.AddAuthorizationBuilder()
.AddPolicy(UserTypes.ADMINISTRATOR, policy =>
{
policy.AuthenticationSchemes.Add(JwtBearerDefaults.AuthenticationScheme);
policy.RequireAuthenticatedUser();
policy.Requirements.Add(new UserTypeRequirement(UserTypes.ADMINISTRATOR));
}
);
options.AddPolicy(
UserTypes.MODERATOR,
policy =>
)
.AddPolicy(UserTypes.MODERATOR, policy =>
{
policy.AuthenticationSchemes.Add(JwtBearerDefaults.AuthenticationScheme);
policy.RequireAuthenticatedUser();
policy.Requirements.Add(new UserTypeRequirement(UserTypes.MODERATOR));
}
);
// Handles empty [Authorize] attributes
options.DefaultPolicy = new AuthorizationPolicyBuilder()
)
.SetDefaultPolicy(new AuthorizationPolicyBuilder()
.AddAuthenticationSchemes(new[] { JwtBearerDefaults.AuthenticationScheme })
.RequireAuthenticatedUser()
.AddRequirements(new[] { new UserTypeRequirement(UserTypes.USER) })
.Build();
options.FallbackPolicy = new AuthorizationPolicyBuilder()
.AddAuthenticationSchemes(new[] { JwtBearerDefaults.AuthenticationScheme })
.RequireAuthenticatedUser()
.Build();
});
.Build());

builder.Services.AddSingleton<IValidatorInterceptor, LeaderboardBackend.Models.Validation.UseErrorCodeInterceptor>();
builder.Services.AddFluentValidationAutoValidation(c =>
Expand Down
6 changes: 3 additions & 3 deletions LeaderboardBackend/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,8 @@
}
}
},
"404": {
"description": "Not Found"
"403": {
"description": "Forbidden"
},
"422": {
"description": "Unprocessable Content",
Expand Down Expand Up @@ -635,7 +635,7 @@
"401": {
"description": "Unauthorized"
},
"404": {
"403": {
"description": "The requesting `User` is unauthorized to create `Leaderboard`s."
},
"422": {
Expand Down

0 comments on commit be5d650

Please sign in to comment.