I was wondering if I really needed the spring security clean up afther the remote call, hence the question: What happens with the thread in which the remote call is done (in RegistrySharedObject.executeRequest method)? If this thread is destroyed I could live with the fact that the spring security clean up is not performed (spring security specific information would be longer on the thread than needed but I don't think this would be an issue).
So if this is the case I think having only the checkRemoteCall method in IRemoteServiceCallPolicy would be ok.
On the other hand, it could be useful to check if the return value of a remote call is authorized or if the returned value needs to be transformed based on the granted authorities of the caller. For this kind of use cases, we could add following method on the IRemoteServiceCallPolicy interface:
The checkRemoteCall method would be indeed a more simple solution but in the use case where you delegate the authorization to spring security you don't have a mean to do spring security specific clean up (i.e. SecurityContextHolder.clearContext()) *after* the invocation of the remote call. Of course this is very specific to spring security use case but it demonstrates that the checkRemoteCall approach is less general than the other approaches.
A couple of thoughts I've had today (I hope you and others don't mind some discussion about the design of this addition...I would like this addition to be as simple as possible...without reducing it's generality...and it's definitely a very important API addition IMHO...so deserves a little public design discussion...
1) After some thinking...I think it's unnecessary to introduce an IRemoteServiceRegistration.callService method to support IRemoteServiceCallPolicy.callWithAuthorization. The reason I now think this is that each provider implementation (i.e. every implementer of IRemoteServiceContainerAdapter) has to provide...and be aware of...their own implementation of IRemoteServiceRegistration anyway...e.g. for the generic implementation it's the org.eclipse.ecf.provider.remoteservice.generic.RemoteServiceRegistrationImpl class. Inside the generic provider implementation there are several cases where the IRemoteServiceRegistration has to be aware of the specific type of the IRemoteServiceRegistration (e.g. RemoteServiceRegistrationImpl).
One of the implications of this is that the code inside the implementation of (e.g.) IRemoteServiceCallPolicy.callWithAuthentication will also be provider-specific...and done differently (or not used at all) for every provider. The provider code will be aware of the real type of IRemoteServiceRegistration, and so casting to that type will not be a problem (for example, it's assumed in both the generic and the r-osgi providers that the implementation class is of the appropriate type...if it's not then there is something seriously wrong (e.g. if callers are passing in r-osgi remote service registrations to generic providers or vice-versa).
2) I'm thinking that perhaps instead of either callWithAuthorization...or the pre* and post*-style approach, that perhaps there could be a single method for the IRemoteServiceCallPolicy that looks something like this
public void checkRemoteCall(IRemoteServiceRegistration registration, IRemoteCall remoteCall, ID fromID) throws SecurityException;
The assumption being that if the IRemoteServiceCallPolicy had been set (via the IRemoteServiceContainerAdapter.setRemoteCallPolicy(IRemoteServiceCallPolicy)), that providers that support using the IRemoteServiceCallPolicy could/would call checkRemoteCall *before* actually invoking the remote call and returning a result. First question: would such an approach work for you (Franky)? Do others see a problem with this approach?
Please let all know what you think.
Thanks,
Scott
On 10/8/2010 3:31 AM, Franky Bridelance wrote:
Hi Scott,
Thanks for the comments.
My use case is quite simple: I'm using spring security framework server side for authentication and authorization because it's quite easy to configure and a lot of the authentication and authorization work is done for you. Of course I still need to provide the authentication information in a spring security manner and because I'm using ECF for client/server communication, it comes down to implement some "binding" code between ECF and spring security.
The "binding" code is a class (see code below) implementing both IConnectHandlerPolicy and IRemoteServiceCallPolicy, it gets the authentication manager of spring security injected to delegate the authentication and it uses SecurityContextHolder to inject the authentication info (granted authorities) right before the service call and clears the context after the call. So in my use case it does not matter which approach is used, both are easy to implement. Currently, only downside of first approach is casting IRemoteServiceRegistration to RemoteServiceRegistrationImpl of ECF generic in order to call callService method but as you said it's easy to solve by adding callService method on IRemoteServiceRegistration interface.
First, I want to say...nice job! This will make a very nice addition to ECF remote services.
I should say though that we are approaching ECF 3.4 minor release (hopefully at the end of this month), and it may not be possible to get these API changes (and consequent changes to provider impls) decided upon and incorporated into the ECF 3.4 release. We've got to have an EF review and stuff like that as part of the release...so I just want to make sure you know that if we don't get this work into that release it's simply because of the mechanics of the releng process...and no reflection on the value of this work. I'm sure that we will be able to get it into later releases in some form.
With my comments below, I'm going to focus on the API additions rather than the implementation...i.e. to the org.eclipse.ecf.remoteservice package...rather than the specifics of the implementation. The reason for this is that I think that the API additions/changes are the most important to think carefully about...as API is forever :), while implementation bugs can be relatively easily fixed.
So...see further comments inline.
On 10/5/2010 6:09 AM, Franky Bridelance wrote:
Hi Scott, all,
<stuff deleted>
- Server creates container - Get ISharedObjectContainerGroupManager interface from the container - Set an implementation of IConnectHandlerPolicy on ISharedObjectContainerGroupManager that will + get the connect data
+ use this connect data to authenticate (via spring security) + if authentication fails throw an exception + if authentication succeeds: retrieve the authorization roles (via spring security) for the authenticated user and link this to the client container ID (fromID arg in IConnectHandlerPolicy.checkConnect) --> question: is the client container ID unique?
Yes.
For the authorization of remote service call there's as far as I know no API available, so I checked what is needed and came up with a proposal API. I tried to make the API generic enough to support several authorization implementations, provider independent and at the same time fitting ECF generic implementation and spring security integration.
This is great...exactly right about the need for generality and cross-provider support.
So what I need is a way to intercept the remote service call at the server side to do some authorization checking (or in my case to provide the needed information to spring security). It's important to have the interception "around" the remote service call such that authorization specific code can be performed before and after the remote service call (in my case some authorization info would be put before the service call for spring security and cleaned up after the call). In ECF generic implementation, what I think would be the right place (but here I'm maybe looking too much at how to integrate with spring security) is in RegistrySharedObject.executeRequest within the run method of the created IProgressRunnable around "localRegistration.callService(call)". I have three reasons to place the authorization interception there:
1. If an exception must be thrown because the call is not authorized the exception will be handled correctly. 2. We're sure not to have any thread context switching from this point to the local service method call (at least not in the ECF layer, you can still have thread context switching within the service method call but that's application specific).
3. It's the last method having all information needed for authorization: ID of the caller (client container), method call and service on which call is performed.
Yes, understood.
For the API, I was thinking to have a policy interface (like the IConnectInitiatorPolicy and IConnectHandlerPolicy) that can be set on IRemoteServiceContainerAdapter interface (question: is this the right place?
Because IRemoteServicecontainerAdapter looks more a client oriented interface while the IRemoteServiceCallPolicy is more a server thing),
Yes, it is the right place for this, I believe. IRemoteServiceContainerAdapter has both remote service 'host' (aka server), and 'consumer' (aka client) methods on it...as there isn't any assumed asymmetry in the remote service API (all just remote service api supporters...able to both register remote services and access the remote service).
for example:
public interface IRemoteServiceCallPolicy { public Object callWithAuthorization(IRemoteServiceRegistration registration, IRemoteCall call, ID fromID) throws Exception; }
and on IRemoteServiceContainerAdapter there should be a new method:
public void setRemoteServiceCallPolicy(IRemoteServiceCallPolicy policy);
<stuff deleted>
There's one thing that bothers me with this solution: an implementation of the IRemoteServicecallPolicy.callWithAuthorization would need to have ECf generic internal knowledge to be able to call localRegistration.callService method because this method is not part of the IRemoteServiceRegistration interface.
Hmm. One possibility is that the IRemoteServiceRegistration interface be enhanced (i.e. to include a callService method).
Another (maybe better) solution could be to have an interface with a pre invocation hook and a post invocation hook. The preinvocation hook can then be used to authorize the service method call and the post invocation hook can be used to clean up authorization data that was set up at the preinvocation hook.
public interface IRemoteServiceCallPolicy { public void preInvocationHook(IRemoteServiceRegistration registration, IRemoteCall call, ID fromID) throws Exception; public void postInocationHook(IRemoteServiceRegistration registration, IRemoteCall call, ID fromID) throws Exception;
}
<stuff deleted>
What's you opinion on this?
This second approach is interesting. It's perhaps a little more complex (two methods rather than one), but in some ways it's more similar to the approach taken in Java 2 security (i.e. the security manager).
My immediate inclination is to prefer the first one (for simplicity). But I would like to discuss with you (Franky) in public (i.e. here on the mailing list) a little bit about your use case...to try to understand which would work better for this and other use cases.
So if you don't mind me asking in public...what are you doing inside of IRemoteServiceCallPolicy for your use case? Is one of the two approaches you list above simpler in your use case? If so, which one?
Thanks again for doing this...and for considering contributing back to the ECF project. This will help all in the community.