dotnet / MQTTnet

MQTTnet is a high performance .NET library for MQTT based communication. It provides a MQTT client and a MQTT server (broker). The implementation is based on the documentation from http://mqtt.org/.
MIT License
4.51k stars 1.07k forks source link

expose more information to ConnectionValidator #346

Closed liuhongbo closed 4 years ago

liuhongbo commented 6 years ago

not sure my request makes sense 😄

currently the MqttConnectionValidatorContext includes ClientId,Username, Password, WillMessage.

Just a thought.

JanEggers commented 6 years ago

you should add client certificates if you just inspect message content that is not save

chkr1011 commented 6 years ago

In general this should be possible (no matter if it makes sense or not). Since version 2.8.0 the adapters have a Endoint as string defined which contains the remote IP and port.

I will check how to add this Endpoint to the validator.

Adding more information from the WebSocket channel is more complicated because it has features which are not avaialble for TCP so me must add interfaces etc.

chkr1011 commented 6 years ago

I added the Endpoint to the validator (for the next release). Hope this helps.

liuhongbo commented 6 years ago

I update to version V2.8.2. Now I can access the MqttConnectionValidatorContext.Endpoint. However, I did a test, the context.Endpoint in my test is string "0HLGFACF8503R" I expect it would be in the format of "ip:port" I did a simple snippet,

var a = new IPEndPoint(new IPAddress(new byte[] { 192, 168, 0, 1 }), 8000);
var s = a.ToString();

s would be "192.168.0.1:8000" Am I missing something? Thanks,

liuhongbo commented 6 years ago

After I looked at the source code, I think the reason the Endpoint is something like "0HLGFACF8503R" rather "ip:port" is because I used the ASP.NET Core 2.1 new TCP transport. In the file MQTTnet\Source\MQTTnet.AspnetCore\MqttConnectionContext.cs

 public string Endpoint => Connection.ConnectionId;

It uses ConnectionId

Should it be something like this instead?

 public string Endpoint => (Connection as TcpConnection)._endPoint.ToString();

Of course, the _endPoint has to be changed to be public first.

JanEggers commented 6 years ago

TcpConnection is client side implementation of Connection.

on the server it is Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionContext.

so please create an issue at https://github.com/aspnet/KestrelHttpServer/

currently I dont see any features that expose information about the remote endpoint (https://github.com/aspnet/KestrelHttpServer/tree/release/2.2/src/Connections.Abstractions/Features)

liuhongbo commented 6 years ago

HttpConnectionContext add feature to expose RemoteEndPoint #2875

Will see what happen. :smiley:

Thanks,

liuhongbo commented 6 years ago

@halter73 suggested a solution in the issue https://github.com/aspnet/KestrelHttpServer/issues/2875#issuecomment-418473684

IHttpConnectionFeature has the remote end point information.

the same way the HttpConnectionMiddleware does

would this work?

JanEggers commented 6 years ago

will try it out

SeppPenner commented 5 years ago

@JanEggers Do you have a status update for this?

JanEggers commented 5 years ago

nope

halter73 commented 5 years ago

At this point, I'd probably just wait for ASP.NET 3.0 to do anything on this, since starting in 3.0, ConnectionContext will directly expose the RemoteEndpoint similarly to HttpContext.Connection.

JanEggers commented 5 years ago

thx for letting us know

SeppPenner commented 4 years ago

This should be fixed with https://github.com/chkr1011/MQTTnet/pull/882 (For NetCore 3.1+).