veliovgroup / Meteor-Files

🚀 Upload files via DDP or HTTP to ☄️ Meteor server FS, AWS, GridFS, DropBox or Google Drive. Fast, secure and robust.
https://packosphere.com/ostrio/files
BSD 3-Clause "New" or "Revised" License
1.11k stars 166 forks source link

Extracting x_mtok authentication into own package? #841

Open jankapunkt opened 2 years ago

jankapunkt commented 2 years ago

I have a suggestion:

I currently implement connect route authentication with ostrio:cookies and basically the code from this package:

From what I can see, the code for authentication could completely be extracted into an own package and being reused for cookies-based auth in routes, right?

Example code could look like this:

Client

import { Accounts } from 'meteor/accounts-base'
import { DDP } from 'meteor/ddp-client'
import { Meteor } from 'meteor/meteor'

// cookie injected from outside to let host control cookie options
export const cookieAuth = cookie => {
  const setTokenCookie = () => {
    if (Meteor.connection._lastSessionId) {
      cookie.set('x_mtok', Meteor.connection._lastSessionId, { path: '/', sameSite: 'Lax' });
      if (Meteor.isCordova && this.allowQueryStringCookies) {
        cookie.send();
      }
    }
  };

  const _accounts = (Package && Package['accounts-base'] && Package['accounts-base'].Accounts) ? Package['accounts-base'].Accounts : undefined;
  if (_accounts) {
    DDP.onReconnect((conn) => {
      conn.onReconnect = setTokenCookie;
    });
    Meteor.startup(setTokenCookie);
    _accounts.onLogin(setTokenCookie);
  }
}

Server

import { Meteor } from 'meteor/meteor'

export const getUser = (req, res, next) => {
  if (req.userId && req.user) { return next() }
  let mtok = null
  if (req.headers['x-mtok']) {
    mtok = req.headers['x-mtok']
  } else {
    const cookie = req.Cookies
    if (cookie.has('x_mtok')) {
      mtok = cookie.get('x_mtok')
    }
  }

  if (mtok) {
    const userId = getUserIdFromToken(mtok)

    if (userId) {
      req.userId = () => userId
      req.user = (() => {
        let user
        return () => {
          if (!user) {
            user = Meteor.users.findOne(userId)
          }
          return user
        }
      })()
    }
  }
  return next()
}
const isObject = obj => typeof obj === 'object'
const getUserIdFromToken = (xmtok) => {
  if (!xmtok) return null

  const sessions = Meteor.server.sessions
  const sessionIsMap = sessions instanceof Map
  const sessionIsObject = isObject(sessions)

  // throw an error upon an unexpected type of Meteor.server.sessions in order to identify breaking changes
  if (!sessionIsMap || !sessionIsObject) {
    throw new Error('Received incompatible type of Meteor.server.sessions')
  }

  if (sessionIsMap && sessions.has(xmtok) && isObject(sessions.get(xmtok))) {
    // to be used with >= Meteor 1.8.1 where Meteor.server.sessions is a Map
    return sessions.get(xmtok).userId
  }
  else if (sessionIsObject && xmtok in sessions && isObject(sessions[xmtok])) {
    // to be used with < Meteor 1.8.1 where Meteor.server.sessions is an Object
    return sessions[xmtok].userId
  }

  return null
}

What do you think about it? Also, do you have any concerns regarding security on that approach?

dr-dimitru commented 2 years ago

@jankapunkt I'd turn it into middleware level API option, and call it http-auth (open to suggestions on name). Would be useful if one want to use Meteor's accounts while building logic on HTTP/REST level

jankapunkt commented 2 years ago

Yes, that's good. Do you think it's possible to do it, without breaking the current functionality of this package?

dr-dimitru commented 2 years ago

@jankapunkt do you have a setup to test such case?

Two options for API:

  1. Would cause a major version, but we will exclude this part from meteor-files and move it to the docs
  2. Minor version bump having http-auth as dependency in meteor-files

wdyt?

jankapunkt commented 2 years ago

I think we make http-auth an extra middleware package but make it remain as dependency of this package so the original functionality is untouched. Other suggestions @s-ol @menelike (mentioned you, since you both were involved in some of the former cookie issues, right)?