Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Closed
2 tasks
dhmlau opened this issue Apr 9, 2019 · 9 comments
Closed
2 tasks

Comments

@dhmlau
Copy link
Member

dhmlau commented Apr 9, 2019

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 [RFC] Introduce express middleware extension point #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

  • Investigate what's the better approach to support authorization - interceptor, middleware or sequence action
  • look into what's the minimal infrasturcture we need to support authorization
@raymondfeng
Copy link
Contributor

See #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
Copy link
Contributor

emonddr commented Apr 11, 2019

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

@JesusTheHun
Copy link

JesusTheHun commented Apr 12, 2019

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 :

  • An authorization definition interface and common adapters : database, authorization server
  • An authorization manager : a service to which authorization definitions and user code annotations will register to. This is the core component.
  • An authorization sequence action : uses the authorization manager to allow or deny the request, provide default rejection response or redirections.

@bajtos
Copy link
Member

bajtos commented Apr 16, 2019

Related: #1462

@raymondfeng
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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

@dhmlau
Copy link
Member Author

dhmlau commented May 14, 2019

See PR #1205

@raymondfeng
Copy link
Contributor

Based on #1205, we can use interceptor to enforce authorization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants