Skip to content
This repository was archived by the owner on Oct 4, 2023. It is now read-only.

[C-781] Add minimum threshold to common password check#1684

Merged
Kyle-Shanks merged 1 commit into
mainfrom
kj-Add-min-threshold-to-common-password-check
Aug 5, 2022
Merged

[C-781] Add minimum threshold to common password check#1684
Kyle-Shanks merged 1 commit into
mainfrom
kj-Add-min-threshold-to-common-password-check

Conversation

@Kyle-Shanks

Copy link
Copy Markdown
Contributor

Description

Add a minimum threshold to the common password check to make it less aggressive

Dragons

N/A

How Has This Been Tested?

Manually tested

How will this change be monitored?

Will watch to see if the check is still too aggressive

Feature Flags

N/A

@sliptype sliptype left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! My other thought is that it could be more performant to only traverse the text once and create an object of { hash: value }. Then could check if the entry exists in the object and what the value is in constant time

const hashArr = text.split(/\s+/g).map((s) => s.slice(0, s.indexOf(':')))

// If there is no match, return false
if (!hashArr.includes(hash.slice(5))) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we be checking for the full hash instead of just the first 5 characters? Here and on line 35

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

opposite. this is slicing off the first 5 chars and checking the rest. this is just because of how the API returns the data

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah gotcha

@audius-infra

Copy link
Copy Markdown
Collaborator

@Kyle-Shanks Kyle-Shanks merged commit 398093d into main Aug 5, 2022
@Kyle-Shanks Kyle-Shanks deleted the kj-Add-min-threshold-to-common-password-check branch August 5, 2022 18:01
audius-infra pushed a commit that referenced this pull request Aug 6, 2022
[c3682b3] [C-780] Update AudiusBackend to work in native context (#1677) Dylan Jeffers
[398093d] [C-781] Add minimum threshold to common password check (#1684) Kyle Shanks
[5a66847] Ensure fetch calls are awaited so errors are caught (#1683) Sebastian Klingler
[d597054] Make audiusBackend configurable for different environments (#1670) Sebastian Klingler
[b5fa3fa] Fix native build with new metro-config dependencies (#1676) Dylan Jeffers
[408a425] [C-771] Update native metro-config to enable @audius/sdk (#1671) Dylan Jeffers
[afde38e] [PAY-416] Early access mode & feature flag refactor (#1654) Michael Piazza
[3b28c7e] Fix typecheck for mobile after AudiusBackend ts migration (#1672) Sebastian Klingler
[e5fbbb5] Revert "Revert "Update explore endpoints in libs to use v1 (#1539)" (#1609)" (#1669) Joseph Lee
[237784b] Default endpoints to empty array instead of null (#1665) Marcus Pasell
[175e5d6] Add ModalContentPages to stems (#1659) Marcus Pasell
[fdadaac] fix mobile menu on narrow (#1664) nicoback2
[3e2b499] Convert AudiusBackend to typescript (#1650) Sebastian Klingler
@AudiusProject AudiusProject deleted a comment from linear Bot Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants