Add password protection to NATS

[Mrxx99]

Description

This adds parameter support to NATS. The postgres implementation was used as inspiration for the implementation.
It uses the user info of the connection string URL to add user name and password, so the connections string looks like
nats://nats:MGBKxjTAjX2stg2wzzjtVW@localhost:52166.
Some NATS clients support this form natively (as can bee seen here) but the .NET one unfortunately does not. It would be nice if this support would be added directly to NATS.Net then that code could be removed from here again.

Contributes to: #6155

The NatsServerResource constructor is changed from (string name) to (string name, ParameterResource? userName, ParameterResource password) to be the same as the PostgresServerResource

Currently there is a inconsistency between the parameters of AddNats(...) and AddPostgres(...).
The postgres method looks like (name, userName = null, password = null, port = null) and the Nats one is changed from (name, port = null) to (name, port = null, userName = null, password = null)', otherwise AddNats("nats", 4222) would break.

So there are 3 options:
a) keep it like it currently is, not breaking, but inconsistent to postgres and forces to use named arguments when user wants to specify password but no port.
b) change the order to be consistent with postgres, breaks existing code that specifies the port without named arguments, but is nicer to use when specifying password and no port
c) keep the existing constructor and add a new one that mimics the postgres one, the existing one would call into that. This would have all the benefits of the two options above and will only break on AddNats("nats", default) (which is unlikely to be used I think).

Edit: seems like all resources have the port at the end in this scenario, except MongoDb which has two constructors, but both have the ports after the name. Maybe it would make sense to also make it consistent for MongoDb and to change MongoDb to option c), should still be possible as the second constructor is not yet shipped.

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
  • No. Follow-up changes expected.
• Are you including unit tests for the changes and scenario tests if relevant?
  • Yes
  • No
• Did you add public API?
  • Yes
    • If yes, did you have an API Review for it?
      • Yes
      • No
    • Did you add <remarks /> and <code /> elements on your triple slash comments?
      • Yes
      • No
  • No
• Does the change make any security assumptions or guarantees?
  • Yes
    • If yes, have you done a threat model and had a security review?
      • Yes
      • No
  • No
• Does the change require an update in our Aspire docs?
  • Yes
    • Link to aspire-docs issue: Add docs about NATS password protection docs-aspire#1817
  • No

[eerhardt]

It uses the user info of the connection string URL to add user name and password, so the connections string looks like
nats://nats:MGBKxjTAjX2stg2wzzjtVW@localhost:52166.
Some NATS clients support this form natively (as can bee seen here) but the .NET one unfortunately does not. It would be nice if this support would be added directly to NATS.Net then that code could be removed from here again.

One interesting behavior is that now that the NATS Hosting has a health check, I am getting the user name and password printed out in my console log:

Is this expected behavior? That doesn't seem right.

cc @mtmk

[eerhardt]

Currently there is a inconsistency between the parameters of AddNats(...) and AddPostgres(...).

Yes, this is an issue, but not one that has a good solution. With MongoDB we decided to go with option (a) keep it like it currently is, not breaking, but inconsistent. This decision was made because we don't make breaking changes to public APIs we've already shipped.

[Mrxx99]

Currently there is a inconsistency between the parameters of AddNats(...) and AddPostgres(...).

Yes, this is an issue, but not one that has a good solution. With MongoDB we decided to go with option (a) keep it like it currently is, not breaking, but inconsistent. This decision was made because we don't make breaking changes to public APIs we've already shipped.

So should I also use a) for Nats?. The password to mongodb was only introduced in aspire 9 RC, so not yet released, so could still be changed to c). Or is having an new amibguity for AddNats/Mongo("name", default) such a problem? Or (for mongo) is breaking change not possible even in RC?

[mtmk]

It uses the user info of the connection string URL to add user name and password, so the connections string looks like
nats://nats:MGBKxjTAjX2stg2wzzjtVW@localhost:52166.
Some NATS clients support this form natively (as can bee seen here) but the .NET one unfortunately does not. It would be nice if this support would be added directly to NATS.Net then that code could be removed from here again.

One interesting behavior is that now that the NATS Hosting has a health check, I am getting the user name and password printed out in my console log:

Is this expected behavior? That doesn't seem right.

cc @mtmk

I might be missing something here but there is no reason to pass user:pass in the url since it won't be used by the client, plus the client doesn't have safeguards to redact the password since it's not a supported method of passing username/password or tokens.

[Mrxx99]

@mtmk

I might be missing something here but there is no reason to pass user:pass in the url since it won't be used by the client, plus the client doesn't have safeguards to redact the password since it's not a supported method of passing username/password or tokens.

It is used because that's how aspire forwards information between resources. Environmental variables would also be possible, but the default aspire approach is using connection strings. The username and password are parsed out in the Aspire.NATS.Net integration package. The user/password information could be stripped out before passing the URL to NATS.Net. An advantage of having this connection string is also easy interopability with clients in other languages that already support this format. It would be nice if NATS.Net would also support it directly.

[mtmk]

@mtmk

I might be missing something here but there is no reason to pass user:pass in the url since it won't be used by the client, plus the client doesn't have safeguards to redact the password since it's not a supported method of passing username/password or tokens.

It is used because that's how aspire forwards information between resources. Environmental variables would also be possible, but the default aspire approach is using connection strings. The username and password are parsed out in the Aspire.NATS.Net integration package. The user/password information could be stripped out before passing the URL to NATS.Net. An advantage of having this connection string is also easy interopability with clients in other languages that already support this format. It would be nice if NATS.Net would also support it directly.

@Mrxx99 thanks for the explanation 👍
sounds like something to be implemented on the client also as @eerhardt suggested. feel free to start a nats.net PR I'm happy to prioritize its release.

[mtmk]

@Mrxx99 NATS .NET with URL auth support released in v2.5.3 thank you for your contribution 👍
https://github.com/nats-io/nats.net/releases/tag/v2.5.3

[Mrxx99]

@eerhardt The 2.5.3 version of Nats.NET (https://www.nuget.org/packages/NATS.Net/2.5.3) needs to be added to the dotnet-public feed so I can use it in this PR.

[sebastienros]

The 2.5.3 version of Nats.NET needs to be added to the dotnet-public feed so I can use it in this PR.

Started job, should be done in 10 minutes

[eerhardt]

LGTM. Thanks for the contribution!