API Call for Promos #157
API Call for Promos #157
Conversation
arpavlic03
requested review from
maharris1011,
jranta,
CoreySchnedl,
seanlawrenz,
slythr,
jthoff10 and
casecadet
| @@ -59,8 +59,14 @@ <h4><br/>Offer valid until May 1st, 2020<br />To submit your business, fill out | |||
| </div> | |||
|
|
|||
|
|
|||
| <script src="https://code.jquery.com/jquery-3.2.1.slim.min.js" | |||
There was a problem hiding this comment.
I think this is not necessary. We load it in the footer.
_includes/promos/10tvpromo.html
Outdated
| submitButton.disabled = true; | ||
| e.preventDefault(); | ||
| var xhr = new XMLHttpRequest(); | ||
| var myAPIKey = process.env.AIRTABLE_KEY; |
There was a problem hiding this comment.
Where is the environment variable set? This code will execute on the client who likely doesn’t know our Airtable key.
|
Hi, once I get back online, I will change the code to use site.env.API_KEY as a hidden element in the template...which is already configured in the jekyll configuration. That way the client can obtain the api key to use to post to the airtable |
|
i'm not sure about that either. That'll work, but the challenge is that it'll generate directly into the site code. The reason it is OK now is that the API key never shows up in the HTML/JS that goes to the client. It's only used at build time by a plugin. That won't work in this case, b/c you need to write to the Airtable directly and to do that the javascript has to have the key. The user could always "View Source" in the browser and see the key. I think opening up an API on Amazon Lambda or Google Functions or Firebase is the right way to go in this case. With the API we could sanitize the input, do some business logic (like sending email), and protect the API key. I'm super happy to work on this with someone if they want to. Like I mentioned, @CoreySchnedl did a killer job with some research into Firebase functions as APIs the other day. |
|
@maharris1011 @arpavlic03 do we want to set up a quick call or slack thread to discuss what the best solution for this is? I am available anytime this evening. Unfortunately, there's no real way to do this 100% securely from what I've seen. Either A, we expose the api key client side and bad users can just make the same api calls with the api key, or b we move the logic to some back end, but that back end will be just as vulnerable as option A. Moving to lambda would only move the vulnerability to someone trying to take advantage of the public aws api gateway endpoint, and would only complicate things even if our api key would technically remain secret.. IMHO, although we probably don't want people having to do manual tasks, the best way to get around this problem is to change that form into an email template that goes to a new email that we don't care if malicious users try to spam it. Or, if that doesn't seem best, we could just add a google recaptcha to at least protect from completely malicious attacks. https://developers.google.com/recaptcha. |
|
Hello, unfortunately tonight is not the best night for me for a discussion. Anytime tomorrow will be better day for me to talk about this in detail |
|
@arpavlic03 - i opened up an api this weekend at https://wduc7ys73l.execute-api.us-east-1.amazonaws.com/dev/promotions/create that takes form input & adds it to the Promotions view of the Airtable base, using the same key that @jranta used in his PRs. if you modify your form to use It solves the problems with api keys we talked about and reduces the need for code in your PR. Attached is a Postman file that you can import to work with the API directly (change to .json extension first) |
_includes/promos/10tvpromo.html
Outdated
| reject("Failure"); | ||
| }; }); | ||
|
|
||
| async function myFunction() { |
There was a problem hiding this comment.
If we want to use async we'll need to add babel to the project.
And if we're doing that, we should use fetch.
_includes/promos/10tvpromo.html
Outdated
| e.preventDefault(); | ||
| console.log("COMMENCING CHECK"); | ||
| myFunction() | ||
| .catch(stuff => { |
There was a problem hiding this comment.
mismatching .catch and async.
On async and await functions, you want to use a pattern of try ... catch.
There was a problem hiding this comment.
Hi Sean! I will be talking to Mark tomorrow but there might be a possibility that we don't need the async/await at all if we call a lambda-oriented api that can handle the post for us and hopefully check for duplicate records.
In any other case, I would definitely agree with you. It worked locally on my machine with the catch, .then, asyncs, awaits and Promises with good ol' jquery but instilling try/catch in all of my functionality will be much more helpful.
|
@maharris1011, just wanted to let you know before our call that the api works! Thank you so much for all of your hard work with this. |
|
I am closing this PR until we get the green light from TV10 regarding the Can't Stop Columbus partnership |
Backend for the Promo Form to post into AirTable - 10TVPromos.