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

Add ModelState validation checks for all controller actions #204

Merged
merged 1 commit into from
Oct 8, 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
2 changes: 2 additions & 0 deletions League.Demo/Controllers/TenantContent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public TenantContent(ICompositeViewEngine viewEngine, IActionContextAccessor act
[HttpGet]
public IActionResult Index(string category = "home", string topic = "index")
{
if (!ModelState.IsValid) return BadRequest();

// Note: Indicate the current controller-specific tenant directory with the "./" prefix
var view = $"./{_tenantContext.SiteContext.FolderName}/{category}_{topic}";
var result = _viewEngine.FindView(_actionContext, view, false);
Expand Down
4 changes: 4 additions & 0 deletions League/Areas/Admin/Controllers/Impersonation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ public Impersonation(SignInManager<ApplicationUser> signInManager, ITenantContex
[HttpGet("")]
public async Task<IActionResult> Index(string search, int limit = 20)
{
if (!ModelState.IsValid) return BadRequest();

limit = Math.Abs(limit);
var users = new List<UserEntity>();
if (!string.IsNullOrWhiteSpace(search))
Expand All @@ -49,6 +51,8 @@ public async Task<IActionResult> Index(string search, int limit = 20)
[HttpGet("[action]/{id}")]
public async Task<IActionResult> Start(long id)
{
if (!ModelState.IsValid) return BadRequest();

var currentUser = User;
var targetUser = await _signInManager.UserManager.FindByIdAsync(id.ToString());

Expand Down
18 changes: 15 additions & 3 deletions League/Controllers/Account.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ public Account(
[AllowAnonymous]
public IActionResult SignIn(string? returnUrl = null)
{
if (!ModelState.IsValid) return BadRequest();

ViewData[ReturnUrl] = returnUrl;
return View();
}
Expand Down Expand Up @@ -181,8 +183,10 @@ public async Task<IActionResult> SignIn(SignInViewModel model, string? returnUrl
[AllowAnonymous]
public IActionResult CreateAccount(string? returnUrl = null)
{
if (!ModelState.IsValid) return BadRequest();

ViewData[ReturnUrl] = returnUrl;
return View();
return View(); // return the view to create a new account
}

[HttpPost("create")]
Expand Down Expand Up @@ -216,6 +220,8 @@ public async Task<IActionResult> CreateAccount(CreateAccountViewModel model, str
[AllowAnonymous]
public IActionResult Register(string? code = null, string? returnUrl = null)
{
if (!ModelState.IsValid) return BadRequest();

if (!_dataProtector.TryDecrypt(code?.Base64UrlDecode() ?? string.Empty, out var email, out var expiration))
{
return Redirect(TenantLink.Action(nameof(Message), nameof(Account), new { messageTypeText = MessageType.ConfirmEmailError })!);
Expand Down Expand Up @@ -295,6 +301,8 @@ public async Task<IActionResult> Register(RegisterViewModel model, CancellationT
[ValidateAntiForgeryToken]
public IActionResult ExternalSignIn(string provider, string? returnUrl = null)
{
if (!ModelState.IsValid) return BadRequest();

// Request a redirect to the external login provider.
var redirectUrl = TenantLink.Action(nameof(ExternalSignInCallback), nameof(Account), new { ReturnUrl = returnUrl });
var properties = _signInManager.ConfigureExternalAuthenticationProperties(provider, redirectUrl);
Expand All @@ -306,7 +314,7 @@ public IActionResult ExternalSignIn(string provider, string? returnUrl = null)
[AllowAnonymous]
public async Task<IActionResult> ExternalSignInCallback(string? returnUrl = null, string? remoteError = null)
{
if (remoteError != null)
if (remoteError != null || !ModelState.IsValid)
{
_logger.LogInformation("{Method} failed. Remote error was supplied.", nameof(ExternalSignInCallback));
return SocialMediaSignInFailure(_localizer["Failed to sign-in with the social network account"].Value + $" ({remoteError})");
Expand Down Expand Up @@ -447,6 +455,8 @@ public async Task<IActionResult> ForgotPassword(ForgotPasswordViewModel model)
[AllowAnonymous]
public IActionResult ResetPassword(string? code = null)
{
if (!ModelState.IsValid) return BadRequest();

var model = new ResetPasswordViewModel {Code = code};
if (code == null)
{
Expand All @@ -460,7 +470,7 @@ public IActionResult ResetPassword(string? code = null)
[ValidateAntiForgeryToken]
public async Task<IActionResult> ResetPassword(ResetPasswordViewModel model)
{
if (model.Code == null)
if (!ModelState.IsValid || model.Code is null)
{
return Redirect(TenantLink.Action(nameof(ResetPassword), nameof(Account))!);
}
Expand Down Expand Up @@ -510,6 +520,8 @@ public async Task<IActionResult> ResetPassword(ResetPasswordViewModel model)
[AllowAnonymous]
public IActionResult Message(string messageTypeText)
{
if (!ModelState.IsValid) return BadRequest();

if (Enum.TryParse(messageTypeText, true, out MessageType messageType))
{
switch (messageType)
Expand Down
4 changes: 4 additions & 0 deletions League/Controllers/Error.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public Error(ILogger<Error> logger, IStringLocalizer<Error> localizer, ILoggerFa
[HttpGet]
public IActionResult Index(string? id)
{
if (!ModelState.IsValid) return BadRequest();

ViewBag.TitleTagText = $"{_tenantContext.OrganizationContext.ShortName} - {_localizer["Error"]}";
id ??= string.Empty;
id = id.Trim();
Expand Down Expand Up @@ -64,6 +66,8 @@ public IActionResult Index(string? id)
[HttpGet]
public IActionResult AccessDenied(string returnUrl)
{
if (!ModelState.IsValid) return View(Views.ViewNames.Error.AccessDenied);

ViewData["ReturnUrl"] = returnUrl;
return View(Views.ViewNames.Error.AccessDenied);
}
Expand Down
2 changes: 2 additions & 0 deletions League/Controllers/Language.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public Language(IOptions<RequestLocalizationOptions> requestLocalizationOptions,
[HttpGet("")]
public IActionResult Index(string? culture, string? uiCulture, string? returnUrl)
{
if (!ModelState.IsValid) return BadRequest();

returnUrl ??= "/";
// QueryStringRequestCultureProvider processes arguments and requires one of both
if (culture == null && uiCulture == null)
Expand Down
8 changes: 6 additions & 2 deletions League/Controllers/Manage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ public async Task<IActionResult> ChangeEmail(ChangeEmailViewModel model)
[HttpGet("[action]/{id}")]
public async Task<IActionResult> ConfirmNewPrimaryEmail(string id, string code, string e)
{
if (!ModelState.IsValid) return BadRequest();

var user = await _userManager.FindByIdAsync(id);
if (user == null)
{
Expand Down Expand Up @@ -609,7 +611,7 @@ public async Task<IActionResult> ManageLogins()
public async Task<IActionResult> RemoveLogin(RemoveLoginViewModel account)
{
var user = await GetCurrentUserAsync();
if (user != null)
if (ModelState.IsValid && user != null)
{
var result = await _userManager.RemoveLoginAsync(user, account.LoginProvider, account.ProviderKey);
if (result.Succeeded)
Expand All @@ -629,6 +631,8 @@ public async Task<IActionResult> RemoveLogin(RemoveLoginViewModel account)
[ValidateAntiForgeryToken]
public IActionResult LinkLogin(string provider)
{
if (!ModelState.IsValid) return BadRequest();

// Request a redirect to the external login provider to link a login for the current user
var redirectUrl = TenantLink.Action(nameof(LinkLoginCallback), nameof(Manage));
var properties = _signInManager.ConfigureExternalAuthenticationProperties(provider, redirectUrl, _userManager.GetUserId(User));
Expand All @@ -639,7 +643,7 @@ public IActionResult LinkLogin(string provider)
public async Task<ActionResult> LinkLoginCallback()
{
var user = await GetCurrentUserAsync();
if (user == null)
if (!ModelState.IsValid || user == null)
{
TempData.Put<ManageMessage>(nameof(ManageMessage), new ManageMessage { AlertType = SiteAlertTagHelper.AlertType.Danger, MessageId = MessageId.AddLoginFailure });
return Redirect(GeneralLink.GetUriByAction(HttpContext, nameof(Index))!);
Expand Down
2 changes: 2 additions & 0 deletions League/Controllers/Map.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public async Task<IActionResult> Index(CancellationToken cancellationToken)
[HttpGet]
public async Task<IActionResult> Venue(long id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NotFound();

var venue = (await _appDb.VenueRepository.GetVenueTeamRowsAsync(
new PredicateExpression(VenueTeamFields.VenueId == id & VenueTeamFields.TournamentId == _tenantContext.TournamentContext.MapTournamentId), cancellationToken)).FirstOrDefault();

Expand Down
17 changes: 15 additions & 2 deletions League/Controllers/Match.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ public async Task<IActionResult> Fixtures(CancellationToken cancellationToken)
[HttpGet("[action]")]
public async Task<IActionResult> Calendar(long id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NoContent();

var matches = await _appDb.MatchRepository.GetMatchCalendarAsync(_tenantContext.TournamentContext.MatchPlanTournamentId, id, null, null, cancellationToken);
if (matches.Count != 1) return NoContent();

Expand Down Expand Up @@ -163,6 +165,8 @@ public async Task<IActionResult> Calendar(long id, CancellationToken cancellatio
[HttpGet("[action]")]
public async Task<IActionResult> PublicCalendar(long? team, long? round, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NoContent();

var matches = await _appDb.MatchRepository.GetMatchCalendarAsync(_tenantContext.TournamentContext.MatchPlanTournamentId, null, team, round, cancellationToken);
if (matches.Count == 0) return NoContent();

Expand Down Expand Up @@ -241,6 +245,8 @@ public async Task<IActionResult> Results(CancellationToken cancellationToken)
[HttpGet("overrule-result/{id:long}")]
public async Task<IActionResult> OverruleResult(long id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return Forbid();

var match = await _appDb.MatchRepository.GetMatchWithSetsAsync(id, cancellationToken);
if (!(await _authorizationService.AuthorizeAsync(User, match, Authorization.MatchOperations.OverruleResult)).Succeeded)
{
Expand All @@ -261,6 +267,8 @@ public async Task<IActionResult> OverruleResult(long id, CancellationToken cance
[HttpGet("enter-result/{id:long}")]
public async Task<IActionResult> EnterResult(long id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return Forbid();

try
{
var match = await _appDb.MatchRepository.GetMatchWithSetsAsync(id, cancellationToken);
Expand Down Expand Up @@ -322,6 +330,8 @@ public async Task<IActionResult> EnterResult(long id, CancellationToken cancella
[ValidateAntiForgeryToken]
public async Task<IActionResult> EnterResult([FromForm] EnterResultViewModel? model, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NotFound();

MatchEntity? match;
try
{
Expand Down Expand Up @@ -427,8 +437,7 @@ public async Task<IActionResult> EnterResult([FromForm] EnterResultViewModel? mo
public async Task<IActionResult> RemoveResult([FromForm] EnterResultViewModel? model,
CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(model);
ArgumentNullException.ThrowIfNull(model.Id);
if (!ModelState.IsValid || model?.Id is null) return NotFound();

try
{
Expand Down Expand Up @@ -467,6 +476,8 @@ public async Task<IActionResult> RemoveResult([FromForm] EnterResultViewModel? m
[HttpGet("edit-fixture/{id:long}")]
public async Task<IActionResult> EditFixture(long id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NotFound();

var model = new EditFixtureViewModel(await GetPlannedMatchFromDatabase(id, cancellationToken), _timeZoneConverter)
{
Tournament = await GetPlanTournament(cancellationToken)
Expand Down Expand Up @@ -506,6 +517,8 @@ public async Task<IActionResult> EditFixture([FromForm] EditFixtureViewModel mod
// [FromBody] => 'content-type': 'application/json'
// [FromForm] => 'content-type': 'application/x-www-form-urlencoded'

if (!ModelState.IsValid) return NotFound();

model = new EditFixtureViewModel(await GetPlannedMatchFromDatabase(model.Id, cancellationToken), _timeZoneConverter)
{
Tournament = await GetPlanTournament(cancellationToken)
Expand Down
4 changes: 4 additions & 0 deletions League/Controllers/Ranking.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ await _appDb.TournamentRepository.GetTournamentAsync(new PredicateExpression(Tou
[HttpGet("all-time/tournament/{id?}")]
public async Task<IActionResult> AllTimeTournament(long?id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) id = null;

try
{
var rankingList = await GetRankingListCached(cancellationToken);
Expand All @@ -101,6 +103,8 @@ public async Task<IActionResult> AllTimeTournament(long?id, CancellationToken ca
[HttpGet("all-time/team/{id?}")]
public async Task<IActionResult> AllTimeTeam(long? id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) id = null;

try
{
var rankingList = await GetRankingListCached(cancellationToken);
Expand Down
8 changes: 8 additions & 0 deletions League/Controllers/Role.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public Role(ITenantContext tenantContext,
public async Task<IActionResult> Remove(string roleName, long uid, long tid, string un,
string returnUrl, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return BadRequest();

var model = new RoleRemoveModel
{
TeamId = tid,
Expand All @@ -62,6 +64,9 @@ public async Task<IActionResult> Remove(string roleName, long uid, long tid, str
[ValidateAntiForgeryToken]
public async Task<IActionResult> Remove([FromForm] RoleRemoveModel model, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return JsonResponseRedirect(Url.Action(nameof(Error.AccessDenied), nameof(Error),
new { _defaultReturnUrl }));

model.ClaimType = Identity.Constants.ClaimType.ManagesTeam.ToLowerInvariant() ==
model.ClaimType?.ToLowerInvariant()
? Identity.Constants.ClaimType.ManagesTeam
Expand Down Expand Up @@ -106,6 +111,9 @@ public async Task<IActionResult> Remove([FromForm] RoleRemoveModel model, Cancel
[HttpGet("[action]/{tid:long}")]
public async Task<IActionResult> Add(long tid, string returnUrl, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return JsonResponseRedirect(Url.Action(nameof(Error.AccessDenied), nameof(Error),
new { _defaultReturnUrl }));

var model = new RoleAddModel
{
TeamId = tid,
Expand Down
17 changes: 16 additions & 1 deletion League/Controllers/Team.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ public async Task<IActionResult> List(CancellationToken cancellationToken)
[HttpGet("my-team/{id:long?}")]
public async Task<IActionResult> MyTeam(long? id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return Redirect(TenantLink.Action(nameof(MyTeam), nameof(Team), new { id = default(long?) })!);

// make sure that any changes are immediately reflected in the user's application cookie
var appUser = await _signInManager.UserManager.GetUserAsync(User);
if (appUser != null)
Expand Down Expand Up @@ -126,6 +128,8 @@ public async Task<IActionResult> MyTeam(long? id, CancellationToken cancellation
[HttpGet("{id:long}")]
public async Task<IActionResult> Single(long id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NotFound();

var model = new TeamSingleModel(_timeZoneConverter)
{
Tournament = await _appDb.TournamentRepository.GetTournamentAsync(
Expand Down Expand Up @@ -154,6 +158,8 @@ public async Task<IActionResult> Single(long id, CancellationToken cancellationT
[HttpGet("[action]/{teamId:long}")]
public async Task<IActionResult> Edit(long teamId, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NotFound();

var team = await _appDb.TeamRepository.GetTeamEntityAsync(new PredicateExpression(TeamFields.Id == teamId), cancellationToken);
if (team == null)
{
Expand All @@ -180,8 +186,10 @@ public async Task<IActionResult> Edit(long teamId, CancellationToken cancellatio
[ValidateAntiForgeryToken]
public async Task<IActionResult> Edit([FromForm] TeamEditModel model, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NotFound();

TeamEntity? team = null;
if (model.Team != null && !model.Team.IsNew)
if (model.Team is { IsNew: false })
{
team = await _appDb.TeamRepository.GetTeamEntityAsync(new PredicateExpression(TeamFields.Id == model.Team.Id), cancellationToken);
if (team == null)
Expand Down Expand Up @@ -259,6 +267,8 @@ public async Task<IActionResult> Edit([FromForm] TeamEditModel model, Cancellati
[HttpGet("[action]/{tid}")]
public async Task<IActionResult> ChangeVenue(long tid, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NotFound();

var teamEntity = await
_appDb.TeamRepository.GetTeamEntityAsync(new PredicateExpression(TeamFields.Id == tid),
cancellationToken);
Expand All @@ -273,6 +283,8 @@ public async Task<IActionResult> ChangeVenue(long tid, CancellationToken cancell
[HttpGet("[action]/{tid}")]
public async Task<IActionResult> SelectVenue(long tid, string returnUrl, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NotFound();

var teamEntity = await
_appDb.TeamRepository.GetTeamEntityAsync(new PredicateExpression(TeamFields.Id == tid),
cancellationToken);
Expand Down Expand Up @@ -305,6 +317,9 @@ public async Task<IActionResult> SelectVenue([FromForm][Bind("TeamId, VenueId")]
{
// model binding is not case-sensitive

if (!ModelState.IsValid) return JsonResponseRedirect(Url.Action(nameof(Error.AccessDenied), nameof(Error),
new { ReturnUrl = TenantLink.Action(nameof(MyTeam), nameof(Team)) }));

var teamEntity = await
_appDb.TeamRepository.GetTeamEntityAsync(new PredicateExpression(TeamFields.Id == model.TeamId),
cancellationToken);
Expand Down
6 changes: 6 additions & 0 deletions League/Controllers/Upload.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public Upload(ITenantContext tenantContext, IWebHostEnvironment webHostingEnviro
[HttpGet("team-photo/{id:long}")]
public async Task<IActionResult> TeamPhoto(long id, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return NotFound();

if (!(await _authorizationService.AuthorizeAsync(User, new TeamEntity(id),
Authorization.TeamOperations.ChangePhoto)).Succeeded)
{
Expand Down Expand Up @@ -78,6 +80,8 @@ public async Task<IActionResult> TeamPhoto(long id, CancellationToken cancellati
[ValidateAntiForgeryToken]
public async Task<IActionResult> TeamPhoto([FromForm] IFormFile file, [FromForm] long teamId, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return Forbid();

if (!(await _authorizationService.AuthorizeAsync(User, new TeamEntity(teamId),
Authorization.TeamOperations.ChangePhoto)).Succeeded)
{
Expand Down Expand Up @@ -148,6 +152,8 @@ await _tenantContext.DbContext.AppDb.TeamRepository.GetTeamEntityAsync(
[HttpGet("remove-team-photo/{id:long}")]
public async Task<IActionResult> RemoveTeamPhoto(long id)
{
if (!ModelState.IsValid) return Forbid();

if (!(await _authorizationService.AuthorizeAsync(User, new TeamEntity(id),
Authorization.TeamOperations.ChangePhoto)).Succeeded)
{
Expand Down
Loading