From 1e97d8e36513808eddeb42382114dad3188e324c Mon Sep 17 00:00:00 2001 From: Giovanni Pellerano Date: Sun, 9 Feb 2025 11:34:17 +0100 Subject: [PATCH] [ci] Improve testing of file api handlers and UI --- backend/globaleaks/handlers/admin/file.py | 16 ++-- .../tests/handlers/admin/test_file.py | 74 +++++++++++++------ .../tests/handlers/recipient/test_rfile.py | 7 +- .../globaleaks/tests/handlers/test_file.py | 2 +- backend/globaleaks/tests/helpers.py | 42 +++++------ .../questionnaires-list.component.html | 2 +- .../admin/settings/tab2/tab2.component.html | 4 +- .../admin/settings/tab2/tab2.component.ts | 3 +- .../admin-file/admin-file.component.ts | 9 ++- .../e2e/12-test-admin-configure-files.cy.ts | 43 +---------- .../e2e/17-test-admin-questionnaires.cy.ts | 12 ++- 11 files changed, 113 insertions(+), 101 deletions(-) diff --git a/backend/globaleaks/handlers/admin/file.py b/backend/globaleaks/handlers/admin/file.py index faedadcf9e..b5a6dbdfe0 100644 --- a/backend/globaleaks/handlers/admin/file.py +++ b/backend/globaleaks/handlers/admin/file.py @@ -108,13 +108,15 @@ class FileInstance(BaseHandler): ] def permission_check(self, name): - if name in ['css', 'favicon', 'script'] and \ - not self.session.has_permission('can_upload_files'): - raise errors.InvalidAuthentication + if self.session.role == 'admin': + if name not in ['logo'] and \ + not self.session.has_permission('can_upload_files'): + raise errors.InvalidAuthentication - if self.session.role != 'admin' and \ - not self.session.has_permission('can_edit_general_settings'): - raise errors.InvalidAuthentication + else: + if name not in ['logo'] or \ + not self.session.has_permission('can_edit_general_settings'): + raise errors.InvalidAuthentication @inlineCallbacks def post(self, name): @@ -130,7 +132,7 @@ def post(self, name): self.allowed_mimetypes = ['text/javascript'] if self.uploaded_file['type'] not in self.allowed_mimetypes: - raise errors.ForbiddenOperation + raise errors.InputValidationError if name in special_files or re.match(requests.uuid_regexp, name): self.uploaded_file['name'] = name diff --git a/backend/globaleaks/tests/handlers/admin/test_file.py b/backend/globaleaks/tests/handlers/admin/test_file.py index 2467152ffd..3d842704f8 100644 --- a/backend/globaleaks/tests/handlers/admin/test_file.py +++ b/backend/globaleaks/tests/handlers/admin/test_file.py @@ -2,48 +2,78 @@ from twisted.internet.defer import inlineCallbacks from globaleaks.handlers.admin import file +from globaleaks.rest import errors from globaleaks.tests import helpers +files = [ + {'handler': 'css', 'name': 'file.css'}, + {'handler': 'script', 'name': 'file.js'}, + {'handler': 'logo', 'name': 'file.png'}, + {'handler': 'favicon', 'name': 'file.ico'}, + {'handler': 'custom', 'name': 'file.txt'}, +] + class TestFileInstance(helpers.TestHandler): _handler = file.FileInstance @inlineCallbacks - def test_post(self): - handler = self.request({}, role='admin') + def test_post_prevent_unauthorized_admin_uploads(self): + for f in files: + handler = self.request({}, role='admin', attachment=self.get_dummy_attachment(f['name'])) + if f['handler'] == 'logo': + yield handler.post(f['handler']) + else: + yield self.assertFailure(handler.post(f['handler']), errors.InvalidAuthentication) + + @inlineCallbacks + def test_post_prevent_wrong_filetypes(self): + permissions = {'can_upload_files': True} - yield handler.post(u'file.pdf') + for f in files: + handler = self.request({}, role='admin', permissions=permissions, attachment=self.get_dummy_attachment(f['name'] + ".wrong.ext")) + yield self.assertFailure(handler.post(f['handler']), errors.InputValidationError) @inlineCallbacks - def test_delete(self): - handler = self.request({}, role='admin') - yield handler.delete(u'file.pdf') + def test_post_accepts_correct_files(self): + permissions = {'can_upload_files': True} + for f in files: + handler = self.request({}, role='admin', permissions=permissions, attachment=self.get_dummy_attachment(f['name'])) + yield handler.post(f['handler']) -class TestFileCollection(helpers.TestHandler): - _handler = file.FileCollection + @inlineCallbacks + def test_post_prevent_unauthorized_recipients_to_upload_any_file(self): + for f in files: + handler = self.request({}, role='receiver', attachment=self.get_dummy_attachment(f['name'])) + yield self.assertFailure(handler.post(f['handler']), errors.InvalidAuthentication) @inlineCallbacks - def test_get(self): - self._handler = file.FileInstance - handler = self.request({}, role='admin') - yield handler.post(u'custom') + def test_post_enable_authorized_recipients_to_upload_the_logo_and_only_it(self): + permissions = {'can_edit_general_settings': True} - self._handler = file.FileCollection - handler = self.request(role='admin') - response = yield handler.get() + for f in files: + handler = self.request({}, role='receiver', permissions=permissions, attachment=self.get_dummy_attachment(f['name'])) + if f['handler'] == 'logo': + yield handler.post(f['handler']) + else: + yield self.assertFailure(handler.post(f['handler']), errors.InvalidAuthentication) - self.assertEqual(len(response), 1) +class TestFileCollection(helpers.TestHandler): + _handler = file.FileCollection + + @inlineCallbacks + def test_get(self): self._handler = file.FileInstance - handler = self.request({}, role='admin') - yield handler.post(u'custom') - handler = self.request({}, role='admin') - yield handler.post(u'custom') + permissions = {'can_upload_files': True} + for f in files: + handler = self.request({}, role='admin', permissions=permissions, attachment=self.get_dummy_attachment(f['name'])) + yield handler.post(f['handler']) self._handler = file.FileCollection - handler = self.request(role='admin') + handler = self.request(role='admin', permissions=permissions) response = yield handler.get() - self.assertEqual(len(response), 3) + self.assertEqual(len(response), 5) diff --git a/backend/globaleaks/tests/handlers/recipient/test_rfile.py b/backend/globaleaks/tests/handlers/recipient/test_rfile.py index b885d7aa16..c1e091edf3 100644 --- a/backend/globaleaks/tests/handlers/recipient/test_rfile.py +++ b/backend/globaleaks/tests/handlers/recipient/test_rfile.py @@ -6,7 +6,7 @@ from globaleaks.jobs.delivery import Delivery from globaleaks.tests import helpers -attachment = b'hello world' +file_content = b'Hello World' class TestWBFileWorkFlow(helpers.TestHandlerWithPopulatedDB): @@ -19,7 +19,8 @@ def test_get(self): self._handler = rtip.ReceiverFileUpload rtips_desc = yield self.get_rtips() for rtip_desc in rtips_desc: - handler = self.request(role='receiver', user_id=rtip_desc['receiver_id'], attached_file=attachment) + attachment = self.get_dummy_attachment(content=file_content) + handler = self.request(role='receiver', user_id=rtip_desc['receiver_id'], attachment=attachment) yield handler.post(rtip_desc['id']) yield Delivery().run() @@ -31,7 +32,7 @@ def test_get(self): for rfile_desc in rfiles_desc: handler = self.request(role='whistleblower', user_id=wbtip_desc['id']) yield handler.get(rfile_desc['id']) - self.assertEqual(handler.request.getResponseBody(), attachment) + self.assertEqual(handler.request.getResponseBody(), file_content) self._handler = rtip.ReceiverFileDownload rtips_desc = yield self.get_rtips() diff --git a/backend/globaleaks/tests/handlers/test_file.py b/backend/globaleaks/tests/handlers/test_file.py index b0ad8fad2e..5be70c9357 100644 --- a/backend/globaleaks/tests/handlers/test_file.py +++ b/backend/globaleaks/tests/handlers/test_file.py @@ -16,7 +16,7 @@ def test_get(self): yield self.assertFailure(handler.get(u'custom'), ResourceNotFound) self._handler = admin_file.FileInstance - handler = self.request({}, role='admin') + handler = self.request({}, role='admin', permissions={'can_upload_files': True}) yield handler.post('custom') self._handler = admin_file.FileCollection diff --git a/backend/globaleaks/tests/helpers.py b/backend/globaleaks/tests/helpers.py index 16c549cdda..b273479202 100644 --- a/backend/globaleaks/tests/helpers.py +++ b/backend/globaleaks/tests/helpers.py @@ -3,6 +3,7 @@ Utilities and basic TestCases. """ import json +import mimetypes import os import shutil @@ -421,13 +422,14 @@ def __init__(self): } -def get_dummy_file(content=None): - filename = generateRandomKey() + ".pdf" +def get_dummy_attachment(name=None, content=None): + if name is None: + name = generateRandomKey() + ".pdf" - content_type = 'application/pdf' + content_type, _ = mimetypes.guess_type(name) if content is None: - content = Base64Encoder.decode(VALID_BASE64_IMG) + content = name.encode() temporary_file = SecureTemporaryFile(Settings.tmp_path) @@ -437,9 +439,9 @@ def get_dummy_file(content=None): State.TempUploadFiles[os.path.basename(temporary_file.filepath)] = temporary_file return { - 'id': filename, + 'id': name, 'date': datetime_now(), - 'name': filename, + 'name': name, 'description': 'description', 'body': temporary_file, 'size': len(content), @@ -458,13 +460,6 @@ def check_confirmation(self): BaseHandler.check_confirmation = check_confirmation -def get_file_upload(self): - return get_dummy_file() - - -BaseHandler.get_file_upload = get_file_upload - - def forge_request(uri=b'https://www.globaleaks.org/', headers=None, body='', args=None, client_addr=b'127.0.0.1', method=b'GET'): """ @@ -718,8 +713,8 @@ def get_dummy_submission(self, context_id): 'receipt': GCE.derive_key(GCE.generate_receipt(), VALID_SALT) }) - def get_dummy_file(self, content=None): - return get_dummy_file(content) + def get_dummy_attachment(self, name=None, content=None): + return get_dummy_attachment(name=name, content=content) def get_dummy_redirect(self, x=''): return { @@ -732,7 +727,7 @@ def emulate_file_upload(self, session, n): This emulates the file upload of an incomplete submission """ for _ in range(n): - session.files.append(self.get_dummy_file()) + session.files.append(self.get_dummy_attachment()) def pollute_events(self, number_of_times=10): for _ in range(number_of_times): @@ -886,7 +881,7 @@ def perform_submission_start(self): def perform_submission_uploads(self, submission_id): for _ in range(self.population_of_attachments): - Sessions.get(submission_id).files.append(self.get_dummy_file()) + Sessions.get(submission_id).files.append(self.get_dummy_attachment()) @inlineCallbacks def perform_submission_actions(self, session_id): @@ -973,14 +968,16 @@ class TestHandler(TestGLWithPopulatedDB): # # } # } + can_upload_files = True def setUp(self): return TestGL.setUp(self) def request(self, body='', uri=b'https://www.globaleaks.org/', - user_id=None, role=None, multilang=False, headers=None, args=None, - client_addr=b'127.0.0.1', handler_cls=None, attached_file=None, - kwargs=None, token=False): + user_id=None, role=None, multilang=False, headers=None, token=False, permissions=None, + client_addr=b'127.0.0.1', + handler_cls=None, attachment=None, + args=None, kwargs=None): """ Constructs a handler for preforming mock requests using the bag of params described below. """ @@ -1012,6 +1009,9 @@ def request(self, body='', uri=b'https://www.globaleaks.org/', else: session = Sessions.new(1, user_id, 1, role, USER_PRV_KEY, USER_ESCROW_PRV_KEY if role == 'admin' else '') + if permissions: + session.permissions = permissions + headers[b'x-session'] = session.id # during unit tests a token is always provided to any handler @@ -1041,7 +1041,7 @@ def request(self, body='', uri=b'https://www.globaleaks.org/', request.language = None if handler.upload_handler: - handler.uploaded_file = self.get_dummy_file(attached_file) + handler.uploaded_file = attachment if attachment else self.get_dummy_attachment() return handler diff --git a/client/app/src/pages/admin/questionnaires/questionnaires-list/questionnaires-list.component.html b/client/app/src/pages/admin/questionnaires/questionnaires-list/questionnaires-list.component.html index 51d92ecc72..b19f3ca9c9 100644 --- a/client/app/src/pages/admin/questionnaires/questionnaires-list/questionnaires-list.component.html +++ b/client/app/src/pages/admin/questionnaires/questionnaires-list/questionnaires-list.component.html @@ -53,4 +53,4 @@ -} \ No newline at end of file +} diff --git a/client/app/src/pages/admin/settings/tab2/tab2.component.html b/client/app/src/pages/admin/settings/tab2/tab2.component.html index 4962a3b8d0..ccc009959d 100644 --- a/client/app/src/pages/admin/settings/tab2/tab2.component.html +++ b/client/app/src/pages/admin/settings/tab2/tab2.component.html @@ -11,7 +11,7 @@
@for (admin_file of admin_files; track admin_file) { - + }
@@ -40,7 +40,7 @@ diff --git a/client/app/src/pages/admin/settings/tab2/tab2.component.ts b/client/app/src/pages/admin/settings/tab2/tab2.component.ts index 2fff57eff0..8192787c15 100644 --- a/client/app/src/pages/admin/settings/tab2/tab2.component.ts +++ b/client/app/src/pages/admin/settings/tab2/tab2.component.ts @@ -90,7 +90,7 @@ export class Tab2Component implements OnInit { flowJsInstance.on("fileSuccess", (_) => { this.appConfigService.reinit(false); - this.utilsService.reloadComponent(); + this.updateFiles(); }); flowJsInstance.on("fileError", (_) => { @@ -111,7 +111,6 @@ export class Tab2Component implements OnInit { this.utilsService.deleteFile(url).subscribe( () => { this.updateFiles(); - this.utilsService.reloadComponent(); } ); } diff --git a/client/app/src/shared/partials/admin-file/admin-file.component.ts b/client/app/src/shared/partials/admin-file/admin-file.component.ts index 175028b5a1..83fb8c5c88 100644 --- a/client/app/src/shared/partials/admin-file/admin-file.component.ts +++ b/client/app/src/shared/partials/admin-file/admin-file.component.ts @@ -26,6 +26,7 @@ export class AdminFileComponent { @Input() adminFile: AdminFile; @Input() present: boolean; + @Input() callback!: () => void; @ViewChild("uploader") uploaderInput!: ElementRef; onFileSelected(files: FileList | null, filetype: string) { @@ -46,7 +47,9 @@ export class AdminFileComponent { flowJsInstance.on("fileSuccess", (_) => { this.appConfigService.reinit(false); - this.utilsService.reloadCurrentRoute(); + if (this.callback) { + this.callback(); + } }); flowJsInstance.on("fileError", (_) => { if (this.uploaderInput) { @@ -61,7 +64,9 @@ export class AdminFileComponent { this.utilsService.deleteFile(url).subscribe( () => { this.appConfigService.reinit(false); - this.utilsService.reloadCurrentRoute(); + if (this.callback) { + this.callback(); + } } ); } diff --git a/client/cypress/e2e/12-test-admin-configure-files.cy.ts b/client/cypress/e2e/12-test-admin-configure-files.cy.ts index 0eb4fc0691..c79ae33f3c 100644 --- a/client/cypress/e2e/12-test-admin-configure-files.cy.ts +++ b/client/cypress/e2e/12-test-admin-configure-files.cy.ts @@ -1,5 +1,5 @@ -describe("Admin configure custom CSS and JS", () => { - it("should be able to configure the file upload", () => { +describe("Admin configure files", () => { + it("should be able to upload, download and delete files", () => { cy.login_admin(); cy.visit("#/admin/settings"); @@ -34,10 +34,6 @@ describe("Admin configure custom CSS and JS", () => { }); }); - cy.get("#project_name").should("be.visible"); - - cy.get('[data-cy="files"]').click(); - const customJSFile = "files/test.js.txt"; cy.fixture(customJSFile).then((fileContent) => { cy.get('div.uploadfile.file-script input[type="file"]').then(($input) => { @@ -52,20 +48,6 @@ describe("Admin configure custom CSS and JS", () => { }); }); - cy.get("#project_name").should("be.visible"); - }); - - it("should upload a file and make it available for download and deletion", () => { - cy.login_admin(); - cy.visit("#/admin/settings"); - cy.get('[data-cy="files"]').click(); - - cy.get("[name='authenticationData.session.permissions.can_upload_files']").should("not.be.checked"); - cy.get("[name='authenticationData.session.permissions.can_upload_files_switch']").click(); - cy.get(".modal").should("be.visible"); - cy.get(".modal [type='password']").type(Cypress.env("user_password")); - cy.get(".modal .btn-primary").click(); - const customFile = "files/test.txt"; cy.fixture(customFile).then((fileContent) => { cy.get("div.file-custom input").then(($input) => { @@ -80,28 +62,11 @@ describe("Admin configure custom CSS and JS", () => { }); }); - cy.get("#project_name").should("be.visible"); - - cy.get('[data-cy="files"]').click(); cy.get('table#fileList').find('td#file_name').should('contain', 'test.txt').should('be.visible'); - cy.get("#fileList").get("#delete").click(); - }); - - - it("should be able to disable the file upload", () => { - cy.login_admin(); - cy.visit("#/admin/settings"); - cy.get('[data-cy="files"]').click(); + cy.get("table#fileList").get(".fa-download").last().click(); + cy.get("table#fileList").get(".fa-trash").last().click(); - cy.get("[name='authenticationData.session.permissions.can_upload_files']").should("not.be.checked"); cy.get("[name='authenticationData.session.permissions.can_upload_files_switch']").click(); - cy.get(".modal").should("be.visible"); - cy.get(".modal [type='password']").type(Cypress.env("user_password")); - cy.get(".modal .btn-primary").click(); - - cy.get("[name='authenticationData.session.permissions.can_upload_files']").should("be.checked"); - cy.get("[name='authenticationData.session.permissions.can_upload_files_switch']").click(); - cy.get('[data-cy="files"]').click(); cy.get("[name='authenticationData.session.permissions.can_upload_files']").should("not.be.checked"); cy.logout(); diff --git a/client/cypress/e2e/17-test-admin-questionnaires.cy.ts b/client/cypress/e2e/17-test-admin-questionnaires.cy.ts index 6c6729037c..5d5bdc4803 100644 --- a/client/cypress/e2e/17-test-admin-questionnaires.cy.ts +++ b/client/cypress/e2e/17-test-admin-questionnaires.cy.ts @@ -85,15 +85,17 @@ describe("admin add, configure and delete questionnaires", () => { cy.get("#modal-action-ok").click(); }); - cy.get('[data-cy="question_templates"]').click(); fieldTypes.forEach((questionType: string, index: number) => { add_question(questionType, index); }); + cy.get("fa-file-export").last().click(); + cy.logout(); }); + it("should import custom questionnaire file", () => { cy.login_admin(); @@ -130,6 +132,7 @@ describe("admin add, configure and delete questionnaires", () => { cy.get("#questionnaire-2").should("be.visible"); cy.logout(); }); + it("should add duplicate questionnaire", function () { cy.login_admin(); cy.visit("/#/admin/questionnaires"); @@ -138,4 +141,11 @@ describe("admin add, configure and delete questionnaires", () => { cy.get("#modal-action-ok").click(); cy.logout(); }); + + it("should export questionnaire", function () { + cy.login_admin(); + cy.visit("/#/admin/questionnaires"); + cy.get(".fa-file-export").first().click(); + cy.logout(); + }); });