[server] don't allow more than three usages per phone number #13186
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
atduarte
reviewed
Sep 21, 2022
There was a problem hiding this comment.
This is awesome! Thank you, Sven 🙏
Looking at the code out of curiosity found something you will notice also pretty quickly :)
PS: Would it make sense and be sensible to also stop overriding the phone number on delete as part of this (here)? Asking because a potential way for an abuser to avoid both these checks is requesting the account to be deleted.
| @@ -59,6 +60,14 @@ export class VerificationService { | |||
| if (!this.verifyService) { | |||
| throw new Error("No verification service configured."); | |||
| } | |||
| const isBlockedNumber = this.userDB.countUsagesOfPhoneNumber(phoneNumber); | |||
There was a problem hiding this comment.
Suggested change
| const isBlockedNumber = this.userDB.countUsagesOfPhoneNumber(phoneNumber); | |
| const isBlockedNumber = this.userDB.isBlockedPhoneNumber(phoneNumber); |
svenefftinge
changed the title
svenefftinge
force-pushed
the
sefftinge/limit-phone-number-re-12883
branch
from
2430319 to
0b53ef5
Compare
roboquat
added
release-note-none
release-note
and removed
do-not-merge/release-note-label-needed
release-note-none
labels
svenefftinge
force-pushed
the
sefftinge/limit-phone-number-re-12883
branch
from
0b53ef5 to
e504643
Compare
easyCZ
reviewed
Sep 22, 2022
components/gitpod-db/src/typeorm/migration/1663784254956-IndexPhoneNumber.ts
Show resolved
Hide resolved
|
/werft run 👍 started the job as gitpod-build-sefftinge-limit-phone-number-re-12883.3 |
|
/hold |
easyCZ
requested changes
Sep 22, 2022
svenefftinge
force-pushed
the
sefftinge/limit-phone-number-re-12883
branch
2 times, most recently
from
ba37cf5 to
6b55247
Compare
easyCZ
approved these changes
Sep 23, 2022
| throw new Error("The given phone number has been used more than three times."); | ||
| } | ||
| if (await isBlockedNumber) { | ||
| throw new Error("The given phone number is blocked due to abuse."); |
There was a problem hiding this comment.
Can we throw this as a user error here? Otherwise it will count towards server errors in our metrics.
| * @param phoneNumber | ||
| * @returns formatted phone number | ||
| */ | ||
| export function formatPhoneNumber(phoneNumber: string): string { |
svenefftinge
force-pushed
the
sefftinge/limit-phone-number-re-12883
branch
from
6b55247 to
dd09f79
Compare
|
/unhold |
roboquat
added
deployed: webapp
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.
Description
Adds checks for phone numbers being used by abusers or used too often (>3)
Related Issue(s)
Fixes #12883
How to test
Release Notes
Documentation
Werft options:
If enabled this will build
install/previewValid options are
all,workspace,webapp,ide