this.userId sometimes null in protected function on server #737
Comments
|
Hello @Lickshotz, While I'm looking in the issue, please help me answering:
|
|
Thanks for the quick reply.
|
|
@Lickshotz on the second: sometimes solutions like MUP, Phusion Passenger, and other launch multiple application instances for balancing and durability |
|
@dr-dimitru I'm using pm2 to manage my application. I'm not running it in cluster mode. |
|
@Lickshotz can it be related to the #738 issue? |
|
@dr-dimitru I don't know, as this has happened on chrome mobile, safari and firefox and I also don't know what he means by: |
|
May this be related to browser's privacy settings? |
|
@jankapunkt du you have any hints on how I could find that out? |
|
Since cookies are involved I would compare browser settings accept all cookies vs rejecting all cookies. |
|
This seems odd, as the cookies are set in the browser of the users having this issue. I will give it a try though |
|
@jankapunkt @dr-dimitru I did indeed check with the user that has the cookie, but does not have the |
|
Any other plugins installed that could break things? |
|
@jankapunkt not that I know of. I have listed my packages above and here are my npm packages |
|
hey @dr-dimitru, |
|
@Lickshotz so far I'm considering something is off on your end. Cookies and authentication pipeline is something we (@jankapunkt, @menelike, and @s-ol) was working on for long time. So far it's well tested (browser/mobile/cordova) and well supported part of the library. You've mentioned it happens only on production, possible scenarios:
|
|
Willing to help if that happens on Cordova, on the browser this should actually be no issue. |
|
@dr-dimitru @menelike I will try to create a minimal reproduction repo.
|
|
Here is a minimal reproduction repo: https://gitlab.com/Lickshotz/files-collection-no-userid-in-protected.git email: test@mail.com Last night, I tested it on production (using the production db as well) Chrome on OS and Windows 10 had no problems. Safari had problems regardless of device. I cannot say about Firefox as I never have issues with firefox, but other users do. In production, I'm running a nginx with a reverse proxy set up, the app is managed by pm2. |
|
@Lickshotz Great, one question though, when you speak about safari, are you speaking about Safari mobile browser only and/or ios Cordova? Just asking to be sure. |
|
@menelike I'm talking only about Safari mobile browser and Safari OS browser. |
|
I still can not reproduce this in my end: I can upload a video file in Safari Update: I need to fix your code first, upload ended somewhere on the disk, URL seems to be malformed hence I can not test the playback e.g. Update: I just used the URL directly ( Update I tried a different video format (MOV file), Safari works fine while uploading and viewing. Result: I can not reproduce this issue. |
|
Works in Xubuntu 18.04. LTS on Firefox (75.0 64bit) and chromium (80.0.3987.163, 64bit) => App running at: http://localhost:3030/
I20200428-08:54:51.266(2)? YFZAcvSz85qLnmAJP
I20200428-08:57:33.878(2)? YFZAcvSz85qLnmAJP |
|
I made it to
|
|
@menelike thank you for testing it. I am completely baffled. Not because you cannot reproduce it, as in my dev environment I am not able to do so either. As to the "." at the end of the link, this equally baffles me, I have noticed it beeing there, but did not know that it shouldn't be there as I'm simply using the Videos.link(file) function to get that link. I removed the I have an android device at home that has that issue running Chrome 81.0.4044.111 I have ran the application without pm2 in production, the issue persists. I will now setup a docker container and build the app locally to see if I can reproduce the issue. |
|
@jankapunkt thank you for testing it. Do you have the cookie set in the browser when using that custom setting? As I have confirmed that:
|
|
Hey @jankapunkt @dr-dimitru @menelike, I was pretty busy setting up the docker image today but in the end, could not reproduce the problem (the device that doesn't work in production had a userId in the protected function). I assumed that nginx is the problem as this was the only thing that I haven't tested. Later today a user was having problem uploading a video with the following error:
which i did do some brief reading on and seems to have something to do with cookies (but im honestly not 100% sure about that). I just reverted back to ddp hoping to solve that issue (without even thinking about the protected function), which it did and the nginx started to show cookies for every request. I assume that it seems to be an issue in the way I have setup https in my nginx. Does any one of you have insight about that? Update: Seems that I have been mistaken, even though it worked on my "problematic" device, I will log it tomorrow during the day and check with some firefox users. |
|
@jankapunkt, @menelike thank you for participating in this one
|
|
I feel your pain bro ;). It had to debug those things so many times, its frustrating, but I always learnt a lot in the end. As already suggested the Nginx configuration might be the issue here, here is the Meteor-Files part from our config (though we use Nginx in docker only locally with self-created SSL certs): and this is our CSP (you need to remove a lot of stuff and replace
Please keep in mind, that once you do scaling in Nginx you need to have sticky sessions, no matter what.
🖖
I highly advocate against that, this is a special dirty workaround designed for Meteor/Cordova, also I am not sure if this helps here at all. |
|
This shouldn't be the problem if it affects desktop Safari, but you should check the CORS headers coming out of and going into nginx if possible.
I don't think so, it is an insecure way of doing it (sensitive data in query string is bad practice) and pollutes the browser URL history. Also the client has to programmed specifically with this in mind. |
|
@dr-dimitru @menelike Btw. After enabling logging in nginx and observing today I noticed that a lot of time a user will have the x_mtok cookie while beeing on /sockjs and it beeing gone once on /cdn. I also found out about ostrio:logger logger today (awesome!) which I'm now using to logg the cookies to the server and I can confirm, that every user has a cookie once they get on a site where a video or an image is. Here the nginx.conf |
|
Can you slim the config for the beginning? e.g. are not crucial here. Your first goal should be to simplify the Nginx config as much as you can. |
|
@menelike I slimmed it down as you suggested. and used your CDN config, while replacing http://app with http://127.0.0.1, right? This did not do the trick. |
|
@Lickshotz You must replace Note: I don't have CSP in here, you might want to add that as well if it doesn't work |
|
@menelike Thank you alot for your effort. I did do that, whilste on the second try stripping down the csp and adding them, without success. After logging from the client to the database for the whole day, I can see that cookies.get('x_mtok') for some users returns a value while the protected function returns It seems that I should focus my attention back onto my application code again, considering that this nginx setup works fine for you. What do you think? |
|
@Lickshotz I'd debug through https://github.com/VeliovGroup/Meteor-Files/blob/master/server.js#L1100-L1128 and try to understand what actually leads to the missing |
|
@menelike @dr-dimitru @s-ol @jankapunkt It looks like I have found the issue, while debugging as suggested, but I don't know if this is then actually an issue with nginx/specific browsers or the package. Whoever acccess the website via example.com gets the cookie on the sever. Once someone access the website via www.example.com has the cookie set in the browser, but will not have it on the server. |
|
Just to be sure, can you confirm that users visiting If you want to work around this issue, just redirect |
|
Every request is made to www.example.com, apart from the one to /cdn, that one is to example.com. Well, the repro above was having this issue in production as well. For now I will redirect users from www.example.com to example.com. |
Well, I think that this is exactly your problem. https://www.thinktecture.com/en/identity/samesite/samesite-in-a-nutshell/ On a general note, you should just use one domain if applicable to make your life easier. https://docs.meteor.com/environment-variables.html#ROOT-URL needs to match as well. |
|
As you suggested, redirecting incoming requests to example.com solved the issue. Thank you! |
|
Maybe this is worth an entry in the wiki? |
Address issue described in #737, thanks to @menelike, @Lickshotz, @s-ol, @jankapunkt
- 🐞 Fix #742, thanks to @menelike and @jankapunkt - 🤓 Update *TypeScript* definitions, thanks to @OliverColeman PR #743, see #226 thread for details - 🤝 Compatibility with `meteor@1.10.2` - 📋 Documentation update to address issue described in #737, thanks to @menelike, @Lickshotz, @s-ol, and @jankapunkt
v1.14.2 - 🐞 Fix #742, thanks to @menelike and @jankapunkt - 🤓 Update *TypeScript* definitions, thanks to @OliverColeman PR #743, see #226 thread for details - 🤝 Compatibility with `meteor@1.10.2` - 📋 Documentation update to address issue described in #737, thanks to @menelike, @Lickshotz, @s-ol, and @jankapunkt
|
@jankapunkt sure, added in the latest release |
|
Hi @dr-dimitru First, I'm impressed how hard you tried to help the OP. I have further hypothesis on this because it does happen to us, slightly differently. The user's But in certain cases, In that case, I will end up with a I'm trying to wrap my head around how this all works but a little help would be really appreciated. So far, I've seen that |
|
@dr-dimitru really simple way to reproduce my issue :
|
|
Hello @armellarcier , Thank you for the feedback and reading this thread. This is expected behaviour for the session-based downloads. Opening new tab/window is a new session, and it's expected behaviour to end previous session. What you need — token/tokenized download links, which can get implemented via Related discussion: #500 |
|
Thx @dr-dimitru , indeed I will need to implement a custom authorization logic for downloads but relying on session ids only seems hacky. As a quick workaround, I'll programmatically set the Something like This is just temporary though. Is there more info about how to implement a custom logic with |
For clarification purposes: Opening a new tab creates a new session. It does not end the previous session. The "problem" lies in the fact that the cookie used for downloads is set by the last open session, and that session will be destroyed when its tab is closed, making download impossible even if another tab is still open. About implementing custom logic : I really think that putting the session_id or a login token in the url is a security issue. I think hiding the authorization logic from the user (in a cookie) is a good think. As users will share these urls eventually. But I guess this should be discussed in another thread ?! |
You're free to set cookie not to a session_id but to some other value (e.g. token) stored on the user's profile, file's record, or somewhere else. Referenced thread and hooks are just for the example
We are good here, feel free to send PR to the documentation if necessary |
I'm having an issue:
I have been reading through the issues concerning the "this.userId null in protected" function for quite abit, but I'm not getting this solved.
On my dev machine, I'm not having this issue, however in production, for some users the issue persists.
On all my machines, regardless if chrome (or chrome mobile), edge or firefox, I do not have this issue.
Other users have this issue in safari, firefox and chrome and chrome mobile.
I have checked with one of the users that have this issue, that the 'x_mok' cookie is set.
I have tried using transport http as well as ddp.
I have added additional logging in the protected function as follows:
which returns the following outcome (given that the problem is present)
Version of Meteor-Files: 1.13.0*
Version of Meteor: METEOR@1.8.1
My package file:
The client side logs do not show anything out of the ordinary.
The server side logs of course show the following:
Thanks in advance and have a nice day! :)
The text was updated successfully, but these errors were encountered: