Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(authentication): Omit query in JWT strategy #2011

Merged
merged 1 commit into from
Jul 12, 2020
Merged

fix(authentication): Omit query in JWT strategy #2011

merged 1 commit into from
Jul 12, 2020

Conversation

daffl
Copy link
Member

@daffl daffl commented Jul 12, 2020

Follow-up for #2009

@daffl daffl merged commit 04ce7e9 into crow Jul 12, 2020
@daffl daffl deleted the jwt-query branch July 12, 2020 17:15
@cantoute
Copy link

cantoute commented Jul 12, 2020

Hi,

I was about to publish a bug report but I see this correction seems to relate to issue I had, but I'm not convinced it'll solve it... I'll have to try (soon I promise).

When calling authenticate (strategy 'jwt')
and that context.params has a sequelize include
in my case a service (tags) including a model alias 'parents' of type belongsToMany(tags, through: tagsTags)
tags and tagsTags have in the model description (but wasn't included in the current call) a relations belongsTo(users, as: author)

As a workaround, I was hiding context.sequelize.include from auth hook and re-injecting it.

here below the hook I used as a 'fix' and below how I implemented $inculde[] if this can help anyone.

hooks/fix-authenticate-sequelize-include.ts

// Use this hook to manipulate incoming or outgoing data.
// For more information on hooks see: http://docs.feathersjs.com/api/hooks.html
import { Hook, HookContext } from '@feathersjs/feathers';
import * as authentication from '@feathersjs/authentication';
import { AuthenticateHookSettings } from '@feathersjs/authentication/lib/hooks/authenticate';

const { authenticate } = authentication.hooks;

/*
When calling authenticate (strategy 'jwt')
and that context.params has a sequelize include 
in my case a service (tags) including a model alias 'parents' of type belongsToMany(tags, through: tagsTags)
tags and tagsTags have in the model description (but wasn't included in the current call) a relations belongsTo(users, as: author)

then an error would emerge out of users service
error: 'invalid reference to FROM-clause entry for table "tags"'
error: error app.service('users').get()

here part of the SQL
FROM
  "users" AS "users"
  LEFT OUTER JOIN (
    "tags_tags" AS "parents->tags_tags"
    INNER JOIN "tags" AS "parents" ON "parents"."id" = "parents->tags_tags"."parentTagId"
  ) ON "users"."id" = "parents->tags_tags"."tagId"
  AND "parents"."id" != "tags"."id"
  LEFT OUTER JOIN "media" AS "parents->featuredImage" ON "parents"."featuredMediumId" = "parents->featuredImage"."id"
WHERE
  ("users"."id" = '1');

So on querying users it would try to link to the models included in the request to 'tags' service

As a workaround I 'proxied' the authenticate() hook with this one, it basically deletes context.params.sequelize.include 

import authenticate from '../../hooks/fix-authenticate-sequelize-include';

that I then use as I would normally use authenticate('jwt')

*/

export default (originalSettings: string | AuthenticateHookSettings, ...originalStrategies: string[]): Hook => {
  return async (context: HookContext) => {
    if (context.params && context.params.sequelize && context.params.sequelize.include) {
      const { sequelize } = context.params;
      const { include } = sequelize;

      let contextWithoutInclude = context;
      delete contextWithoutInclude.params.sequelize.include;

      let authContext = await authenticate(originalSettings, ...originalStrategies)(contextWithoutInclude);
      authContext.params.sequelize.include = include;

      return authContext;
    } else {
      return authenticate(originalSettings, ...originalStrategies)(context);
    }
  };
};

hooks/sequelize-include-remote-models.ts

import { Hook, HookContext } from '@feathersjs/feathers';
import { mergeDeep, castToBoolean } from '../shared/utils';
import logger from '../logger';
import { merge } from 'lodash';
// import { Sequelize, DataTypes, Op } from 'sequelize';

const hydrate = require('feathers-sequelize/hooks/hydrate');

// inspired by https://stackoverflow.com/questions/48602085/using-feathers-client-and-sequelize-many-to-many-relation

// TODO: find how to do this the right way ( app.hooks(appHooks) in app.ts isn't so happy about this )
type HookContextWithAssociations = HookContext & { associations: { include: any[] } };

export default (options = {}): any => {
  return async function (this: any, context: HookContextWithAssociations) {
    switch (context.type) {
      case 'before':
        if (context.params && context.params.query) {
          if ('$include' in context.params.query) {
            const { $include, ...query } = context.params.query;

            context.associations = context.associations || {};
            context.associations.include = context.associations.include || [];

            // thanks https://jsperf.com/array-coerce
            (Array.isArray($include) ? $include : [$include]).forEach((name: string) => {
              if (name in context.service.Model.associations) {
                context.associations.include.push({
                  association: context.service.Model.associations[name],
                  as: name,
                });
              } else {
                logger.error(
                  `Requested association '${name}' of model '%s' doesn't exist. Available associations are: %O`,
                  context.service.Model.name,
                  context.service.Model.associations
                );
              }
            });

            // Update the query to not include `$include`
            context.params.query = query;

            context.params.sequelize = context.params.sequelize || {};

            merge(context.params.sequelize, context.associations, { raw: false });
          }

          if ('$raw' in context.params.query) {
            const { $raw, ...query } = context.params.query;

            // Update the query to not include `$raw`
            context.params.query = query;

            context.params.sequelize = context.params.sequelize || {};
            Object.assign(context.params.sequelize, { raw: castToBoolean($raw) });
          }
        }

        break;

      case 'after':
        // I still don't understand why we have to do this... but they say so :-/
        if (context.associations && context.params && context.params.sequelize && !context.params.sequelize.raw) {
          hydrate(context.associations).call(this, context);
        }

        break;
    }

    // return Promise.resolve(context);
    return context;
  };
};

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants