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.

Sign up for GitHub

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 Role-based restriction to backend mutations #594

Merged
merged 16 commits into from
Sep 7, 2021
Merged

Add Role-based restriction to backend mutations #594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
ccf9b49
Generalize Role based access in backend
ErlendHaa Sep 1, 2021
7af642f
Allow different signed-in users in AuthMock
ErlendHaa Sep 2, 2021
7e17215
Re-purpose MutationTest class
ErlendHaa Sep 6, 2021
eefa34b
Restrict access to mutation CreateParticipant
ErlendHaa Sep 1, 2021
85b3332
Restrict access to mutation SetAnswer
ErlendHaa Sep 1, 2021
3d98e5d
Restrict access to mutation CreateAction
ErlendHaa Sep 3, 2021
586ea3b
Restrict access to mutation EditAction
ErlendHaa Sep 1, 2021
aa7129b
Restrict access to mutation DeleteAction
ErlendHaa Sep 1, 2021
e6a17f0
Restrict access to mutation CreateNote
ErlendHaa Sep 1, 2021
07b4143
Delete mutation EditNote from backend
ErlendHaa Sep 1, 2021
e1b58ab
Restrict access to mutation CreateClosingRemark
ErlendHaa Sep 1, 2021
abd5012
Restrict access to mutation DeleteParticipant
ErlendHaa Sep 2, 2021
e72da85
Add backend auth-tests for ProgressEvaluation
ErlendHaa Sep 3, 2021
9f90a42
Restrict access to mutation ProgressParticipant
ErlendHaa Sep 3, 2021
82f57c6
Add backend auth-tests for SetSummary mutation
ErlendHaa Sep 3, 2021
53e42d7
Safeguard against deleting last Facilitator
ErlendHaa Sep 6, 2021
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
13 changes: 0 additions & 13 deletions backend/api/Authorization/AuthService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ namespace api.Authorization
public interface IAuthService
{
public string GetOID();
public void AssertIsFacilitator(string evaluationId);
}

public class AuthService : IAuthService
Expand All @@ -33,17 +32,5 @@ public string GetOID()
string oid = httpContext.User.FindFirst("http://schemas.microsoft.com/identity/claims/objectidentifier").Value;
return oid;
}

public void AssertIsFacilitator(string evaluationId)
{
string oid = GetOID();
Evaluation evaluation = _evaluationService.GetEvaluation(evaluationId);
Participant participant = _participantService.GetParticipant(oid, evaluation);
if (participant.Role != Role.Facilitator)
{
throw new UnauthorizedAccessException($"Current user is not Facilitator");

}
}
}
}
89 changes: 81 additions & 8 deletions backend/api/GQL/Mutation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,11 @@ public Evaluation CreateEvaluation(string name,

public Evaluation ProgressEvaluation(string evaluationId, Progression newProgression)
{
_authService.AssertIsFacilitator(evaluationId);
Evaluation evaluation = _evaluationService.GetEvaluation(evaluationId);

Role[] canBePerformedBy = { Role.Facilitator };
AssertCanPerformMutation(evaluation, canBePerformedBy);

_evaluationService.ProgressEvaluation(evaluation, newProgression);

_participantService.ProgressAllParticipants(evaluation, newProgression);
Expand All @@ -87,9 +89,11 @@ public Evaluation ProgressEvaluation(string evaluationId, Progression newProgres

public Evaluation SetSummary(string evaluationId, string summary)
{
_authService.AssertIsFacilitator(evaluationId);
Evaluation evaluation = _evaluationService.GetEvaluation(evaluationId);

Role[] canBePerformedBy = { Role.Facilitator };
AssertCanPerformMutation(evaluation, canBePerformedBy);

_evaluationService.SetSummary(evaluation, summary);
return evaluation;
}
Expand All @@ -100,6 +104,9 @@ public Participant ProgressParticipant(string evaluationId, Progression newProgr
Evaluation evaluation = _evaluationService.GetEvaluation(evaluationId);
Participant participant = _participantService.GetParticipant(azureUniqueId, evaluation);

Role[] canBePerformedBy = { Role.Facilitator, Role.OrganizationLead, Role.Participant };
AssertCanPerformMutation(evaluation, canBePerformedBy);

Participant progressedParticipant = _participantService.ProgressParticipant(participant, newProgression);

return progressedParticipant;
Expand All @@ -108,11 +115,42 @@ public Participant ProgressParticipant(string evaluationId, Progression newProgr
public Participant CreateParticipant(string azureUniqueId, string evaluationId, Organization organization, Role role)
{
Evaluation evaluation = _evaluationService.GetEvaluation(evaluationId);

Role[] canBePerformedBy = { Role.Facilitator, Role.OrganizationLead };
AssertCanPerformMutation(evaluation, canBePerformedBy);

return _participantService.Create(azureUniqueId, evaluation, organization, role);
}

public Participant DeleteParticipant(string participantId)
{
Evaluation evaluation = _participantService.GetAll()
.Where(p => p.Id.Equals(participantId))
.Select(p => p.Evaluation)
.First()
;

Role[] canBePerformedBy = { Role.Facilitator, Role.OrganizationLead };
AssertCanPerformMutation(evaluation, canBePerformedBy);

Participant subject = _participantService.GetParticipant(participantId);

/* Safeguard against deleting the last Facilitator */
if (subject.Role.Equals(Role.Facilitator))
{
int facilitators = evaluation.Participants
.Where(p => p.Role.Equals(Role.Facilitator))
.Count()
;

if (facilitators < 2)
{
string msg = "Cannot delete last Facilitator in Evaluation";
throw new InvalidOperationException(msg);

}
}

return _participantService.Remove(participantId);
}

Expand Down Expand Up @@ -157,6 +195,10 @@ public Answer SetAnswer(string questionId, Severity severity, string text, Progr
IQueryable<Question> queryableQuestion = _questionService.GetQuestion(questionId);
Question question = queryableQuestion.First();
Evaluation evaluation = queryableQuestion.Select(q => q.Evaluation).First();

Role[] canBePerformedBy = { Role.Facilitator, Role.Participant, Role.OrganizationLead };
AssertCanPerformMutation(evaluation, canBePerformedBy);

Participant currentUser = CurrentUser(evaluation);
Answer answer;
try
Expand All @@ -178,6 +220,9 @@ public Action CreateAction(string questionId, string assignedToId, string descri
Question question = queryableQuestion.First();
Evaluation evaluation = queryableQuestion.Select(q => q.Evaluation).First();

Role[] canBePerformedBy = { Role.Facilitator, Role.Participant, Role.OrganizationLead };
AssertCanPerformMutation(evaluation, canBePerformedBy);

Participant assignedTo = _participantService.GetParticipant(assignedToId);

return _actionService.Create(CurrentUser(evaluation), assignedTo, description, dueDate, title, priority, question);
Expand All @@ -187,6 +232,10 @@ public Action EditAction(string actionId, string assignedToId, string descriptio
{
IQueryable<Action> queryableAction = _actionService.GetAction(actionId);
Action action = queryableAction.First();
Evaluation evaluation = queryableAction.Select(q => q.Question.Evaluation).First();

Role[] canBePerformedBy = { Role.Facilitator, Role.Participant, Role.OrganizationLead };
AssertCanPerformMutation(evaluation, canBePerformedBy);

Participant assignedTo = _participantService.GetParticipant(assignedToId);

Expand All @@ -198,6 +247,10 @@ public Action DeleteAction(string actionId)
/* Note that no related fields are loaded */
IQueryable<Action> queryableAction = _actionService.GetAction(actionId);
Action action = queryableAction.First();
Evaluation evaluation = queryableAction.Select(q => q.Question.Evaluation).First();

Role[] canBePerformedBy = { Role.Facilitator };
AssertCanPerformMutation(evaluation, canBePerformedBy);

_actionService.Remove(action);
return action;
Expand All @@ -209,13 +262,10 @@ public Note CreateNote(string actionId, string text)
Action action = queryableAction.First();
Evaluation evaluation = queryableAction.Select(a => a.Question.Evaluation).First();

return _noteService.Create(CurrentUser(evaluation), text, action);
}
Role[] canBePerformedBy = { Role.Facilitator, Role.Participant, Role.OrganizationLead };
AssertCanPerformMutation(evaluation, canBePerformedBy);

public Note EditNote(string noteId, string text)
{
Note note = _noteService.GetNote(noteId);
return _noteService.EditNote(note, text);
Comment on lines -215 to -218
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we are not using, nor want to have the possibility to edit Notes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's my so-far conclusion, yes.

This mutation is unused and there are currently no UI support for
editing notes - nor is any such feature planned.

Updating and maintaining this mutation without any current or future planned use-case for it seams like unnecessary work. If we ever need it, we can revert this commit

return _noteService.Create(CurrentUser(evaluation), text, action);
}

public ClosingRemark CreateClosingRemark(string actionId, string text)
Expand All @@ -224,6 +274,9 @@ public ClosingRemark CreateClosingRemark(string actionId, string text)
Action action = queryableAction.First();
Evaluation evaluation = queryableAction.Select(a => a.Question.Evaluation).First();

Role[] canBePerformedBy = { Role.Facilitator, Role.Participant, Role.OrganizationLead };
AssertCanPerformMutation(evaluation, canBePerformedBy);

return _closingRemarkService.Create(CurrentUser(evaluation), text, action);
}

Expand All @@ -232,5 +285,25 @@ private Participant CurrentUser(Evaluation evaluation)
string azureUniqueId = _authService.GetOID();
return _participantService.GetParticipant(azureUniqueId, evaluation);
}

private void AssertCanPerformMutation(Evaluation evaluation, Role[] validRoles) {
string oid = _authService.GetOID();
Role userRoleInEvaluation;

try
{
userRoleInEvaluation = _participantService.GetParticipant(oid, evaluation).Role;
}
catch(NotFoundInDBException) {
string msg = "Non-participants cannot perform mutations on Evaluation";
throw new UnauthorizedAccessException(msg);
}

if (!validRoles.Contains(userRoleInEvaluation))
{
string msg = "{0} are not allowed to perform this operation";
throw new UnauthorizedAccessException(String.Format(msg, userRoleInEvaluation));
};
}
}
}
19 changes: 14 additions & 5 deletions backend/tests/DbContextTestSetup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

using api.Authorization;
using api.Context;
using api.Models;

namespace tests
{
Expand Down Expand Up @@ -33,15 +34,23 @@ public void Dispose()
}
}

class MockAuthService : IAuthService
public class MockAuthService : IAuthService
{
public string GetOID()
private string _loggedInUser = "1";

public void LoginUser(string azureUniqueId)
{
return "1";
_loggedInUser = azureUniqueId;
}
public void AssertIsFacilitator(string evaluationId)

public void LoginUser(Participant participant)
{
_loggedInUser = participant.AzureUniqueId;
}

public string GetOID()
{
// Do nothing
return _loggedInUser;
}
}
}
Loading