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

Implement LDAP Authentication #748

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AndrewJackson2020
Copy link
Contributor

@AndrewJackson2020 AndrewJackson2020 commented May 25, 2024

This PR implements trust (no password), and LDAP authentication with pgcat. Postgres offers a number of authentication options that are not available with pgcat with trust being the most basic. Added a configuration item to allow the auth_type to be set at the user level.

@AndrewJackson2020 AndrewJackson2020 changed the title Implement trust (no password) authentication Implement trust (no password) authentication and LDAP Aug 26, 2024
@AndrewJackson2020
Copy link
Contributor Author

@drdrsh any chance that this functionality could get merged in? Happy to make any changes, add documentation, tests, split up the PR(one for trust, one for LDAP), etc.

@drdrsh
Copy link
Collaborator

drdrsh commented Sep 3, 2024

I haven't had the time to review this yet.
I will take a look later this week or on the weekend.

@drdrsh
Copy link
Collaborator

drdrsh commented Sep 3, 2024

I will appreciate it if you could add some tests.
I'll update the CONTRIBUTING file to show how you can add tests in Ruby/Python/Go/Rust and how you can run them locally.

@drdrsh
Copy link
Collaborator

drdrsh commented Sep 3, 2024

I updated the Contribution guide to show how you can write integration tests.

@AndrewJackson2020
Copy link
Contributor Author

AndrewJackson2020 commented Sep 3, 2024

Thank for updating those docs. Added some basic unit tests for LDAP and trust. Added onto the python unit tests since that is the testing framework that I am most familiar with.

One thing I notice is that the python tests are currently organized as a script, might be worth refactoring it to use pytest to get better reporting, test isolation, parallelism, amongst other things. I opened #790 which modifies the tests to use the pytest framework. If you want to go that route I can change the tests in this PR as well if/when you want to merge that.

Since LDAP requires a dedicated web service I incorporated GLauth in the docker container since it can be installed via a very straightforward binary. GLauth also provides a docker container as well (https://hub.docker.com/r/glauth/glauth) if this fits better with the pgcat testing framework.

@AndrewJackson2020
Copy link
Contributor Author

I see the #790 was merged. Will work on converting these tests to use pytest today.

@AndrewJackson2020
Copy link
Contributor Author

Just a heads up, removed the trust auth (passwordless) stuff out of this PR. Will raise a new PR with that stuff. I figure its easier that way as the LDAP functionality requires changes to the dockerfile (and thus the CI) in order to run whereas the trust functionality is a lot more limited in scope.

@AndrewJackson2020 AndrewJackson2020 changed the title Implement trust (no password) authentication and LDAP Implement LDAP Authentication Sep 7, 2024
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.

2 participants