You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "tolbertam (via GitHub)" <gi...@apache.org> on 2024/08/23 04:12:29 UTC
[PR] Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
tolbertam opened a new pull request, #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis merged PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1728387648
##########
conn.go:
##########
@@ -43,25 +43,11 @@ import (
"github.com/gocql/gocql/internal/streams"
)
-var (
- defaultApprovedAuthenticators = []string{
- "org.apache.cassandra.auth.PasswordAuthenticator",
- "com.instaclustr.cassandra.auth.SharedSecretAuthenticator",
- "com.datastax.bdp.cassandra.auth.DseAuthenticator",
- "io.aiven.cassandra.auth.AivenAuthenticator",
- "com.ericsson.bss.cassandra.ecaudit.auth.AuditPasswordAuthenticator",
- "com.amazon.helenus.auth.HelenusAuthenticator",
- "com.ericsson.bss.cassandra.ecaudit.auth.AuditAuthenticator",
- "com.scylladb.auth.SaslauthdAuthenticator",
- "com.scylladb.auth.TransitionalAuthenticator",
- "com.instaclustr.cassandra.auth.InstaclustrPasswordAuthenticator",
- }
-)
-
-// 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 {
Review Comment:
alternatively, could test for nil, but felt would keep it this way for consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1831201958
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
lgtm 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1831112564
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
Is the comment I changed at line 46 in conn.go ok or were you looking for documentation elsewhere (like on the `AllowedAuthenticators` field in `PasswordAuthenticator`?
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
Is the comment I changed at line 46 in conn.go ok or were you looking for documentation elsewhere (like on the `AllowedAuthenticators` field in `PasswordAuthenticator`?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#issuecomment-2457820171
I'd rather merge this instead of #1800 but if there's an objection to this one we can merge #1800
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1831251143
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
Thanks, i'll squash the commits and update the patch by/reviewed by :+1:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1831139504
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
:+1: sounds great, agree that this is the most appropriate place. Will make that change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1831257780
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
done, hopefully this is good to go!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1831137264
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
I was thinking about a comment on the type itself ( `conn.go` ) so that this information shows up on the docs section for the type (in pkg.go.dev) and when using an IDE:
```
type PasswordAuthenticator struct {
Username string
Password string
// Setting this to nil or empty will allow any authenticator provided by the server
AllowedAuthenticators []string
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1829779060
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
nit: Maybe a comment on the interface function itself could also be useful
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1831140543
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
Maybe a comment on the type itself too.
```
// PasswordAuthenticator can be configured with an "allow list" of authenticators (can be set to nil or empty to allow all)
type PasswordAuthenticator struct {
Username string
Password string
// Setting this to nil or empty will allow any authenticator provided by the server
AllowedAuthenticators []string
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#discussion_r1831161889
##########
doc.go:
##########
@@ -81,6 +81,16 @@
// }
// defer session.Close()
//
+// By default, PasswordAuthenticator will attempt to authenticate regardless of what implementation the server returns
+// in its AUTHENTICATE message as its authenticator, (e.g. org.apache.cassandra.auth.PasswordAuthenticator). If you
+// wish to restrict this you may use PasswordAuthenticator.AllowedAuthenticators:
Review Comment:
made an attempt to document both. Thought it would be good to make it clear that the default behavior of other drivers is to allow any authenticator, generally I don't think people should configure this, and the presence of documentation may lead them to think they need to do so, so felt it was good to clarify that. Hopefully that looks ok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-19 Don't restrict server authenticator in PasswordAuthenticator [cassandra-gocql-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on PR #1801:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1801#issuecomment-2459887589
> I'd rather merge this instead of https://github.com/apache/cassandra-gocql-driver/pull/1800 but if there's an objection to this one we can merge https://github.com/apache/cassandra-gocql-driver/pull/1800
I very much agree with that sentiment, would prefer it be consistent with other drivers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org