Repository navigation
NCBC-4375: Add PasswordAuthenticator.LdapCompatible - #235
Draft
davidkelly wants to merge 1 commit into
Draft
davidkelly wants to merge 1 commit into
davidkelly wants to merge 1 commit into
Conversation
Motivation ========== NCBC-4331 stops PasswordAuthenticator sending SASL PLAIN on non-TLS connections. Users with no SCRAM credentials, such as LDAP users, then cannot connect with couchbase://, and have no way to opt in. Modification ============ - PasswordAuthenticator.LdapCompatible(username, password) uses PLAIN on every KV connection, as Java's ldapCompatible does. Its docs warn that PLAIN sends the password in cleartext without TLS. - On a non-TLS connection the factory requires the server to advertise PLAIN, and logs a warning once per cluster that the password is sent in cleartext. - The default authenticator is unchanged. There is no general allowed-mechanisms setting. Results ======= New unit tests cover PLAIN on both transports, a server that does not offer PLAIN, and the warning (once, never on TLS, never by default). Each fails against code with the matching defect. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
FIT performer imagePublished: ghcr.io/couchbase/dotnet-fit-performer:NCBC-4375Run FIT locally against this PR: fit run preset <preset-name> --performer dotnet-fit-performer:NCBC-4375Or run it from the workflow here. Note Each push to this PR replaces the image and edits this comment. The image is deleted 7 days after the last push. |
This file contains hidden or 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
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.
The issue in brief
After #232 (NCBC-4331),
PasswordAuthenticatornever sends SASL PLAIN on a non-TLS connection, because PLAIN sends the password in cleartext. Users with no SCRAM credentials, such as LDAP users, then cannot connect withcouchbase://, and have no way to opt in. This PR adds that opt-in:PasswordAuthenticator.LdapCompatible(username, password).Jira: NCBC-4375. Stacked on #232: the base is
NCBC-4331. Review only the last commit. I will retarget it tomasterafter #232 merges.Change
PasswordAuthenticator.LdapCompatible(username, password): a static factory that uses PLAIN on every KV connection, TLS or not. It matches Java'sldapCompatible(builder().onlyEnablePlainSaslMechanism()) and C++'spassword_authenticator::ldap_compatible. Its XML docs warn that PLAIN sends the password in cleartext without TLS, and recommendcouchbases://.SaslMechanismFactory.CreateLdapCompatibleMechanism, on a non-TLS connection:SASL_LIST_MECHS, as Java does, and otherwise throwsAuthenticationFailureExceptionthat names the advertised list. If the list is empty (the fetch failed), it still tries PLAIN, and the server decides.LdapCompatiblebehaves the same as the default authenticator: PLAIN, no warning.Deliberately left out: a general allowed-mechanisms setting (Java's
allowedSaslMechanisms/enablePlainSaslMechanism). It is more public API, mostly for edge cases, andenablePlainSaslMechanismbrings back the silent downgrade to PLAIN that #232 removed. No SDK RFC defines either API. RFC 0048 mentions mechanisms "enabled by the user" but defines no API for it. We can add it when someone asks.Tests
SaslMechanismFactoryTests(8 new): PLAIN on non-TLS when the server offers it (with and without SCRAM); throws, with no warning, when the server does not offer PLAIN; still tries PLAIN when the list is empty; warns once over three connections; PLAIN with no warning over TLS; the default path never warns.PasswordAuthenticatorTests(3 new):LdapCompatibleuses PLAIN on both transports, through the real factory, and keeps the credentials.Release note
PasswordAuthenticator.LdapCompatible(username, password)authenticates with SASL PLAIN on every connection, for users that have no SCRAM credentials, such as LDAP users. Oncouchbase://this sends the password in cleartext, and the SDK logs a warning the first time it does. Usecouchbases://when possible.🤖 Generated with Claude Code