nats-io / nats.java

Java client for NATS
Apache License 2.0
563 stars 154 forks source link

Add quickfix to be able to use auth handler and user info #1129

Closed ryad-eldajani closed 4 months ago

ryad-eldajani commented 4 months ago

When trying to connect to a NATS server with Auth Callout, the client may be required to use a combination of an auth handler (NKey + Seed) plus user info (username + password). This is working as expected in the go reference implementation.

However, this is not working in the current nats.java implementation, as user info is stripped out when building connection options:

AuthHandler authHandler = Nats.staticCredentials(jwt, nkey);
Options connectOptions = Options.builder()
        .server(config.getUrl())
        .authHandler(authHandler)
        .userInfo(username, password)  
        .build();

This PR fixes the behavior in the client options to allow the usage of an auth handler in combination with username and password.

scottf commented 4 months ago

@ryad-eldajani I messed up your pr, I had made a change to options and it got merged here and overwrote your change and reverted back to before your change. I need to close this PR. Can you please make a new one with the changes discussed.

scottf commented 4 months ago

@ryad-eldajani So my commit contains my original modifying of the includeAuth check, which came before I got the feedback from the developers that it's not one or the other, but you can send both. Please make that change, again sorry for the screwup.

ryad-eldajani commented 4 months ago

@scottf Thanks for your valuable feedback. I'll try to redo this change within the next few days, as I'm currently out of office for vacation.

Also, sorry for the missing verification. I have committed this change using my working email address, which is currently not assigned to my personal GPG key. Will keep that in mind on the next PR.

Best regards!

ryad-eldajani commented 4 months ago

Hi @scottf thanks for your patience. I have created a new PR based on this discussion. Please see https://github.com/nats-io/nats.java/pull/1138