armada-ths / ais

Armada Internal Systems (AIS)
https://ais.armada.nu
Other
17 stars 6 forks source link

Feature request from android #19

Closed jeppes closed 8 years ago

jeppes commented 8 years ago

The /companies and /events endpoints follow the format: { companies: [...] }

In the next version of the api can we just get the JSON array directly without this wrapper?

macaullyjames commented 8 years ago

Sorry for the late response 🙈 This is already in the spec for /companies and I can't imagine /events would be handled differently. See #16 for details and further discussion :)

macaullyjames commented 8 years ago

Wait that wasn't actually so late 🤔 Must have been thinking of another issue 😛

apals commented 8 years ago

https://www.owasp.org/index.php/OWASP_AJAX_Security_Guidelines#Always_return_JSON_with_an_Object_on_the_outside

http://stackoverflow.com/questions/3503102/what-are-top-level-json-arrays-and-why-are-they-a-security-risk

"We wrap the heroes array in an object with a data property for the same reason that a data server does: to mitigate the security risk posed by top-level JSON arrays." https://angular.io/docs/ts/latest/guide/server-communication.html

macaullyjames commented 8 years ago

This vulnerability was fixed in all major browsers in 2009-2010 according to the flask documentation and MDN. Some random people on Github had the same problem and mentioned this SO thread that says something to the same effect. Even if it wasn't fixed the attack wouldn't apply here since it's "just" a breach of confidentiality. The data served by the API is always non-confidential since there's no authentication going on (at least for the time being) :)

macaullyjames commented 8 years ago

Is it OK to close this again @Jeppes?

macaullyjames commented 8 years ago

Woops, I meant @apals ^^ You OK with that too?

apals commented 8 years ago

yup just thought id mention it. :)

macaullyjames commented 8 years ago

I'd actually never heard of this exploit before so thanks for pointing it out :) 👍