Bug 36177 - We need integration tests to cover CSRF checks
Summary: We need integration tests to cover CSRF checks
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 36148
Blocks: 36355
  Show dependency treegraph
 
Reported: 2024-02-27 14:39 UTC by Nick Clemens (kidclamp)
Modified: 2024-05-01 13:15 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00
Circulation function:


Attachments
Bug 36177: Cypress tests - yarn add cypress-mysql (8.32 KB, patch)
2024-02-28 14:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36177: Cypress tests - cypress-mysql config (1.78 KB, patch)
2024-02-28 14:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36177: Add Cypress tests (6.45 KB, patch)
2024-02-28 14:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36177: Add Cypress tests (6.45 KB, patch)
2024-02-29 08:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36177: Cypress tests - yarn add cypress-mysql (8.36 KB, patch)
2024-03-08 21:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 36177: Cypress tests - cypress-mysql config (1.82 KB, patch)
2024-03-08 21:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 36177: Add Cypress tests (6.49 KB, patch)
2024-03-08 21:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 36177: Cypress tests - yarn add cypress-mysql (8.42 KB, patch)
2024-03-11 14:16 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36177: Cypress tests - cypress-mysql config (1.88 KB, patch)
2024-03-11 14:16 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36177: Add Cypress tests (6.55 KB, patch)
2024-03-11 14:16 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2024-02-27 14:39:26 UTC
Bug 36148 will be adding a check in our middleware for csrf coverage on post operations - we need selenium tests to cover all scenarios
Comment 1 Jonathan Druart 2024-02-28 14:44:15 UTC
Created attachment 162532 [details] [review]
Bug 36177: Cypress tests - yarn add cypress-mysql
Comment 2 Jonathan Druart 2024-02-28 14:44:18 UTC
Created attachment 162533 [details] [review]
Bug 36177: Cypress tests - cypress-mysql config
Comment 3 Jonathan Druart 2024-02-28 14:44:20 UTC
Created attachment 162534 [details] [review]
Bug 36177: Add Cypress tests

This should not be pushed, we are not ready on jenkins.

csrf.ts must be run within ktd (not on the host, or 'db' must resolve)
Comment 4 Jonathan Druart 2024-02-28 14:46:16 UTC
Tomas, can you help here? 'db' needs to be resolved from the container.
Comment 5 Tomás Cohen Arazi (tcohen) 2024-02-28 18:37:09 UTC
(In reply to Jonathan Druart from comment #4)
> Tomas, can you help here? 'db' needs to be resolved from the container.

'db' is resolved inside the container, so we should be fine.

That said... cannot we ship some template instead, and have a helper script that picks env variables and does what is needed instead? Having that also for the baseUrl and friends would also make it easier for running Cypress on the sandboxes, etc.
Comment 6 Jonathan Druart 2024-02-29 08:02:53 UTC
Created attachment 162576 [details] [review]
Bug 36177: Add Cypress tests
Comment 7 Jonathan Druart 2024-02-29 08:04:53 UTC
(In reply to Tomás Cohen Arazi from comment #5)
> (In reply to Jonathan Druart from comment #4)
> > Tomas, can you help here? 'db' needs to be resolved from the container.
> 
> 'db' is resolved inside the container, so we should be fine.

Indeed, the tests are passing inside the container. No idea what changed since I wrote them.

> That said... cannot we ship some template instead, and have a helper script
> that picks env variables and does what is needed instead? Having that also
> for the baseUrl and friends would also make it easier for running Cypress on
> the sandboxes, etc.

Yes, maybe. Feel free to open a new bug and provide a patch. I don't see it as an important need right now.
Comment 8 David Nind 2024-03-08 21:35:11 UTC
Created attachment 162984 [details] [review]
Bug 36177: Cypress tests - yarn add cypress-mysql

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2024-03-08 21:35:14 UTC
Created attachment 162985 [details] [review]
Bug 36177: Cypress tests - cypress-mysql config

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 David Nind 2024-03-08 21:35:17 UTC
Created attachment 162986 [details] [review]
Bug 36177: Add Cypress tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2024-03-08 21:38:58 UTC
Testing notes (using KTD):

1. Ran cypress tests before applying the patches:
   perl /kohadevbox/misc4dev/run_tests.pl --run-cypress-tests-only

2. Ran the tests again after patches applied (I think I did a yarn there as well to add the new module/package, but I did a reset_all and shut down KTD and restarted as well - not sure of the correct way to do this).

Results after patches applied:

Spec                                              Tests  Passing  Failing  Pending  Skipped  
  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ ✔  InfiniteScrollSelect_spec.ts             00:14        4        4        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  KohaTable_spec.ts                        00:03        1        1        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  Auth/csrf.ts                             00:18        6        6        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  ERM/Agreements_spec.ts                   00:31        5        5        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  ERM/DataProviders_spec.ts                00:49       13       13        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  ERM/Dialog_spec.ts                       00:11        4        4        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  ERM/Licenses_spec.ts                     00:21        5        5        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  ERM/Packages_spec.ts                     00:22        5        5        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  ERM/Searchbar_spec.ts                    00:11        4        4        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  ERM/Titles_spec.ts                       00:34        6        6        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  ERM/UsageReports_spec.ts                 00:42       13       13        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  ERM/UserRoles_spec.ts                    00:03        1        1        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  Preservation/Settings.ts                 00:15        5        5        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  Preservation/Trains.ts                   00:20        8        8        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  Preservation/WaitingList.ts              00:08        3        3        -        -        - │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘
    ✔  All specs passed!                        05:09       83       83        -        -        -  

Done in 349.96s.
all good
Running [koha-shell kohadev -c "touch testing.success"]...
Comment 12 Nick Clemens (kidclamp) 2024-03-11 14:16:28 UTC
Created attachment 163035 [details] [review]
Bug 36177: Cypress tests - yarn add cypress-mysql

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Nick Clemens (kidclamp) 2024-03-11 14:16:30 UTC
Created attachment 163036 [details] [review]
Bug 36177: Cypress tests - cypress-mysql config

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Nick Clemens (kidclamp) 2024-03-11 14:16:33 UTC
Created attachment 163037 [details] [review]
Bug 36177: Add Cypress tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 15 Katrin Fischer 2024-03-12 16:49:30 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 16 Fridolin Somers 2024-03-13 06:58:12 UTC
Depends on Bug 36148 not in 23.11.x