Skip to content

Commit

Permalink
Merge pull request #1059 from openkfw/1053-document-bug
Browse files Browse the repository at this point in the history
Add document checks before sharing
  • Loading branch information
georgimld committed Mar 29, 2022
2 parents fdf12d2 + 2243b47 commit 9e1a6d7
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 24 deletions.
17 changes: 17 additions & 0 deletions api/src/service/document_share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
},
});
},
});
});

Expand Down
28 changes: 26 additions & 2 deletions api/src/service/domain/document/document_share.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand All @@ -64,7 +85,7 @@ const baseWorkflowitem: Workflowitem = {
displayName: "dummy",
description: "dummy",
amountType: "N/A",
documents: [],
documents,
permissions: { "workflowitem.intent.grantPermission": ["alice"] },
log: [],
additionalData: {},
Expand All @@ -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 () => {
Expand All @@ -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")),
},
);
Expand Down
27 changes: 21 additions & 6 deletions api/src/service/domain/document/document_share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -32,6 +33,7 @@ interface Repository {
subprojectId: string,
workflowitemId: string,
): Promise<Result.Type<Workflowitem.Workflowitem>>;
getDocumentInfo(docId: string): Promise<Result.Type<DocumentUploaded.Document | undefined>>;
}

export async function shareDocument(
Expand All @@ -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;
}

Expand Down Expand Up @@ -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(
Expand Down
17 changes: 17 additions & 0 deletions api/src/service/workflowitem_permission_grant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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),
Expand Down
16 changes: 5 additions & 11 deletions e2e-test/cypress/integration/documents_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -34,7 +34,7 @@ describe("Attaching a document to a workflowitem.", function() {
});
});

beforeEach(function() {
beforeEach(function () {
cy.login();
cy.visit(`/projects/${projectId}/${subprojectId}`);
});
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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})`);
}
});
});
10 changes: 5 additions & 5 deletions e2e-test/cypress/integration/status_spec.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
describe("Component Versions", function() {
describe("Component Versions", function () {
let exportUrl, apiBaseUrl, apiUrl;
before(() => {
apiBaseUrl = Cypress.env("API_BASE_URL") || `${Cypress.config("baseUrl")}/test`;
apiUrl = apiBaseUrl + "/api";
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());
Expand All @@ -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`);
Expand Down

0 comments on commit 9e1a6d7

Please sign in to comment.