Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expose the nonce to custom authentication #2699

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

ripienaar
Copy link
Contributor

Signed-off-by: R.I.Pienaar [email protected]

/cc @nats-io/core

@ripienaar
Copy link
Contributor Author

This is a companion to #2696 that ensure custom auth methods can actually get the nonce, I assumed it was on ClientOpts but not.

@ripienaar ripienaar requested a review from kozlovic November 18, 2021 12:49
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but we have a test called TestCustomClientAuthentication that you could extend/copy form to actually verify that a nonce is specified?

@ripienaar
Copy link
Contributor Author

LGTM, but we have a test called TestCustomClientAuthentication that you could extend/copy form to actually verify that a nonce is specified?

OK, added a bit to that test

@ripienaar ripienaar requested a review from kozlovic November 18, 2021 16:58
@@ -629,6 +638,14 @@ func TestCustomClientAuthentication(t *testing.T) {
if _, err := nats.Connect(addr, nats.UserInfo("invalid", "")); err == nil {
t.Fatal("Expected client to fail to connect")
}

s.opts.AlwaysEnableNonce = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am getting races when running your test, so need to revisit that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I was less lazy now, think it probably fixed now

Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ripienaar ripienaar merged commit 3665d1d into nats-io:main Nov 18, 2021
@ripienaar ripienaar deleted the client_auth_nonce branch November 18, 2021 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants