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

fix: restrict state to active and inactive in url search #2050

Merged
merged 2 commits into from
Nov 16, 2022

Conversation

halfwhole
Copy link
Collaborator

@halfwhole halfwhole commented Oct 26, 2022

Problem

The query conditions for state when searching for URLs in the user page are overly permissive, and should be restricted to either active or inactive states.

See Snyk dashboard - Improper Type Validation

Solution

Use Joi validator to allow the optional state field to be only one of ACTIVE, INACTIVE or ''.

I'm perhaps less comfortable with allowing state to be '', but the frontend inserts &state= by default so disallowing it will break the frontend search edit: removed '' from state following the comments below!

Tests:

  • Added tests for an invalid and empty state field in UserController.test.ts
  • (Improvement) Renamed test cases that return a bad request response code to begin with "reports bad request", in line with other test cases in the file that do this

@yong-jie
Copy link
Member

yong-jie commented Oct 26, 2022

but the frontend inserts &state= by default

random passing thought, but would changing the default frontend behavior to insert &state=ACTIVE instead, help? then our backend api's accepted inputs can be cleaner

edit: this comment might also apply to #2051 as well

@halfwhole
Copy link
Collaborator Author

it'd be good to preserve the functionality of searching for both active and inactive links in some way though (currently done through &state=), and changing the default behaviour to &state=ACTIVE may not be able to help with that :(

agree that the inputs aren't really clean though - maybe we can try dropping &state= altogether if the user is searching for both active and inactive links, then the backend won't need to accept '' as an input?

@yong-jie
Copy link
Member

oh yes good idea!

@halfwhole halfwhole merged commit aa2bbd6 into develop Nov 16, 2022
@halfwhole halfwhole deleted the fix/url-search-restrict-state branch November 16, 2022 03:36
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

Successfully merging this pull request may close these issues.

3 participants