search

mountetna / etna

Base gem for Mount Etna applications
GNU General Public License v2.0
0 stars 0 forks source link

Disable cookie auth #60

Open graft opened 4 years ago

graft commented 4 years ago

Etna::Auth currently accepts cookies as a valid way to present your token. While Janus token cookies are marked with secure: true and same-site: strict, they are still vulnerable to CSRF attacks (i.e., they will be posted automatically with any request to an etna service).

We may instead completely disable cookie-based authentication and reform all JS clients to add Authorization headers to requests. Most other clients (e.g. Etna::Client or MetisClient) already don't use cookies, so this avenue should only involve some JS changes. However, clients in Timur, Janus and Metis (e.g. anything using fetch) will all have to change to reflect this amendment.

coleshaw commented 4 years ago

Is the goal to roll this into the current Magma - Metis work, or should I close that out for now using credentials: include and we'll pick this up later?

corps commented 4 years ago

I am in favor of this change for sure. Cookies are decent for storage client side, but explicitly reading and setting headers is more controllable authentication broadly.

corps commented 4 years ago

Is the goal to roll this into the current Magma - Metis work, or should I close that out for now using credentials: include and we'll pick this up later?

Personally I think it's okay to roll out as is. The credentials include in this case has minimum risk because the url is hard coded, not suer supplied. Then we could roll out the header change widely and separately from other work.

graft commented 4 years ago

Yeah there's little risk in the magma-metis case because of all the signing; we can fix this up later.