Closed pmlopes closed 2 years ago
In order to have the split, we will have to refactor the base interceptor
interface. While currently, an interceptor is:
Function<Message<JsonObject>, Future<Message<JsonObject>>>
This cannot work with a split of authn
and authz
as the 2 concerns have inner dependencies. To perform authz
we must have a user
which is created by a authn
interceptor.
The executor of all interceptors, needs to keep state between all calls, until all interceptors are executed, so for example, authn
creates a user and passes it down to the next interceptor.
Maintaining a context, allows any order of interceptors, with the drawback that it cannot be type checked at compile time.
We should define an interceptor as:
@VertxGen
@FunctionalInterface
public interface ServiceInterceptor {
Future<Message<JsonObject>> intercept(Context ctx, Message<JsonObject> msg);
}
The next question is: What should be the shape of the context? (just a Map?) Probably it's the simplest implementation and works.
To keep the current API backwards compatible, we may need to deprecate the old interceptor code and have an adapter to wrap old interceptors as the new one (which should be straighforward).
@pmlopes How do you think will be better to separate interceptor's logic inside? Is it ok to look at something like msg.headers['action'] here to know what checkup to perform? Or by having multiple interface implementations (in that case we will able to check interceptor's type during the compilation).
I think we can do it similar to the new secure config system in web. Define a set of priorities:
and a weight function:
So when interceptors are added to the proxy they assert that the system is valid:
Regarding the context, I think we are probably engineering it, as all we need to have is a reference to a User in during the interceptor run. A user is created from a Authn handler and uses by an authz handler to perform RBAC. So i think the context should be a simple Object where we can get and set a User
and later if needed we add more properties.
I don't think the action should be a concern of the executor, I think each interceptor should have in its logic whether or not the action is important.
@pmlopes Right, What should be the set of priorities initially?
@Vorimo if we build on the previous PR (that was just merged) I think that around the code:
if (interceptorHolders == null) {
interceptorHolders = new ArrayList<>();
}
// HERE!
interceptorHolders.add(new InterceptorHolder(action, interceptor));
return this;
We need to perform a simple weight analysis to the interceptor type.
We should define at least 2 interfaces:
interface AuthenticationInterception extends Function<Message<JsonObject>, Future<Message<JsonObject>>> {}
interface AuthorizationInterception extends Function<Message<JsonObject>, Future<Message<JsonObject>>> {}
Then we should have 3 weights:
Where USER
is the simple: Function<Message<JsonObject>, Future<Message<JsonObject>>>
. This allows us to ensure that interceptors are added in the right order so authn/authz is not broken by mistaken.
I'm thinking we should define this enum inside a impl
package and move the InterceptorHolder
that too, as this is code that is not expected to be used by end users.
Once we have this impl
package I may need to do a review and probably move the utils
classes there too as that was a bad design from us initially but don't worry about this for now as it requires checks for backwards compatibility breaks.
When the basic functionality is there we can work on introducing a Context
to allow multiple interceptors to share context (e.g.: User
for state between authn/authz).
Fixed by #135
https://github.com/vert-x3/vertx-service-proxy/blob/master/src/main/java/io/vertx/serviceproxy/ServiceAuthInterceptor.java
Handles both
authn
andauthz
, we need to investigate if this solution is scalable enough. Will this solution allow us to perform both, any or exclusivelyauthn
andauthz
? (we need more use cases and tests)Splitting would probably make sense and allow us to support the full security model we have in web for example. One missing feature is the support for variable-based authorization, as there is no variable extractor support here.
Having the interceptor divided into 2 would allow reusability and composition, specially in cases where
authz
is not applicable "globally" but on a "method" level.