From ccf9b49b82497ce3596090e854e3d2ca125f3c8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Wed, 1 Sep 2021 16:14:06 +0200 Subject: [PATCH 01/16] Generalize Role based access in backend The goal is to implement restrictions for which roles can do what mutations in the backend. To that end the AssertIsFacilitator is replaced by the more general AssertCanPerformMutation. To avoid having to replicate AssertCanPerformMutation in the mocked version of the AuthService, the method is also moved to the Mutation class. --- backend/api/Authorization/AuthService.cs | 13 ----------- backend/api/GQL/Mutation.cs | 28 ++++++++++++++++++++++-- backend/tests/DbContextTestSetup.cs | 3 ++- backend/tests/GQL/Mutation.cs | 2 +- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/backend/api/Authorization/AuthService.cs b/backend/api/Authorization/AuthService.cs index 4a05ee1f..a914b42a 100644 --- a/backend/api/Authorization/AuthService.cs +++ b/backend/api/Authorization/AuthService.cs @@ -9,7 +9,6 @@ namespace api.Authorization public interface IAuthService { public string GetOID(); - public void AssertIsFacilitator(string evaluationId); } public class AuthService : IAuthService @@ -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"); - - } - } } } diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index c34d5c8f..cf49e318 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -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); @@ -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; } @@ -232,5 +236,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)); + }; + } } } diff --git a/backend/tests/DbContextTestSetup.cs b/backend/tests/DbContextTestSetup.cs index ca4968be..2955d9e7 100644 --- a/backend/tests/DbContextTestSetup.cs +++ b/backend/tests/DbContextTestSetup.cs @@ -5,6 +5,7 @@ using api.Authorization; using api.Context; +using api.Models; namespace tests { @@ -39,7 +40,7 @@ public string GetOID() { return "1"; } - public void AssertIsFacilitator(string evaluationId) + public void AssertCanPerformMutation(Evaluation evaluation, Role[] allowdRoles) { // Do nothing } diff --git a/backend/tests/GQL/Mutation.cs b/backend/tests/GQL/Mutation.cs index 60f5ce60..892c4723 100644 --- a/backend/tests/GQL/Mutation.cs +++ b/backend/tests/GQL/Mutation.cs @@ -67,7 +67,7 @@ public void ProgressEvaluationToFollowup() { Project project = _projectService.Create("ProgressEvaluationToFollowup"); Evaluation evaluation = _evaluationService.Create("ProgressEvaluationToFollowup", project, ""); - Participant participant = _participantService.Create("ProgressEvaluationToFollowup", evaluation, Organization.All, Role.Facilitator); + Participant participant = _participantService.Create("1", evaluation, Organization.All, Role.Facilitator); List questions = _questionService.CreateBulk(_questionTemplateService.GetAll().ToList(), evaluation); _answerService.Create(participant, questions[0], Severity.High, "test_answer_0", Progression.Workshop); From 7af642f2b5ce8d58059d657d2a1d2a287cbd2f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Thu, 2 Sep 2021 14:50:35 +0200 Subject: [PATCH 02/16] Allow different signed-in users in AuthMock Extend the functionallity of the MockAuthService class to allow setting different "signed-in"-users. --- backend/tests/DbContextTestSetup.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/backend/tests/DbContextTestSetup.cs b/backend/tests/DbContextTestSetup.cs index 2955d9e7..f305b758 100644 --- a/backend/tests/DbContextTestSetup.cs +++ b/backend/tests/DbContextTestSetup.cs @@ -34,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 AssertCanPerformMutation(Evaluation evaluation, Role[] allowdRoles) + + public void LoginUser(Participant participant) + { + _loggedInUser = participant.AzureUniqueId; + } + + public string GetOID() { - // Do nothing + return _loggedInUser; } } } From 7e172158d11135c8e58050276b3d9c9272b4d1fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Mon, 6 Sep 2021 08:14:02 +0200 Subject: [PATCH 03/16] Re-purpose MutationTest class --- backend/tests/GQL/Mutation.cs | 69 ++++++++++------------------------- 1 file changed, 19 insertions(+), 50 deletions(-) diff --git a/backend/tests/GQL/Mutation.cs b/backend/tests/GQL/Mutation.cs index 892c4723..105b2b27 100644 --- a/backend/tests/GQL/Mutation.cs +++ b/backend/tests/GQL/Mutation.cs @@ -14,16 +14,22 @@ namespace tests [Collection("UsesDbContext")] public class MutationTest : DbContextTestSetup { - private readonly Mutation _mutation; - private readonly ProjectService _projectService; - private readonly EvaluationService _evaluationService; - private readonly ParticipantService _participantService; - private readonly QuestionService _questionService; - private readonly AnswerService _answerService; - private readonly QuestionTemplateService _questionTemplateService; - private readonly ActionService _actionService; - private readonly NoteService _noteService; - private readonly ClosingRemarkService _closingRemarkService; + /* Services */ + protected readonly Mutation _mutation; + protected readonly ProjectService _projectService; + protected readonly EvaluationService _evaluationService; + protected readonly ParticipantService _participantService; + protected readonly QuestionService _questionService; + protected readonly AnswerService _answerService; + protected readonly QuestionTemplateService _questionTemplateService; + protected readonly ActionService _actionService; + protected readonly NoteService _noteService; + protected readonly ClosingRemarkService _closingRemarkService; + protected readonly MockAuthService _authService; + + /* Helpers */ + private readonly Project _project; + public MutationTest() { ILoggerFactory factory = new NullLoggerFactory(); @@ -36,6 +42,7 @@ public MutationTest() _actionService = new ActionService(_context); _noteService = new NoteService(_context); _closingRemarkService = new ClosingRemarkService(_context); + _authService = new MockAuthService(); _mutation = new Mutation( _projectService, _evaluationService, @@ -46,49 +53,11 @@ public MutationTest() _actionService, _noteService, _closingRemarkService, - new MockAuthService(), + _authService, new Logger(factory) ); - } - - [Fact] - public void CreateEvaluation() - { - string projectId = _context.Projects.First().Id; - int evaluationsBefore = _context.Evaluations.Count(); - _mutation.CreateEvaluation("CreateEvaluation", projectId, ""); - int evaluationsAfter = _context.Evaluations.Count(); - - Assert.Equal(evaluationsBefore + 1, evaluationsAfter); - } - - [Fact] - public void ProgressEvaluationToFollowup() - { - Project project = _projectService.Create("ProgressEvaluationToFollowup"); - Evaluation evaluation = _evaluationService.Create("ProgressEvaluationToFollowup", project, ""); - Participant participant = _participantService.Create("1", evaluation, Organization.All, Role.Facilitator); - - List questions = _questionService.CreateBulk(_questionTemplateService.GetAll().ToList(), evaluation); - _answerService.Create(participant, questions[0], Severity.High, "test_answer_0", Progression.Workshop); - _answerService.Create(participant, questions[1], Severity.High, "test_answer_1", Progression.Workshop); - _answerService.Create(participant, questions[2], Severity.High, "test_answer_2", Progression.Workshop); - - int nAnswersWorkshop = _context.Answers.Where( - a => (a.Progression.Equals(Progression.Workshop) && a.Question.Evaluation.Equals(evaluation)) - ).Count(); - - // Forces null on db relations - // To simulate behavior of normal API call - _context.ChangeTracker.Clear(); - - _mutation.ProgressEvaluation(evaluation.Id, Progression.FollowUp); - - int nAnswersFollowup = _context.Answers.Where( - a => (a.Progression.Equals(Progression.FollowUp) && a.Question.Evaluation.Equals(evaluation)) - ).Count(); - Assert.Equal(nAnswersWorkshop, nAnswersFollowup); + _project = _projectService.Create("Project"); } } } From eefa34bd9e1c8b716eec5dcf9a64261312f64421 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Wed, 1 Sep 2021 16:16:19 +0200 Subject: [PATCH 04/16] Restrict access to mutation CreateParticipant This commit implements role-based restrictions on the CreateParticipant mutation. Only Facilitator and OrganizationLead can use this mutation. As a consequence, the seeding logic in the cypress test needs to be updated to ensure that the mutation is only performed by a Facilitator. --- backend/api/GQL/Mutation.cs | 4 + backend/tests/GQL/Mutation.cs | 65 ++++++++++++++ .../tests/GQL/Mutations/CreateParticipant.cs | 86 +++++++++++++++++++ backend/tests/Randomize.cs | 50 +++++++++++ frontend/cypress/support/evaluation_seed.ts | 37 ++++---- 5 files changed, 221 insertions(+), 21 deletions(-) create mode 100644 backend/tests/GQL/Mutations/CreateParticipant.cs create mode 100644 backend/tests/Randomize.cs diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index cf49e318..a2d20d03 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -112,6 +112,10 @@ 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); } diff --git a/backend/tests/GQL/Mutation.cs b/backend/tests/GQL/Mutation.cs index 105b2b27..63598660 100644 --- a/backend/tests/GQL/Mutation.cs +++ b/backend/tests/GQL/Mutation.cs @@ -59,5 +59,70 @@ public MutationTest() _project = _projectService.Create("Project"); } + + /* Mutation wrappers with default values */ + + protected Evaluation CreateEvaluation( + string name = null, + string projectId = null, + string previousEvaluationId = null) + { + if (name == null) + { + name = Randomize.String(); + } + + if (projectId == null) + { + projectId = _project.Id; + } + + if (previousEvaluationId == null) + { + previousEvaluationId = ""; + } + + Evaluation evaluation = _mutation.CreateEvaluation( + name: name, + projectId: projectId, + previousEvaluationId: previousEvaluationId + ); + + return evaluation; + } + + protected Participant CreateParticipant( + Evaluation evaluation, + string azureUniqueId = null, + Organization? organization = null, + Role? role = null) + { + if (azureUniqueId == null) + { + azureUniqueId = Randomize.Integer().ToString(); + } + + Participant participant = _mutation.CreateParticipant( + azureUniqueId: azureUniqueId, + evaluationId: evaluation.Id, + organization: organization.GetValueOrDefault(Randomize.Organization()), + role: role.GetValueOrDefault(Randomize.Role()) + ); + + return participant; + } + + /* Helper methods */ + + protected int NumberOfParticipants(Evaluation evaluation) + { + int participants = _participantService + .GetAll() + .Where(p => p.Evaluation == evaluation) + .Count() + ; + + return participants; + } } } diff --git a/backend/tests/GQL/Mutations/CreateParticipant.cs b/backend/tests/GQL/Mutations/CreateParticipant.cs new file mode 100644 index 00000000..ec58f13a --- /dev/null +++ b/backend/tests/GQL/Mutations/CreateParticipant.cs @@ -0,0 +1,86 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class CreateParticipantMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + + public CreateParticipantMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanCreate(_facilitator); + } + + [Fact] + public void OrganizationLeadCanUseMutation() + { + AssertCanCreate(_organizationLead); + } + + [Fact] + public void ParticipantIsUnauthorized() + { + AssertIsNotAuthorized(_participant); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanCreate(Participant user) + { + _authService.LoginUser(user); + int participants = NumberOfParticipants(_evaluation); + CreateParticipant(_evaluation); + Assert.True(NumberOfParticipants(_evaluation) == participants + 1); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + CreateParticipant(_evaluation) + ); + } + + private void AssertIsNotAuthorized(Participant user) + { + AssertIsNotAuthorized(user.AzureUniqueId); + } + } +} diff --git a/backend/tests/Randomize.cs b/backend/tests/Randomize.cs new file mode 100644 index 00000000..f9ea8f37 --- /dev/null +++ b/backend/tests/Randomize.cs @@ -0,0 +1,50 @@ +using System; + +using api.Models; + + +namespace tests +{ + public static class Randomize + { + private static Random random = new Random(); + private static string alphanumeric = + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + + "abcdefghijklmnopqrstuvwxyz" + + "0123456789"; + + public static string Integer() + { + return random.Next(Int32.MinValue, Int32.MaxValue).ToString(); + } + + /* Random alphanumeric string of 8-32 characters */ + public static string String() + { + char[] chars = new char[random.Next(8, 32)]; + + for (int i = 0; i < chars.Length; i++) + { + chars[i] = alphanumeric[random.Next(alphanumeric.Length)]; + } + + return new String(chars); + } + + public static Role Role() + { + return RandomEnumValue(); + } + + public static Organization Organization() + { + return RandomEnumValue(); + } + + private static T RandomEnumValue() + { + Array values = Enum.GetValues(typeof(T)); + return (T)values.GetValue(random.Next(values.Length)); + } + } +} diff --git a/frontend/cypress/support/evaluation_seed.ts b/frontend/cypress/support/evaluation_seed.ts index 089b4413..92618167 100644 --- a/frontend/cypress/support/evaluation_seed.ts +++ b/frontend/cypress/support/evaluation_seed.ts @@ -207,39 +207,34 @@ const populateDB = (seed: EvaluationSeed) => { }) }) .then(() => { - cy.log(`EvaluationSeed: Progressing evaluation for creator ${seed.participants[0].user}`) - cy.gql(PROGRESS_PARTICIPANT, { + return seed.participants.slice(1) + }) + .each((participant: Participant) => { + cy.log(`EvaluationSeed: Adding Participants`) + cy.gql(ADD_PARTICIPANT, { variables: { + azureUniqueId: participant.user.id, evaluationId: seed.evaluationId, - newProgression: seed.participants[0].progression, + organization: participant.organization, + role: participant.role, }, + }).then(res => { + participant.id = res.body.data.createParticipant.id }) }) .then(() => { - return seed.participants.slice(1) + return seed.participants }) .each((participant: Participant) => { - cy.log(`EvaluationSeed: Adding and progressing additional Participants`) - return cy - .gql(ADD_PARTICIPANT, { + cy.login(participant.user).then(() => { + cy.log(`EvaluationSeed: Progressing Participant`) + cy.gql(PROGRESS_PARTICIPANT, { variables: { - azureUniqueId: participant.user.id, evaluationId: seed.evaluationId, - organization: participant.organization, - role: participant.role, + newProgression: participant.progression, }, }) - .then(res => { - participant.id = res.body.data.createParticipant.id - cy.login(participant.user).then(() => { - cy.gql(PROGRESS_PARTICIPANT, { - variables: { - evaluationId: seed.evaluationId, - newProgression: participant.progression, - }, - }) - }) - }) + }) }) .then(() => { return seed.answers From 85b3332295067c2cea97d6542faa887375429269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Wed, 1 Sep 2021 16:28:48 +0200 Subject: [PATCH 05/16] Restrict access to mutation SetAnswer This commit implements role-based restrictions on the SetAnswer mutation. This effectively excludes Read-Only participants and non-participating users from using this mutation. --- backend/api/GQL/Mutation.cs | 4 ++ backend/tests/GQL/Mutation.cs | 42 +++++++++++ backend/tests/GQL/Mutations/SetAnswer.cs | 88 ++++++++++++++++++++++++ backend/tests/Randomize.cs | 10 +++ 4 files changed, 144 insertions(+) create mode 100644 backend/tests/GQL/Mutations/SetAnswer.cs diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index a2d20d03..cea531ce 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -165,6 +165,10 @@ public Answer SetAnswer(string questionId, Severity severity, string text, Progr IQueryable 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 diff --git a/backend/tests/GQL/Mutation.cs b/backend/tests/GQL/Mutation.cs index 63598660..3518d1e9 100644 --- a/backend/tests/GQL/Mutation.cs +++ b/backend/tests/GQL/Mutation.cs @@ -112,6 +112,28 @@ protected Participant CreateParticipant( return participant; } + protected Answer SetAnswer( + string questionId, + Severity? severity = null, + string text = null, + Progression? progression = null) + { + + if (text == null) + { + text = Randomize.String(); + } + + Answer answer= _mutation.SetAnswer( + questionId: questionId, + severity: severity.GetValueOrDefault(Randomize.Severity()), + text: text, + progression: progression.GetValueOrDefault(Randomize.Progression()) + ); + + return answer; + } + /* Helper methods */ protected int NumberOfParticipants(Evaluation evaluation) @@ -124,5 +146,25 @@ protected int NumberOfParticipants(Evaluation evaluation) return participants; } + + protected int NumberOfAnswers(Question question) + { + int answers = _answerService + .GetAll() + .Where(a => a.Question == question) + .Count() + ; + + return answers; + } + + protected Question GetFirstQuestion(Evaluation evaluation) + { + return _questionService + .GetAll() + .Where(q => q.Evaluation.Id == evaluation.Id) + .First() + ; + } } } diff --git a/backend/tests/GQL/Mutations/SetAnswer.cs b/backend/tests/GQL/Mutations/SetAnswer.cs new file mode 100644 index 00000000..e64655d5 --- /dev/null +++ b/backend/tests/GQL/Mutations/SetAnswer.cs @@ -0,0 +1,88 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class SetAnswerMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + private readonly Question _question; + + public SetAnswerMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + _question = GetFirstQuestion(_evaluation); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanSet(_facilitator); + } + + [Fact] + public void OrganizationLeadCanUseMutation() + { + AssertCanSet(_organizationLead); + } + + [Fact] + public void ParticipantCanUseMutation() + { + AssertCanSet(_participant); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanSet(Participant user) + { + _authService.LoginUser(user); + int answers = NumberOfAnswers(_question); + SetAnswer(_question.Id); + Assert.True(NumberOfAnswers(_question) == answers + 1); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + SetAnswer(_question.Id) + ); + } + + private void AssertIsNotAuthorized(Participant user) + { + AssertIsNotAuthorized(user.AzureUniqueId); + } + } +} diff --git a/backend/tests/Randomize.cs b/backend/tests/Randomize.cs index f9ea8f37..6a17e614 100644 --- a/backend/tests/Randomize.cs +++ b/backend/tests/Randomize.cs @@ -41,6 +41,16 @@ public static Organization Organization() return RandomEnumValue(); } + public static Progression Progression() + { + return RandomEnumValue(); + } + + public static Severity Severity() + { + return RandomEnumValue(); + } + private static T RandomEnumValue() { Array values = Enum.GetValues(typeof(T)); From 3d98e5df1830074499ee0f5c79189742f82a70f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Fri, 3 Sep 2021 13:54:50 +0200 Subject: [PATCH 06/16] Restrict access to mutation CreateAction This commit implements role-based restrictions on the CreateAction mutation. This effectively excludes Read-Only participants and non-participating users from using this mutation. Whether Role.Participant should be able to create actions is currently up for debate, so this might be subject to change. --- backend/api/GQL/Mutation.cs | 3 + backend/tests/GQL/Mutation.cs | 43 +++++++++- backend/tests/GQL/Mutations/CreateAction.cs | 89 +++++++++++++++++++++ backend/tests/Randomize.cs | 15 ++++ frontend/cypress/support/testdata.ts | 2 +- 5 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 backend/tests/GQL/Mutations/CreateAction.cs diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index cea531ce..59cc11ee 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -190,6 +190,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); diff --git a/backend/tests/GQL/Mutation.cs b/backend/tests/GQL/Mutation.cs index 3518d1e9..ef61ae64 100644 --- a/backend/tests/GQL/Mutation.cs +++ b/backend/tests/GQL/Mutation.cs @@ -4,7 +4,6 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; -using System; using System.Linq; using System.Collections.Generic; using Xunit; @@ -134,6 +133,37 @@ protected Answer SetAnswer( return answer; } + protected Action CreateAction( + string questionId, + string assignedToId, + string description = null, + System.DateTimeOffset? dueDate = null, + Priority? priority = null, + string title = null) + { + + if (title == null) + { + title = Randomize.String(); + } + + if (description == null) + { + description = Randomize.String(); + } + + Action action = _mutation.CreateAction( + questionId: questionId, + assignedToId: assignedToId, + description: description, + dueDate: dueDate.GetValueOrDefault(System.DateTimeOffset.Now), + priority: priority.GetValueOrDefault(Randomize.Priority()), + title: title + ); + + return action; + } + /* Helper methods */ protected int NumberOfParticipants(Evaluation evaluation) @@ -158,6 +188,17 @@ protected int NumberOfAnswers(Question question) return answers; } + protected int NumberOfActions(Question question) + { + int actions = _actionService + .GetAll() + .Where(a => a.Question == question) + .Count() + ; + + return actions; + } + protected Question GetFirstQuestion(Evaluation evaluation) { return _questionService diff --git a/backend/tests/GQL/Mutations/CreateAction.cs b/backend/tests/GQL/Mutations/CreateAction.cs new file mode 100644 index 00000000..c13f92ac --- /dev/null +++ b/backend/tests/GQL/Mutations/CreateAction.cs @@ -0,0 +1,89 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class CreateActionMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + private readonly Question _question; + + public CreateActionMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + _question = GetFirstQuestion(_evaluation); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanCreate(_facilitator); + } + + [Fact] + public void OrganizationLeadCanUseMutation() + { + AssertCanCreate(_organizationLead); + } + + [Fact] + public void ParticipantIsCanUseMutation() + { + AssertCanCreate(_participant); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly.AzureUniqueId); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanCreate(Participant user) + { + _authService.LoginUser(user); + int answers = NumberOfActions(_question); + CreateAction( + questionId: _question.Id, + assignedToId: Randomize.Value(_evaluation.Participants).Id + ); + Assert.True(NumberOfActions(_question) == answers + 1); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + CreateAction( + questionId: _question.Id, + assignedToId: Randomize.Value(_evaluation.Participants).Id + ) + ); + } + } +} diff --git a/backend/tests/Randomize.cs b/backend/tests/Randomize.cs index 6a17e614..b655cf22 100644 --- a/backend/tests/Randomize.cs +++ b/backend/tests/Randomize.cs @@ -1,4 +1,8 @@ using System; +using System.Linq; +using System.Collections; +using System.Collections.Generic; + using api.Models; @@ -51,6 +55,17 @@ public static Severity Severity() return RandomEnumValue(); } + public static Priority Priority() + { + return RandomEnumValue(); + } + + public static T Value(ICollection values) + { + int pick = random.Next(values.Count); + return values.ElementAt(pick); + } + private static T RandomEnumValue() { Array values = Enum.GetValues(typeof(T)); diff --git a/frontend/cypress/support/testdata.ts b/frontend/cypress/support/testdata.ts index 0bac24b5..9a4e9490 100644 --- a/frontend/cypress/support/testdata.ts +++ b/frontend/cypress/support/testdata.ts @@ -20,7 +20,7 @@ export function createAction( this: EvaluationSeed, { assignedTo = faker.random.arrayElement(this!.participants), - createdBy = faker.random.arrayElement(this!.participants), + createdBy = faker.random.arrayElement(this!.participants.filter(x => x.role !== Role.ReadOnly)), // no access to questions as creation is run before plant. Reconsider? questionOrder = faker.datatype.number({ min: 1, max: 3 }), dueDate = faker.date.future(), From 586ea3b90343fd2a91a2420504335485d9271de3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Wed, 1 Sep 2021 16:42:08 +0200 Subject: [PATCH 07/16] Restrict access to mutation EditAction This commit implements role-based restrictions on the EditAction mutation. This effectively excludes Read-Only participants and non-participating users from using this mutation. Whether Role.Participant should be able to create actions is currently up for debate, so this might be subject to change. --- backend/api/GQL/Mutation.cs | 4 + backend/tests/GQL/Mutation.cs | 35 ++++++++ backend/tests/GQL/Mutations/EditAction.cs | 100 ++++++++++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 backend/tests/GQL/Mutations/EditAction.cs diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index 59cc11ee..85782e67 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -202,6 +202,10 @@ public Action EditAction(string actionId, string assignedToId, string descriptio { IQueryable 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); diff --git a/backend/tests/GQL/Mutation.cs b/backend/tests/GQL/Mutation.cs index ef61ae64..fd2698b4 100644 --- a/backend/tests/GQL/Mutation.cs +++ b/backend/tests/GQL/Mutation.cs @@ -164,6 +164,41 @@ protected Action CreateAction( return action; } + protected Action EditAction( + string actionId, + string assignedToId, + string description = null, + System.DateTimeOffset? dueDate = null, + Priority? priority = null, + string title = null, + bool onHold = false, + bool completed = false) + { + + if (title == null) + { + title = Randomize.String(); + } + + if (description == null) + { + description = Randomize.String(); + } + + Action action = _mutation.EditAction( + actionId: actionId, + assignedToId: assignedToId, + description: description, + dueDate: dueDate.GetValueOrDefault(System.DateTimeOffset.Now), + priority: priority.GetValueOrDefault(Randomize.Priority()), + title: title, + onHold: onHold, + completed: completed + ); + + return action; + } + /* Helper methods */ protected int NumberOfParticipants(Evaluation evaluation) diff --git a/backend/tests/GQL/Mutations/EditAction.cs b/backend/tests/GQL/Mutations/EditAction.cs new file mode 100644 index 00000000..d3bf58b8 --- /dev/null +++ b/backend/tests/GQL/Mutations/EditAction.cs @@ -0,0 +1,100 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class EditActionMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + private readonly Question _question; + private readonly api.Models.Action _action; + + public EditActionMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + _question = GetFirstQuestion(_evaluation); + _action = CreateAction( + questionId: _question.Id, + assignedToId: Randomize.Value(_evaluation.Participants).Id + ); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanEdit(_facilitator); + } + + [Fact] + public void OrganizationLeadCanUseMutation() + { + AssertCanEdit(_organizationLead); + } + + [Fact] + public void ParticipantIsCanUseMutation() + { + AssertCanEdit(_participant); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly.AzureUniqueId); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanEdit(Participant user) + { + _authService.LoginUser(user); + int answers = NumberOfActions(_question); + string description = _action.Description; + string actionId = _action.Id; + + var action = EditAction( + actionId: actionId, + assignedToId: Randomize.Value(_evaluation.Participants).Id, + description: Randomize.String() + ); + Assert.True(NumberOfActions(_question) == answers); + Assert.True(actionId == action.Id); + Assert.False(description == action.Description); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + EditAction( + actionId: _action.Id, + assignedToId: Randomize.Value(_evaluation.Participants).Id + ) + ); + } + } +} From aa7129b830fe31758b64669936cecff1d3f261f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Wed, 1 Sep 2021 16:44:35 +0200 Subject: [PATCH 08/16] Restrict access to mutation DeleteAction This commit implements role-based restrictions on the DeleteAction mutation. Only Facilitators are permitted to use this mutation. --- backend/api/GQL/Mutation.cs | 4 + backend/tests/GQL/Mutation.cs | 5 ++ backend/tests/GQL/Mutations/DeleteAction.cs | 93 ++++++++++++++++++++ frontend/cypress/integration/actions_spec.ts | 5 +- 4 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 backend/tests/GQL/Mutations/DeleteAction.cs diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index 85782e67..d9375dd8 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -217,6 +217,10 @@ public Action DeleteAction(string actionId) /* Note that no related fields are loaded */ IQueryable 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; diff --git a/backend/tests/GQL/Mutation.cs b/backend/tests/GQL/Mutation.cs index fd2698b4..ac632005 100644 --- a/backend/tests/GQL/Mutation.cs +++ b/backend/tests/GQL/Mutation.cs @@ -199,6 +199,11 @@ protected Action EditAction( return action; } + protected void DeleteAction(string actionId) + { + _mutation.DeleteAction(actionId); + } + /* Helper methods */ protected int NumberOfParticipants(Evaluation evaluation) diff --git a/backend/tests/GQL/Mutations/DeleteAction.cs b/backend/tests/GQL/Mutations/DeleteAction.cs new file mode 100644 index 00000000..350db8f1 --- /dev/null +++ b/backend/tests/GQL/Mutations/DeleteAction.cs @@ -0,0 +1,93 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class DeleteActionMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + private readonly Question _question; + private readonly api.Models.Action _action; + + public DeleteActionMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + _question = GetFirstQuestion(_evaluation); + _action = CreateAction( + questionId: _question.Id, + assignedToId: Randomize.Value(_evaluation.Participants).Id + ); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanDelete(_facilitator); + } + + [Fact] + public void OrganizationLeadIsUnauthorized() + { + AssertIsNotAuthorized(_organizationLead.AzureUniqueId); + } + + [Fact] + public void ParticipantIsUnauthorized() + { + AssertIsNotAuthorized(_participant.AzureUniqueId); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly.AzureUniqueId); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanDelete(Participant user) + { + var toDelete = CreateAction( + questionId: _question.Id, + assignedToId: Randomize.Value(_evaluation.Participants).Id + ); + + _authService.LoginUser(user); + int answers = NumberOfActions(_question); + DeleteAction(toDelete.Id); + Assert.True(NumberOfActions(_question) == answers - 1); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + DeleteAction(_action.Id) + ); + } + } +} diff --git a/frontend/cypress/integration/actions_spec.ts b/frontend/cypress/integration/actions_spec.ts index f2ae4e33..07635368 100644 --- a/frontend/cypress/integration/actions_spec.ts +++ b/frontend/cypress/integration/actions_spec.ts @@ -1,5 +1,5 @@ import { EvaluationSeed } from '../support/evaluation_seed' -import { Progression, Priority } from '../../src/api/models' +import { Progression, Priority, Role } from '../../src/api/models' import { FUSION_DATE_LOCALE } from '../support/helpers' import { barrierToString, organizationToString } from '../../src/utils/EnumToString' import { Action, Note, Participant } from '../support/mocks' @@ -130,7 +130,7 @@ describe('Actions', () => { ;({ seed, actionToDelete, actionToStay } = createDeleteSeed()) seed.plant().then(() => { - const user = faker.random.arrayElement(seed.participants).user + const user = faker.random.arrayElement(seed.participants.filter(x => x.role === Role.Facilitator)).user cy.visitEvaluation(seed.evaluationId, user) evaluationPage.progressionStepLink(deleteActionFrom).click() cy.contains(actionToDelete.title).should('exist') @@ -176,6 +176,7 @@ describe('Actions', () => { } }) + cy.login(seed.participants[0].user) deleteAction() cy.testCacheAndDB( From e6a17f07690720ac4ea4106e1643a097b00469bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Wed, 1 Sep 2021 16:48:09 +0200 Subject: [PATCH 09/16] Restrict access to mutation CreateNote This commit implements role-based restrictions on the CreateNote mutation. This effectively excludes Read-Only participants and non-participating users from using this mutation. --- backend/api/GQL/Mutation.cs | 3 + backend/tests/GQL/Mutation.cs | 29 +++++++ backend/tests/GQL/Mutations/CreateNote.cs | 94 +++++++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 backend/tests/GQL/Mutations/CreateNote.cs diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index d9375dd8..7216e575 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -232,6 +232,9 @@ public Note CreateNote(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 _noteService.Create(CurrentUser(evaluation), text, action); } diff --git a/backend/tests/GQL/Mutation.cs b/backend/tests/GQL/Mutation.cs index ac632005..e173d410 100644 --- a/backend/tests/GQL/Mutation.cs +++ b/backend/tests/GQL/Mutation.cs @@ -204,6 +204,24 @@ protected void DeleteAction(string actionId) _mutation.DeleteAction(actionId); } + protected Note CreateNote( + string actionId, + string text = null) + { + + if (text == null) + { + text = Randomize.String(); + } + + Note note = _mutation.CreateNote( + actionId: actionId, + text: text + ); + + return note; + } + /* Helper methods */ protected int NumberOfParticipants(Evaluation evaluation) @@ -239,6 +257,17 @@ protected int NumberOfActions(Question question) return actions; } + protected int NumberOfNotes(Action action) + { + int notes = _noteService + .GetAll() + .Where(a => a.Action == action) + .Count() + ; + + return notes; + } + protected Question GetFirstQuestion(Evaluation evaluation) { return _questionService diff --git a/backend/tests/GQL/Mutations/CreateNote.cs b/backend/tests/GQL/Mutations/CreateNote.cs new file mode 100644 index 00000000..37c2bf38 --- /dev/null +++ b/backend/tests/GQL/Mutations/CreateNote.cs @@ -0,0 +1,94 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class CreateNoteMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + private readonly Question _question; + private readonly api.Models.Action _action; + + public CreateNoteMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + _question = GetFirstQuestion(_evaluation); + _action = CreateAction( + questionId: _question.Id, + assignedToId: Randomize.Value(_evaluation.Participants).Id + ); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanCreate(_facilitator); + } + + [Fact] + public void OrganizationLeadCanUseMutation() + { + AssertCanCreate(_organizationLead); + } + + [Fact] + public void ParticipantCanUseMutation() + { + AssertCanCreate(_participant); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly.AzureUniqueId); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanCreate(Participant user) + { + _authService.LoginUser(user); + int notes = NumberOfNotes(_action); + CreateNote( + actionId: _action.Id, + text: Randomize.String() + ); + Assert.True(NumberOfNotes(_action) == notes + 1); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + CreateNote( + actionId: _action.Id, + text: Randomize.String() + ) + ); + } + } +} From 07b4143f53287758ff9cd499cb36bf6e1e46bd27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Wed, 1 Sep 2021 16:52:58 +0200 Subject: [PATCH 10/16] Delete mutation EditNote from backend This mutation is unused and there are currently no UI support for editing notes - nor is any such feature planned. --- backend/api/GQL/Mutation.cs | 6 ------ frontend/src/api/models.ts | 7 ------- 2 files changed, 13 deletions(-) diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index 7216e575..0b4a1328 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -238,12 +238,6 @@ public Note CreateNote(string actionId, string text) return _noteService.Create(CurrentUser(evaluation), text, action); } - public Note EditNote(string noteId, string text) - { - Note note = _noteService.GetNote(noteId); - return _noteService.EditNote(note, text); - } - public ClosingRemark CreateClosingRemark(string actionId, string text) { IQueryable queryableAction = _actionService.GetAction(actionId); diff --git a/frontend/src/api/models.ts b/frontend/src/api/models.ts index a6b22c38..fd352506 100644 --- a/frontend/src/api/models.ts +++ b/frontend/src/api/models.ts @@ -327,7 +327,6 @@ export type Mutation = { editAction?: Maybe; deleteAction?: Maybe; createNote?: Maybe; - editNote?: Maybe; createClosingRemark?: Maybe; }; @@ -435,12 +434,6 @@ export type MutationCreateNoteArgs = { }; -export type MutationEditNoteArgs = { - noteId?: Maybe; - text?: Maybe; -}; - - export type MutationCreateClosingRemarkArgs = { actionId?: Maybe; text?: Maybe; From e1b58ab73ca68a4e4536ab35e6ea9dd4d1add810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Wed, 1 Sep 2021 16:57:26 +0200 Subject: [PATCH 11/16] Restrict access to mutation CreateClosingRemark This commit implements role-based restrictions on the CreateClosingRemark mutation. This effectively excludes Read-Only participants and non-participating users from using this mutation. Whether Role.Participant should be able to create closing remarks is currently up for debate, so this might be subject to change. --- backend/api/GQL/Mutation.cs | 3 + backend/tests/GQL/Mutation.cs | 29 ++++++ .../GQL/Mutations/CreateClosingRemark.cs | 94 +++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 backend/tests/GQL/Mutations/CreateClosingRemark.cs diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index 0b4a1328..05e36351 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -244,6 +244,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); } diff --git a/backend/tests/GQL/Mutation.cs b/backend/tests/GQL/Mutation.cs index e173d410..99eef449 100644 --- a/backend/tests/GQL/Mutation.cs +++ b/backend/tests/GQL/Mutation.cs @@ -222,6 +222,24 @@ protected Note CreateNote( return note; } + protected ClosingRemark CreateClosingRemark( + string actionId, + string text = null) + { + + if (text == null) + { + text = Randomize.String(); + } + + ClosingRemark remark = _mutation.CreateClosingRemark( + actionId: actionId, + text: text + ); + + return remark; + } + /* Helper methods */ protected int NumberOfParticipants(Evaluation evaluation) @@ -268,6 +286,17 @@ protected int NumberOfNotes(Action action) return notes; } + protected int NumberOfClosingRemarks(Action action) + { + int remarks = _closingRemarkService + .GetAll() + .Where(a => a.Action == action) + .Count() + ; + + return remarks; + } + protected Question GetFirstQuestion(Evaluation evaluation) { return _questionService diff --git a/backend/tests/GQL/Mutations/CreateClosingRemark.cs b/backend/tests/GQL/Mutations/CreateClosingRemark.cs new file mode 100644 index 00000000..dd717062 --- /dev/null +++ b/backend/tests/GQL/Mutations/CreateClosingRemark.cs @@ -0,0 +1,94 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class CreateClosingRemarkMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + private readonly Question _question; + private readonly api.Models.Action _action; + + public CreateClosingRemarkMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + _question = GetFirstQuestion(_evaluation); + _action = CreateAction( + questionId: _question.Id, + assignedToId: Randomize.Value(_evaluation.Participants).Id + ); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanCreate(_facilitator); + } + + [Fact] + public void OrganizationLeadCanUseMutation() + { + AssertCanCreate(_organizationLead); + } + + [Fact] + public void ParticipantCanUseMutation() + { + AssertCanCreate(_participant); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly.AzureUniqueId); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanCreate(Participant user) + { + _authService.LoginUser(user); + int notes = NumberOfClosingRemarks(_action); + CreateClosingRemark( + actionId: _action.Id, + text: Randomize.String() + ); + Assert.True(NumberOfClosingRemarks(_action) == notes + 1); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + CreateClosingRemark( + actionId: _action.Id, + text: Randomize.String() + ) + ); + } + } +} From abd5012d151bb0a6e9a5b264c98aa17d1d1bfea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Thu, 2 Sep 2021 10:08:53 +0200 Subject: [PATCH 12/16] Restrict access to mutation DeleteParticipant This commit implements role-based restrictions on the DeleteParticipant mutation. Only Facilitator and OrganizationLead are permitted to use this mutation. --- backend/api/GQL/Mutation.cs | 9 ++ .../tests/GQL/Mutations/DeleteParticipant.cs | 91 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 backend/tests/GQL/Mutations/DeleteParticipant.cs diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index 05e36351..59f2baba 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -121,6 +121,15 @@ public Participant CreateParticipant(string azureUniqueId, string evaluationId, 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); + return _participantService.Remove(participantId); } diff --git a/backend/tests/GQL/Mutations/DeleteParticipant.cs b/backend/tests/GQL/Mutations/DeleteParticipant.cs new file mode 100644 index 00000000..5b811e9d --- /dev/null +++ b/backend/tests/GQL/Mutations/DeleteParticipant.cs @@ -0,0 +1,91 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class DeleteParticipantMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + + public DeleteParticipantMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanDelete(_facilitator); + } + + [Fact] + public void OrganizationLeadCanUseMutation() + { + AssertCanDelete(_organizationLead); + } + + [Fact] + public void ParticipantIsUnauthorized() + { + AssertIsNotAuthorized(_participant.AzureUniqueId); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly.AzureUniqueId); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanDelete(Participant user) + { + var toDelete = CreateParticipant( + evaluation: _evaluation, + azureUniqueId: Randomize.Integer().ToString() + ); + + _authService.LoginUser(user); + int participants = NumberOfParticipants(_evaluation); + _mutation.DeleteParticipant(toDelete.Id); + Assert.True(NumberOfParticipants(_evaluation) == participants - 1); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + var toDelete = CreateParticipant( + evaluation: _evaluation, + azureUniqueId: Randomize.Integer().ToString() + ); + + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + _mutation.DeleteParticipant(toDelete.Id) + ); + } + } +} From e72da853b71f1ee9d40f58f37340b74687946c19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Fri, 3 Sep 2021 16:13:18 +0200 Subject: [PATCH 13/16] Add backend auth-tests for ProgressEvaluation --- .../tests/GQL/Mutations/ProgressEvaluation.cs | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 backend/tests/GQL/Mutations/ProgressEvaluation.cs diff --git a/backend/tests/GQL/Mutations/ProgressEvaluation.cs b/backend/tests/GQL/Mutations/ProgressEvaluation.cs new file mode 100644 index 00000000..c5bb29f8 --- /dev/null +++ b/backend/tests/GQL/Mutations/ProgressEvaluation.cs @@ -0,0 +1,94 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class ProgressEvaluationMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + + public ProgressEvaluationMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanProgress(_facilitator); + } + + [Fact] + public void OrganizationLeadIsUnauthorized() + { + AssertIsNotAuthorized(_organizationLead); + } + + [Fact] + public void ParticipantIsUnauthorized() + { + AssertIsNotAuthorized(_participant); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanProgress(Participant user) + { + _authService.LoginUser(user); + Progression newProgression = Randomize.Progression(); + + _mutation.ProgressEvaluation( + evaluationId: _evaluation.Id, + newProgression: newProgression + ); + + Assert.True(_evaluation.Progression == newProgression); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + _mutation.ProgressEvaluation( + evaluationId: _evaluation.Id, + newProgression: Randomize.Progression() + ) + ); + } + + private void AssertIsNotAuthorized(Participant user) + { + AssertIsNotAuthorized(user.AzureUniqueId); + } + } +} From 9f90a42e37cb790e9b02ff7eb176d76a328a374e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Fri, 3 Sep 2021 16:25:40 +0200 Subject: [PATCH 14/16] Restrict access to mutation ProgressParticipant This commit implements role-based restrictions on the ProgressParticipant mutation. This effectively excludes Read-Only participants and non-participating users from using this mutation --- backend/api/GQL/Mutation.cs | 3 + .../GQL/Mutations/ProgressParticipant.cs | 82 +++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 backend/tests/GQL/Mutations/ProgressParticipant.cs diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index 59f2baba..87955212 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -104,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; diff --git a/backend/tests/GQL/Mutations/ProgressParticipant.cs b/backend/tests/GQL/Mutations/ProgressParticipant.cs new file mode 100644 index 00000000..0bd3cf02 --- /dev/null +++ b/backend/tests/GQL/Mutations/ProgressParticipant.cs @@ -0,0 +1,82 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class ProgressParticipantMutation : MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + + public ProgressParticipantMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanProgress(_facilitator); + } + + [Fact] + public void OrganizationLeadCanUseMutation() + { + AssertCanProgress(_organizationLead); + } + + [Fact] + public void ParticipantCanUseMutation() + { + AssertCanProgress(_participant); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly); + } + + /* Helper methods */ + + private void AssertCanProgress(Participant user) + { + _authService.LoginUser(user); + Progression newProgression = Randomize.Progression(); + + _mutation.ProgressParticipant( + evaluationId: _evaluation.Id, + newProgression: newProgression + ); + + Assert.True(user.Progression == newProgression); + } + + private void AssertIsNotAuthorized(Participant user) + { + _authService.LoginUser(user.AzureUniqueId); + Assert.Throws(() => + _mutation.ProgressParticipant( + evaluationId: _evaluation.Id, + newProgression: Randomize.Progression() + ) + ); + } + } +} From 82f57c6b8ad29c6719b6dc3347ba2df8cf06498d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Fri, 3 Sep 2021 16:32:30 +0200 Subject: [PATCH 15/16] Add backend auth-tests for SetSummary mutation --- backend/tests/GQL/Mutations/SetSummary.cs | 94 +++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 backend/tests/GQL/Mutations/SetSummary.cs diff --git a/backend/tests/GQL/Mutations/SetSummary.cs b/backend/tests/GQL/Mutations/SetSummary.cs new file mode 100644 index 00000000..572472ce --- /dev/null +++ b/backend/tests/GQL/Mutations/SetSummary.cs @@ -0,0 +1,94 @@ +using Xunit; +using System; +using System.Linq; + +using api.Models; +using api.Services; +using api.GQL; + + +namespace tests +{ + public class SetSummaryMutation: MutationTest + { + private readonly Evaluation _evaluation; + private readonly Participant _facilitator; + private readonly Participant _organizationLead; + private readonly Participant _participant; + private readonly Participant _readonly; + + public SetSummaryMutation() { + _evaluation = CreateEvaluation(); + _facilitator = _evaluation.Participants.First(); + _authService.LoginUser(_facilitator); + + _organizationLead = CreateParticipant(_evaluation, role: Role.OrganizationLead); + _participant = CreateParticipant(_evaluation, role: Role.Participant); + _readonly = CreateParticipant(_evaluation, role: Role.ReadOnly); + } + + /* Tests */ + + [Fact] + public void FacilitatorCanUseMutation() + { + AssertCanSet(_facilitator); + } + + [Fact] + public void OrganizationLeadIsUnauthorized() + { + AssertIsNotAuthorized(_organizationLead); + } + + [Fact] + public void ParticipantIsUnauthorized() + { + AssertIsNotAuthorized(_participant); + } + + [Fact] + public void ReadOnlyIsUnauthorized() + { + AssertIsNotAuthorized(_readonly); + } + + [Fact] + public void NonParcipantIsUnauthorized() + { + string AzureUniqueId = Randomize.Integer().ToString(); + AssertIsNotAuthorized(AzureUniqueId); + } + + /* Helper methods */ + + private void AssertCanSet(Participant user) + { + _authService.LoginUser(user); + + string summary = Randomize.String(); + _mutation.SetSummary( + evaluationId: _evaluation.Id, + summary: summary + ); + + Assert.True(_evaluation.Summary == summary); + } + + private void AssertIsNotAuthorized(string azureUniqueId) + { + _authService.LoginUser(azureUniqueId); + Assert.Throws(() => + _mutation.SetSummary( + evaluationId: _evaluation.Id, + summary: Randomize.String() + ) + ); + } + + private void AssertIsNotAuthorized(Participant user) + { + AssertIsNotAuthorized(user.AzureUniqueId); + } + } +} From 53e42d7b0620f82e663db4a127fb0941c16dc755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erlend=20H=C3=A5rstad?= Date: Mon, 6 Sep 2021 08:59:05 +0200 Subject: [PATCH 16/16] Safeguard against deleting last Facilitator --- backend/api/GQL/Mutation.cs | 18 +++++++++++ .../tests/GQL/Mutations/DeleteParticipant.cs | 30 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/backend/api/GQL/Mutation.cs b/backend/api/GQL/Mutation.cs index 87955212..212e53a0 100644 --- a/backend/api/GQL/Mutation.cs +++ b/backend/api/GQL/Mutation.cs @@ -133,6 +133,24 @@ public Participant DeleteParticipant(string participantId) 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); } diff --git a/backend/tests/GQL/Mutations/DeleteParticipant.cs b/backend/tests/GQL/Mutations/DeleteParticipant.cs index 5b811e9d..d39ed8b3 100644 --- a/backend/tests/GQL/Mutations/DeleteParticipant.cs +++ b/backend/tests/GQL/Mutations/DeleteParticipant.cs @@ -60,6 +60,36 @@ public void NonParcipantIsUnauthorized() AssertIsNotAuthorized(AzureUniqueId); } + [Fact] + public void CannotPerformOnNonLastFacilitator() + { + Evaluation evaluation = CreateEvaluation(); + Participant first = evaluation.Participants.First(); + _authService.LoginUser(first); + Participant second = CreateParticipant(evaluation, role: Role.Facilitator); + + int participants = evaluation.Participants.Count(); + Assert.Equal(2, participants); + + _authService.LoginUser(second); + _mutation.DeleteParticipant(first.Id); + + participants = evaluation.Participants.Count(); + Assert.Equal(1, participants); + } + + [Fact] + public void CannotPerformOnLastFacilitator() + { + Evaluation evaluation = CreateEvaluation(); + Participant facilitator = evaluation.Participants.First(); + _authService.LoginUser(facilitator); + + Assert.Throws(() => + _mutation.DeleteParticipant(facilitator.Id) + ); + } + /* Helper methods */ private void AssertCanDelete(Participant user)