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 for GL3 #5483

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Ithanil
Copy link
Contributor

@Ithanil Ithanil commented Oct 26, 2023

This PR enables LDAP authentication for GL3 based on a forked omniauth-ldap (because it is currently unmaintained).
It was not developed with the goal to be merged upstream, but I'd like to provide our development to the public for anyone in need of this.

This PR is part of a series of similar PR submissions (Redis Sentinel, SAML integration, LDAP integration).

Details / How to use:

  • LDAP is added analogous to OIDC via omniauth
  • OIDC takes precedence, so configure any OIDC variables if you want to use LDAP
  • Find the necessary configuration parameters in sample.env
  • Some documentation on the parameters can be found here: https://github.com/omniauth/omniauth-ldap
  • The PR also removes any Sign-Up buttons if external authentication is used

Copy link

sonarcloud bot commented Nov 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Ithanil Ithanil force-pushed the ldap branch 2 times, most recently from 50369f8 to 3a06655 Compare January 9, 2024 15:20
Copy link

sonarcloud bot commented Jan 9, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Mar 12, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Apr 22, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@pizkaz
Copy link

pizkaz commented Jun 6, 2024

Contrary to what I expected and how GL2 handled this, GL3 stores the full DN in the external_id field. This is how the user table of my test instance looks like after

  1. Setting up GL2 and logging in with two LDAP users
  2. Migrating to GL3
  3. Logging in to GL3 as user1:
greenlight3-ldap-greenlight3-db=> select name,email,external_id,provider from users;
       name        |            email             |             external_id             |  provider
-------------------+------------------------------+-------------------------------------+------------
 User2             | [email protected]               | User2                               | greenlight
 User1             | [email protected]                  | cn=user1,ou=users,dc=censored,dc=de | greenlight

Greenlight LDAP configuration (unchanged from GL2 to GL3):

$ env | grep '^LDAP_'
LDAP_BASE=ou=users,dc=censored,dc=de
LDAP_AUTH=simple
LDAP_FILTER=
LDAP_PASSWORD=censored
LDAP_BIND_DN=cn=admin,dc=censored,dc=de
LDAP_UID=cn
LDAP_ROLE_FIELD=
LDAP_TLS_NO_VERIFY=true
LDAP_PORT=636
LDAP_ATTRIBUTE_MAPPING=
LDAP_METHOD=ssl
LDAP_SERVER=greenlight3-openldap

Is this expected behavior? Am I doing something wrong?

Also:
GL2 stored provider=ldap for LDAP users, migration converts this to provider=greenlight. Trying to patch the omniauth callback handler resulted in Greenlight complaining about a provider mismatch between the user and the default role, though.
Maybe I don't fully understand the purpose of the provider field but I feel like this should still read ldap for LDAP users. If that means we need to create separate user roles in every Greenlight 3 instance, I guess it's not worth the effort, though.

@Ithanil
Copy link
Contributor Author

Ithanil commented Jun 6, 2024

Contrary to what I expected and how GL2 handled this, GL3 stores the full DN in the external_id field. This is how the user table of my test instance looks like after

1. Setting up GL2 and logging in with two LDAP users

2. Migrating to GL3

3. Logging in to GL3 as `user1`:
greenlight3-ldap-greenlight3-db=> select name,email,external_id,provider from users;
       name        |            email             |             external_id             |  provider
-------------------+------------------------------+-------------------------------------+------------
 User2             | [email protected]               | User2                               | greenlight
 User1             | [email protected]                  | cn=user1,ou=users,dc=censored,dc=de | greenlight

Greenlight LDAP configuration (unchanged from GL2 to GL3):

$ env | grep '^LDAP_'
LDAP_BASE=ou=users,dc=censored,dc=de
LDAP_AUTH=simple
LDAP_FILTER=
LDAP_PASSWORD=censored
LDAP_BIND_DN=cn=admin,dc=censored,dc=de
LDAP_UID=cn
LDAP_ROLE_FIELD=
LDAP_TLS_NO_VERIFY=true
LDAP_PORT=636
LDAP_ATTRIBUTE_MAPPING=
LDAP_METHOD=ssl
LDAP_SERVER=greenlight3-openldap

Is this expected behavior? Am I doing something wrong?

Also: GL2 stored provider=ldap for LDAP users, migration converts this to provider=greenlight. Trying to patch the omniauth callback handler resulted in Greenlight complaining about a provider mismatch between the user and the default role, though. Maybe I don't fully understand the purpose of the provider field but I feel like this should still read ldap for LDAP users. If that means we need to create separate user roles in every Greenlight 3 instance, I guess it's not worth the effort, though.

First of all, please understand that the developers of GL3 decided to not support multiple different external authentication methods anymore (and also either local OR external authentication). Instead they implemented only a single external method, namely OpenID Connect, with the suggestion to use Keycloak to support other protocols.
Why they still added the 'provider' attribute/column to multiple objects/tables is beyond me, but in any case it is currently always 'greenlight'. That's why the migration also maps everything to 'greenlight'.

Now, this PR adds unofficial support for LDAP without having a Keycloak in between the LDAP server and Greenlight. However, it does so using a different Ruby gem (omniauth-ldap) than what was used in Greenlight 2. This alone might lead to slightly different behavior. Also, maybe not all the environment variables from GL2 are supported here, best is to have a look at config/initializers/omniauth.rb to see what variable does what.

Regarding the database entries with full DN, I also can't tell anymore how it looked in our GL2 database. But in our current GL3 database, all users have the full DN as external_id regardless of whether they have logged in after the migration or not. So I'm pretty sure it was the same before in our case and also would assume that I would have noticed the difference when migrating. Unfortunately, my knowledge about the LDAP protocol or how omniauth-ldap implements it is extremely limited, so I can't really give you advice how to achieve a different outcome.

EDIT: Also, can you please check how the table looks before having User1 logged in after migration and also double-check that it isn't actually the same in the GL2 DB? If I'm not completely confused right now, it should create an entirely new user if the external_id didn't match on login.

@pizkaz
Copy link

pizkaz commented Jun 7, 2024

First of all, please understand that the developers of GL3 decided to not support multiple different external authentication methods anymore (and also either local OR external authentication). Instead they implemented only a single external method, namely OpenID Connect, with the suggestion to use Keycloak to support other protocols. Why they still added the 'provider' attribute/column to multiple objects/tables is beyond me, but in any case it is currently always 'greenlight'. That's why the migration also maps everything to 'greenlight'.

Ah, thank you for clarifying this.

Now, this PR adds unofficial support for LDAP without having a Keycloak in between the LDAP server and Greenlight. However, it does so using a different Ruby gem (omniauth-ldap) than what was used in Greenlight 2. This alone might lead to slightly different behavior. Also, maybe not all the environment variables from GL2 are supported here, best is to have a look at config/initializers/omniauth.rb to see what variable does what.

Yes, the different ruby gem might explain everything. Frankly, I don't really care how the external_id is stored, as long as no data is lost and users don't need to recreate rooms and such from scratch.
As long as e-mail addresses do not change, the external_id is automatically updated and in conjunction with greenlight-ldap-sync, which synchronizes name and e-mail regardless of the external_id format, the whole setup should be pretty robust.

EDIT: Also, can you please check how the table looks before having User1 logged in after migration and also double-check that it isn't actually the same in the GL2 DB? If I'm not completely confused right now, it should create an entirely new user if the external_id didn't match on login.

Already did that: Before first LDAP-login in GL3 the "User1" row looks exactly the same as the "User2" row, which in turn is the same as in GL2 (except for provider).

Copy link

sonarcloud bot commented Sep 19, 2024

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

Successfully merging this pull request may close these issues.

3 participants