From 0a87fdd764bf45cdba7b271855fd46200a36a656 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 9 Oct 2023 10:03:24 -0400 Subject: [PATCH 1/5] Add User.isSuperUser to User class Signed-off-by: Craig Perkins --- .../opensearch/commons/ConfigConstants.java | 2 ++ .../org/opensearch/commons/authuser/User.java | 8 +++++ .../opensearch/commons/authuser/UserTest.java | 36 +++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/src/main/java/org/opensearch/commons/ConfigConstants.java b/src/main/java/org/opensearch/commons/ConfigConstants.java index 2fcf9dae..9ec38a63 100644 --- a/src/main/java/org/opensearch/commons/ConfigConstants.java +++ b/src/main/java/org/opensearch/commons/ConfigConstants.java @@ -54,5 +54,7 @@ private static Setting createFallbackInsecureSetting(String key) { public static final String INJECTED_USER = "injected_user"; public static final String OPENSEARCH_SECURITY_USE_INJECTED_USER_FOR_PLUGINS = "plugins.security_use_injected_user_for_plugins"; public static final String OPENSEARCH_SECURITY_SSL_HTTP_ENABLED = "plugins.security.ssl.http.enabled"; + public static final String OPENSEARCH_SECURITY_AUTHCZ_ADMIN_DN = "plugins.security.authcz.admin_dn"; public static final String OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT = "_opendistro_security_user_info"; + } diff --git a/src/main/java/org/opensearch/commons/authuser/User.java b/src/main/java/org/opensearch/commons/authuser/User.java index 79242559..c254a3ce 100644 --- a/src/main/java/org/opensearch/commons/authuser/User.java +++ b/src/main/java/org/opensearch/commons/authuser/User.java @@ -10,6 +10,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -19,8 +20,10 @@ import org.opensearch.client.Response; import org.opensearch.common.Nullable; import org.opensearch.common.inject.internal.ToStringBuilder; +import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.commons.ConfigConstants; import org.opensearch.core.common.Strings; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -250,4 +253,9 @@ public List getCustomAttNames() { public String getRequestedTenant() { return requestedTenant; } + + public static boolean isSuperUser(User user, Settings settings) { + List adminDns = settings.getAsList(ConfigConstants.OPENSEARCH_SECURITY_AUTHCZ_ADMIN_DN, Collections.emptyList()); + return adminDns.contains(user.getName()); + } } diff --git a/src/test/java/org/opensearch/commons/authuser/UserTest.java b/src/test/java/org/opensearch/commons/authuser/UserTest.java index 56152477..03449207 100644 --- a/src/test/java/org/opensearch/commons/authuser/UserTest.java +++ b/src/test/java/org/opensearch/commons/authuser/UserTest.java @@ -13,11 +13,13 @@ import java.io.IOException; import java.util.Arrays; +import java.util.List; import org.junit.jupiter.api.Test; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.commons.ConfigConstants; import org.opensearch.core.common.Strings; import org.opensearch.core.common.io.stream.StreamInput; @@ -202,4 +204,38 @@ public void testParseUserStringMalformed() { User user = User.parse(str); assertNull(user); } + + @Test + public void testUserIsSuperUserTrue() { + Settings settings = Settings + .builder() + .putList(ConfigConstants.OPENSEARCH_SECURITY_AUTHCZ_ADMIN_DN, List.of("CN=kirk,OU=client,O=client,L=test, C=de")) + .build(); + ThreadContext tc = new ThreadContext(Settings.EMPTY); + tc + .putTransient( + OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, + "CN=kirk,OU=client,O=client,L=test, C=de|backendrole1,backendrole2|role1,role2" + ); + String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); + User user = User.parse(str); + assertTrue(User.isSuperUser(user, settings)); + } + + @Test + public void testUserIsSuperUserFalse() { + Settings settings = Settings + .builder() + .putList(ConfigConstants.OPENSEARCH_SECURITY_AUTHCZ_ADMIN_DN, List.of("CN=spock,OU=client,O=client,L=test, C=de")) + .build(); + ThreadContext tc = new ThreadContext(Settings.EMPTY); + tc + .putTransient( + OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, + "CN=kirk,OU=client,O=client,L=test, C=de|backendrole1,backendrole2|role1,role2" + ); + String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); + User user = User.parse(str); + assertFalse(User.isSuperUser(user, settings)); + } } From e5aaee80a68ff5d7aa86450fc0210d5ce2657142 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 9 Oct 2023 23:21:41 -0400 Subject: [PATCH 2/5] Add null check Signed-off-by: Craig Perkins --- src/main/java/org/opensearch/commons/authuser/User.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/opensearch/commons/authuser/User.java b/src/main/java/org/opensearch/commons/authuser/User.java index c254a3ce..4e61b817 100644 --- a/src/main/java/org/opensearch/commons/authuser/User.java +++ b/src/main/java/org/opensearch/commons/authuser/User.java @@ -255,6 +255,9 @@ public String getRequestedTenant() { } public static boolean isSuperUser(User user, Settings settings) { + if (user == null || settings == null) { + return false; + } List adminDns = settings.getAsList(ConfigConstants.OPENSEARCH_SECURITY_AUTHCZ_ADMIN_DN, Collections.emptyList()); return adminDns.contains(user.getName()); } From afdf043191831fd08fc8ac35c821550b28d7b1b8 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 10 Oct 2023 07:01:13 -0400 Subject: [PATCH 3/5] Add another test Signed-off-by: Craig Perkins --- .../java/org/opensearch/commons/authuser/UserTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/java/org/opensearch/commons/authuser/UserTest.java b/src/test/java/org/opensearch/commons/authuser/UserTest.java index 03449207..e82c8967 100644 --- a/src/test/java/org/opensearch/commons/authuser/UserTest.java +++ b/src/test/java/org/opensearch/commons/authuser/UserTest.java @@ -238,4 +238,12 @@ public void testUserIsSuperUserFalse() { User user = User.parse(str); assertFalse(User.isSuperUser(user, settings)); } + + @Test + public void testUserOrSettingsAreNull() { + Settings settings = Settings.EMPTY; + User user = User.parse("username|backend_role1|role1"); + assertFalse(User.isSuperUser(null, settings)); + assertFalse(User.isSuperUser(user, null)); + } } From 527e2d263e8148265be19c3210bedef4d1919345 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 12 Oct 2023 11:52:34 -0400 Subject: [PATCH 4/5] Make method non-static and require a user to exist to call method Signed-off-by: Craig Perkins --- .../java/org/opensearch/commons/authuser/User.java | 6 +++--- .../java/org/opensearch/commons/authuser/UserTest.java | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/opensearch/commons/authuser/User.java b/src/main/java/org/opensearch/commons/authuser/User.java index 4e61b817..233e3cc9 100644 --- a/src/main/java/org/opensearch/commons/authuser/User.java +++ b/src/main/java/org/opensearch/commons/authuser/User.java @@ -254,11 +254,11 @@ public String getRequestedTenant() { return requestedTenant; } - public static boolean isSuperUser(User user, Settings settings) { - if (user == null || settings == null) { + public boolean isSuperUser(Settings settings) { + if (settings == null) { return false; } List adminDns = settings.getAsList(ConfigConstants.OPENSEARCH_SECURITY_AUTHCZ_ADMIN_DN, Collections.emptyList()); - return adminDns.contains(user.getName()); + return adminDns.contains(this.name); } } diff --git a/src/test/java/org/opensearch/commons/authuser/UserTest.java b/src/test/java/org/opensearch/commons/authuser/UserTest.java index e82c8967..9fe1cb15 100644 --- a/src/test/java/org/opensearch/commons/authuser/UserTest.java +++ b/src/test/java/org/opensearch/commons/authuser/UserTest.java @@ -219,7 +219,7 @@ public void testUserIsSuperUserTrue() { ); String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); User user = User.parse(str); - assertTrue(User.isSuperUser(user, settings)); + assertTrue(user.isSuperUser(settings)); } @Test @@ -236,14 +236,14 @@ public void testUserIsSuperUserFalse() { ); String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); User user = User.parse(str); - assertFalse(User.isSuperUser(user, settings)); + assertFalse(user.isSuperUser(settings)); } @Test - public void testUserOrSettingsAreNull() { + public void testUserOrSettingsAreNullOrEmpty() { Settings settings = Settings.EMPTY; User user = User.parse("username|backend_role1|role1"); - assertFalse(User.isSuperUser(null, settings)); - assertFalse(User.isSuperUser(user, null)); + assertFalse(user.isSuperUser(null)); + assertFalse(user.isSuperUser(settings)); } } From dc0047c36b654341f5ac1268a57069345fedbf33 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 9 Nov 2023 11:19:35 -0500 Subject: [PATCH 5/5] Change to isAdminDn Signed-off-by: Craig Perkins --- .../java/org/opensearch/commons/authuser/User.java | 2 +- .../org/opensearch/commons/authuser/UserTest.java | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/opensearch/commons/authuser/User.java b/src/main/java/org/opensearch/commons/authuser/User.java index 233e3cc9..3e507857 100644 --- a/src/main/java/org/opensearch/commons/authuser/User.java +++ b/src/main/java/org/opensearch/commons/authuser/User.java @@ -254,7 +254,7 @@ public String getRequestedTenant() { return requestedTenant; } - public boolean isSuperUser(Settings settings) { + public boolean isAdminDn(Settings settings) { if (settings == null) { return false; } diff --git a/src/test/java/org/opensearch/commons/authuser/UserTest.java b/src/test/java/org/opensearch/commons/authuser/UserTest.java index 9fe1cb15..df4e6602 100644 --- a/src/test/java/org/opensearch/commons/authuser/UserTest.java +++ b/src/test/java/org/opensearch/commons/authuser/UserTest.java @@ -206,7 +206,7 @@ public void testParseUserStringMalformed() { } @Test - public void testUserIsSuperUserTrue() { + public void testUserIsAdminDnTrue() { Settings settings = Settings .builder() .putList(ConfigConstants.OPENSEARCH_SECURITY_AUTHCZ_ADMIN_DN, List.of("CN=kirk,OU=client,O=client,L=test, C=de")) @@ -219,11 +219,11 @@ public void testUserIsSuperUserTrue() { ); String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); User user = User.parse(str); - assertTrue(user.isSuperUser(settings)); + assertTrue(user.isAdminDn(settings)); } @Test - public void testUserIsSuperUserFalse() { + public void testUserIsAdminDnFalse() { Settings settings = Settings .builder() .putList(ConfigConstants.OPENSEARCH_SECURITY_AUTHCZ_ADMIN_DN, List.of("CN=spock,OU=client,O=client,L=test, C=de")) @@ -236,14 +236,14 @@ public void testUserIsSuperUserFalse() { ); String str = tc.getTransient(OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); User user = User.parse(str); - assertFalse(user.isSuperUser(settings)); + assertFalse(user.isAdminDn(settings)); } @Test public void testUserOrSettingsAreNullOrEmpty() { Settings settings = Settings.EMPTY; User user = User.parse("username|backend_role1|role1"); - assertFalse(user.isSuperUser(null)); - assertFalse(user.isSuperUser(settings)); + assertFalse(user.isAdminDn(null)); + assertFalse(user.isAdminDn(settings)); } }