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

Add ABA account number assert #240

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

acostalima
Copy link
Contributor

@acostalima acostalima commented Jun 16, 2023

Add ABA account number assert.

The number of digits in a Bank of America account number can vary depending on the type of account. Typically, account numbers for personal checking and savings accounts have between 8 and 12 digits. Business accounts may have a different number of digits.

Chase bank account numbers typically consist of eight to 12 digits, but some account numbers could even contain up to 17 digits.

Plaid: https://plaid.com/resources/banking/account-numbers-explained/#what-is-an-account-number

An account number is a unique string of numbers, letters, and other characters that identify a specific financial account. Almost all financial transactions make use of account numbers.

In the United States, each bank assigns account numbers using its own methodology, kept private for security reasons. Most bank account numbers have between 8 and 12 digits, though they can range from 5 to 17.

* Account number regex.
*/

const accountNumberRegex = /^\d{8,17}$/;
Copy link

Choose a reason for hiding this comment

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

Are we sure about the minimum limit? It seems at ach gateway we're letting min=1...
Can we double check production's db for account number minimum number of digits before deploying this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll double check.

Copy link
Contributor Author

@acostalima acostalima Jun 19, 2023

Choose a reason for hiding this comment

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

Based on the information I've read on Plaid's website, I've updated the regexp range from 5 to 17 characters where each character can be:

  • a digit
  • a lowercase letter
  • an uppercase letter

src/asserts/aba-account-number-assert.js Show resolved Hide resolved
* Account number regex.
*/

const accountNumberRegex = /^\d{8,17}$/;
Copy link

Choose a reason for hiding this comment

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

Also, we don't seem to be validating the digit-only part. Both ach gateway and backend seem to allow letters, are we sure about this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll double check.

Copy link
Contributor Author

@acostalima acostalima Jun 19, 2023

Choose a reason for hiding this comment

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

Based on the information I've read on Plaid's website, I've updated the regexp range from 5 to 17 characters where each character can be:

  • a digit
  • a lowercase letter
  • an uppercase letter

@acostalima acostalima force-pushed the feature/aba-account-number-assert branch from b1ff424 to 3f005dc Compare June 19, 2023 11:03
Copy link

@jcm300 jcm300 left a comment

Choose a reason for hiding this comment

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

See @arop comments.

@acostalima acostalima force-pushed the feature/aba-account-number-assert branch from 6e04f4d to cf97b56 Compare June 19, 2023 16:16
@acostalima acostalima force-pushed the feature/aba-account-number-assert branch from cf97b56 to b5c7c73 Compare June 19, 2023 16:18
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.

3 participants