Ldap integration tests by zachjsh · Pull Request #10901 · apache/druid

zachjsh · 2021-02-19T02:28:49Z

Description

Added integration tests for testing the druid-basic-security extension in ldap mode. Modeled heavily after the existing ITBasicAuthConfigurationTest. Added new test group ldap-security as we need specific common druid properties to be set in order configure druid to use ldap auth z/n

Running the osixia/docker-openldap docker image as an ldap server for the tests
users and groups are preloaded into the ldap server, as defined in bootstrap.ldif
refactored common functionality between ITBasicAuthConfigurationTest and ITBasicAuthLdapConfigurationTest into base class AbstractAuthConfigurationTest

zachjsh · 2021-02-19T02:43:30Z

integration-tests/docker/docker-compose.base.yml

      - ./environment-configs/router-custom-check-tls
+
+  druid-openldap:
+    image: osixia/openldap:1.4.0


This uses MIT license. Do we need to add this to any license file or anything like that?

Since this is only pulled while running the integration tests, I think it can be omitted, since it won't actually ever be included in a source/binary/docker distribution, which is what the license and notice files cover.

zachjsh · 2021-02-19T02:44:50Z

integration-tests/docker/test-data/ldap-security-sample-data.sql

+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+INSERT INTO druid_tasks (id, created_date, datasource, payload, status_payload, active) VALUES ('index_auth_test_2030-04-30T01:13:31.893Z', '2030-04-30T01:13:31.893Z', 'auth_test', '{\"id\":\"index_auth_test_2030-04-30T01:13:31.893Z\",\"created_date\":\"2030-04-30T01:13:31.893Z\",\"datasource\":\"auth_test\",\"active\":0}', '{\"id\":\"index_auth_test_2030-04-30T01:13:31.893Z\",\"status\":\"SUCCESS\",\"duration\":1}', 0);


This is same as security-sample-data.sql . There is a script file that is expecting a file with name {GROUP}-sample-data.sql. maybe can change logic specifically for security or ldap-security group tests to just look for the security-sample-data.sql instead?

Hmm, I checked the test-data dir and it looks like the 3 files below (out of 5) are identical:
high-availability-sample-data.sql
query-retry-sample-data.sql
query-sample-data.sql

I'm not sure at this point how likely it is for these sample data sets to diverge in the future, but it seems like it might be reasonable to change the structure in a way such that the individual groups can specify directly what sample data they need preloaded (and those 3 files, and the security files, could be shared).

That said, I don't think it's a huge deal if we introduce another copy now.

Left as is. Think fixing this otherwise would require a lot of code churn. If we're going to fix this, I think it should be in another pr.

suneet-s · 2021-02-23T01:03:38Z

...on-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthLdapConfigurationTest.java

+  }
+
+  @Test
+  public void testSystemSchemaAccess() throws Exception


This test seems to be doing many things. Can we break it up into smaller tests?

Similar comment for other examples in this test. It seems like each time the logger is used, that could be a separate test

broke up a lot of the tests.

jon-wei · 2021-02-23T00:04:56Z

integration-tests/docker/test-data/ldap-security-sample-data.sql

+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+INSERT INTO druid_tasks (id, created_date, datasource, payload, status_payload, active) VALUES ('index_auth_test_2030-04-30T01:13:31.893Z', '2030-04-30T01:13:31.893Z', 'auth_test', '{\"id\":\"index_auth_test_2030-04-30T01:13:31.893Z\",\"created_date\":\"2030-04-30T01:13:31.893Z\",\"datasource\":\"auth_test\",\"active\":0}', '{\"id\":\"index_auth_test_2030-04-30T01:13:31.893Z\",\"status\":\"SUCCESS\",\"duration\":1}', 0);


Hmm, I checked the test-data dir and it looks like the 3 files below (out of 5) are identical:
high-availability-sample-data.sql
query-retry-sample-data.sql
query-sample-data.sql

I'm not sure at this point how likely it is for these sample data sets to diverge in the future, but it seems like it might be reasonable to change the structure in a way such that the individual groups can specify directly what sample data they need preloaded (and those 3 files, and the security files, could be shared).

That said, I don't think it's a huge deal if we introduce another copy now.

jon-wei · 2021-02-23T00:24:48Z

integration-tests/docker/ldap-configs/bootstrap.ldif

+objectClass: inetOrgPerson
+loginShell: /bin/bash
+homeDirectory: /home/admin
+uidNumber: 14583100


Can you add some comments to the file on how the uid/gid values here were determined? Could they ever change?

Oh I think I just took it from some example I found. I think the can be whatever. To make this simpler let me just start from 1 instead.

jon-wei · 2021-02-23T01:29:12Z

...on-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthLdapConfigurationTest.java

+      {
+      };
+
+  private static final String SYSTEM_SCHEMA_SEGMENTS_RESULTS_RESOURCE =


These look identical to the ones in the abstract base class

jon-wei · 2021-02-23T01:32:41Z

...on-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthLdapConfigurationTest.java

+{
+  private static final Logger LOG = new Logger(ITBasicAuthLdapConfigurationTest.class);
+
+  private static final String LDAP_AUTHENTICATOR = "ldap";


I think you could avoid some test redundancy if you added methods like getAuthenticatorName, getAuthorizerName, getExpectedAvaticaAuthError and had the basic/ldap tests provide implementations for those that returned these strings

good point.

* refactor * address review comments

zachjsh added 3 commits February 18, 2021 14:31

Add integration tests for ldap extension

77050ed

* refactor

923d84b

* add ldap-security integration test to travis

a02d447

zachjsh commented Feb 19, 2021

View reviewed changes

clintropolis added Area - Testing Security labels Feb 19, 2021

zachjsh added 2 commits February 19, 2021 14:10

* fix license error

04b9c7c

* Fix failing other integration test

18b56b7

clintropolis approved these changes Feb 23, 2021

View reviewed changes

suneet-s reviewed Feb 23, 2021

View reviewed changes

jon-wei reviewed Feb 23, 2021

View reviewed changes

zachjsh added 3 commits February 23, 2021 03:23

* break up large tests

fb37128

* refactor * address review comments

* fix intellij inspections failure

3d5ea6d

* remove dead code

367e67a

jon-wei approved these changes Feb 23, 2021

View reviewed changes

jon-wei merged commit 553f5c8 into apache:master Feb 23, 2021

zachjsh deleted the ldap_integration_tests branch February 24, 2021 06:41

clintropolis added this to the 0.22.0 milestone Aug 12, 2021

Conversation

zachjsh commented Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zachjsh Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zachjsh Feb 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zachjsh commented Feb 19, 2021 •

edited

Loading

zachjsh Feb 19, 2021 •

edited

Loading

zachjsh Feb 23, 2021 •

edited

Loading