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

Support LDAP authentication #144

Closed
7 tasks
Tracked by #172 ...
lfrancke opened this issue Feb 7, 2022 · 7 comments
Closed
7 tasks
Tracked by #172 ...

Support LDAP authentication #144

lfrancke opened this issue Feb 7, 2022 · 7 comments
Assignees

Comments

@lfrancke
Copy link
Member

lfrancke commented Feb 7, 2022

As a user I'd like to use my existing LDAP/AD credentials to log into Druid. This was already done in e.g. NiFi or Trino. This can be especially helpful for writing tests.

The LDAP support should be integrated in the structure from PR #6 which must be finished first.

apiVersion: druid.stackable.tech/v1alpha1
kind: DruidCluster
metadata:
  name: druid
spec:
  version: 24.0.0-stackable0.1.0
  clusterConfig:
    tls:
      serverSecretClass: String # defaults to "tls"
      internalSecretClass: String # defaults to "tls"
    authentication: 
      - authenticationClass: druid-tls-authentication-class # String
      - authenticationClass: druid-ldap-authentication-class # String

This is done when

  • LDAP is configurable in the CRD using the LDAP structs from operator-rs
  • A user/admin can configure Druid to use a LDAP server for authentication (while still offering existing authentication methods)
  • There is documentation on how to configure Druid with LDAP using the Custom Resource
  • Optional: There is an example demonstrating Druid with LDAP (docs, or example folder)
  • There are tests which include:
    • OpenLDAP is installed and accessible via Druid
    • LDAP authenticated access to Druid works
  • It is added to the feature tracker (ask Lars for help)

This depends on the reference architecture developed in stackabletech/issues#170

@fhennig
Copy link
Contributor

fhennig commented Feb 16, 2022

Druid already supports LDAP authentication https://druid.apache.org/docs/latest/operations/auth-ldap.html

Edit: I meant by this that at least we don't have to write an extension for it - we just need to configure it properly

@lfrancke
Copy link
Member Author

Blocked by #6

@sbernauer
Copy link
Member

Blocked by #365

@vsupalov
Copy link
Contributor

The first PR #374 is in review now. To close this ticket, we'll need to add ldaps:// in a later PR. They are separated for the sake of reviewer convenience, as the second part will require refactoring TLS-related code as well.

bors bot pushed a commit that referenced this issue 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.
@lfrancke
Copy link
Member Author

lfrancke commented Feb 2, 2023

I briefly talked to Vladislav today and he said that we might want to do more work here before we finish this ticket.
Can someone give an update on this issue?

@vsupalov
Copy link
Contributor

vsupalov commented Feb 6, 2023

My current understanding: there is the intention to add ldaps support to this ticket before closing it. The consensus was, that it probably would not be a lot of effort and supporting ldaps is something people expect.

The previous PR was very long-running and already quite big, which is why I opted to close it instead of cramming #382 into it as well. Adding ldaps could turn out to be a relatively simple PR, but from what I could tell a lot of existing TLS functionality would need to be adjusted as well (creating and handling a common trust store mostly), making it seem larger than it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants