Hello again!
It's been a while and over at @risetechnologies we have been doing pretty well with Meteor-Files and the ?xmtok=... authentication approach, as necessitated by cordova-plugin-meteor-webapp.
Recently I have been researching the topic again because we faced a similar issue on another front - the AWS loadbalancer only supports 'sticky sessions' using Cookies, but as we know, these are tricky with cordova.
In the process of researching this (details in cordova-plugin-meteor-webapp#64), I have found two things:
img.src etc) could totally be authenticated with Cookies, it's just that ostrio:cookies sets the Cookie from the client side, and so it ends up being set on the localhost:12008 proxy server and not on the alternate-origin ROOT_URL. Clientside JS is disallowed from reading or manipulating cookies there but server responses can read and write cookies..withCredentials is set on the XHR before sending they behave just like described above. Most web packages that use either expose this option or are easily modified to set it. This is the case for all our bundled asset-viewers (threejs, hlsjs, pdfjs).With the above information, the only missing piece to fix 'The Cookie Experience' is a serverside route, e.g. ${downloadRoute}/_cookie, that responds with the correct Set-Cookie: x_mtok=... header. On cordova (or in general), the client bundle can then trigger a .withCredentials XHR there when the connection is established, and all further requests, browser- or XHR-initiated, will be authenticated.
We are thinking about implementing this ourselves, but it sounds mostly trivial to add to Meteor-Files and I am sure others could be interested in this approach.
A more widely-scoped solution would change the way ostrio:cookies works on Cordova, to something like this: (pseudocode)
client:
Meteor.call('cookies.set', { [name]: value }, (err, res) => {
if (res) {
const xhr = new XmlHTTPRequest();
xhr.withCredentials = true;
xhr.open('GET', res.pickupURL);
xhr.send(null);
}
});
server:
Meteor.methods({
'cookies.set': (to_set) => {
// store cookies temporarily
const pickupId = Random.ID();
return `${pickupRoute}?id=${pickupId}`;
}
});
....onRequest(pickupRoute, (req, res) => {
const pickupId = req.params.id;
// retrieve cookies from temp storage
res.set_cookies(cookies);
})
Hey @s-ol ,
Great Idea. I've been thinking to move all XHR and WebSockets logic to WebWorkers, your contribution is good reason to stop thinking and implement it, or maybe we shall finally jump to v2.0?
As I know there is no issues running WebWorkers on Cordova, right?
@dr-dimitru do you mean the logic concerning uploads? This is more about a one-time setup request that can establish your x_mtok cookie on the correct origin domain.
I have some time allocated today to investigating a possible implementation, expect a PR later :)
@s-ol you know what? I think you're right.
We should just extract hostname out of ROOT_URL or MOBILE_ROOT_URL and pass it to Cookies constructor. wdyt?
@dr-dimitru please check out the new PRs VeliovGroup/Meteor-Cookies#11 and #676.
@dr-dimitru @s-ol
how do we want to continue here? I think this issue is related to the Meteor core (cordova) as well.
I followed the discussion on https://github.com/VeliovGroup/Meteor-Cookies/pull/11 and want to put some questions here:
ostrio:files-cordova and ostrio:cookies-cordova. Those package could provide additional logic that is only related to cordova to fix these issues while keeping the basic structures of both packages backwards-compatible.@jankapunkt
Does it make sense to get in touch with MDG about the underlying issue in cordova and security implications?
I tried, it seems they don't care:
meteor/cordova-plugin-meteor-webapp#64
Is it possible to come up with a more decoupled solution, that does not break the current cookies structure?
This solution does not break the current cookies structure - my implementation may have introduced a bug, but it is 100% fixable and non critical. None of the changes I made (should) affect the API the plugin exposes in any way.
Does it make sense to rewrite this package and the cookies package in a way, that cordova-specific code / behavior can be handled by adding some additional packages...
Splitting is probably possible, but also highly likely to be an over-engineered solution, it probably takes a lot more time getting the packages to cooperate rather than just having a boolean option in ostrio:cookies that enables the cordova specific api endpoint.
@s-ol @jankapunkt
Does it make sense to get in touch with MDG about the underlying issue in cordova and security implications?
Well, MDG team went against cookies since day zero of Meteor. You can easily find a lot of articles why they aren't using and not going to support Cookies in any way.
This solution does not break the current cookies structure - my implementation may have introduced a bug, but it is 100% fixable and non critical. None of the changes I made (should) affect the API the plugin exposes in any way.
It does break current auto-tests, and who knows what is it going to break in current implementations around existent apps. We should keep in mind security and stability as this is one of the most popular community packages used in many production apps.
@s-ol @jankapunkt made first step with releasing ostrio:[email protected]. Please, review its changes and let me know if any further work is required, if not — I'm ready to integrate with Meteor-Files library
@dr-dimitru I currently can't test the mobile part (@s-ol how about you?) but I could test the default browser-server integration.
Hello @jankapunkt
Yes, please, test as much as you can
@s-ol @jankapunkt friendly ping, any news on your end?
@dr-dimitru sorry for the delay, I'm having a busy week. I hope that I can take a look at everything tomorrow.
Okay @dr-dimitru, finally found the time to take a look and try it. The changes in meteor-cookies:4.2.0 are not enough - withCredentials is activated as necessary in cookies.set() but the cookies are never set on $METEOR_URL; they are only set via document.cookie which saves them for localhost:12*** but does not save them for $METEOR_URL.
Therefore when the cookies.set() request arrives at $METEOR_URL, even though withCredentials: true is set, since the browser hasn't seen any credentials/cookies for $METEOR_URL it also doesn't send any.
https://github.com/VeliovGroup/Meteor-Cookies/blob/2.4.0/cookies.js#L506
Here you set the Set-Cookie response-header value to what the Cookie request-header was.
This has no effect! The client already has these cookies, he just sent them to you!
If the client wanted to set cookies, but can't (since he is on cordova and on a different origin), then he cannot send the cookies via the Cookie header (or any other header)! That is why I sent the cookies via the URI parameters in my PR. The server then has to turn the parameters into a Set-Cookie header. That is the only case where a Set-Cookie header is necessary.
If the client wanted to set cookies, but can't (since he is on cordova and on a different origin), then he cannot send the cookies via the Cookie header (or any other header)! That is why I sent the cookies via the URI parameters in my PR. The server then has to turn the parameters into a Set-Cookie header. That is the only case where a Set-Cookie header is necessary.
Got that part. How we'll be able to provide security over it? For example do not execute hooks when cookies passed via get query?
For example do not execute hooks when cookies passed via get query?
which hooks? This is supposded to happen in Meteor-Cookies on the /__cookies__/set endpoint. Then if Set-Cookie is properly used, Meteor-Files will get actual cookies and not care.
How we'll be able to provide security over it? For example do not execute hooks when cookies passed via get query?
Regular clients can already set Cookies as they want - Cookies can never be trusted. Passing cookies via the query parameter is not insecure, as long as the same origin policy is properly set up to prevent malicious JS on other domains from overriding the cookies (or theoretically reading them, if a bug exposes them).
Thank you guys (@s-ol and @jankapunkt) for contribution, finally it's here 🥳
Released as v1.12.1.
Feel free to close it in case if the issue is solved on your end.
Hello everyone! v1.13.0 is out as I understood it is finally solves Cordova issue.
Feel free to close it in case if the issue is solved on your end.
Dear friends @s-ol @jankapunkt ,
Let me know if this one is ready to get closed, if you have any further concerns — I'm open for discussion
@menelike are you able to use stock meteor-files + meteor-cookies in production on Cordova now?
@menelike friendly ping 🖖
@s-ol sorry I have missed your question.
@dr-dimitru Thanks for the heads up.
This issue can be closed as this works now (tested against 1.13.0).
Thanks @s-ol @dr-dimitru for all the effort you've put into this! 🚀❤️
@menelike no worries.
Thank you for testing and confirming this one as fixed.
This is our team effort guys!
Thank you all @s-ol @menelike @jankapunkt
Feel free to reopen it in case if the issue is still persists on your end.
Most helpful comment
@s-ol sorry I have missed your question.
@dr-dimitru Thanks for the heads up.
This issue can be closed as this works now (tested against 1.13.0).
Thanks @s-ol @dr-dimitru for all the effort you've put into this! 🚀❤️