Load balancer and cookie proposal

[Kaczkazniszczenia]

I would like to suggest changing the content of the cookies x_mtok to 'LoginToken' stored in local storage. Alternatively, make this change optional by selecting flags in the global configuration for the package (not sure if there is any).

Using session key stored in the DB would make it easier to recognize the user. If you allow cookie parameters to be defined in the global configuration, it is also possible to separate the file server from the rest.

Of course, if the change gains the interest and consent of the owners of the package, I will be happy to deliver the change.

My reasons for change:
The package doesn't work since my application uses multiple servers + load balancer. Because of reasons I won't use sticky sessions.

[dr-dimitru]

Hello @Kaczkazniszczenia ,

I'm open to make this package better, also I'd like to keep it solid, so far that wasn't an issue. To start we need to compare pros and cons using Meteor.connection._lastSessionId vs Meteor.loginToken.

From my point of view Meteor.loginToken carries the way more Security vulnerables, and this is exactly why Meteor Dev Group initially discarded using cookies for this purpose.

I've been helping commercial projects going thought security audit and compliance — using temporary Meteor.connection._lastSessionId accepted just fine, while using Meteor.loginToken would reject such compliance it for sure.

To proceed further, please tell us what are reasons made you to step away from sticky sessions? Note: Upload won't work without sticky sessions, it might, but would eventually fail from client to client.

[Kaczkazniszczenia]

Please do not close issue, I should be able to prepare an answer tomorrow.

[Kaczkazniszczenia]

Hello @dr-dimitru, sorry for the long response time.

Sticky session is not suitable for my system, we have a 'relatively' low number of users, with the possibility of generating a lot of traffic by each of them.

Can you please elaborate on the upload topic? Fortunately(or not), I didn't run into this problem.

Even though direct use of Meteor.loginToken is convenient for me, I must admit that it can reduce security. It is also hard to propose specific solutions that would suit all library users.

So it might be a good idea to let users define their own recognizeUser function. Using this feature would disable setting cookies on the client side, and replacing '_getUser' on the server side. The solution would add flexibility to the library, allowing everyone to make their own mistakes.

I hope you will like the idea and be willing to discuss it further.

[dr-dimitru]

Hello @Kaczkazniszczenia ,

1. I don't understand how sticky sessions are related to the visitors/session; However this is beyond this package and you know better specific needs of your application;
2. Upload would fail in case if load-balance would send requests to different servers, that's why you would need to implement sticky-sessions;
3. Yes, you implement whatever authentication you want, here's a good example

Hope that help.
Please, share your solution in this thread once found

[Kaczkazniszczenia]

User injection inside the 'protected' function looks like a working solution, ... but I think it could be solved better
The x_mtok cookie is still being set, and authorization is performed based on it each time a download is attempted (before 'protected' can overwrite user). Not to mention that the 'additional' authorization method must be defined separately for each collection. (A statement based on a quick code check, I hope I didn't twist anything.)

I would like the library to have a global override of the user authorization method for all file collections in system(_getUser), and the cookie management would be handled entirely outside the library.
I already have a cookie in my system that allows me to authorize the user. It would be a shame to be forced to store another with the same purpose.

[dr-dimitru]

I already have a cookie in my system that allows me to authorize the user. It would be a shame to be forced to store another with the same purpose.

You can use it in server hooks where http object (with request and response) passed as an argument

[Kaczkazniszczenia]

The x_mtok cookie is still being set, and authorization is performed based on it each time a download is attempted (before 'protected' can overwrite user). Not to mention that the 'additional' authorization method must be defined separately for each collection.

I don't see any hook allowing me to skip these issues. Can you point me in right direction ?

[Kaczkazniszczenia]

added PR.
The introduced changes will allow me to write my own authorization.
I gave up on setting global hooks, I hope the changes are acceptable.

[jankapunkt]

Related #778

[Kaczkazniszczenia]

@dr-dimitru can I ask for your opinions on the proposed changes ? I am happy to answer any questions to speed up the entire process.

[dr-dimitru]

@Kaczkazniszczenia I usually give time up to two-three weeks on all PRs and critical changes (especially related to security) at this repository.

Meanwhile I advise to you to use it as much as possible to find and eliminate possible bugs and issues.

I'll be back once thoroughly tested on our end.

[Kaczkazniszczenia]

@dr-dimitru below I am sending custom auth class wrapper that I am currently using. I hope it helps.

import { FilesCollection } from 'meteor/ostrio:files';
import { Cookies } from 'meteor/ostrio:cookies';
const cookies = new Cookies();

import { Meteor } from 'meteor/meteor';
import { Random } from 'meteor/random';
import { Accounts } from 'meteor/accounts-base';

import crypto from 'crypto';
function hashToken(token) {
  const hash = crypto.createHash('sha256');
  hash.update(token);
  return hash.digest('base64');
}

/**
 * Use this class instead of your default FileCollection
 */
export class FileCollectionWrapper extends FilesCollection {

  constructor(config) {
    super({
      /* disable x_mtok cookie */
      disableSetTokenCookie: true,
      /**
       * User recognition based on the content of 'fileToken' cookie and 
       *  the value of the 'services.resume.loginTokens.$.hashedFileToken' 
       *  field that should contain the hashed value from fileToken.
       */
      getUser(http) {
        const cookie = http.request.Cookies;
        const token = cookie.get('fileToken');
        let user = null;
        if (token) {
          const hashedToken = hashToken(token);
          user = Meteor.users.findOne({
            'services.resume.loginTokens.hashedFileToken': hashedToken
          }, {
            fields: { services: 0 }
          }) || null;
        }
        return {
          userId: user && user._id,
          user() { return user; }
        };
      },
      ...config
    });
  }

}


/**
 * Client only code
 * ================
 * 
 * On password login, send request for new fileToken and set it in cookie.
 * 
 */
if (Meteor.isClient) {
  Meteor.startup(() => {
    Accounts.onLogin((obj) => {
      /**
       * We ask to generate fileToken right after creation of loginToken by meteor. 
       */
      if (obj.type === 'password') {
        const loginToken = localStorage.getItem('Meteor.loginToken');
        Meteor.call('generateNewFileToken', loginToken, (error, resp) => {
          if (resp) {
            const { fileToken, expires } = resp;
            cookies.set('fileToken', fileToken, { expires, secure: true, sameSite: 'strict' });
            return;
          }
          console.error('Failed on geting new fileToken', { error, resp });
        });
      }
    });
  });
}


/**
 * Server only code
 * ================
 * 
 * Creating method for generating new fileToken and placing it in db in way it will be connected to loginToken.
 * 
 */
if (Meteor.isServer) {
  Meteor.methods({
    /**
     * Method generating new fileToken (if not set before)
     * @param {String} loginToken loginToken to which the newly generated fileToken should be assigned to
     * @returns {{fileToken: String, expires: Date}}
     * @throws on alot of things 
     */
    generateNewFileToken(loginToken) {

      if (!this.userId) {
        throw new Meteor.Error('Access blocked');
      }
      if (!loginToken || typeof loginToken !== 'string') {
        throw new Meteor.Error('Access blocked');
      }

      const userId = this.userId;

      const fileToken = Random.secret();

      /**
       * Expires will be used in cookie and is set based on meteor/accounts configuration.
       * The goal is that the loginToken and fileToken cookie will expire on the same day.
       */
      const expiresInDays = Accounts._options.loginExpirationInDays || Accounts.DEFAULT_LOGIN_EXPIRATION_DAYS || 90;
      const expires = new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * expiresInDays);

      const hashedLoginToken = hashToken(loginToken);
      const hashedFileToken = hashToken(fileToken);

      const numberOfDocsUpdated = Meteor.users.update({
        _id: userId,
        'services.resume.loginTokens': {
          $elemMatch: {
            'hashedToken': hashedLoginToken,
            'hashedFileToken': { $exists: 0 }
          }
        }
      }, {
        $set: {
          'services.resume.loginTokens.$.hashedFileToken': hashedFileToken
        }
      });

      if (numberOfDocsUpdated === 0) {
        throw new Meteor.Error('No file Token was set');
      }

      return {
        fileToken: fileToken,
        expires: expires
      };

    }
  });
}

[Kaczkazniszczenia]

Any new updates ?

[dr-dimitru]

@Kaczkazniszczenia merged. Testing pending

[dr-dimitru]

Closed due to silence at issue owner end.
Feel free to reopen it in case if the issue still persists on your end.