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

[#12048] Add SQL injection tests in FeedbackSessionsDbIT #12857

Merged

Conversation

jayasting98
Copy link
Contributor

@jayasting98 jayasting98 commented Feb 28, 2024

Part of #12048

Outline of Solution

SQL injection tests were added into FeedbackSessionsDbIT, covering almost every method, and every input, except non-string input, input strings which disallowed single quotation marks, and methods with no user input.

Also, the column type of the instructions of a feedback session was changed to be TEXT, instead of VARCHAR, to allow for much longer text, which was used in the tests. I believe the previous limit was not intended.

@jayasting98 jayasting98 added s.Ongoing The PR is being worked on by the author(s) c.Task Other non-user-facing works, e.g. refactoring, adding tests labels Feb 28, 2024
@jayasting98 jayasting98 added this to the V9.0.0-beta.0 milestone Feb 28, 2024
@jayasting98 jayasting98 self-assigned this Feb 28, 2024
@jayasting98 jayasting98 marked this pull request as ready for review February 28, 2024 16:40
@jayasting98 jayasting98 added s.ToReview The PR is waiting for review(s) and removed s.Ongoing The PR is being worked on by the author(s) labels Feb 28, 2024
@jayasting98 jayasting98 requested a review from EuniceSim142 March 8, 2024 10:03
@weiquu weiquu removed this from the V9.0.0-beta.0 milestone Mar 10, 2024
Copy link
Contributor

@EuniceSim142 EuniceSim142 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks

Copy link
Contributor

@weiquu weiquu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@weiquu weiquu added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels Mar 18, 2024
@weiquu weiquu merged commit ffb07dd into TEAMMATES:master Mar 18, 2024
11 checks passed
jayasting98 added a commit to jayasting98/teammates that referenced this pull request Mar 18, 2024
…AMMATES#12857)

* Add createTypicalXyz methods

* Add SQL injection tests for createFeedbackSession

* Add SQL injection tests for getFeedbackSession(String, String)

* Add SQL injection tests for getSoftDeletedFeedbackSession

* Add SQL injection test for getSoftDeletedFeedbackSessionsForCourse

* Add SQL injection tests from restoreDeletedFeedbackSession

* Change SQL injection for restoreDeletedFeedbackSession

* Add SQL injection test for getFeedbackSessionEntitiesForCourse

* Add SQL injection test for get ... for course starting after

* Add comment for SQL injection attempts on deleteFeedbackSession

* Add SQL injection tests for updateFeedbackSession

* Add SQL injection tests for softDeleteFeedbackSession

* Fix lint

* Fix PMD

* Make SQL injection query more concise

---------

Co-authored-by: Wei Qing <[email protected]>
@cedricongjh cedricongjh added this to the V9.0.0-beta.1 milestone Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants