Release/0.70.0 #1202
Merged
Release/0.70.0 #1202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* fix(otp): increment instead of update for concurrency * ref(otp): migrate to use neverthrow instead * chore: remove unneeded check for empty string
* chore: add validators * fix: tests * fix: add test * chore: enforce 6 length otp
## Problem There was an issue with #1183 due to the removal of the `trust proxy` setting in express. There are two packages that rely on express apis, `express-rate-limit` and `express-session`. due to changes in the way we used `express-rate-limit`, I thought that this setting can be removed, and this was only done with the intention of a cleanup and does not affect the functionality of how we are currently using `express-rate-limit`. The [documentation](https://github.com/expressjs/session?tab=readme-ov-file#cookiesecure) for `express-session` also states that: > If secure is set, and you access your site over HTTP, the cookie will not be set. If you have your node.js behind a proxy and are using secure: true, you need to set "trust proxy" in express Checking in with the vapt folks to sense check this fix as well ## Solution add back trust proxy **Breaking Changes** <!-- Does this PR contain any backward incompatible changes? If so, what are they and should there be special considerations for release? --> - [ ] Yes - this PR contains breaking changes - Details ... - [X] No - this PR is backwards compatible with ALL of the following feature flags in this [doc](https://www.notion.so/opengov/Existing-feature-flags-518ad2cdc325420893a105e88c432be5) # Note: This test will take a while, and requires at worse 15 mins to conduct ## Tests - [ ] create a file called ddos.js ``` const stg = "https://staging-cms-api.isomer.gov.sg/v2/auth/verify" async function send() { try { const resp = await fetch(stg, { method: "POST", body: JSON.stringify({ email: "[email protected]", otp: "111111", }), headers: { "Content-Type": "application/json", "X-Forwarded-For": generateRandomIp(), }, }) const text = await resp.text() console.log(text) console.log({ Limit: resp.headers.get("Ratelimit-Limit"), Remaining: resp.headers.get("Ratelimit-Remaining"), Reset: resp.headers.get("Ratelimit-Reset"), }) } catch (err) { console.log(err.message) } } for (let i = 1; i <= 25; i++) { send() } ``` - [ ] connect to ogp vpn - [ ] run `node ddos.js` - [ ] assert that the remaining counter fell from 100  - [ ] note the reset time (this is the window time, and by extension the amount of time to wait for this test) - [ ] unconnect from vpn - [ ] run `node ddos.js` - [ ] assert that the remaining counter fell from 100  - [ ] After the reset time is achieved, do above steps again and verify that after the reset time, the counters for both the simulated user resets. 
|
|
* fix: schema * fix: make feedback optional * chore: make feedback optional
|
|
alexanderleegs
approved these changes
Mar 11, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's Changed
Full Changelog: v0.69.0...v0.70.0
Tests
#1186 - @alexanderleegs
#1196 - @alexanderleegs
node ddos.jsnode ddos.js#1197 - @dcshzj
Check that the following endpoints do not throw an error from validation:
/mobile/verifyOtpendpoint no longer accepts an array formobile