-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI should use authorization header instead of cookie #2085
Comments
How do you imagine logon working for this for UI users? HTTP Basic? |
I've taken this out of v1.3. I think we have mitigated for the time being. |
@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. |
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. |
https://github.com/slackhq/csp-html-webpack-plugin - maybe this? |
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? |
Isn't session-storage safe? https://stackoverflow.com/questions/5523140/html5-local-storage-vs-session-storage |
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. |
@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) |
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. |
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. |
Following #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.
The text was updated successfully, but these errors were encountered: