Skip to content

Conversation

@tolbertam
Copy link
Contributor

@tolbertam tolbertam commented Aug 23, 2024

Allow MutualTlsWithPasswordFallbackAuthenticator and MutualTlsAuthenticator as possible authenticators.

MutualTlsWithPasswordFallbackAuthenticator should behave functionally the same as PasswordAuthenticator.

MutualTlsAuthenticator's current implementation doesn't send AUTHENTICATE messages to the client, but felt it was worth including here in case it is ever enhanced to possibly also require credentials.

patch by Andy Tolbert; reviewed by Martin Sucha for CASSANDRA-19858

Copy link
Contributor

@martin-sucha martin-sucha left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! Please update the reviewed by in the commit message.

@tolbertam
Copy link
Contributor Author

will do! Apologies for missing this, I'll update my commit shortly.

Allow MutualTlsWithPasswordFallbackAuthenticator and
MutualTlsAuthenticator as possible authenticators.

MutualTlsWithPasswordFallbackAuthenticator should behave
functionally the same as PasswordAuthenticator.

MutualTlsAuthenticator's current implementation doesn't send
AUTHENTICATE messages to the client, but felt it was worth
including here in case it is ever enhanced to possibly
also require credentials.

patch by Andy Tolbert; reviewed by Martin Sucha for CASSANDRA-19858
@tolbertam
Copy link
Contributor Author

commit updated with no other changes than the commit message itself 👍

Copy link
Contributor

@martin-sucha martin-sucha left a comment

Choose a reason for hiding this comment

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

Thank you!

@joao-r-reis joao-r-reis changed the title Add MutualTls authenticators to defaultApprovedAuthenticators CASSGO-18 Add MutualTls authenticators to defaultApprovedAuthenticators Nov 5, 2024
@joao-r-reis
Copy link
Contributor

I'm +1 of merging #1801 instead of this PR but if there's concerns about #1800 I'm ok with merging this one instead.

@joao-r-reis
Copy link
Contributor

Closing this as #1801 was merged.

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.

4 participants