Custom alias do not call authorize

[felipegcampos]

Hi all,

I've noticed custom alias handler is not calling the authorize method. First I thought of using onBeforeCall; however, it's not called as well. My temporary solution is to create a custom handler that sets an 'auth' property and creates the Context to invoke the authorize, such as follow:

'POST /files/upload': {
  auth: 'required',
  handler: async function(req, res) {
    const { broker } = req.$service;
    const ctx = Context.create(broker, '', broker.nodeID, req.$params, req.$route.callOptions || {});
    try {
      if (await this.authorize(ctx, req.$route, req)) {
        this.uploadFile(req, res);
      }
    } catch (err) {
      this.sendError(req, res, err);
    }
  },
},

Then, in the authorize method I added the following logic:

// Check if route requires auth
if ((req.$endpoint && req.$endpoint.action.auth === 'required') || (req.$alias && req.$alias.auth === 'required')) {

I guess custom alias/actions should be part of the action lifecycle somehow.

Thanks

[icebob]

Good solution. The current call flow doesn't enable to implement is, only in a new breaking version.

[felipegcampos]

@icebob , don't you think would be valid to add the authorization options to the aliases?

Something like:

aliases: {
    'REST user auth': 'users'
}

or

aliases: {
  'GET user': {
    auth: 'required', // could have optional as well
    action: 'users.get'
    // or
    action: handler(req, res) {
      // Custom logic
    },
  },
}

It would make it easier to create custom functions to retrieve data from multiple services with a single round-trip.

I don't understand why the services should be aware of authorization if the communication among them is private.

Let me know if I can help with that

[icebob]

What's the use case? In the route authorization: false but you can overwrite it in aliases?

[felipegcampos]

Actually, in my understanding authorization is just Gateway API related. Supposing you're using moleculer-db to create your default CRUD actions and have 3 services ( A, B and C ) running on the same basis. You want your Gateway API to retrieve data from all of them ( ie: they have an id relationship ) and return to the user. First of all, the way it works I would have to extend the CRUD actions in the services to add the authorization property. For example, supposing I want the default behaviour:

{
  create: {
    auth: 'required'
  },
  update: {
    auth: 'required'
  },
  delete: {
    auth: 'required'
  },
  [...others]
} 

An then I found I want service A list action to filter just property1. I would have to rewrite my list action in service A jut because of the Gateway API when actually I want my services to be able to query the service using all properties. It would be easier If I could just create a custom action in the Gateway API where I could use the default ctx.call('A.list', { query: { property1: value1 } }) and then go over the list and do smth with service B and C. I would them just add an authorization to the alias and create my custom action. It would also be more readable if I could write authorization for the route and just disable in the aliases where I don' t want it ( or the opposite ). It comes to:

routes: [{
  authorization: 'required',
  aliases: {
    'GET myAction': {
      action: function handler(req, res) {
        const docs = ctx.call('A.list', { query: { property1: value1 } });
        // do smth with services B and C
        return smthElse;
      },
    },
    'GET user': {
      required: false, // override default behaviour
      action: 'users.get',
    },
    // or sugar
    'GET user': 'users.get', // would inherit route authorization
  },  
}]

I'll have to fork the project anyways and make some changes, but I would like to contribute to the project. I wanna make sure I can write a code that can be used by everyone.

[icebob]

OK. If you have some working solution, please open a PR, and we can continue the discussion about it.

[felipegcampos]

Ok . I should have it soon . Thank you

[felipegcampos]

@icebob ... Just to let you know, I'm trying to break it into parts. The first PR I will do is to add the custom aliases in the current call flow.

[icebob]

ok, in this case, I will create a new branch: https://github.com/moleculerjs/moleculer-web/tree/alias-next

[felipegcampos]

Hey @icebob . I need your help here. I have it already working on my repo but I had to chop off the middleware from aliases. Actually, I can't think of a use case for it if you have the custom function that receives the context and the route. Here is the link to the custom test:

https://github.com/felipegcampos/moleculer-web/blob/939d34ddf8f735846dfa22594e627aca9ffdc47a/test/integration/index.spec.js#L842

I am trying to think whether I should add the middlewares again somehow or just ignore it and let the implementation through the custom function since it's in the call flow now.

It' s important to notice the custom handler now returns a value that later will be processed by the sendResponse function.

I can' t think of a nice way to add the middleware pipeline to the flow. Can you share your expectations?

Regards

[icebob]

Hi @felipegcampos. First of all, please don't remove any existing feature from the source. Just extend the functionality. Thanks.

I think the main problem is that you can't create Context without a valid Action. But if you have a custom alias, there is no action for Context.

If I were you, I'd create actions in the GW service which makes what you want (move custom alias logic to an action. E.g.:

routes: [{
  authorization: 'required',
  aliases: {
    'GET myAction': 'api-gw.myAction'
  },
//....
actions: {
  myAction(ctx) {
        const docs = ctx.call('A.list', { query: { property1: value1 } });
        // do smth with services B and C
        return smthElse;
  }
}

So in this case the GW runs the normal flow, so it will call authorization too. Can it solve your problem?

[felipegcampos]

Hey @icebob . I appreciate your answer. I am not removing or adding anything without asking you first. Don't worry about it. It seems like creating an action could solve the first issue.

[icebob]

Currently the GW creates a "wrapper" context with a fake action. But in the next Moleculer version the context action will be more strictly, so I will have to change this part in the future. And I would like to create a regular action in GW service, and move the all "context" code part into this action. After that, maybe we can create an other api.custom action which can handle the custom aliases. Once it will be a regular action, we will have Context and we can create authorization, onBeforeCall...etc functions. What do you think?

[felipegcampos]

Do you mean to split concerns? In this case, we would have different setups for "regular actions" and custom actions?

[icebob]

We split only what is different. The "normal" action will call ctx.call after hooks & middlewares, the "custom" action will call the custom alias function after hooks & middlewares. I think....

[felipegcampos]

It sounds like what I've done in my solution. Did you have the chance to check it out?

[icebob]

Yes, I will check.

[felipegcampos]

@icebob , I decided to move our GW to work with HapiJS and use plugins to initialize the broker. I'm more experienced with it and it has everything we need for this first sprint. Anyways, I'm available to help with anything you need. I know sometimes it's hard to handle both a full-time job and open source project. You're doing a nice job with moleculer.

[icebob]

Thanks @felipegcampos! Yes, it's not easy :) Maybe if you can share some code parts, how you use HapiJS with Moleculer, it would be awesome! (I have no experience with HapiJS).

[felipegcampos]

Yes, it's really easy.

Using Hapi17, you basically create one plugin and then register it. The plugin would be smth like:

export const plugin = {
  name: 'hapi-moleculer',
  version: '1.0.0',
  register: async function register(server) {
    const broker = new ServiceBroker({});

    server.decorate('server', 'broker', broker);
    server.decorate('request', 'broker', broker);

    await broker.start();
  },
};

Of course there are many things one can do. It's possible to handle the moleculer errors in the onPreResponse lifecycle and even create the routes for your REST actions within moleculer.

A really simple example:

    const aliases = [{ method: 'REST', path: '/user', action: 'users' }];
    let routes = [];
    aliases.forEach((alias) => {
      if (alias.method === 'REST') {
        routes.push({ method: 'GET', path: `${alias.path}/{id}`, action: `${alias.action}.get` });
        routes.push({ method: 'GET', path: alias.path, action: `${alias.action}.list` });
        routes.push({ method: 'POST', path: alias.path, action: `${alias.action}.create` });
        routes.push({ method: 'PUT', path: `${alias.path}/{id}`, action: `${alias.action}.update` });
        routes.push({ method: 'DELETE', path: `${alias.path}/{id}`, action: `${alias.action}.remove` });
      } else {
        routes.push(alias);
      }
    });
    routes = routes.map(route => ({
      path: route.path,
      method: route.method,
      options: {
        tags: ['api', 'user'],
      },
      handler: async function action(request) {
        const params = _.extend(request.params, request.query, request.payload);
        const ret = await request.broker.call(route.action, params);
        return ret;
      },
    }));

I'll see if I can create a hapi-moleculer plugin to publish.

[icebob]

Wow, it would be amazing!

[felipegcampos]

Hey @icebob . I've finished the first version of the plugin.

https://github.com/felipegcampos/hapi-moleculer

Could you please add to the 'Other modules' section? I did my best to document and test.

[icebob]

Nice repo @felipegcampos, thanks! :tada: I'm adding it to the site.

[icebob]

Issue is fixed & released in 0.8.0