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(canSendEmailOtp): use a query to find whitelist match, rather than fetching the whole table #1296

Merged
merged 2 commits into from
Apr 12, 2024

Conversation

timotheeg
Copy link
Contributor

Problem

We have a whitelist of domains and specific allowed emails in DB. Before we allow a OTP to be sent to an email, we check against the whitelist if the input email is allowed.

The way this is done is to fetch the entire table content (with a slight filter for expiry) and do a match in code 😑

It would be much more efficient to use the database query capabilities to run the match on the DB directly. We can expect that to be both faster and transfer much less data.

Solution

Run a raw efficient query to return an easy yes/no state on whether an email matches an entry in the whitelist.

We can not use the ORM capability because we are not matching a cell value to some input. Instead we are matching the input against the cell value with a endsWith behaviour. This is done by using the <input> like concat ('%', field) syntax.

As we want the query to exit as early as possible (any match is enough), we run the query to return 1 on match and limit 1.

While DB engine do not offer guarantees of early exit, in principle, this can exit on the first match found and is thus as efficient as it can be made.

For email which do not match, there will be a full traversal of the table, which is about the same as the previous code, which was also doing a full table scan.

We return the whitelist entry that matched so we can log and verify

@timotheeg timotheeg requested a review from a team April 12, 2024 13:09
@timotheeg timotheeg merged commit 5a14d72 into develop Apr 12, 2024
20 of 21 checks passed
@mergify mergify bot deleted the staging branch April 12, 2024 14:15
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.

1 participant