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

Allow deanonymizing phone numbers for abuse handling #169

Open
scy opened this issue Oct 5, 2023 · 2 comments
Open

Allow deanonymizing phone numbers for abuse handling #169

scy opened this issue Oct 5, 2023 · 2 comments
Labels
area:backend Related to the server component flag:caveat Might be confusing or cause problems for third-party users flag:security Impacts the overall security of the system prio:B Major impact, but not essential type:enhancement New feature or request

Comments

@scy
Copy link
Collaborator

scy commented Oct 5, 2023

  • Have a list of “abusive” phone number hashes (initially empty).
  • The admin can add hashes to that list if they abuse the system.
  • Every time a raw phone number is being hashed (by the number class, I guess?) it will be compared to this list.
  • If the hash matches an entry on the list, store the hash and its corresponding number in a (to-be-created) “de-anonymizing” database table.

A good spot to implement this would probably be in the is_allowed (or something like that) check in the number class, as it checks the number for policy reasons anyway and thus should be called in central places. Also, it does have database access (or at least it had once while I implemented it, don’t think it does currently … 🤔 but it should).

@scy scy added type:enhancement New feature or request area:backend Related to the server component flag:security Impacts the overall security of the system labels Oct 5, 2023
@scy scy added this to the 2.0: instant call support milestone Oct 5, 2023
@scy scy added the prio:B Major impact, but not essential label Oct 11, 2023
@jbethune
Copy link
Collaborator

jbethune commented Oct 18, 2023

I see that there is a check for blocked numbers:

if self.matches_filter(config.blocked_numbers):

However, this check is done against the values in the config file. Should the "abusive" phone number hashes also be stored in the config file or should we use a database table? If it is the latter case: Once we have a match, should the entry in the abusive-hashes table be removed once we have written an entry (hashed+unhashed) in the "de-anonymizing” database table?

Should we maybe also move the blocked phone numbers into the database?

Also, it does have database access (or at least it had once while I implemented it, don’t think it does currently … 🤔 but it should).

It currently doesn't. Should we pass in a Session as a method parameter?

@scy scy added the flag:caveat Might be confusing or cause problems for third-party users label Dec 15, 2024
@scy
Copy link
Collaborator Author

scy commented Dec 15, 2024

I see that there is a check for blocked numbers:

if self.matches_filter(config.blocked_numbers):

Yeah, but I'm not 100 % sure if hooking the de-anonymizing into that place is the best way to do it. There are some additional spots in the code that deal with unhashed phone numbers (though not many), e.g. the scheduler.

However, this check is done against the values in the config file. Should the "abusive" phone number hashes also be stored in the config file or should we use a database table? If it is the latter case: Once we have a match, should the entry in the abusive-hashes table be removed once we have written an entry (hashed+unhashed) in the "de-anonymizing” database table?

Good questions. I don't have a strong opinion on either of these. If we're using the database instead of the config file, we should provide a CLI command, too.

Should we maybe also move the blocked phone numbers into the database?

No. We can talk about having an additional block list in the database, for numbers that are added and deleted on the fly (if there ever is such a thing), but having at least some of them in the config allows adding them to version control.

Also, it does have database access (or at least it had once while I implemented it, don’t think it does currently … 🤔 but it should).

It currently doesn't. Should we pass in a Session as a method parameter?

Guess we'd have to. Note that it's also called from UserPhone.is_allowed().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:backend Related to the server component flag:caveat Might be confusing or cause problems for third-party users flag:security Impacts the overall security of the system prio:B Major impact, but not essential type:enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants