-
-
Notifications
You must be signed in to change notification settings - Fork 170
Set httpOnly to false #903
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
base: migration-branch/3.0
Are you sure you want to change the base?
Set httpOnly to false #903
Conversation
httpOnly is a serverside flag and adding it to the cookie make ostrio:files not add it
I am also wondering why this token is needed in the first place? Is it to make it work when the CDN is served from a different domain? Why not just rely on the Meteor session? |
@make-github-pseudonymous-again It's a very interesting way. My theory is it's to only allow users who're currently active (in a session) to view the files AND logged in, because a user could be logged in long time ago (Meteor logins last around ~90 days by default). But I'd like to hear from @dr-dimitru |
@harryadel After some research, I think I have answers to your questions: From looking at Meteor's sources, it looks like the login token is stored in A new "session" id is generated for each tab, and regenerated on refresh, and stored at
The login token or the session id can be used to identify the user. The session id has the advantage of being short-lived and more identifiable, since you can distinguish between different browser tabs. The login action (whether active, or passive on tab refresh) links the login token to each session here:
Since everything in vanilla Meteor happens over DDP (methods and subscriptions over WebSocket), there is no way for the server to drive the creation of the corresponding cookie on the client.
You could expose an HTTP endpoint that responds with a set-cookie instruction for one of those, but pay attention to how this HTTP request does not escape the DDP-curse: Line 535 in 1ed12d1
It does not work if you do not pass a way to identify the user first. You could make the endpoint accept the "login token" or "session id" as an authorization header token though. |
const setTokenCookie = () => { | ||
if (Meteor.connection._lastSessionId) { | ||
cookie.set('x_mtok', Meteor.connection._lastSessionId, { path: '/', sameSite: 'Lax', secure: true, httpOnly: true }); | ||
cookie.set('x_mtok', Meteor.connection._lastSessionId, { path: '/', sameSite: 'Lax', secure: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this is not just a local testing issue? Are you using localhost
? Would the following be more appropriate:
cookie.set('x_mtok', Meteor.connection._lastSessionId, { path: '/', sameSite: 'Lax', secure: true }); | |
cookie.set('x_mtok', Meteor.connection._lastSessionId, { path: '/', sameSite: 'Lax', secure: Meteor.isProduction, httpOnly: Meteor.isProduction }); |
See also https://forums.meteor.com/t/security-dont-store-tokens-in-localstorage/50539. |
More precisely,
Admitedly, the current docs at MDN are not crystal clear, but they do say:
Here is a backup of the old docs that are a bit more explicit:
You can actually see the following in the client's browser logs: Cookie “x_mtok” has been rejected because there is already an HTTP-Only cookie but script tried to store a new one. and you can easily reproduce that: >> document.cookie = "foo=bar; HttpOnly";
Cookie “foo” has been rejected because there is already an HTTP-Only cookie but script tried to store a new one. Essentially, the approach is: "If an HttpOnly cookie cannot be accessed from client code, then it should not be created from client code, so we forbid that.". |
It looks like that feature is already available via
It's just that Lines 119 to 121 in 1ed12d1
|
Thank you for the deep dive and thoughtful answer @make-github-pseudonymous-again. It's rare that I come across Meteor users this knowledge. I'd love to get in touch. Do you have an email? I'm still interested to hear from the big dawg @dr-dimitru and possible a new rc till we get to the bottom of this, maybe? |
You can reach me at [email protected] |
@harryadel @make-github-pseudonymous-again thank you for this detailed discussion, pardon me for joining it so much later.
P.S. I'm going to keep |
@harryadel do we have |
httpOnly flag is a server side flag, adding it to the client side cookie creation code invalidates it.
It's definitely possible that we can move the token creation to the server-side for more secure tokens but it's necessary to have you green light it first @dr-dimitru
I'm thinking we inject the token somewhere here:
Meteor-Files/server.js
Line 526 in 1ed12d1
and we completely remove the need to instantiate the token on the client side. Do you have any objections? Let me know about the best route and I'll gladly implement it.
I'm even more interested as to why you did it this way. Why was it created on the client side then sent to the server. Another peculiar problem is why do you rely on the
lastSessionId
to validate the request and locate the user. Why not store the usual Meteor local storage token in the cookie and use it later on to retrieve the user. This is one of the most unique was I've ever encountered in Meteor.Thanks to @fidelsam1992 / @determinds for pointing this out and the time/money. 😁