-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Ldap Group Provider #17518
Ldap Group Provider #17518
Conversation
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. It would be nice if we could have a PTs for this.
...o-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapGroupProvider.java
Outdated
Show resolved
Hide resolved
...o-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapGroupProvider.java
Outdated
Show resolved
Hide resolved
...o-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapGroupProvider.java
Outdated
Show resolved
Hide resolved
...o-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapGroupProvider.java
Outdated
Show resolved
Hide resolved
...o-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapGroupProvider.java
Show resolved
Hide resolved
...word-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapGroupProviderConfig.java
Outdated
Show resolved
Hide resolved
plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/LdapUtil.java
Outdated
Show resolved
Hide resolved
.../trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapUtil.java
Outdated
Show resolved
Hide resolved
ah yup .. Product Tests .. also i will write some Doco ;) for this .. WIP. |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
4 similar comments
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
BTW i have filled in the CLA and emailed it back am ignoring the bot ;) |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
hi @Praveen2112 PTAL, i have added in support for these three common ldap schema use cases now. (1) RFC2307 (posix)
The group is a first-class entry in the LDAP server (e.g. "cn=admins,ou=groups,dc=example,dc=com") (2) Active Directory
The user’s group memberships are listed as attributes on the user, and the group does not exist as an entry on the server. e.g "memberOf" (3) RFC2307bis (FreeIPA), Augmented Active Directory
The user’s group memberships are listed as attributes on the user. e.g. "memberOf: cn=admins,ou=groups,dc=example,dc=com" The group is a first-class entry on the LDAP server. e.g. "dn: cn=admins,ou=groups,dc=example,dc=com" |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
4 similar comments
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Commit 0d0c56a Added testing support for active directory schema based testing. Tests will need this test image PR to merge - trinodb/docker-images#166 |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
2 similar comments
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Documentation for LDAP Group provider now added in this commit. |
CLA - i signed this and returned it on 16/5/23 cheers. |
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
1 similar comment
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
161e5e2
to
d145448
Compare
81540be
to
e4dc470
Compare
Done. |
Hi there! This is a very exciting PR (thank you so much for working on it @eformat ). Is there anything I could help here with? We would find this particular group provider extremely helpful for our own needs. I've not yet gone through the contributor agreements, but am happy to try to help if I'm able to get approved. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
@eformat Any updates on this PR ? |
hi, no. it is complete from a while back ? just waiting to be merged really. |
Some of the review comments were not yet addressed. Can we address them and rebase the PR ? |
which comments please ?
also it has been a long time .. so i dont have a lot of bandwidth on this
at the moment
so if you do intend on merging it , please let me know as i do not have
bandwidth to continually rebase this over several months
…On Thu, 2 Nov 2023 at 17:00, Praveen Krishna ***@***.***> wrote:
Some of the review comments were not yet addressed. Can we address them
and rebase the PR ?
—
Reply to this email directly, view it on GitHub
<#17518 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFN7IH25ZRPLMTRFUVW2WTYCNAJPAVCNFSM6AAAAAAYDNP4L6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJQGE3TMMRZGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Like this one - #17518 (comment), In this case can other members (or myself) work on top of your PR for applying the comments and rebasing, I will make sure you will be added as a co-author in the new PR. |
this is endless. i did not see these on june 12 - sorry. sure - feel free to work on top of the existing code. |
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
@Praveen2112 could you work with @eformat and continue this PR? |
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time. |
FYI .. i am still keen to get this merged .. looks like its going to take a bit more effort .. i may have some bandwidth coming up to get that last mile in on this PR .... |
BTW, there's a competing PR with similar functionality: #20157 |
thats fine - @Praveen2112 as discussed - is that a clone of this PR ? if so i will close this and you can attribute as appropriate |
No, it is an independent effort |
Reopening work previously closed - this PR supersedes #8335 #10116
Adds
cc: @bitsondatadev
Please advise on what you think needs finishing off. I have tested this against a local FreeIPA ldap setup OK and have joined your trino slack dev channel as well. cheers.