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

Removed ccard.js in favor of public domain code #3983

Merged
merged 2 commits into from
May 20, 2024
Merged

Removed ccard.js in favor of public domain code #3983

merged 2 commits into from
May 20, 2024

Conversation

fballiano
Copy link
Contributor

@fballiano fballiano commented May 8, 2024

This PR targets next

We're this weird ccard.js file that has a weird copyright and the license is not clear, you can see that:
Screenshot 2024-05-14 alle 12 41 58
This seems to be copyrighted code and has to be removed from openmage

Credit cards use Luhn algothithm which is public domain.

This PR removes this file (a network connection just for a single javascript function is not a good idea) and reimplemented the validation method using the public domain code.

@github-actions github-actions bot added Template : admin Relates to admin template Template : rwd Relates to rwd template Template : base Relates to base template Component: Adminhtml Relates to Mage_Adminhtml Component: Page Relates to Mage_Page Component: Oauth Relates to Mage_Oauth JavaScript Relates to js/* XML Layout labels May 8, 2024
@@ -916,6 +916,25 @@ function parseNumber(v)
return parseFloat(v);
}

function validateCreditCard(input) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Test
validateCreditCard('') returns true.
validateCreditCard('222 2222') returns true.

Need some tweaking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kiatng did you run the same tests on the old version?

Screenshot 2024-05-20 alle 11 15 30

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the new version behaves exactly like the old version, nothing less nothing more

@kiatng
Copy link
Contributor

kiatng commented May 20, 2024

I asked Google Gemini to code:

function validateCreditCardNumber(inputString) {
  // Remove non-numeric characters from the input string
  const sanitizedString = inputString.replace(/\D/g, "");

  // Check for empty string or all repeated digits
  if (!sanitizedString || sanitizedString.split("").every(char => char === sanitizedString[0])) {
    return false;
  }

  // Check the length based on common card lengths
  const validLengths = [13, 15, 16];
  if (!validLengths.includes(sanitizedString.length)) {
    return false;
  }

  // Check the starting digits based on common card issuers
  const validStartingDigits = {
    "4": [ // Visa
      "40", "41", "42", "43", "44", "45", "46", "47", "48", "49"
    ],
    "5": [ // MasterCard
      "51", "52", "53", "54", "55"
    ],
    "3": [ // American Express, Diners Club
      "34", "37" // Amex
      // "300", "305", "36", "38" // Diners Club (potentially not widely used)
    ],
    "6": [ // Discover
      "6011", "622126", "622925", "644", "645", "646", "647", "648", "649",
      "65"
    ]
  };

  const firstDigit = sanitizedString.charAt(0);
  const startingDigits = sanitizedString.substring(0, 2);

  if (!Object.keys(validStartingDigits).includes(firstDigit) ||
      !validStartingDigits[firstDigit].includes(startingDigits)) {
    return false;
  }

  let sum = 0;
  let isDouble = false;

  // Iterate through the digits from the rightmost side
  for (let i = sanitizedString.length - 1; i >= 0; i--) {
    let digit = parseInt(sanitizedString.charAt(i));

    // Double every second digit (starting from the rightmost)
    if (isDouble) {
      digit *= 2;
      // If the doubled digit is greater than 9, add the digits individually
      if (digit > 9) {
        digit = 1 + (digit % 10);
      }
    }
    sum += digit;
    isDouble = !isDouble; // Toggle the doubling flag for the next iteration
  }

  // Check if the sum is a multiple of 10
  return sum % 10 === 0;
}

Tested with real visa cards, amex, and master card. Also tested invalid number by editing the real ones.

@fballiano
Copy link
Contributor Author

@kiatng no man that's done in another part of our codebase, the length and prefix check, that's precisely why I didn't change the logic and how the functions work.

again, prefix and length check is already done somewhere else.

@fballiano
Copy link
Contributor Author

check validate-cc-number in validation.js

@fballiano fballiano merged commit afd4ba3 into OpenMage:next May 20, 2024
14 checks passed
@fballiano fballiano deleted the modernccard branch May 20, 2024 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Oauth Relates to Mage_Oauth Component: Page Relates to Mage_Page JavaScript Relates to js/* Template : admin Relates to admin template Template : base Relates to base template Template : rwd Relates to rwd template XML Layout
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants