Skip to content

Commit

Permalink
[PermissionService] Replace brittle project id check (#2712)
Browse files Browse the repository at this point in the history
  • Loading branch information
imnasnainaec authored Oct 20, 2023
1 parent 5494338 commit 28f5ce6
Show file tree
Hide file tree
Showing 13 changed files with 71 additions and 90 deletions.
3 changes: 2 additions & 1 deletion Backend.Tests/Mocks/PermissionServiceMock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ public bool IsCurrentUserAuthorized(HttpContext? request)
/// to support unit testing when `HttpContext`s are not available.
/// </param>
/// <param name="permission"> Same as the real implementation. </param>
/// <param name="projectId"> Same as the real implementation. </param>
/// </summary>
public Task<bool> HasProjectPermission(HttpContext? request, Permission permission)
public Task<bool> HasProjectPermission(HttpContext? request, Permission permission, string projectId)
{
return Task.FromResult(IsAuthorizedHttpContext(request));
}
Expand Down
2 changes: 1 addition & 1 deletion Backend.Tests/Services/PermissionServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void IsSiteAdminTestTrue()
public void HasProjectPermissionTestAdmin()
{
var httpContext = createHttpContextWithUser(new User { IsAdmin = true });
Assert.That(_permService.HasProjectPermission(httpContext, Permission.Archive).Result, Is.True);
Assert.That(_permService.HasProjectPermission(httpContext, Permission.Archive, "ProjId").Result, Is.True);
}

[Test]
Expand Down
4 changes: 2 additions & 2 deletions Backend/Controllers/AudioController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public IActionResult DownloadAudioFile(string projectId, string wordId, string f
public async Task<IActionResult> UploadAudioFile(string projectId, string wordId,
[FromForm] FileUpload fileUpload)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -127,7 +127,7 @@ public async Task<IActionResult> UploadAudioFile(string projectId, string wordId
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(string))]
public async Task<IActionResult> DeleteAudioFile(string projectId, string wordId, string fileName)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry, projectId))
{
return Forbid();
}
Expand Down
6 changes: 3 additions & 3 deletions Backend/Controllers/InviteController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ public InviteController(
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(string))]
public async Task<IActionResult> EmailInviteToProject([FromBody, BindRequired] EmailInviteData data)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.DeleteEditSettingsAndUsers))
var projectId = data.ProjectId;
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.DeleteEditSettingsAndUsers, projectId))
{
return Forbid();
}

var projectId = data.ProjectId;
if (!await _permissionService.ContainsProjectRole(HttpContext, data.Role, projectId))
{
return Forbid();
Expand Down
10 changes: 5 additions & 5 deletions Backend/Controllers/LiftController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ internal async Task<IActionResult> UploadLiftFileAndGetWritingSystems(FileUpload
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(int))]
public async Task<IActionResult> FinishUploadLiftFile(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Import))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Import, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -148,7 +148,7 @@ internal async Task<IActionResult> FinishUploadLiftFile(string projectId, string
[RequestSizeLimit(250_000_000)] // 250MB.
public async Task<IActionResult> UploadLiftFile(string projectId, [FromForm] FileUpload fileUpload)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Import))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Import, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -283,7 +283,7 @@ public async Task<IActionResult> ExportLiftFile(string projectId)

private async Task<IActionResult> ExportLiftFile(string projectId, string userId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Export))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Export, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -368,7 +368,7 @@ public async Task<IActionResult> DownloadLiftFile(string projectId)

internal async Task<IActionResult> DownloadLiftFile(string projectId, string userId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Export))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Export, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -412,7 +412,7 @@ internal IActionResult DeleteLiftFile(string userId)
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(bool))]
public async Task<IActionResult> CanUploadLift(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Import))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Import, projectId))
{
return Forbid();
}
Expand Down
18 changes: 12 additions & 6 deletions Backend/Controllers/MergeController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ public MergeController(IMergeService mergeService, IPermissionService permission
public async Task<IActionResult> MergeWords(
string projectId, [FromBody, BindRequired] List<MergeWords> mergeWordsList)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.MergeAndReviewEntries))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
{
return Forbid();
}
Expand All @@ -53,7 +54,8 @@ public async Task<IActionResult> MergeWords(
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(bool))]
public async Task<IActionResult> UndoMerge(string projectId, [FromBody, BindRequired] MergeUndoIds merge)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.MergeAndReviewEntries))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
{
return Forbid();
}
Expand All @@ -68,7 +70,8 @@ public async Task<IActionResult> UndoMerge(string projectId, [FromBody, BindRequ
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List<string>))]
public async Task<IActionResult> BlacklistAdd(string projectId, [FromBody, BindRequired] List<string> wordIds)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.MergeAndReviewEntries))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
{
return Forbid();
}
Expand All @@ -84,7 +87,8 @@ public async Task<IActionResult> BlacklistAdd(string projectId, [FromBody, BindR
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List<string>))]
public async Task<IActionResult> GraylistAdd(string projectId, [FromBody, BindRequired] List<string> wordIds)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.MergeAndReviewEntries))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
{
return Forbid();
}
Expand All @@ -105,7 +109,8 @@ public async Task<IActionResult> GraylistAdd(string projectId, [FromBody, BindRe
public async Task<IActionResult> GetPotentialDuplicates(
string projectId, int maxInList, int maxLists, string userId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.MergeAndReviewEntries))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
{
return Forbid();
}
Expand All @@ -124,7 +129,8 @@ public async Task<IActionResult> GetPotentialDuplicates(
public async Task<IActionResult> getGraylistEntries(
string projectId, int maxLists, string userId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.MergeAndReviewEntries))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
{
return Forbid();
}
Expand Down
12 changes: 7 additions & 5 deletions Backend/Controllers/ProjectController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ public async Task<IActionResult> GetAllProjects()
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List<User>))]
public async Task<IActionResult> GetAllProjectUsers(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.DeleteEditSettingsAndUsers))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.DeleteEditSettingsAndUsers, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -76,7 +77,7 @@ public async Task<IActionResult> DeleteAllProjects()
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(Project))]
public async Task<IActionResult> GetProject(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -139,7 +140,8 @@ public async Task<IActionResult> CreateProject([FromBody, BindRequired] Project
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(string))]
public async Task<IActionResult> UpdateProject(string projectId, [FromBody, BindRequired] Project project)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.DeleteEditSettingsAndUsers))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.DeleteEditSettingsAndUsers, projectId))
{
return Forbid();
}
Expand All @@ -158,7 +160,7 @@ public async Task<IActionResult> UpdateProject(string projectId, [FromBody, Bind
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(Project))]
public async Task<IActionResult> PutChars(string projectId, [FromBody, BindRequired] Project project)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.CharacterInventory))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.CharacterInventory, projectId))
{
return Forbid();
}
Expand All @@ -181,7 +183,7 @@ public async Task<IActionResult> PutChars(string projectId, [FromBody, BindRequi
[ProducesResponseType(StatusCodes.Status200OK)]
public async Task<IActionResult> DeleteProject(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Archive))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Archive, projectId))
{
return Forbid();
}
Expand Down
10 changes: 5 additions & 5 deletions Backend/Controllers/StatisticsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public StatisticsController(
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List<SemanticDomainCount>))]
public async Task<IActionResult> GetSemanticDomainCounts(string projectId, string lang)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics, projectId))
{
return Forbid();
}
Expand All @@ -55,7 +55,7 @@ public async Task<IActionResult> GetSemanticDomainCounts(string projectId, strin
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List<WordsPerDayPerUserCount>))]
public async Task<IActionResult> GetWordsPerDayPerUserCounts(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics, projectId))
{
return Forbid();
}
Expand All @@ -76,7 +76,7 @@ public async Task<IActionResult> GetWordsPerDayPerUserCounts(string projectId)
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(ChartRootData))]
public async Task<IActionResult> GetProgressEstimationLineChartRoot(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics, projectId))
{
return Forbid();
}
Expand All @@ -97,7 +97,7 @@ public async Task<IActionResult> GetProgressEstimationLineChartRoot(string proje
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(ChartRootData))]
public async Task<IActionResult> GetLineChartRootData(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics, projectId))
{
return Forbid();
}
Expand All @@ -120,7 +120,7 @@ public async Task<IActionResult> GetLineChartRootData(string projectId)
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List<SemanticDomainUserCount>))]
public async Task<IActionResult> GetSemanticDomainUserCounts(string projectId, string lang)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.Statistics, projectId))
{
return Forbid();
}
Expand Down
16 changes: 10 additions & 6 deletions Backend/Controllers/UserEditController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public UserEditController(IUserEditRepository userEditRepo, IUserEditService use
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List<UserEdit>))]
public async Task<IActionResult> GetProjectUserEdits(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry, projectId))
{
return Forbid();
}
Expand All @@ -58,7 +58,7 @@ public async Task<IActionResult> GetProjectUserEdits(string projectId)
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(UserEdit))]
public async Task<IActionResult> GetUserEdit(string projectId, string userEditId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry))
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry, projectId))
{
return Forbid();
}
Expand All @@ -84,7 +84,8 @@ public async Task<IActionResult> GetUserEdit(string projectId, string userEditId
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(User))]
public async Task<IActionResult> CreateUserEdit(string projectId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.MergeAndReviewEntries))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -122,7 +123,8 @@ public async Task<IActionResult> CreateUserEdit(string projectId)
public async Task<IActionResult> UpdateUserEditGoal(
string projectId, string userEditId, [FromBody, BindRequired] Edit newEdit)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.MergeAndReviewEntries))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -170,7 +172,8 @@ public async Task<IActionResult> UpdateUserEditGoal(
public async Task<IActionResult> UpdateUserEditStep(string projectId, string userEditId,
[FromBody, BindRequired] UserEditStepWrapper stepEdit)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.MergeAndReviewEntries))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
{
return Forbid();
}
Expand Down Expand Up @@ -227,7 +230,8 @@ await _userEditService.UpdateStepInGoal(
[ProducesResponseType(StatusCodes.Status200OK)]
public async Task<IActionResult> DeleteUserEdit(string projectId, string userEditId)
{
if (!await _permissionService.HasProjectPermission(HttpContext, Permission.DeleteEditSettingsAndUsers))
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.DeleteEditSettingsAndUsers, projectId))
{
return Forbid();
}
Expand Down
Loading

0 comments on commit 28f5ce6

Please sign in to comment.