eraeco / pay

Checkard is a peer-to-peer check payment system for ERA PAY.
MIT License
4 stars 0 forks source link

Feature/rememberme #11

Closed jazzyjackson closed 1 month ago

jazzyjackson commented 1 month ago

I don't know if this is how you want to do modules, but I just created a 'rememberme.js' script that loads with a deferred script tag in the head.

This module attaches a second 'click' listener on the 'send' button as the point where the check is assumed to be filled out and personal details are committed to localStorage

On load, we check if localStorage.isPremium is truthy and make sure we're not in flip mode (indicating that we're looking at a check that's already been filled out and just needs to be endorsed, not edited)

If so inject the values into the check and set the classes to hide the placeholders, as well as triggering the micr function to render.

amark commented 1 month ago

please:

  1. rename to remember.js
  2. move to bottom (not head/top) Many modules will have UIs that get injected into the Tray, this one does not, so this approach is fine for now, tho other modules will be with the jquery $.load( we discussed before, which I might want everything switched over to later but let's roll for now.

not sure I want mod as the folder name (i'm anal about naming lol) but admittedly its pretty good. I kinda like /+/ since PLUS is URL allowed, but if you feel strongly against it let me know.

Generally I dislike 2 word (anything that could cause camelCase or camel_gap arguments) variables. So for instance, isPremium should just be Proprietor.

Note, you did everything right, I didn't want to get into the following details until after the initial prototype was sketched out, so now all my annoying disclosures:

(3) to comply with regulations, tho this doesn't apply because its their physical device, but likewise holding ourselves to higher standards, the banking information will need to be encrypted before saved. For reals, this should be a PBKDF2 extension of a PIN they provide, and that is a feature that'll work when registered user accounts exist - for now, since this is local to their device only, we're just gonna encrypt with uniform key "rest", the point of this is not to prevent manual attackers or brute force, but OS or system level sniffers that are scraping unencrypted data at rest in appData folders and stuff. I'm fairly certain the Browser also encrypts this data at rest likewise, but it doesn't matter, I want us to do our best - especially since this may (?) be the world's 1st Open Source payment processor, so I want to set the highest standards. await SEA.encrypt(json, "rest") (I hate JSON but silly to use Book here, so shrug for now, I'll regret this later lol).

(4) similar to (3) we're only going to locally store their data on their device IF they're already a Proprietor, rather than storing ahead of time & then checking on read (altho this is easier, and I'm happy you did it). So the way this should work is... remember module will inject a "remember me" checkbox after/below the "send" button, when clicked that will scroll people to the Tray page with the "blah blah here's why you should pay $99/mo for the Proprietor features: SUBSCRIBE BUTTON", when they click the subscribe/upgrade (whatever, naming TBA) button we will immediately give them a free trial of Proprietor, locally stash their information (rather than again & again on each time they hit send), and open a new tab which is our paylink for them to pay us. On that new page, since they got immediately free trial... the full check should be fully auto-filled in from both our paylink + their rememberme, this will also let us test that it is working. Of course, they can exit at any point and go back to sending their original check as normal.

cloudflare-pages[bot] commented 1 month ago

Deploying pay with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1a1db3e
Status: ✅  Deploy successful!
Preview URL: https://f4571174.pay-cwc.pages.dev
Branch Preview URL: https://feature-rememberme.pay-cwc.pages.dev

View logs

jazzyjackson commented 1 month ago

This is looking pretty good now. Couple of caveats:

I decided to test whether values are non-empty before allowing 'remember me' to be checked. Consequence tho is if you uncheck the box and refresh the page, there's no way to recheck the box to fill in the remembered details. You can manually set localStorage.Proprietor to true and refresh and then everything is filled in again. I guess I could check if details have already been saved and fill them in when you check the box?

Also I'm not proud of line 37 in remember.js, to await the decryption I use an async IIFE because the containing function is a callback that jquery calls once loading html is complete - if I make this callback async it doesn't get called at all

amark commented 1 month ago

Thank you for the progress @jazzyjackson !

Testing: Check # was remembered (incorrect, or if we do, we should ++1 each time), personal address remembered (correct, altho the label is wrong... tho fix could be in any branch) but doesn't remember bank address, MICR correct. Let's have it remember signature too. I change my mind, Remember Me checkbox should go ABOVE email/send .submit line (it being below, made me think to remember who I'm paying, not the check itself).

Code: (1) Sorry I made it awkward, there should be +/remember.html and the JS inlined there, with bottom of index.html having a 1 line jquery load for it (no callback needed, as the remember.html JS will run upon load). Not index loading JS that loads HTML. The .remember label CSS can also go in remember.html. We should either make en/de be on a shared app global (maybe ard is cute?)` or just call SEA directly no util.

(2) Oof PII is extra scary sounding lol rather just call it remember or save or local variable, or Proprietor. The localStorage should only be ONE item Proprietor you can do if(!!localStorage.Proprietor for existence check to cast to Boolean.

(3) Oof I hear you about uncheck (hadn't thought about that), and I agree (it may just mean temporary check being written not meant to be saved), but I am scared of liability so if they uncheck it let's purge/delete/wipe everything for now (in future we'll add a tray prompt that gives options of "cancel plan, delete data, add new check account, etc.").

Did I miss anything?

jazzyjackson commented 1 month ago

I had check # auto incrementing but its a little trickier with it stored as an encrypted blob, will have to decrypt, increment and encrypt again. Otherwise it will just be one more than it was when it was remembered the first time. Would that be allright?

Turning the import process into one step instead of two makes sense, should be no problem.

I think I'll just call SEA directly and drop the util functions.

Will push a commit now that stores a single Proprietor object instead of separate boolean, that makes sense too. Been awake a bit too long so will double check everything tomorrow.

jazzyjackson commented 1 month ago

what do you think of this solution to check #: increment when writing props from localstorage to DOM, and then on clicking the send button, if Proprietor exists then overwrite it with current contents of check (including the incremented check number)

this ensures we always remembers the most recent data entered (most time, only check number has changed) -- if Proprietor already exists on localstorage, update it with contents of check - if Proprietor does not exist skip it.

amark commented 1 month ago

hmm not seeing the commit (?) or maybe haven't done the import changes yet.

amark commented 1 month ago

Yesssss!!! I think this is ready to pull.

(what the heck happened to the HTML diff lol, I actually don't care... I assume just an ident level?)

will pull without, but a nit-pick might be

$('.remember input').on('change', async function(eve){
  if(!eve.target.checked){
    localStorage.Proprietor = "";
    return;
  }
  /* ... positive case logic not needing to be IF nested idented ... */
})
jazzyjackson commented 1 month ago

ugh, yeah, I lost a DIV at some point and auto-formatted the document to find it, thought I had reverted it.

amark commented 1 month ago

Tested it, working. Tho uggh noticed 2 things:

(1) Upon pulling, the filler text is gonna get published/deployed, so let's CSS hide that for now since tray kinda ugly currently. (2) Need to save a began property as +new Date so we can later write code to check for free trial length.

And might as well toss in (4) fix labels top left "Your Name \n Your Address"etc. not bank, bottom left "Your Bank \n Bank's Address"etc. and (5) I don't really like console logs/warns published, fine in dev, but comment them out for deploy/prod?

amark commented 1 month ago

WAHOOOOO 🎉 !!!!!!!!!!!!!