Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SASL/SCRAM authentication #1835

Merged
merged 1 commit into from
Mar 7, 2019
Merged

SASL/SCRAM authentication #1835

merged 1 commit into from
Mar 7, 2019

Conversation

andreme
Copy link
Contributor

@andreme andreme commented Feb 16, 2019

@@ -0,0 +1,40 @@
'use strict'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how this can be integrated into the test run? The role needs to be created with a scram-sha-256 password

@andreme andreme marked this pull request as ready for review February 16, 2019 02:38
@andreme andreme force-pushed the scramv1 branch 2 times, most recently from 0c8a03e to 5f51f7f Compare February 16, 2019 04:08
@andreme
Copy link
Contributor Author

andreme commented Feb 16, 2019

The failing test for node 11 fails in other pr's too, so I assume it's not something I did

@brianc
Copy link
Owner

brianc commented Feb 20, 2019

ohh very very nice.

@brianc
Copy link
Owner

brianc commented Feb 21, 2019

Hey if you rebase on master now the test should pass!

@andreme
Copy link
Contributor Author

andreme commented Feb 22, 2019

@brianc thanks, rebased

@demisx
Copy link

demisx commented Feb 26, 2019

Can this please be merged? We are stuck because of #1508 right now. Thank you!

@brianc
Copy link
Owner

brianc commented Mar 6, 2019

yup! Will look @ this today. Apologies for the delay!

@brianc
Copy link
Owner

brianc commented Mar 6, 2019

@andreme this is really awesome! Thanks a lot for taking the time on this. Do you have any documentation on how to get the integration test to pass? I'd like to at least be able to run the integration test locally & once I figure that out I'll work out how to add a hook to run it in CI (if possible). Since it's auth related I'd really like to get this running "end-to-end" in tests so we can be sure we don't regress on this at any point. I think I need postgres@11.x at least...anything else I need to do? I'll see how much I can modify the postgres hba

edit: it looks like I basically have full access to the pg_hba.conf file on the travis box...so just gotta figure out some bash-foo & travis-ci incantations to get it set up correctly.

@andreme
Copy link
Contributor Author

andreme commented Mar 6, 2019

@brianc pg10 is the first version that has scram auth.
Basically, the setup is creating a user with a scram encrypted password, and adding the entries in the pg_hba for that user with scram auth. The template for that is in the test file.

Make sure the user entries come before any trust entries in the pg_hba.

@brianc
Copy link
Owner

brianc commented Mar 7, 2019

gotcha!

Make sure the user entries come before any trust entries in the pg_hba.

good point. will do.

I'll start trying to get the CI stuff working today so we can merge this thing!

@brianc
Copy link
Owner

brianc commented Mar 7, 2019

I'm going to merge this but hold off on releasing semver minor w/ this feature until I can get some tests running in CI. Reading about setting up postgres-10 in travis & configuring pg_hba.conf and stuff now...will hopefully have it set up today.

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.

3 participants