ER-1446 Added check on vacancy owner in authorization (#851) +semver:…

… patch * ER-1446 Added check on vacancy owner in authorization * ER-1446 Added ownership check in the command handler
SkillsFundingAgency · Oct 25, 2019 · d237be1 · d237be1
1 parent 6fa2c62
commit d237be1
Show file tree

Hide file tree

Showing 18 changed files with 162 additions and 60 deletions.
diff --git a/src/.vscode/tasks.json b/src/.vscode/tasks.json
@@ -189,7 +189,7 @@
             "type": "process",
             "args": [
                 "test",
-                "${workspaceFolder}/Jobs/Recruit.Vacancies.Jobs.Tests/Recruit.Vacancies.Jobs.Tests.csproj",
+                "${workspaceFolder}/Jobs/Recruit.Vacancies.Jobs.UnitTests/Recruit.Vacancies.Jobs.UnitTests.csproj",
                 "/p:GenerateFullPaths=true"
             ]
         }

diff --git a/src/Employer/Employer.Web/Orchestrators/VacancyPreviewOrchestrator.cs b/src/Employer/Employer.Web/Orchestrators/VacancyPreviewOrchestrator.cs
@@ -10,9 +10,11 @@
 using Esfa.Recruit.Shared.Web.Helpers;
 using Esfa.Recruit.Shared.Web.Orchestrators;
 using Esfa.Recruit.Shared.Web.Services;
+using Esfa.Recruit.Vacancies.Client.Application.Commands;
 using Esfa.Recruit.Vacancies.Client.Application.Validation;
 using Esfa.Recruit.Vacancies.Client.Domain.Entities;
 using Esfa.Recruit.Vacancies.Client.Domain.Exceptions;
+using Esfa.Recruit.Vacancies.Client.Domain.Messaging;
 using Esfa.Recruit.Vacancies.Client.Infrastructure.Client;
 using Microsoft.Extensions.Logging;
 using ErrMsg = Esfa.Recruit.Shared.Web.ViewModels.ErrorMessages;
@@ -29,20 +31,23 @@ public class VacancyPreviewOrchestrator : EntityValidatingOrchestrator<Vacancy,
         private readonly DisplayVacancyViewModelMapper _vacancyDisplayMapper;
         private readonly IReviewSummaryService _reviewSummaryService;
         private readonly ILegalEntityAgreementService _legalEntityAgreementService;
+        private readonly IMessaging _messaging;
 
         public VacancyPreviewOrchestrator(
             IEmployerVacancyClient client,
             IRecruitVacancyClient vacancyClient,
             ILogger<VacancyPreviewOrchestrator> logger,
             DisplayVacancyViewModelMapper vacancyDisplayMapper, 
             IReviewSummaryService reviewSummaryService, 
-            ILegalEntityAgreementService legalEntityAgreementService) : base(logger)
+            ILegalEntityAgreementService legalEntityAgreementService,
+            IMessaging messaging) : base(logger)
         {
             _client = client;
             _vacancyClient = vacancyClient;
             _vacancyDisplayMapper = vacancyDisplayMapper;
             _reviewSummaryService = reviewSummaryService;
             _legalEntityAgreementService = legalEntityAgreementService;
+            _messaging = messaging;
         }
 
         public async Task<VacancyPreviewViewModel> GetVacancyPreviewViewModelAsync(VacancyRouteModel vrm)
@@ -117,7 +122,9 @@ private async Task<SubmitVacancyResponse> SubmitActionAsync(Vacancy vacancy, Vac
             if (response.HasLegalEntityAgreement == false)
                 return response;
 
-            await _client.SubmitVacancyAsync(vacancy.Id, vacancy.EmployerDescription, user);
+            var command = new SubmitVacancyCommand(vacancy.Id, user, vacancy.EmployerDescription, OwnerType.Employer);
+
+            await _messaging.SendCommandAsync(command);
 
             response.IsSubmitted = true;
 

diff --git a/src/Employer/Employer.Web/Utility.cs b/src/Employer/Employer.Web/Utility.cs
@@ -46,6 +46,8 @@ public static void CheckAuthorisedAccess(Vacancy vacancy, string employerAccount
         {
             if (!vacancy.EmployerAccountId.Equals(employerAccountId, StringComparison.OrdinalIgnoreCase))
                 throw new AuthorisationException(string.Format(ExceptionMessages.VacancyUnauthorisedAccess, employerAccountId, vacancy.EmployerAccountId, vacancy.Title, vacancy.Id));
+            if (vacancy.OwnerType != OwnerType.Employer)
+                throw new AuthorisationException(string.Format(ExceptionMessages.UserIsNotTheOwner, OwnerType.Employer));
         }
 
         public static void CheckRouteIsValidForVacancy(Vacancy vacancy, string currentRouteName)

diff --git a/src/Provider/Provider.Web/Orchestrators/VacancyPreviewOrchestrator.cs b/src/Provider/Provider.Web/Orchestrators/VacancyPreviewOrchestrator.cs
@@ -8,9 +8,11 @@
 using Esfa.Recruit.Provider.Web.ViewModels.VacancyPreview;
 using Esfa.Recruit.Shared.Web.Orchestrators;
 using Esfa.Recruit.Shared.Web.Services;
+using Esfa.Recruit.Vacancies.Client.Application.Commands;
 using Esfa.Recruit.Vacancies.Client.Application.Validation;
 using Esfa.Recruit.Vacancies.Client.Domain.Entities;
 using Esfa.Recruit.Vacancies.Client.Domain.Exceptions;
+using Esfa.Recruit.Vacancies.Client.Domain.Messaging;
 using Esfa.Recruit.Vacancies.Client.Infrastructure.Client;
 using Microsoft.Extensions.Logging;
 using ErrMsg = Esfa.Recruit.Shared.Web.ViewModels.ErrorMessages;
@@ -28,6 +30,7 @@ public class VacancyPreviewOrchestrator : EntityValidatingOrchestrator<Vacancy,
         private readonly IReviewSummaryService _reviewSummaryService;
         private readonly ILegalEntityAgreementService _legalEntityAgreementService;
         private readonly ITrainingProviderAgreementService _trainingProviderAgreementService;
+        private readonly IMessaging _messaging;
 
         public VacancyPreviewOrchestrator(
             IProviderVacancyClient client,
@@ -36,14 +39,16 @@ public VacancyPreviewOrchestrator(
             DisplayVacancyViewModelMapper vacancyDisplayMapper, 
             IReviewSummaryService reviewSummaryService,
             ILegalEntityAgreementService legalEntityAgreementService,
-            ITrainingProviderAgreementService trainingProviderAgreementService) : base(logger)
+            ITrainingProviderAgreementService trainingProviderAgreementService,
+            IMessaging messaging) : base(logger)
         {
             _client = client;
             _vacancyClient = vacancyClient;
             _vacancyDisplayMapper = vacancyDisplayMapper;
             _reviewSummaryService = reviewSummaryService;
             _legalEntityAgreementService = legalEntityAgreementService;
             _trainingProviderAgreementService = trainingProviderAgreementService;
+            _messaging = messaging;
         }
 
         public async Task<VacancyPreviewViewModel> GetVacancyPreviewViewModelAsync(VacancyRouteModel vrm)
@@ -94,7 +99,7 @@ public async Task<OrchestratorResponse<SubmitVacancyResponse>> SubmitVacancyAsyn
             return await ValidateAndExecute(
                 vacancy,
                 v => ValidateVacancy(v, SubmitValidationRules),
-                v => SubmitActionAsync(v, user)
+                v => SubmitActionAsync(vacancy, user)
                 );
         }
 
@@ -114,7 +119,9 @@ private async Task<SubmitVacancyResponse> SubmitActionAsync(Vacancy vacancy, Vac
 
             if (response.HasLegalEntityAgreement && response.HasProviderAgreement)
             {
-                await _client.SubmitVacancyAsync(vacancy.Id, user);
+                var command = new SubmitVacancyCommand(vacancy.Id, user, OwnerType.Provider);
+
+                await _messaging.SendCommandAsync(command);
                 response.IsSubmitted = true;
             }
 

diff --git a/src/Provider/Provider.Web/Utility.cs b/src/Provider/Provider.Web/Utility.cs
@@ -46,6 +46,8 @@ public static void CheckAuthorisedAccess(Vacancy vacancy, long ukprn)
         {
             if (vacancy.TrainingProvider.Ukprn.Value != ukprn)
                 throw new AuthorisationException(string.Format(ExceptionMessages.VacancyUnauthorisedAccessForProvider, ukprn, vacancy.TrainingProvider.Ukprn, vacancy.Title, vacancy.Id));
+            if (vacancy.OwnerType != OwnerType.Provider)
+                throw new AuthorisationException(string.Format(ExceptionMessages.UserIsNotTheOwner, OwnerType.Provider));
         }
 
         public static void CheckRouteIsValidForVacancy(Vacancy vacancy, string currentRouteName, VacancyRouteModel vrm)

diff --git a/...ovider.Web/Orchestrators/CloneVacancyOrchestratorTest/CloneVacancyOrchestratorTestBase.cs b/...ovider.Web/Orchestrators/CloneVacancyOrchestratorTest/CloneVacancyOrchestratorTestBase.cs
@@ -26,7 +26,8 @@ public abstract class CloneVacancyOrchestratorTestBase
             TrainingProvider = TrainingProvider,
             Status = VacancyStatus.Live,
             StartDate = SourceStartDate,
-            ClosingDate = SourceClosingDate
+            ClosingDate = SourceClosingDate,
+            OwnerType = OwnerType.Provider
         };
         internal VacancyRouteModel VRM => new VacancyRouteModel
         {

diff --git a/src/Provider/UnitTests/Provider.Web/Orchestrators/Part1/LegalEntityOrchestratorTests.cs b/src/Provider/UnitTests/Provider.Web/Orchestrators/Part1/LegalEntityOrchestratorTests.cs
@@ -72,7 +72,8 @@ private Vacancy GetTestVacancy()
                 Wage = new Wage
                 {
                     WageType = WageType.NationalMinimumWage
-                }
+                },
+                OwnerType = OwnerType.Provider
             };
         }
     }

diff --git a/src/Provider/UnitTests/Provider.Web/Orchestrators/Part2/SkillOrchestratorTests.cs b/src/Provider/UnitTests/Provider.Web/Orchestrators/Part2/SkillOrchestratorTests.cs
@@ -196,7 +196,8 @@ private static Vacancy GetTestVacancy()
                     Duration = 1,
                     WageType = WageType.NationalMinimumWage
                 },
-                StartDate = DateTime.Now
+                StartDate = DateTime.Now,
+                OwnerType = OwnerType.Provider
             };
         }
 

diff --git a/src/Provider/UnitTests/Provider.Web/Orchestrators/VacancyPreviewOrchestratorTests.cs b/src/Provider/UnitTests/Provider.Web/Orchestrators/VacancyPreviewOrchestratorTests.cs
@@ -12,6 +12,7 @@
 using Esfa.Recruit.Vacancies.Client.Application.Rules.VacancyRules;
 using Esfa.Recruit.Vacancies.Client.Application.Validation;
 using Esfa.Recruit.Vacancies.Client.Domain.Entities;
+using Esfa.Recruit.Vacancies.Client.Domain.Messaging;
 using Esfa.Recruit.Vacancies.Client.Infrastructure.Client;
 using Esfa.Recruit.Vacancies.Client.Infrastructure.Services.PasAccount;
 using FluentAssertions;
@@ -29,7 +30,8 @@ public class VacancyPreviewOrchestratorTests
         [InlineData(true, false, false)]
         [InlineData(false, true, false)]
         [InlineData(false, false, false)]
-        public async Task SubmitVacancyAsync_ShouldNotSubmitWhenMissingAgreements(bool hasLegalEntityAgreement, bool hasProviderAgreement, bool shouldBeSubmitted)
+        public async Task SubmitVacancyAsync_ShouldNotSubmitWhenMissingAgreements(
+            bool hasLegalEntityAgreement, bool hasProviderAgreement, bool shouldBeSubmitted)
         {
             var vacancyId = Guid.NewGuid();
             const long ukprn = 12345678;
@@ -44,9 +46,9 @@ public async Task SubmitVacancyAsync_ShouldNotSubmitWhenMissingAgreements(bool h
             vacancy.IsDeleted = false;
             vacancy.EmployerAccountId = employerAccountId;
             vacancy.LegalEntityId = legalEntityId;
+            vacancy.OwnerType = OwnerType.Provider;
 
             var client = new Mock<IProviderVacancyClient>();
-
 
             var vacancyClient = new Mock<IRecruitVacancyClient>();
             vacancyClient.Setup(c => c.GetVacancyAsync(vacancyId))
@@ -74,7 +76,11 @@ public async Task SubmitVacancyAsync_ShouldNotSubmitWhenMissingAgreements(bool h
             agreementServiceMock.Setup(t => t.HasAgreementAsync(ukprn))
                 .ReturnsAsync(hasProviderAgreement);
 
-            var orch = new VacancyPreviewOrchestrator(client.Object, vacancyClient.Object, logger.Object, mapper, review.Object, legalEntityAgreement.Object, agreementServiceMock.Object);
+            var messagingMock = new Mock<IMessaging>();
+
+            var orch = new VacancyPreviewOrchestrator(
+                client.Object, vacancyClient.Object, logger.Object, mapper, review.Object, 
+                legalEntityAgreement.Object, agreementServiceMock.Object, messagingMock.Object);
 
             var m = new SubmitEditModel
             {
@@ -87,7 +93,7 @@ public async Task SubmitVacancyAsync_ShouldNotSubmitWhenMissingAgreements(bool h
             var actualResponse = await orch.SubmitVacancyAsync(m, user);
 
             var submittedTimes = shouldBeSubmitted ? Times.Once() : Times.Never();
-            client.Verify(c => c.SubmitVacancyAsync(vacancyId, user), submittedTimes);
+            messagingMock.Verify(c => c.SendCommandAsync(It.IsAny<ICommand>()), submittedTimes);
 
             actualResponse.Data.IsSubmitted.Should().Be(shouldBeSubmitted);
             actualResponse.Data.HasLegalEntityAgreement.Should().Be(hasLegalEntityAgreement);

diff --git a/src/Provider/UnitTests/Provider.Web/UtilityTests/GetAuthorisedApplicationReviewAsyncTests.cs b/src/Provider/UnitTests/Provider.Web/UtilityTests/GetAuthorisedApplicationReviewAsyncTests.cs
@@ -34,7 +34,8 @@ public GetAuthorisedApplicationReviewAsyncTests()
             new Vacancy()
             {
                 Id = _vacancyId,
-                TrainingProvider = new TrainingProvider { Ukprn = _applicationReviewUkprn }
+                TrainingProvider = new TrainingProvider { Ukprn = _applicationReviewUkprn },
+                OwnerType = OwnerType.Provider
             });
         }
 

diff --git a/...hared/Recruit.Vacancies.Client/Application/CommandHandlers/SubmitVacancyCommandHandler.cs b/...hared/Recruit.Vacancies.Client/Application/CommandHandlers/SubmitVacancyCommandHandler.cs
@@ -15,6 +15,10 @@ namespace Esfa.Recruit.Vacancies.Client.Application.CommandHandlers
 {
     public class SubmitVacancyCommandHandler : IRequestHandler<SubmitVacancyCommand>
     {
+        public const string VacancyNotFoundExceptionMessageFormat = "Vacancy {0} not found";
+        public const string InvalidStateExceptionMessageFormat = "Unable to submit vacancy {0} due to vacancy having a status of {1}.";
+        public const string InvalidOwnerExceptionMessageFormat = "The vacancy {0} owner has changed from {1} to {2} and hence cannot be submitted.";
+        public const string MissingReferenceNumberExceptionMessageFormat = "Cannot submit vacancy {0} without a vacancy reference number";
         private readonly ILogger<SubmitVacancyCommandHandler> _logger;
         private readonly IVacancyRepository _vacancyRepository;
         private readonly IMessaging _messaging;
@@ -41,14 +45,17 @@ public async Task Handle(SubmitVacancyCommand message, CancellationToken cancell
 
             var vacancy = await _vacancyRepository.GetVacancyAsync(message.VacancyId);
 
-            if (vacancy == null || vacancy.CanSubmit == false)
-            {
-                _logger.LogWarning($"Unable to submit vacancy {{vacancyId}} due to vacancy having a status of {vacancy?.Status}.", message.VacancyId);
-                return;
-            }
+            if (vacancy == null) 
+                throw new ArgumentException(string.Format(VacancyNotFoundExceptionMessageFormat, message.VacancyId));
 
             if (vacancy.VacancyReference.HasValue == false)
-                throw new Exception("Cannot submit vacancy without a vacancy reference");
+                throw new InvalidOperationException(string.Format(MissingReferenceNumberExceptionMessageFormat, vacancy.Id));
+
+            if(vacancy.CanSubmit == false)
+                throw new InvalidOperationException(string.Format(InvalidStateExceptionMessageFormat, vacancy.Id, vacancy.Status));
+
+            if(vacancy.OwnerType != message.SubmissionOwner)
+                throw new InvalidOperationException(string.Format(InvalidOwnerExceptionMessageFormat, vacancy.Id, message.SubmissionOwner, vacancy.OwnerType));
 
             var now = _timeProvider.Now;
 

diff --git a/src/Shared/Recruit.Vacancies.Client/Application/Commands/SubmitVacancyCommand.cs b/src/Shared/Recruit.Vacancies.Client/Application/Commands/SubmitVacancyCommand.cs
@@ -7,21 +7,24 @@ namespace Esfa.Recruit.Vacancies.Client.Application.Commands
 {
     public class SubmitVacancyCommand : ICommand, IRequest
     {
-        public SubmitVacancyCommand(Guid vacancyId, VacancyUser user)
+        public SubmitVacancyCommand(Guid vacancyId, VacancyUser user, OwnerType submissionOwner)
         {
             VacancyId = vacancyId;
             User = user;
+            SubmissionOwner = submissionOwner;
         }
 
-        public SubmitVacancyCommand(Guid vacancyId, VacancyUser user, string employerDescription)
+        public SubmitVacancyCommand(Guid vacancyId, VacancyUser user, string employerDescription, OwnerType submissionOwner)
         {
             VacancyId = vacancyId;
             User = user;
             EmployerDescription = employerDescription;
+            SubmissionOwner = submissionOwner;
         }
 
         public Guid VacancyId { get;}
         public VacancyUser User { get; }
         public string EmployerDescription { get; }
+        public OwnerType SubmissionOwner { get; }
     }
 }
diff --git a/src/Shared/Recruit.Vacancies.Client/Application/Exceptions/ExceptionMessages.cs b/src/Shared/Recruit.Vacancies.Client/Application/Exceptions/ExceptionMessages.cs
@@ -6,6 +6,7 @@ public static class ExceptionMessages
     {
         public const string VacancyUnauthorisedAccess = "The employer account '{0}' cannot access employer account '{1}' for vacancy '{2} ({3})'.";
         public const string VacancyUnauthorisedAccessForProvider = "The provider account '{0}' cannot access provider account '{1}' for vacancy '{2} ({3})'.";
+        public const string UserIsNotTheOwner = "The {0} user is not the owner of the vacancy";
         public const string VacancyWithReferenceNotFound = "Unable to find vacancy with reference: {0}.";
         public const string VacancyWithIdNotFound = "Unable to find vacancy with id: {0}.";
         public const string ApplicationReviewUnauthorisedAccess = "The employer account '{0}' cannot access employer account '{1}' application '{2}' for vacancy '{3}'.";

diff --git a/src/Shared/Recruit.Vacancies.Client/Infrastructure/Client/IEmployerVacancyClient.cs b/src/Shared/Recruit.Vacancies.Client/Infrastructure/Client/IEmployerVacancyClient.cs
@@ -13,7 +13,6 @@ public interface IEmployerVacancyClient
     {
         Task<Guid> CreateVacancyAsync(string title, string employerAccountId, VacancyUser user);
         Task GenerateDashboard(string employerAccountId);
-        Task SubmitVacancyAsync(Guid vacancyId, string employerDescription, VacancyUser user);
         Task DeleteVacancyAsync(Guid vacancyId, VacancyUser user);
         Task<EmployerDashboard> GetDashboardAsync(string employerAccountId, bool createIfNonExistent = false);
         Task<EditVacancyInfo> GetEditVacancyInfoAsync(string employerAccountId);

diff --git a/src/Shared/Recruit.Vacancies.Client/Infrastructure/Client/IProviderVacancyClient.cs b/src/Shared/Recruit.Vacancies.Client/Infrastructure/Client/IProviderVacancyClient.cs
@@ -16,7 +16,6 @@ public interface IProviderVacancyClient
         Task SetupProviderAsync(long ukprn);
         Task<ProviderEditVacancyInfo> GetProviderEditVacancyInfoAsync(long ukprn);
         Task<EmployerInfo> GetProviderEmployerVacancyDataAsync(long ukprn, string employerAccountId);
-        Task SubmitVacancyAsync(Guid vacancyId, VacancyUser user);
         Task DeleteVacancyAsync(Guid vacancyId, VacancyUser user);
         Task<Guid> CreateProviderApplicationsReportAsync(long ukprn, DateTime fromDate, DateTime toDate, VacancyUser user, string reportName);
         Task<List<ReportSummary>> GetReportsForProviderAsync(long ukprn);

diff --git a/src/Shared/Recruit.Vacancies.Client/Infrastructure/Client/ProviderVacancyClient.cs b/src/Shared/Recruit.Vacancies.Client/Infrastructure/Client/ProviderVacancyClient.cs
@@ -64,13 +64,6 @@ public Task SetupProviderAsync(long ukprn)
             return _messaging.SendCommandAsync(command);
         }
 
-        public Task SubmitVacancyAsync(Guid vacancyId, VacancyUser user)
-        {
-            var command = new SubmitVacancyCommand(vacancyId, user);
-
-            return _messaging.SendCommandAsync(command);
-        }
-
         public async Task<Guid> CreateProviderApplicationsReportAsync(long ukprn, DateTime fromDate, DateTime toDate, VacancyUser user, string reportName)
         {
             var reportId = Guid.NewGuid();

diff --git a/src/Shared/Recruit.Vacancies.Client/Infrastructure/Client/VacancyClient.cs b/src/Shared/Recruit.Vacancies.Client/Infrastructure/Client/VacancyClient.cs
@@ -167,13 +167,6 @@ private Guid GenerateVacancyId()
             return Guid.NewGuid();
         }
 
-        public Task SubmitVacancyAsync(Guid vacancyId, string employerDescription, VacancyUser user)
-        {
-            var command = new SubmitVacancyCommand(vacancyId, user, employerDescription);
-
-            return _messaging.SendCommandAsync(command);
-        }
-
         public Task DeleteVacancyAsync(Guid vacancyId, VacancyUser user)
         {
             var command = new DeleteVacancyCommand
-Original file line number
+Diff line change
@@ Expand Up / @@ -72,7 +72,8 @@ private Vacancy GetTestVacancy() @@
                     Wage = new Wage
                     {
                         WageType = WageType.NationalMinimumWage
-                    }
+                    },
+                    OwnerType = OwnerType.Provider
                 };
             }
         }
@@ Expand Down @@