Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redesigned Dashboard & Workflow - Persisting state in URL #10771

Closed
jardakotesovec opened this issue Jan 5, 2025 · 3 comments
Closed

Redesigned Dashboard & Workflow - Persisting state in URL #10771

jardakotesovec opened this issue Jan 5, 2025 · 3 comments
Assignees
Milestone

Comments

@jardakotesovec
Copy link
Contributor

jardakotesovec commented Jan 5, 2025

To make it easier to bookmark and share urls in dashboard and workflow, following items are being persisted to the URL as query params.

This is new feature compared to 3.4.

  • selected view in dashboard
  • selected filters in dashboard
  • search phrase in dashboard
  • which column to sort by in dashboard
  • opened submission id in workflow
  • selected menu in workflow

Testing is mostly just copying url and making sure that the state was applied accurately.

@jardakotesovec jardakotesovec self-assigned this Jan 5, 2025
jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Jan 5, 2025
jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Jan 5, 2025
jardakotesovec added a commit to pkp/ui-library that referenced this issue Jan 5, 2025
* pkp/pkp-lib#10771 Persist also selected menu in workflow page

* pkp/pkp-lib#10771 remove correct query param when redirecting to decision page
@jardakotesovec jardakotesovec changed the title Redesigned Dashboard & Workflow - persisting state in URL Redesigned Dashboard & Workflow - Persisting state in URL Jan 5, 2025
@jardakotesovec jardakotesovec added this to the 3.5.0 LTS milestone Jan 7, 2025
@Tribunal33
Copy link

Tribunal33 commented Jan 11, 2025

@jardakotesovec One possible efficiency (I don't know what to call it) would be if the user has opened up a workflow pane the search Phrase shouldn't be kept since it doesn't do anything when you copy and paste the urls for state?

Here is an example with searchPhrase being kept in the URL but the workflow is opened and the search doesn't have anything to do with it.
/ojs-main/ojs/index.php/publicknowledge/en/dashboard/editorial?currentViewId=active&sectionIds=1&daysInactive=16&searchPhrase=learning+trends&workflowSubmissionId=11&workflowMenuKey=publication_jats

It also introduces a bit of a security flaw in that I can send you a link with a searchPhrase that could potentially allow a user to put malicious code into the string.
Something like this doesn't work but you get the idea :) :
/ojs-main/ojs/index.php/publicknowledge/en/dashboard/editorial?currentViewId=active&sectionIds=1&daysInactive=16&searchPhrase=<script>alert('work')</script>&workflowSubmissionId=11&workflowMenuKey=publication_jats

This is more of a generalized method for all parameters but I'm thinking about this : https://pragmaticwebsecurity.com/articles/apisecurity/tamper-proof-params-jwt#:~:text=URL%20parameters%20are%20present%20in,a%20URL%20containing%20sensitive%20parameters.

@jardakotesovec
Copy link
Contributor Author

@Tribunal33 regarding the search phrase. That still gets applied, but to the dashboard state, not workflow as all other filters. I don't think thats causing any problems? Even if someone is sharing url with opened workflow page - keeping the dashboard state increase the chance that after closing workflow page - that particular submission will be somewhere in that listing.

Regarding the xss attacks, these are prevented (escaped) on rendering layer (vue.js), so we don't do any explicit handling of that.

@Tribunal33
Copy link

Tribunal33 commented Jan 14, 2025

@jardakotesovec this case passes QA. The tricky part was trying to figure out how these URLs interact with certain users. Which users are able to see what. I did not see requirements on them so I tried a few that I knew shouldn't be allow. It blocks them correctly -- gives this error response index.php/publicknowledge/en/user/authorizationDenied?message=user.authorization.roleBasedAccessDenied

These params are easy to manipulate because of the readability but if the search bar is escaping properly then I don't really see a strong attack vector here. Might be something to monitor on hosting side to scan for any malicious looking urls.

@pmangahis I believe that PS team wanted to make sure copy pasting urls was working well. Are there any other cases I might have missed?

Test Case Overview

  1. Test each of the states by copying to incognito browser tab
    a. The view
    b. The filters
    c. Search phrases
    d. Column sorting
    e. Opened submission id
    f. Selected side menu in submission workflow
  2. Reverse the param states beginning with full params in the URL
    a. Close the submission
    b. Clear filters
    c. Cancel search
  3. Test user access to these URLs
    a. Author shouldn’t have Access to wrong submission ids
    b. Readers shouldn’t have access to certain views

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants