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

Update handling of account authentication expired error #695

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

wallyqs
Copy link
Member

@wallyqs wallyqs commented Mar 30, 2021

Applies similar handling -ERR 'Account Authentication Expired error as with others auth expired errors.

Signed-off-by: Waldemar Quevedo wally@synadia.com

@wallyqs
Copy link
Member Author

wallyqs commented Mar 30, 2021

After this merge, this test should be updated in the server: nats-io/nats-server@master...wallyqs:auth-acc-expired-err

nats_test.go Outdated
@@ -1651,6 +1651,107 @@ func TestRevokedUserCredentials(t *testing.T) {
wg.Wait()
}

func TestExpiredAccountAuthentication(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Think some of these tests should be refactored into table driven tests since too similar, I'll update the PR

@coveralls
Copy link

coveralls commented Mar 30, 2021

Coverage Status

Coverage decreased (-0.08%) to 86.592% when pulling bd7b51f on wallyqs:auth-acc-expired-err into 7013a58 on nats-io:master.

@kozlovic
Copy link
Member

@wallyqs I don't understand.. you already merged a PR that did that: #684

Signed-off-by: Waldemar Quevedo <wally@synadia.com>
@wallyqs
Copy link
Member Author

wallyqs commented Mar 30, 2021

@kozlovic there were three types of expired/revoked errors that had to be handled in the same way, I changed into table driven tests so that it is easier to add/remove these special cases /cc @matthiashanel

		{"expired users credentials", AUTHENTICATION_EXPIRED_ERR, ErrAuthExpired},
		{"revoked users credentials", AUTHENTICATION_REVOKED_ERR, ErrAuthRevoked},
		{"expired account", ACCOUNT_AUTHENTICATION_EXPIRED_ERR, ErrAccountAuthExpired},

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 (sorry I got confused with 684 and thought that you added the same auth error that was there, but it is different).

Copy link
Contributor

@matthiashanel matthiashanel left a comment

Choose a reason for hiding this comment

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

LGTM

@wallyqs wallyqs merged commit a0b1f60 into nats-io:master Mar 30, 2021
@wallyqs wallyqs deleted the auth-acc-expired-err branch March 30, 2021 22:54
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.

None yet

4 participants