[DEPRECATED] LDAP authentication #341
Closed
Conversation
This file contains 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
vsupalov
force-pushed
the
feature/144-ldap-authentication
branch
4 times, most recently
from
97e4200 to
b6412d7
Compare
added 3 commits
November 28, 2022 11:05
# Conflicts: # CHANGELOG.md # Conflicts: # CHANGELOG.md
Squashed commit of the following: commit 5ba0132 Author: Razvan-Daniel Mihai <[email protected]> Date: Fri Nov 18 17:33:47 2022 +0100 Updates and cleanups. commit 2e7cd7a Author: Razvan-Daniel Mihai <[email protected]> Date: Fri Nov 18 16:58:10 2022 +0100 Remove OPA and update autocheck.py. commit 5f3aa96 Author: Razvan-Daniel Mihai <[email protected]> Date: Fri Nov 18 14:33:06 2022 +0100 Add leftover from previous comit and experiment with autocheck.py commit 71b9c43 Author: Razvan-Daniel Mihai <[email protected]> Date: Fri Nov 18 11:59:11 2022 +0100 Install openldap in the kuttl test namespace. commit 5e0e594 Author: Razvan-Daniel Mihai <[email protected]> Date: Thu Nov 17 17:47:39 2022 +0100 Almost working kuttl test (missing ldap users).
vsupalov
force-pushed
the
feature/144-ldap-authentication
branch
from
3ce21f5 to
c467de4
Compare
This reverts commit e50555f.
Co-authored-by: Razvan-Daniel Mihai <[email protected]>
…etech/druid-operator into feature/144-ldap-authentication
added 5 commits
December 13, 2022 09:26
This was referenced Jan 5, 2023
bors bot
pushed a commit
that referenced
this pull request
Jan 30, 2023
# Description This will resolve part of #144 The ticket can be merged once the stretch goals are reached as well. A new iteration on the changes prototyped in #341 This iteration will include: * A closer resemblance to the ticket requirements - using a list of authenticators * Non-usage of LDAP for inter-node authentication (basic authentication instead) * Erroring out if both TLS auth and LDAP auth are configured ## Follow-up Work * Interconnection with an OPA authorization config, if provided (former stretch goal) * Adding ldaps:// support (former stretch goal) * Druid does not like anonymous LDAP access (without bind credentials). I have not found a way to configure it. This however, seems to be a usecase we want to support generally.
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.
Description
Implementing LDAP according to #144
Trying a few things in the process: (UPDATE: the first two were dropped after teaming up)
Overview of PR Scope and Choices
The PR implements LDAP authentication, leaving TLS-authenticated LDAP
as an exercise for the readerfor a later PR, as it would clash with existing functionality. We decided to first finish the non-TLS LDAP functionality here, as it is a good scope for a single PR when it comes to reviewer-friendliness.Tradeoff made in this PR: secrets related to LDAP are written in plain-text into the druid runtime.properties config. Unfortunately, the druid features which were supposed to be able to read ENV variables which are referenced in runtime.properties simply refused to work.
Most obvious possible follow-up tasks for future work packages:
LDAP vs TLS Auth
The PR contains a commented-out test for TLS while LDAP is enabled. It's failing at the moment, but could prove as a useful starting point if the topic warrants more attention.
Review Checklist
Once the review is done, comment
bors r+(orbors merge) to merge. Further information