Support Authentication with SCRAM #26
Merged
Conversation
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
tobyhede
force-pushed
the
CIP-1034/support-scram-sha-256-plus-auth
branch
3 times, most recently
from
4fe90f6 to
38ab2b1
Compare
tobyhede
force-pushed
the
CIP-1034/support-scram-sha-256-plus-auth
branch
5 times, most recently
from
83d729e to
69f03b0
Compare
auxesis
reviewed
Dec 16, 2024
Contributor
auxesis
left a comment
auxesis
left a comment
There was a problem hiding this comment.
Thanks for this @tobyhede, there's a lot of work that's gone into this.
I have left a few questions, added a few commits, and left some comments.
The integration tests are currently failing in CI because mise --silent --env tcp run proxy & is failing.
That command is failing because it's looking for cipherstash-proxy.toml, which is not committed to the repo.
I need to look into why the environment variables aren't being picked up correctly.
.github/workflows/test.yml
Outdated
Contributor
There was a problem hiding this comment.
For reference, upgrading mise has changed how variables are interpolated, which is causing build failures like this:
run mise run up --extra-args "--detach --wait"
[up] $ chmod 600 tls/*
Running in CI: setting file permissions for container
unknown flag: --detach --wait
[up] ERROR task failed
Error: Process completed with exit code 16.
Contributor
There was a problem hiding this comment.
Timeline:
- November 4: jdx/mise#2906 was a request for string escaping to be the default behaviour
- December 11: jdx/mise#3453 implemented the new behaviour, but in a way that was backwards compatible and worked with our use of mise
- December 11: jdx/mise#3455 modified the behaviour in a way that breaks our use of mise
- December 11: those last two changes (3453 + 3455) go out in the 2024.12.5 release
The workaround is to echo the command and pipe to bash to execute.
Contributor
|
CI is now running reliably, and Proxy is not starting due to a configuration error: Looks like additional config is needed in |
tobyhede
force-pushed
the
CIP-1034/support-scram-sha-256-plus-auth
branch
from
4c3da4f to
4ad4ded
Compare
freshtonic
approved these changes
Dec 16, 2024
Contributor
freshtonic
left a comment
freshtonic
left a comment
There was a problem hiding this comment.
Thank you for figuring all this out!
auxesis
added a commit
that referenced
this pull request
Mar 6, 2025
* auth message parsing and scrambling * extract handler into module * add require_tls option to server config * read_message_with_timeout * PasswordMessage and SASLinitialresponse * Passthrough SCRAM_SHA_256 auth * add postgres-protocol * Pull log into module * Add dev option to disable database tls * remove unused tests * rename dataset to encrypt_config * rename test config files to disambiguate * add channel binding to tlsconfig on init * Improve error message on db connect failure * channelbinding for asyncstream * Auth module & SASL messages * read_auth_message * cleanup * SASL & MD5 Auth actually working * Do not require_tls by default * Handle database Md5 and cleartext auth * disable mapping * SASLMechanism as enum * log targets * Use correct sasl mechanism * updated cargo * Cleanup * psql tests * Maybe integration tests * README on int tests * Ignore cipherstash-proxy.local.toml * just enough shutdown for tests * attempt integration test in ci * clippy cleanup * Filter integration tests * Install latest stable rust * Explain what this config is for * Bump to latest mise, in attempt to work around bug * Do something unholy to make the command execute correctly * Fix use ordering to satisfy the `cargo fmt` gods * Move the test tasks to a more discoverable place * Explicitly call nextest tests unit tests * Run the integration tests as part of the CI tests * option to disable mapping * More logging * Fix bug introduced by bad merge conflict * Add a task for testing if Proxy has started * Make nc work on GNU-based systems * Try and determine if the wait is working * Report when successful * Disable running up a container Because we do that later in the integration tests * Test for the right port t 🤦 * wtf is going on * GitHub: tell me how can you fuck up a loop * Remove debugging. Fuck you GitHub * setup proxy env from secrets * setup proxy env from secrets * un-silent proxy run in tests * Install rust via mise * more clippy --------- Co-authored-by: Lindsay Holmwood <[email protected]> Co-authored-by: Lindsay Holmwood <[email protected]>
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.
No description provided.