loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.96k stars 1.07k forks source link

[Spike] Investigate the minimal infrastructure we need to support authorization #2718

Closed dhmlau closed 5 years ago

dhmlau commented 5 years ago

Description / Steps to reproduce / Feature proposal

Capturing the discussion with @raymondfeng @bajtos

There are different choices to support authorization:

  1. interceptor
  2. middleware (our original approach, that's why we have https://github.com/strongloop/loopback-next/pull/2434)
  3. sequence action

@raymondfeng did some initial investigation and think interceptor might be a better approach, because middleware doesn't have access to the target method.

Acceptance Criteria

raymondfeng commented 5 years ago

See https://github.com/strongloop/loopback-next/pull/2687.

Please note the middleware layer does not have information about the target method and corresponding arguments and context. It's probably not suitable for authorization decisions.

emonddr commented 5 years ago

Investigate minimal infrastructure to support authorization, using interceptors and a community authorization example as inspiration for the POC.

JesusTheHun commented 5 years ago

Interceptors are great because they are quite simple, they are just annotations with a clear semantic purpose : modify the incoming request and/or the out coming response. However this simplicity implies a few limitations :

  1. You cannot add default behaviour in the absence of these annotations, so a controller without the proper annotation will not have its access denied, which is the security standard.
  2. You do not control the chain of execution. If you add a class level interceptor and then you add a different method level interceptor, you don't know which one will be executed first, which can be important if not a true issue.

These problems can be over come, of course :

  1. you can add the default annotation to a base controller you will use as a default, but as an architect I don't like this approach since it's a security related feature, I don't want to rely on developers not forgetting something. Security should be the default behaviour.
  2. you can add a priority number in your declaration but you will never know if some third party interceptor is not hijacking your position, and maybe you will encounter equalities.

For those reasons I think the sequence approach is the most reasonable : your sequence action is always called, so you have a default security behaviour. You can declare specific authorization behaviours by using annotation on the class or the method you want. You always know when your sequence action will be executed ; if you need extra context execution such as loading database-stored ACLs, you can declare those by using previously mentioned annotations.

EDIT : the minimal infrastructure required for my proposal is :

bajtos commented 5 years ago

Related: https://github.com/strongloop/loopback-next/issues/1462

raymondfeng commented 5 years ago

You cannot add default behaviour in the absence of these annotations, so a controller without the proper annotation will not have its access denied, which is the security standard.

We support global interceptors that are bound the context with a special tag.

You do not control the chain of execution. If you add a class level interceptor and then you add a different method level interceptor, you don't know which one will be executed first, which can be important if not a true issue.

We define the order of execution.

See https://github.com/strongloop/loopback-next/blob/interceptor/docs/site/Interceptors.md for more details.

JesusTheHun commented 5 years ago

I've think about it lot. Global interceptors solve the first issue I mentioned, but the second about third parties was not answered. I was unable to find the definition of order execution of global interceptors in your doc, beside local interceptors.

The order of execution must be defined in user land, not in the interceptor itself. This will prevent third-party interceptor order overlap, and add a bit of security preventing malicious invisible interception embodied into a third-party interceptor.

This being said, interceptor pattern seems fine to me.

raymondfeng commented 5 years ago

Interceptors are now supported. We also allow you to control order of global interceptors.

dhmlau commented 5 years ago

See PR https://github.com/strongloop/loopback-next/pull/1205

raymondfeng commented 5 years ago

Based on https://github.com/strongloop/loopback-next/pull/1205, we can use interceptor to enforce authorization.