From 6d38124620db593e3711e456adab3f21c73cd4ae Mon Sep 17 00:00:00 2001 From: Kirill Lakhov Date: Thu, 19 Sep 2024 17:45:37 +0300 Subject: [PATCH 01/10] Tests for basic `Management` tab on `Quality control` page (#8447) ### Motivation and context This PR adds tests for `management` tab that was introduced in #8329 ### How has this been tested? ### Checklist - [x] I submit my changes into the `develop` branch - ~~[ ] I have created a changelog fragment ~~ - ~~[ ] I have updated the documentation accordingly~~ - [x] I have added tests to cover my changes - ~~[ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword))~~ - ~~[ ] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/cvat-ai/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/cvat-ai/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/cvat-ai/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/cvat-ai/cvat/tree/develop/cvat-ui#versioning))~~ ### License - [x] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. ## Summary by CodeRabbit - **New Features** - Enhanced styling for the Allocation Table and Summary components with new CSS class names for better visual representation. - Introduced a new Cypress command for creating jobs, improving the testing process. - Added a new testing suite for ground truth management, validating key functionalities in the management page. - **Bug Fixes** - Updated test data for regression tests to ensure accuracy in testing scenarios. --- .../task-quality/allocation-table.tsx | 8 +- .../quality-control/task-quality/summary.tsx | 6 +- .../cypress/e2e/features/ground_truth_jobs.js | 131 +++++++++++++++++- tests/cypress/support/commands.js | 19 ++- 4 files changed, 153 insertions(+), 11 deletions(-) diff --git a/cvat-ui/src/components/quality-control/task-quality/allocation-table.tsx b/cvat-ui/src/components/quality-control/task-quality/allocation-table.tsx index e1a4862ad822..b1619bf6fcd3 100644 --- a/cvat-ui/src/components/quality-control/task-quality/allocation-table.tsx +++ b/cvat-ui/src/components/quality-control/task-quality/allocation-table.tsx @@ -109,10 +109,12 @@ function AllocationTable(props: Readonly): JSX.Element { render: (active: boolean, record: RowData): JSX.Element => ( active ? ( { onDeleteFrames([record.frame]); }} /> ) : ( { onRestoreFrames([record.frame]); }} component={RestoreIcon} /> @@ -130,7 +132,7 @@ function AllocationTable(props: Readonly): JSX.Element { { selection.selectedRowKeys.length !== 0 ? ( <> - + { const framesToUpdate = selection.selectedRows @@ -141,7 +143,7 @@ function AllocationTable(props: Readonly): JSX.Element { }} /> - + { const framesToUpdate = selection.selectedRows @@ -163,7 +165,7 @@ function AllocationTable(props: Readonly): JSX.Element { if (!rowData.active) { return 'cvat-allocation-frame-row cvat-allocation-frame-row-excluded'; } - return 'cvat-allocation-frame'; + return 'cvat-allocation-frame-row'; }} columns={columns} dataSource={data} diff --git a/cvat-ui/src/components/quality-control/task-quality/summary.tsx b/cvat-ui/src/components/quality-control/task-quality/summary.tsx index be249341b37d..59767192ed0d 100644 --- a/cvat-ui/src/components/quality-control/task-quality/summary.tsx +++ b/cvat-ui/src/components/quality-control/task-quality/summary.tsx @@ -20,14 +20,14 @@ export default function SummaryComponent(props: Readonly): JSX.Element { - + Excluded count: {' '} {excludedCount} - + Total count: {' '} @@ -36,7 +36,7 @@ export default function SummaryComponent(props: Readonly): JSX.Element { - + Active count: {' '} diff --git a/tests/cypress/e2e/features/ground_truth_jobs.js b/tests/cypress/e2e/features/ground_truth_jobs.js index 9eba445b76a2..dfd8a8fe2b52 100644 --- a/tests/cypress/e2e/features/ground_truth_jobs.js +++ b/tests/cypress/e2e/features/ground_truth_jobs.js @@ -88,6 +88,14 @@ context('Ground truth jobs', () => { .should('be.visible'); } + function openManagementTab() { + cy.clickInTaskMenu('Quality control', true); + cy.get('.cvat-task-control-tabs') + .within(() => { + cy.contains('Management').click(); + }); + } + before(() => { cy.visit('auth/login'); cy.login(); @@ -187,6 +195,121 @@ context('Ground truth jobs', () => { }); }); + describe('Testing ground truth management basics', () => { + const serverFiles = ['images/image_1.jpg', 'images/image_2.jpg', 'images/image_3.jpg']; + + before(() => { + cy.headlessCreateTask({ + labels: [{ name: labelName, attributes: [], type: 'any' }], + name: taskName, + project_id: null, + source_storage: { location: 'local' }, + target_storage: { location: 'local' }, + }, { + server_files: serverFiles, + image_quality: 70, + use_zip_chunks: true, + use_cache: true, + sorting_method: 'lexicographical', + }).then((taskResponse) => { + taskID = taskResponse.taskID; + [jobID] = taskResponse.jobIDs; + }).then(() => ( + cy.headlessCreateJob({ + task_id: taskID, + frame_count: 3, + type: 'ground_truth', + frame_selection_method: 'random_uniform', + }) + )).then((jobResponse) => { + groundTruthJobID = jobResponse.jobID; + }).then(() => { + cy.visit(`/tasks/${taskID}`); + cy.get('.cvat-task-details').should('exist').and('be.visible'); + openManagementTab(); + }); + }); + + after(() => { + cy.headlessDeleteTask(taskID); + }); + + it('Check management page contents.', () => { + cy.get('.cvat-annotations-quality-allocation-table-summary').should('exist'); + cy.contains('.cvat-allocation-summary-excluded', '0').should('exist'); + cy.contains('.cvat-allocation-summary-total', '3').should('exist'); + cy.contains('.cvat-allocation-summary-active', '3').should('exist'); + + cy.get('.cvat-frame-allocation-table').should('exist'); + cy.get('.cvat-allocation-frame-row').should('have.length', 3); + cy.get('.cvat-allocation-frame-row').each(($el, index) => { + cy.wrap($el).within(() => { + cy.contains(`#${index}`).should('exist'); + cy.contains(`images/image_${index + 1}.jpg`).should('exist'); + }); + }); + }); + + it('Check link to frame.', () => { + cy.get('.cvat-allocation-frame-row').last().within(() => { + cy.get('.cvat-open-frame-button').first().click(); + }); + cy.get('.cvat-spinner').should('not.exist'); + cy.url().should('contain', `/tasks/${taskID}/jobs/${groundTruthJobID}`); + cy.checkFrameNum(2); + + cy.interactMenu('Open the task'); + openManagementTab(); + }); + + it('Disable single frame, enable it back.', () => { + cy.get('.cvat-allocation-frame-row').last().within(() => { + cy.get('.cvat-allocation-frame-delete').click(); + }); + cy.get('.cvat-spinner').should('not.exist'); + + cy.get('.cvat-allocation-frame-row-excluded').should('exist'); + cy.contains('.cvat-allocation-summary-excluded', '1').should('exist'); + cy.contains('.cvat-allocation-summary-active', '2').should('exist'); + + cy.get('.cvat-allocation-frame-row-excluded').within(() => { + cy.get('.cvat-allocation-frame-restore').click(); + }); + cy.get('.cvat-spinner').should('not.exist'); + cy.get('.cvat-allocation-frame-row-excluded').should('not.exist'); + cy.contains('.cvat-allocation-summary-excluded', '0').should('exist'); + cy.contains('.cvat-allocation-summary-active', '3').should('exist'); + }); + + it('Select several frames, use group operations.', () => { + function selectFrames() { + cy.get('.cvat-allocation-frame-row').each(($el, index) => { + if (index !== 0) { + cy.wrap($el).within(() => { + cy.get('.ant-table-selection-column input[type="checkbox"]').should('not.be.checked').check(); + }); + } + }); + } + + selectFrames(); + cy.get('.cvat-allocation-selection-frame-delete').click(); + cy.get('.cvat-spinner').should('not.exist'); + + cy.get('.cvat-allocation-frame-row-excluded').should('have.length', 2); + cy.contains('.cvat-allocation-summary-excluded', '2').should('exist'); + cy.contains('.cvat-allocation-summary-active', '1').should('exist'); + + selectFrames(); + cy.get('.cvat-allocation-selection-frame-restore').click(); + cy.get('.cvat-spinner').should('not.exist'); + + cy.get('.cvat-allocation-frame-row-excluded').should('not.exist'); + cy.contains('.cvat-allocation-summary-excluded', '0').should('exist'); + cy.contains('.cvat-allocation-summary-active', '3').should('exist'); + }); + }); + describe('Regression tests', () => { const imagesCount = 20; const imageFileName = 'ground_truth_2'; @@ -205,8 +328,12 @@ context('Ground truth jobs', () => { cy.imageGenerator(imagesFolder, imageFileName, width, height, color, posX, posY, labelName, imagesCount); cy.createZipArchive(directoryToArchive, archivePath); cy.createAnnotationTask( - taskName, labelName, attrName, - textDefaultValue, archiveName, false, + taskName, + labelName, + attrName, + textDefaultValue, + archiveName, + false, { multiJobs: true, segmentSize: 1 }, ); cy.openTask(taskName); diff --git a/tests/cypress/support/commands.js b/tests/cypress/support/commands.js index 83628abfb66b..76f7ffe2640c 100644 --- a/tests/cypress/support/commands.js +++ b/tests/cypress/support/commands.js @@ -171,11 +171,11 @@ Cypress.Commands.add( attrName = 'Some attr name', textDefaultValue = 'Some default value for type Text', image = 'image.png', - multiAttrParams, - advancedConfigurationParams, + multiAttrParams = null, + advancedConfigurationParams = null, forProject = false, attachToProject = false, - projectName, + projectName = '', expectedResult = 'success', projectSubsetFieldValue = 'Test', ) => { @@ -365,6 +365,19 @@ Cypress.Commands.add('headlessLogout', () => { cy.clearAllLocalStorage(); }); +Cypress.Commands.add('headlessCreateJob', (jobSpec) => { + cy.window().then(async ($win) => { + const data = { + ...jobSpec, + }; + + const job = new $win.cvat.classes.Job(data); + + const result = await job.save(data); + return cy.wrap({ jobID: result.id }); + }); +}); + Cypress.Commands.add('openTask', (taskName, projectSubsetFieldValue) => { cy.contains('strong', new RegExp(`^${taskName}$`)) .parents('.cvat-tasks-list-item') From 0bf45fd5de08a652dffbfb517318a64c2fdbc5cf Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Fri, 20 Sep 2024 15:56:18 +0300 Subject: [PATCH 02/10] Merge commit from fork When a view returns an `HttpResponseNotFound` object, the `Content-Type` header is set to `text/html` by default. Therefore, including external data (in this case, the request ID) in the output without any escaping or validation leads to a vulnerability. If an attacker can trick a user into following a malicious link, they can include HTML elements in the request ID that execute malicious JavaScript code in the victim's browser. That code will be able to make requests to the CVAT API with the user's privileges. The simplest fix for this is to not include variable data in the error message, which is what I did. In the long run, I think we need to get rid of these HTML responses, since it doesn't make sense for a JSON API to return HTML. --- changelog.d/20240912_201524_roman_xss_requests.md | 4 ++++ cvat/apps/engine/views.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 changelog.d/20240912_201524_roman_xss_requests.md diff --git a/changelog.d/20240912_201524_roman_xss_requests.md b/changelog.d/20240912_201524_roman_xss_requests.md new file mode 100644 index 000000000000..584291de39a2 --- /dev/null +++ b/changelog.d/20240912_201524_roman_xss_requests.md @@ -0,0 +1,4 @@ +### Security + +- Fixed an XSS vulnerability in request-related endpoints + () diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 05a50857b28f..528c8314b677 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -3391,7 +3391,7 @@ def retrieve(self, request: HttpRequest, pk: str): job = self._get_rq_job_by_id(pk) if not job: - return HttpResponseNotFound(f"There is no request with specified id: {pk}") + return HttpResponseNotFound("There is no request with specified id") self.check_object_permissions(request, job) @@ -3428,7 +3428,7 @@ def cancel(self, request: HttpRequest, pk: str): rq_job = self._get_rq_job_by_id(pk) if not rq_job: - return HttpResponseNotFound(f"There is no request with specified id: {pk!r}") + return HttpResponseNotFound("There is no request with specified id") self.check_object_permissions(request, rq_job) From 75c3d573bc9468b718f53b442c2ef69ad1d5de12 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Fri, 20 Sep 2024 15:57:15 +0300 Subject: [PATCH 03/10] Merge commit from fork The default `Content-Type` for `HttpResponse` is `text/html`, so an attacker can create a label a `