e-a-h / That-Sky-Bot

That Sky [Bot] for thatgamecompany and #thatskygame
Creative Commons Attribution Share Alike 4.0 International
14 stars 3 forks source link

Deploy secret key for Music Cog to send link to sky-music.herokuapp.com #51

Open jmmelko opened 4 years ago

jmmelko commented 4 years ago

A secret key must be generated by @Specy-wot, transmitted to @e-a-h, and stored somewhere, either in config.json, or as a GitHub secret.

Then the music cog can interrogate config.json to insert the key in a POST request to the website.

This key should not be visible in the public code of this repository.

Specy commented 4 years ago

I already sent the key to @e-a-h

jmmelko commented 4 years ago

Specy, could you please send me the API key to skymusicwebsite __at__ gmail __dot__ com ? I don’t think you can send PM on GitHub and I have disabled my discord temporarily.

I need it to test the program locally. I’ll try to set it as a GitHub secret because I think we can also make the link available to command-line users.

Tx.

Specy commented 4 years ago

Done, sent the email

jmmelko commented 4 years ago

Could you please provide me with a full request example (excluding the api key)?

I am currently submitting the request as “x-www-form-urlencoded”, with the parameters encoded in bytes.

The parameters look like this after url-encoding, and before bytes encoding:

API_KEY=[LONG KEY GOES HERE]&song=%7B%22name%22%3A+%22Untitled%22%2C+%22author%22%3A+%22d%22%2C+%22arrangedBy%22%3A+%22%22%2C+%22transcribedBy%22%3A+%22d%22%2C+%22permission%22%3A+%22%22%2C+%22isComposed%22%3A+true%2C+%22bpm%22%3A+220%2C+%22pitchLevel%22%3A+0%2C+%22bitsPerPage%22%3A+16%2C+%22songNotes%22%3A+%5B%7B%22time%22%3A+68%2C+%22key%22%3A+%221Key4%22%7D%2C+%7B%22time%22%3A+136%2C+%22key%22%3A+%221Key5%22%7D%2C+%7B%22time%22%3A+204%2C+%22key%22%3A+%221Key6%22%7D%2C+%7B%22time%22%3A+272%2C+%22key%22%3A+%221Key4%22%7D%2C+%7B%22time%22%3A+340%2C+%22key%22%3A+%221Key7%22%7D%2C+%7B%22time%22%3A+409%2C+%22key%22%3A+%221Key9%22%7D%2C+%7B%22time%22%3A+ ....

and I am using the following headers:

"Content-type": "application/x-www-form-urlencoded" "Accept": "text/plain"

I have to emphasize that the JSON wrapping is coded as a python dict but the song itself is a string:

{"API_KEY": 1234577, "‘Song’: "‘Author‘: 'Elvis', 'Song': {'time':60, 'Key': '1Key5'

What I mean is that after decoding your website will receive the song as a long string, just as if someone copy paste the notes somewhere. Is that okay?

Also, could you give me the possible errors returned? Is it always HTTP503 Service Univailable? What happens if the HTTP request is valid but the song is badly formatted? And vice-versa?

Specy commented 4 years ago

The server answers with a 500 when there was an error, a 403 if the api key is wrong, if you are getting a 503 then that comes from express, so probably something wrong with sending the information to the server.

i found this example on SO that shows (i think) how it should be done

The header should have: "Content-Type", "application/json; charset=utf-8"

This is an example of the json string that the server expects {"API_KEY":"keyHere","song":{"name":"Alone","bpm":200,"isComposed":true"bitsPerPage":16,"pitchLevel":0,"songNotes":[{"key":"1Key3","time":500},{"key":"1Key7","time":800}]}}

the post request should be done at https://sky-music.herokuapp.com/api/generateTempSong

jmmelko commented 4 years ago

I have succeeded in getting a link following your example but the song seems to be broken:

Can you have a look on your side? Cause I can’t access the data:

https://sky-music.herokuapp.com/?tempSong=ZTpuczC3 https://sky-music.herokuapp.com/?tempSong=JG83UVPi

Specy commented 4 years ago

Yes it seems to fail at parsing the song and returns undefined, I'm not at home so I can't check what's going on, very probably an issue on my end though. Can we organise a day and hour so we can test it?

jmmelko commented 4 years ago

Yes sure, probably next week though.

Another possibility is for you to create another url where the website would return a text answer, with the text being the processed data (=string output of the json after decoding).

Specy commented 4 years ago

Sure, I'm going to a birthday right now so I can't do it, I'll add a route when I come back home. In the meanwhile, if you want, you can check how data is handled in the server or our website here https://github.com/thoseSkyModders/SkyMusic/blob/e202897037f99caab43799cc455860bdba8a149e/index.js#L104

Specy commented 4 years ago

ok looks like i forgot to parse the json in the website before handling the song object so it returned undefined, i added the parsing and probably the issue was there but just for debugging purposes i changed the response the server gives, it will now send back an object like this: { url:"https://sky-music.herokuapp.com/?tempSong=someID", receivedData: {} } the "receivedData" will be what the server received so you can compare it to what you sent, i will remove this once everything is set up and working and will send back just the URL

jmmelko commented 4 years ago

Great!

here is what I got back, seems correct to me, unless I missed some subtle quote / bracket / comma typo?

(I assumed your script was smart enough to skip optional dictionary entries)

{"url":"https://sky-music.herokuapp.com/?tempSong=QWZJO1vK","receivedData":{"API_KEY":"V3...............................&","song":{"name":"Untitled","author":"d","arrangedBy":"","transcribedBy":"","permission":"","isComposed":true,"bpm":220,"pitchLevel":0,"bitsPerPage":16,"songNotes":[{"time":68,"key":"1Key4"},{"time":136,"key":"1Key5"},{"time":204,"key":"1Key6"},{"time":272,"key":"1Key4"},{"time":340,"key":"1Key7"},{"time":409,"key":"1Key9"},{"time":477,"key":"1Key8"},{"time":545,"key":"1Key6"},{"time":681,"key":"1Key4"},{"time":750,"key":"1Key5"},{"time":818,"key":"1Key6"},{"time":886,"key":"1Key8"},{"time":954,"key":"1Key6"},{"time":1022,"key":"1Key7"},{"time":1090,"key":"1Key5"},{"time":1159,"key":"1Key4"},{"time":1227,"key":"1Key5"},{"time":1295,"key":"1Key6"},{"time":1363,"key":"1Key4"},{"time":1431,"key":"1Key7"},{"time":1500,"key":"1Key9"},{"time":1568,"key":"1Key8"},{"time":1636,"key":"1Key6"},{"time":1704,"key":"1Key4"},{"time":1772,"key":"1Key5"},{"time":1840,"key":"1Key6"},{"time":1909,"key":"1Key8"},{"time":1977,"key":"1Key5"},{"time":1987,"key":"1Key4"},{"time":1997,"key":"1Key3"},{"time":2007,"key":"1Key2"},{"time":2045,"key":"1Key3"},{"time":2113,"key":"1Key4"},{"time":2181,"key":"1Key4"},{"time":2250,"key":"1Key5"},{"time":2318,"key":"1Key5"},{"time":2386,"key":"1Key4"},{"time":2396,"key":"1Key3"},{"time":2406,"key":"1Key2"},{"time":2454,"key":"1Key3"},{"time":2522,"key":"1Key4"},{"time":2590,"key":"1Key2"},{"time":2600,"key":"1Key1"},{"time":2610,"key":"1Key0"},{"time":2659,"key":"1Key0"},{"time":2727,"key":"1Key3"},{"time":2795,"key":"1Key2"},{"time":2863,"key":"1Key2"},{"time":2999,"key":"1Key0"},{"time":3068,"key":"1Key1"},{"time":3136,"key":"1Key1"},{"time":3204,"key":"1Key2"},{"time":3272,"key":"1Key5"},{"time":3340,"key":"1Key4"},{"time":3409,"key":"1Key4"},{"time":3477,"key":"1Key3"},{"time":3545,"key":"1Key2"},{"time":3613,"key":"1Key3"},{"time":3681,"key":"1Key4"},{"time":3749,"key":"1Key4"},{"time":3886,"key":"1Key5"},{"time":3954,"key":"1Key5"},{"time":4022,"key":"1Key5"},{"time":4090,"key":"1Key4"},{"time":4100,"key":"1Key3"},{"time":4110,"key":"1Key2"},{"time":4159,"key":"1Key3"},{"time":4227,"key":"1Key3"},{"time":4363,"key":"1Key2"},{"time":4431,"key":"1Key2"},{"time":4441,"key":"1Key1"},{"time":4451,"key":"1Key0"},{"time":4499,"key":"1Key3"},{"time":4568,"key":"1Key2"},{"time":4636,"key":"1Key2"},{"time":4704,"key":"1Key0"},{"time":4772,"key":"1Key0"},{"time":4840,"key":"1Key0"},{"time":4909,"key":"1Key1"},{"time":4977,"key":"1Key1"},{"time":5045,"key":"1Key2"},{"time":5113,"key":"1Key4"},{"time":5181,"key":"1Key2"},{"time":5191,"key":"1Key1"},{"time":5201,"key":"1Key0"},{"time":5318,"key":"1Key4"},{"time":5386,"key":"1Key2"},{"time":5454,"key":"1Key1"},{"time":5522,"key":"1Key1"},{"time":5590,"key":"1Key0"},{"time":5727,"key":"1Key0"},{"time":5795,"key":"1Key2"},{"time":5863,"key":"1Key4"},{"time":5931,"key":"1Key5"},{"time":5999,"key":"1Key6"},{"time":6068,"key":"1Key5"},{"time":6136,"key":"1Key0"},{"time":6204,"key":"1Key2"},{"time":6272,"key":"1Key4"},{"time":6340,"key":"1Key1"},{"time":6409,"key":"1Key0"},{"time":6545,"key":"1Key0"},{"time":6613,"key":"1Key2"},{"time":6681,"key":"1Key4"},{"time":6749,"key":"1Key1"},{"time":6818,"key":"1Key0"}]}}}

Specy commented 4 years ago

Did the url work?

Specy commented 4 years ago

ah nvm i had another issue in the front-end, should be fixed now, since we know that the server is handling the response how it should be, i removed the "receivedSong" and now the server will just answer back with the url.

The issue was that the parsing of a song in my website is done by array as it can accept an array of songs, i instead sent just the song itself, now it works, i tested it with your example and it works perfectly.

Remember, the website is cached and i didn't update the service worker version to prevent having all users download the resources again, you have to paste the link in a private tab or disable cache to see the difference (or delete the cache). As soon as it works i will refresh the cache for all users in the next update

jmmelko commented 4 years ago

Ok it works now.

I have 2 questions:

1/ the tempo is too fast, I will revert back to 120 bpm or maybe check my conversion, could you please confirm the elapsed time in ms between two notes for 220 bpm and a 4:4 beat?

2/ it seems that your website currently responds to the query by submitting the url as text. Another way to do that would be to do an HTTP redirect to the new url. Please tell me which way you will choose in the end, because it will define whether I do response.url or response.text (I could do some smart guess but let’s agree first).

Thanks.

Specy commented 4 years ago

Maybe you could add a question for the bpm information, also the example you gave me was really really fast, not even at 220bpm it should be that fast (look at the library script I made to download the song, that one uses fixed 200 and sometimes it's almost slow) so maybe you have an issue somewhere else? 220bpm should be 270ms per tick

What do you mean in question 2? If the website will just send an URL back or if it will send an object with the URL inside like this: {"url":"example.com"}?

Specy commented 4 years ago

By the way, this is not really important but if you can, can you make the first property of the song object the name of the song? (If it isn't already)

jmmelko commented 4 years ago

By question 2 I mean that, instead of sending an normal content, such as HTML, text, or a binary file, your website could send a redirection to a new url, that url being the one of the temporary song.

“In HTTP, redirection is triggered by a server sending a special redirect response to a request. Redirect responses have status codes that start with 3, and a Location header holding the URL to redirect to.“

https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections

About the BPM: I made a confusion with tempo, so I did a division by 4, never mind it’s corrected now.

(Btw yes ‘name’ is already the first dictionary entry for the ‘song’ object, even though in Python dictionaries are not strictly ordered, this entries goes out first)

jmmelko commented 4 years ago

@e-a-h I have finished implementing Specy’s URL generation. Could you please tell me how you will store the API KEY? Can I assume that I can do: Configuration.get_var('API_KEY')?

(or any other name you would like to give it).

Thank you.