Code-Sharp / WampSharp

A C# implementation of WAMP (The Web Application Messaging Protocol)
http://wampsharp.net
Other
385 stars 84 forks source link

Host with multiple transports having different auth types #328

Closed esso23 closed 2 years ago

esso23 commented 3 years ago

Hello,

I need to create a Host that has multiple transports having different types of authentication. My case:

I tried using code from WampAuthenticationHost but I ended up with endless internal dependency chain (I gave up after copying more than dozen classes).

I just need to have 2 different RegisterTransport methods, one which uses CreateAuthenticationBinding (this one uses WampAuthenticationBinaryBinding, WampAuthenticationBinaryBinding and WampAuthenticationBinding which are all internal and have other internal dependencies) and one which does not.

I also tried to use implementation of WampHost and edit that but that's even worse, almost everything is internal.

Is there a simple way to do this? Or is it at least possible to make most of those things public?

Thanks Esso

darkl commented 3 years ago

It seems to me like the problem is that you have a single IWampSessionAuthenticatorFactory so apparently you can't use two authentication methods? But IWampSessionAuthenticatorFactory actually receives as an argument of the type WampPendingClientDetails so in theory we can just add the details about the underlying transport there and then you can choose your authenticator factory based on that?

Elad

esso23 commented 3 years ago

Oh I didn't know it's sent in the arguments like that. That's very convenient. I actually have two separatate IWampSessionAuthenticatorFactorys so I created one Master factory which has a dictionary to switch the factory based on transport. Is there some list where all the transport strings are listed?

darkl commented 3 years ago

Can you attach here your master factory code please? For future reference and for understanding what you exactly did.

Thanks Elad

esso23 commented 3 years ago

Well it does not work since the TransportDetails object is null.

Screenshot 2020-12-10 22 58 16

darkl commented 3 years ago

Yeah, I'll fix that. Maybe the WampPendingClientDetails class should also include the transport itself for the case where you have two instances of a transport of the same type with different authorization methods.

Elad

On Thu, Dec 10, 2020, 16:59 Andrej Kmetík notifications@github.com wrote:

Well it does not work since the TransportDetails object is null.

[image: Screenshot 2020-12-10 22 58 16] https://user-images.githubusercontent.com/5548662/101834975-5def3880-3b3b-11eb-85b1-23ab65f69f2d.png

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Code-Sharp/WampSharp/issues/328#issuecomment-742826577, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIS75QSPRU4EBJRVO65ILTSUFAF7ANCNFSM4UTXX54A .

esso23 commented 3 years ago

I was thinking about it and in my opinion it's a hacky solution. Ideally we should be able to just attach IWampSessionAuthenticatorFactory to a binding just like it's done in WampAuthenticationHost and that would solve all the issues. In that case I would be able to not only have different authentication methods for multiple instances of the same transport, but even specify different authenticators for each binding on the same transport.

darkl commented 3 years ago

We'll have to change some api.

// Allows to map different bindings to different authentication methods
public void RegisterTransport(IWampTransport transport, IDictonary<IWampBinding, IWampSessionAuthenticatorFactory> bindingToAuthenticationFactory);

If we remove the original overload:

public void RegisterTransport(IWampTransport transport, IEnumerable<IWampBinding> bindings)

this will break api and also won't allow WampAuthenticationHost to implement IWampHost. Another option is just to add a new class instead of WampAuthenticationHost that has this functionality. I'll have to do some refactoring tricks but this is doable.

I also checked and authentication for crossbario is also defined per transport so we should probably support this.

darkl commented 3 years ago

Hi Esso,

I just pushed a commit with this feature. Can you try it out? The version is 21.0.0-develop-33 from GitHub and the new class is called WampMultiAuthenticationHost.

Usage:

WampMultiAuthenticationHost host =
    new WampMultiAuthenticationHost();

host.RegisterTransport(new FleckWebSocketTransport("ws://127.0.0.1:8080/ws"),
                       new Dictionary<IWampBinding, IWampSessionAuthenticatorFactory>()
                       {
                           {new JTokenJsonBinding(), new WampCraUserDbAuthenticationFactory(new MyAuthenticationProvider(), new MyUserDb())},
                           {new JTokenMessagePackBinding(), new MyAuthenticatorFactory()}
                       });

Thanks Elad

esso23 commented 3 years ago

Hi Elad, sorry for answering so late. I tried this (using 21.0.0-develop-34) and it seems to work without any issues (I will do more testing in the following weeks).

I propose adding another overload:

void RegisterTransport(IWampTransport transport, IEnumerable<IWampBinding> bindings, IWampSessionAuthenticatorFactory sessionAuthenticatorFactory);

Since it's likely going to be the most common use-case to add the same auth factory to all the bindings on the given transport.

esso23 commented 2 years ago

I did some tests in production environment with this and found no issues.

darkl commented 2 years ago

Great! I plan to release a new version of WampSharp at some point containing these changes and also the changes of #330, but it will take me some time unfortunately.

esso23 commented 2 years ago

Hi @darkl, any ETA on when we can expect this to be merged to master?

Thanks, Esso

darkl commented 2 years ago

Hi Esso,

I'm currently very busy. I'll have more time after March 15th.

Elad

esso23 commented 2 years ago

Alright, we'll be patiently waiting then :)

darkl commented 2 years ago

Version 22.5.1 is now available on NuGet!