argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.47k stars 5.31k forks source link

UI should use authorization header instead of cookie #2085

Open jessesuen opened 5 years ago

jessesuen commented 5 years ago

Following https://github.com/argoproj/argo-cd/issues/1642, the UI should switch to authenticating calls using HTTP Authorization headers instead of a cookie.

This will allow us to double the size of the JWT token to 8KB (from 4KB), which users hitting when attempting to login and they are a member of too many groups.

alexec commented 5 years ago

How do you imagine logon working for this for UI users? HTTP Basic?

alexec commented 5 years ago

I've taken this out of v1.3. I think we have mitigated for the time being.

jannfis commented 4 years ago

@jessesuen @alexec How do we proceed with this topic? If we don't switch from authentication cookie to authentication header in the UI, then we really should implement additional CSRF protection for the API when being used by a browser which sends cookies (even if we now have SameSite attribute on the authentication cookie). If this issue stays wontfix, I'll start working on implementing proper CSRF protection using something like gorilla-csrf, but I really don't want to do this for the trashcan - i.e. if we switch from cookie to auth header.

alexmt commented 4 years ago

If we stop using cookie than UI will have to store token somewhere else. The local storage is definitely not secure. Honestly, I don't which other options are available.

I would keep using cookie in UI and implement CSRF protection.

alexec commented 4 years ago

https://github.com/slackhq/csp-html-webpack-plugin - maybe this?

alexmt commented 4 years ago

I've never heard about CSP before. Briefly checked this link from plugin readme: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy . Seems like CSP allows to control which files browser can load for the page. I could not figure out how it helps to store token. @alexec Am I missing something?

davidkarlsen commented 4 years ago

Isn't session-storage safe? https://stackoverflow.com/questions/5523140/html5-local-storage-vs-session-storage

mhamann commented 4 years ago

What about converting to session IDs that are associated with a JWT stored on the server? Cookies are a good solution, but in my experience, storing JWTs in cookies isn't great. Session IDs are much more compact.

davidkarlsen commented 4 years ago

@mhamann you then need a server-side session-store to have the link between sessionId and token (which has the claims) - leading to server-side state, which requires a store like redis or similar and also influences scalability (maybe not a big issue though)

RichiCoder1 commented 4 years ago

It'd be a bit more complicated, but another answer might be "chunking" the JWT, or splitting it across multiple cookies and then stitching it back together if it's larger then the 4k limit. That's how AspNet Core manages it.

alexmt commented 4 years ago

Ah, that is awesome suggestion @RichiCoder1 ! I really like it. It might be much easier to do because all the logic resides on server side.

pranaypathik commented 3 years ago

Ah, that is awesome suggestion @RichiCoder1 ! I really like it. It might be much easier to do because all the logic resides on server side.

I was wondering if this was still looked into. I was very interested in a fix for this.