Don't restrict server authenticator in PasswordAuthenticator #1801
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently gocql will only allow authenticating with authenticators defined in defaultApprovedAuthenticators in conn.go. There have been multiple occurrences of implementers needing to update this list, either when a vendor would like to add their authenticator, or a new authenticator being added. It would probably reduce friction to just accept any authenticator provided by the server. From what I know, other drivers behave in this way. If a user wanted to restrict this, they could use the existing configuration PasswordAuthenticator.AllowedAuthenticators. patch by Andy Tolbert; reviewed by for CASSANDRA-19859
tolbertam
commented
Aug 23, 2024
|
|
||
| // approve the authenticator with the list of allowed authenticators or default list if approvedAuthenticators is empty. | ||
| // approve the authenticator with the list of allowed authenticators. If the provided list is empty, | ||
| // the given authenticator is allowed. | ||
| func approve(authenticator string, approvedAuthenticators []string) bool { | ||
| if len(approvedAuthenticators) == 0 { |
There was a problem hiding this comment.
alternatively, could test for nil, but felt would keep it this way for consistency.
lukasz-antoniak
approved these changes
Aug 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently gocql will only allow authenticating with authenticators defined in defaultApprovedAuthenticators in conn.go.
There have been multiple occurrences of implementers needing to update this list, either when a vendor would like to add their authenticator, or a new authenticator being added.
It would probably reduce friction to just accept any authenticator provided by the server. From what I know, other drivers behave in this way.
If a user wanted to restrict this, they could use the existing configuration PasswordAuthenticator.AllowedAuthenticators.
patch by Andy Tolbert; reviewed by for CASSANDRA-19859
This is an alternative pr to #1800; since this is a behavioral change, it merits some discussion and may take longer to review.