From d897e0e065e4e4335b0286e347bed1f232b8e396 Mon Sep 17 00:00:00 2001 From: "Moldovan, Georgia" Date: Wed, 23 Mar 2022 17:35:03 +0100 Subject: [PATCH 1/3] api: add document checks before sharing - before sharing a document it should be checked if document exists and if it is a document stored in the storage service --- api/src/service/document_share.ts | 17 ++++++++++++ .../service/domain/document/document_share.ts | 27 ++++++++++++++----- .../service/workflowitem_permission_grant.ts | 17 ++++++++++++ 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/api/src/service/document_share.ts b/api/src/service/document_share.ts index 47098b0f9..aef366988 100644 --- a/api/src/service/document_share.ts +++ b/api/src/service/document_share.ts @@ -8,6 +8,7 @@ import * as Cache from "./cache2"; import { ConnToken } from "./conn"; import { sourceSecrets } from "./domain/document/document_eventsourcing"; import * as DocumentShare from "./domain/document/document_share"; +import * as DocumentGet from "./domain/document/document_get"; import * as SecretGet from "./domain/document/secret_get"; import { ServiceUser } from "./domain/organization/service_user"; import * as PublicKeyGet from "./public_key_get"; @@ -54,6 +55,22 @@ export async function documentShare( getWorkflowitem: async (projectId, subprojectId, workflowitemId) => { return cache.getWorkflowitem(projectId, subprojectId, workflowitemId); }, + getDocumentInfo: async (docId) => { + return DocumentGet.getDocumentInfo(ctx, docId, { + getDocumentsEvents: async () => { + return cache.getDocumentUploadedEvents(); + }, + getAllProjects: async () => { + return cache.getProjects(); + }, + getAllSubprojects: async (projectId) => { + return cache.getSubprojects(projectId); + }, + getAllWorkflowitems: async (projectId, subprojectId) => { + return cache.getWorkflowitems(projectId, subprojectId); + }, + }); + }, }); }); diff --git a/api/src/service/domain/document/document_share.ts b/api/src/service/domain/document/document_share.ts index cee2f8a5a..b96434b22 100644 --- a/api/src/service/domain/document/document_share.ts +++ b/api/src/service/domain/document/document_share.ts @@ -9,6 +9,7 @@ import * as Workflowitem from "../workflow/workflowitem"; import { NotAuthorized } from "../errors/not_authorized"; import { PreconditionError } from "../errors/precondition_error"; import logger from "lib/logger"; +import * as DocumentUploaded from "../document/document_uploaded"; type Base64String = string; @@ -32,6 +33,7 @@ interface Repository { subprojectId: string, workflowitemId: string, ): Promise>; + getDocumentInfo(docId: string): Promise>; } export async function shareDocument( @@ -48,6 +50,25 @@ export async function shareDocument( // if secret is already published for this document and organization no event is created const alreadyPublished = await repository.secretAlreadyExists(docId, organization); if (alreadyPublished) { + logger.debug( + { docId, publisherOrganization }, + "Secret is already shared with this organization", + ); + return undefined; + } + + const workflowitem = await repository.getWorkflowitem(projectId, subprojectId, workflowitemId); + if (Result.isErr(workflowitem)) { + return new VError(" Error while fetching workflowitem!"); + } + + const { documents } = workflowitem; + if (!documents.some((doc) => doc.id === docId)) { + return new VError(`No documents with id ${docId} found in workflowitem ${workflowitemId}`); + } + const documentInfo = await repository.getDocumentInfo(docId); + if (!documentInfo) { + logger.debug({ docId, workflowitemId }, "No such document attached to this workflowitem"); return undefined; } @@ -114,12 +135,6 @@ export async function shareDocument( ); } - const workflowitem = await repository.getWorkflowitem(projectId, subprojectId, workflowitemId); - - if (Result.isErr(workflowitem)) { - return new VError(" Error while fetching workflowitem!"); - } - const intent = "workflowitem.intent.grantPermission"; logger.trace( diff --git a/api/src/service/workflowitem_permission_grant.ts b/api/src/service/workflowitem_permission_grant.ts index 82aca0bfe..56512940a 100644 --- a/api/src/service/workflowitem_permission_grant.ts +++ b/api/src/service/workflowitem_permission_grant.ts @@ -9,6 +9,7 @@ import * as Result from "../result"; import * as Cache from "./cache2"; import { ConnToken } from "./conn"; import * as DocumentShare from "./domain/document/document_share"; +import * as DocumentGet from "./domain/document/document_get"; import * as SecretGet from "./domain/document/secret_get"; import { Identity } from "./domain/organization/identity"; import { ServiceUser } from "./domain/organization/service_user"; @@ -92,6 +93,22 @@ export async function grantWorkflowitemPermission( getWorkflowitem: async (projectId, subprojectId, workflowitemId) => { return cache.getWorkflowitem(projectId, subprojectId, workflowitemId); }, + getDocumentInfo: async (docId) => { + return DocumentGet.getDocumentInfo(ctx, docId, { + getDocumentsEvents: async () => { + return cache.getDocumentUploadedEvents(); + }, + getAllProjects: async () => { + return cache.getProjects(); + }, + getAllSubprojects: async (projectId) => { + return cache.getSubprojects(projectId); + }, + getAllWorkflowitems: async (projectId, subprojectId) => { + return cache.getWorkflowitems(projectId, subprojectId); + }, + }); + }, }, ), groupExists: async (group) => GroupQuery.groupExists(conn, ctx, serviceUser, group), From 8dee8f34951d0d13871a05155f3509be72bf1936 Mon Sep 17 00:00:00 2001 From: "Moldovan, Georgia" Date: Tue, 29 Mar 2022 09:58:51 +0200 Subject: [PATCH 2/3] api: update unit tests --- .../domain/document/document_share.spec.ts | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/api/src/service/domain/document/document_share.spec.ts b/api/src/service/domain/document/document_share.spec.ts index 7bf12aed6..192918e38 100644 --- a/api/src/service/domain/document/document_share.spec.ts +++ b/api/src/service/domain/document/document_share.spec.ts @@ -3,11 +3,12 @@ import { Ctx } from "lib/ctx"; import * as Result from "../../../result"; import { ServiceUser } from "../organization/service_user"; import { UserRecord } from "../organization/user_record"; -import { UploadedDocument } from "./document"; +import { StoredDocument, UploadedDocument } from "./document"; import { RequestData, shareDocument } from "./document_share"; import { Workflowitem } from "../workflow/workflowitem"; import { NotAuthorized } from "../errors/not_authorized"; import { PreconditionError } from "../errors/precondition_error"; +import * as DocumentUploaded from "./document_uploaded"; const ctx: Ctx = { requestId: "test", @@ -54,6 +55,26 @@ const requestData: RequestData = { subprojectId, workflowitemId, }; + +const documentId = "1"; +const documentHash = "hash"; +const documentFileName = "name"; +const organization = "organization"; +const organizationUrl = "url"; + +const documents: StoredDocument[] = [ + { + id: documentId, + hash: documentHash, + fileName: documentFileName, + }, +]; +const documentInfo: DocumentUploaded.Document = { + id: documentId, + fileName: documentFileName, + organization, + organizationUrl, +}; const baseWorkflowitem: Workflowitem = { isRedacted: false, id: workflowitemId, @@ -64,7 +85,7 @@ const baseWorkflowitem: Workflowitem = { displayName: "dummy", description: "dummy", amountType: "N/A", - documents: [], + documents, permissions: { "workflowitem.intent.grantPermission": ["alice"] }, log: [], additionalData: {}, @@ -79,6 +100,7 @@ const repository = { getSecret: (docId, organization) => Promise.resolve(secret), secretAlreadyExists: (docId, organization) => Promise.resolve(false), getWorkflowitem: (projectId, subprojectId, workflowitemId) => Promise.resolve(baseWorkflowitem), + getDocumentInfo: (docId) => Promise.resolve(documentInfo), }; describe("Share a document", async () => { @@ -104,6 +126,8 @@ describe("Share a document", async () => { { ...requestData, docId: "-1" }, { ...repository, + getWorkflowitem: (projectId, subprojectId, workflowitemId) => + Promise.resolve({ ...baseWorkflowitem, documents: [] }), getSecret: (docId, organization) => Promise.resolve(new Error("Could not find")), }, ); From 2243b479eddbf1fbef1fa23a275d37406febaeed Mon Sep 17 00:00:00 2001 From: "Moldovan, Georgia" Date: Tue, 29 Mar 2022 11:19:05 +0200 Subject: [PATCH 3/3] e2e-test: Fix tests related to storage service --- e2e-test/cypress/integration/documents_spec.js | 16 +++++----------- e2e-test/cypress/integration/status_spec.js | 10 +++++----- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/e2e-test/cypress/integration/documents_spec.js b/e2e-test/cypress/integration/documents_spec.js index 826898ec2..377f1fc86 100644 --- a/e2e-test/cypress/integration/documents_spec.js +++ b/e2e-test/cypress/integration/documents_spec.js @@ -9,7 +9,7 @@ let isExternalStorageEnabled = false; // Actual file in fixture folder const fileName = "documents_test.json"; -describe("Attaching a document to a workflowitem.", function() { +describe("Attaching a document to a workflowitem.", function () { before(() => { baseUrl = Cypress.env("API_BASE_URL") || `${Cypress.config("baseUrl")}/test`; apiRoute = baseUrl.toLowerCase().includes("test") ? "/test/api" : "/api"; @@ -34,7 +34,7 @@ describe("Attaching a document to a workflowitem.", function() { }); }); - beforeEach(function() { + beforeEach(function () { cy.login(); cy.visit(`/projects/${projectId}/${subprojectId}`); }); @@ -60,7 +60,7 @@ describe("Attaching a document to a workflowitem.", function() { return cy.get("[data-test=workflowitemDocumentFileName]").should("contain", fileName); }; - it("A document can be validated.", function() { + it("A document can be validated.", function () { cy.intercept(apiRoute + "/workflowitem.update*").as("update"); cy.intercept(apiRoute + "/subproject.viewDetails*").as("viewDetails"); cy.intercept(apiRoute + "/workflowitem.validate*").as("validate"); @@ -97,7 +97,7 @@ describe("Attaching a document to a workflowitem.", function() { .should("contain", "Identical"); }); - it("Validation of wrong document fails.", function() { + it("Validation of wrong document fails.", function () { cy.intercept(apiRoute + "/workflowitem.update*").as("update"); cy.intercept(apiRoute + "/subproject.viewDetails*").as("viewDetails"); cy.intercept(apiRoute + "/workflowitem.validate*").as("validate"); @@ -135,7 +135,7 @@ describe("Attaching a document to a workflowitem.", function() { .should("contain", "Different"); }); - it("The filename and document name are shown correctly", function() { + it("The filename and document name are shown correctly", function () { cy.intercept(apiRoute + "/workflowitem.update*").as("update"); cy.intercept(apiRoute + "/subproject.viewDetails*").as("viewDetails"); cy.intercept(apiRoute + "/workflowitem.validate*").as("validate"); @@ -163,11 +163,5 @@ describe("Attaching a document to a workflowitem.", function() { .should("be.visible") .contains(fileName); - // Check filename if saved on external storage (minio) - if (isExternalStorageEnabled) { - cy.get("[data-test=workflowitemDocumentFileName]") - .should("be.visible") - .contains(`(${fileName})`); - } }); }); diff --git a/e2e-test/cypress/integration/status_spec.js b/e2e-test/cypress/integration/status_spec.js index c76b34045..e0af5d7b0 100644 --- a/e2e-test/cypress/integration/status_spec.js +++ b/e2e-test/cypress/integration/status_spec.js @@ -1,4 +1,4 @@ -describe("Component Versions", function() { +describe("Component Versions", function () { let exportUrl, apiBaseUrl, apiUrl; before(() => { apiBaseUrl = Cypress.env("API_BASE_URL") || `${Cypress.config("baseUrl")}/test`; @@ -6,16 +6,16 @@ describe("Component Versions", function() { exportUrl = Cypress.env("EXPORT_SERVICE_BASE_URL") || `${Cypress.config("baseUrl")}/test/api/export/xlsx`; }); - it("Shows status list", function() { + it("Shows status list", function () { cy.login(); cy.visit(`/status`); cy.get("[data-test=status-table-body]") .should("be.visible") .children() - .should("have.length", 4); + .should("have.length", 5); }); - it("Shows connection of export-service", function() { + it("Shows connection of export-service", function () { cy.intercept(exportUrl + "/readiness*").as("isExportReady"); cy.visit("/"); cy.wait("@isExportReady").then(() => loginUi()); @@ -25,7 +25,7 @@ describe("Component Versions", function() { .should("be.visible"); }); - it("Shows versions of basic services (frontend,api,blockchain,multichain) correctly", function() { + it("Shows versions of basic services (frontend,api,blockchain,multichain) correctly", function () { cy.intercept(apiUrl + "/version").as("fetchVersions"); cy.login(); cy.visit(`/status`);