From f4572d5b104984365de8fcc7235a77ba9eea8707 Mon Sep 17 00:00:00 2001 From: Sridhar <srnidhi@88665a377311.ant.amazon.com> Date: Mon, 13 Sep 2021 17:30:24 -0700 Subject: [PATCH 1/5] add non-null to store non-default values when patching security config Signed-off-by: Nidhi Sridhar <srnidhi@amazon.com> --- .../securityconf/impl/v6/ConfigV6.java | 34 +++- .../securityconf/impl/v7/ConfigV7.java | 34 +++- .../dlic/rest/api/SecurityConfigApiTest.java | 34 ++++ .../restapi/securityconfig_nondefault.json | 173 ++++++++++++++++++ 4 files changed, 255 insertions(+), 20 deletions(-) create mode 100644 src/test/resources/restapi/securityconfig_nondefault.json diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java b/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java index 32649ef1b9..54b49baa28 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java @@ -40,6 +40,7 @@ import com.fasterxml.jackson.annotation.JsonAnyGetter; import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.core.JsonProcessingException; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.auth.internal.InternalAuthenticationBackend; @@ -55,6 +56,7 @@ public String toString() { return "Config [dynamic=" + dynamic + "]"; } + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Dynamic { @@ -81,6 +83,7 @@ public String toString() { } } + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Kibana { public boolean multitenancy_enabled = true; @@ -97,7 +100,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Http { public boolean anonymous_auth_enabled = false; public Xff xff = new Xff(); @@ -108,7 +112,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthFailureListeners { @JsonIgnore private final Map<String, AuthFailureListener> listeners = new HashMap<>(); @@ -125,7 +130,8 @@ public Map<String, AuthFailureListener> getListeners() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthFailureListener { public String type; public String authentication_backend; @@ -148,7 +154,8 @@ public String asJson() { } } } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Xff { public boolean enabled = true; public String internalProxies = Pattern.compile( @@ -170,7 +177,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Authc { @JsonIgnore @@ -193,7 +201,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthcDomain { public boolean http_enabled= true; public boolean transport_enabled= true; @@ -210,6 +219,7 @@ public String toString() { } + @JsonInclude(JsonInclude.Include.NON_NULL) public static class HttpAuthenticator { public boolean challenge = true; public String type; @@ -231,7 +241,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthzBackend { public String type = "noop"; public Map<String, Object> config = Collections.emptyMap(); @@ -252,7 +263,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthcBackend { public String type = InternalAuthenticationBackend.class.getName(); public Map<String, Object> config = Collections.emptyMap(); @@ -273,7 +285,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Authz { @JsonIgnore private final Map<String, AuthzDomain> domains = new HashMap<>(); @@ -295,7 +308,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthzDomain { public boolean http_enabled = true; public boolean transport_enabled = true; diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index 83724bd7d5..2d91ff2cd1 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -40,6 +40,7 @@ import com.fasterxml.jackson.annotation.JsonAnyGetter; import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.core.JsonProcessingException; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.auth.internal.InternalAuthenticationBackend; @@ -106,6 +107,7 @@ public String toString() { return "Config [dynamic=" + dynamic + "]"; } + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Dynamic { @@ -132,6 +134,7 @@ public String toString() { } } + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Kibana { public boolean multitenancy_enabled = true; @@ -147,7 +150,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Http { public boolean anonymous_auth_enabled = false; public Xff xff = new Xff(); @@ -158,7 +162,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthFailureListeners { @JsonIgnore private final Map<String, AuthFailureListener> listeners = new HashMap<>(); @@ -175,7 +180,8 @@ public Map<String, AuthFailureListener> getListeners() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthFailureListener { public String type; public String authentication_backend; @@ -211,7 +217,8 @@ public String asJson() { } } } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Xff { public boolean enabled = false; public String internalProxies = Pattern.compile( @@ -230,7 +237,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Authc { @JsonIgnore @@ -254,7 +262,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthcDomain { public boolean http_enabled= true; @@ -293,6 +302,7 @@ public String toString() { } + @JsonInclude(JsonInclude.Include.NON_NULL) public static class HttpAuthenticator { public boolean challenge = true; public String type; @@ -327,7 +337,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthzBackend { public String type = "noop"; public Map<String, Object> config = Collections.emptyMap(); @@ -365,7 +376,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthcBackend { public String type = InternalAuthenticationBackend.class.getName(); public Map<String, Object> config = Collections.emptyMap(); @@ -403,7 +415,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Authz { @JsonIgnore private final Map<String, AuthzDomain> domains = new HashMap<>(); @@ -425,7 +438,8 @@ public String toString() { } - + + @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthzDomain { public boolean http_enabled = true; public boolean transport_enabled = true; diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java index 93fa02df06..27d78ddd47 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java @@ -23,6 +23,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.test.helper.file.FileHelper; import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; @@ -108,4 +109,37 @@ public void testSecurityConfigApiWrite() throws Exception { Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); } + + @Test + public void testSecurityConfigPatch() throws Exception { + + Settings settings = Settings.builder().put(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build(); + setup(settings); + + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = true; + + //non-default config + String updatedConfig = FileHelper.loadFile("restapi/securityconfig_nondefault.json"); + + //update config + HttpResponse response = rh.executePutRequest(ENDPOINT + "/securityconfig/config", updatedConfig, new Header[0]); + Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); + + //make patch request + response = rh.executePatchRequest(ENDPOINT + "/securityconfig", "[{\"op\": \"add\",\"path\": \"/config/dynamic/do_not_fail_on_forbidden\",\"value\": \"false\"}]", new Header[0]); + Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); + + //get config + response = rh.executeGetRequest(ENDPOINT + "/securityconfig", new Header[0]); + Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); + System.out.print("nidhitest" + response.getBody()); + + // verify configs are same + Assert.assertEquals(DefaultObjectMapper.readTree(updatedConfig), DefaultObjectMapper.readTree(response.getBody()).get("config")); + + + } } + + diff --git a/src/test/resources/restapi/securityconfig_nondefault.json b/src/test/resources/restapi/securityconfig_nondefault.json new file mode 100644 index 0000000000..3870330044 --- /dev/null +++ b/src/test/resources/restapi/securityconfig_nondefault.json @@ -0,0 +1,173 @@ +{ + "dynamic" : { + "filtered_alias_mode" : "warn", + "disable_rest_auth" : false, + "disable_intertransport_auth" : false, + "respect_request_indices_options" : false, + "kibana" : { + "multitenancy_enabled" : true, + "server_username" : "kibanaserver", + "index" : ".kibana" + }, + "http" : { + "anonymous_auth_enabled" : false, + "xff" : { + "enabled" : false, + "internalProxies" : "192\\.168\\.0\\.10|192\\.168\\.0\\.11", + "remoteIpHeader" : "X-Forwarded-For" + } + }, + "authc" : { + "jwt_auth_domain" : { + "http_enabled" : true, + "transport_enabled" : true, + "order" : 0, + "http_authenticator" : { + "challenge" : false, + "type" : "jwt", + "config" : { + "signing_key" : "base64 encoded HMAC key or public RSA/ECDSA pem key", + "jwt_header" : "Authorization" + } + }, + "authentication_backend" : { + "type" : "noop", + "config" : { } + }, + "description" : "Authenticate via Json Web Token" + }, + "ldap" : { + "http_enabled" : false, + "transport_enabled" : false, + "order" : 5, + "http_authenticator" : { + "challenge" : false, + "type" : "basic", + "config" : { } + }, + "authentication_backend" : { + "type" : "ldap", + "config" : { + "enable_ssl" : false, + "enable_start_tls" : false, + "enable_ssl_client_auth" : false, + "verify_hostnames" : true, + "hosts" : [ + "localhost:8389" + ], + "userbase" : "ou=people,dc=example,dc=com", + "usersearch" : "(sAMAccountName={0})" + } + }, + "description" : "Authenticate via LDAP or Active Directory" + }, + "basic_internal_auth_domain" : { + "http_enabled" : true, + "transport_enabled" : true, + "order" : 4, + "http_authenticator" : { + "challenge" : true, + "type" : "basic", + "config" : { } + }, + "authentication_backend" : { + "type" : "intern", + "config" : { } + }, + "description" : "Authenticate via HTTP Basic against internal users database" + }, + "proxy_auth_domain" : { + "http_enabled" : false, + "transport_enabled" : false, + "order" : 3, + "http_authenticator" : { + "challenge" : false, + "type" : "proxy", + "config" : { + "user_header" : "x-proxy-user", + "roles_header" : "x-proxy-roles" + } + }, + "authentication_backend" : { + "type" : "noop", + "config" : { } + }, + "description" : "Authenticate via proxy" + }, + "clientcert_auth_domain" : { + "http_enabled" : false, + "transport_enabled" : false, + "order" : 2, + "http_authenticator" : { + "challenge" : false, + "type" : "clientcert", + "config" : { + "username_attribute" : "cn" + } + }, + "authentication_backend" : { + "type" : "noop", + "config" : { } + }, + "description" : "Authenticate via SSL client certificates" + }, + "kerberos_auth_domain" : { + "http_enabled" : false, + "transport_enabled" : false, + "order" : 6, + "http_authenticator" : { + "challenge" : true, + "type" : "kerberos", + "config" : { + "krb_debug" : false, + "strip_realm_from_principal" : true + } + }, + "authentication_backend" : { + "type" : "noop", + "config" : { } + } + } + }, + "authz" : { + "roles_from_another_ldap" : { + "http_enabled" : false, + "transport_enabled" : false, + "authorization_backend" : { + "type" : "ldap", + "config" : { } + }, + "description" : "Authorize via another Active Directory" + }, + "roles_from_myldap" : { + "http_enabled" : false, + "transport_enabled" : false, + "authorization_backend" : { + "type" : "ldap", + "config" : { + "enable_ssl" : false, + "enable_start_tls" : false, + "enable_ssl_client_auth" : false, + "verify_hostnames" : true, + "hosts" : [ + "localhost:8389" + ], + "rolebase" : "ou=groups,dc=example,dc=com", + "rolesearch" : "(member={0})", + "userrolename" : "disabled", + "rolename" : "cn", + "resolve_nested_roles" : true, + "userbase" : "ou=people,dc=example,dc=com", + "usersearch" : "(uid={0})" + } + }, + "description" : "Authorize via LDAP or Active Directory" + } + }, + "auth_failure_listeners" : { }, + "do_not_fail_on_forbidden" : false, + "multi_rolespan_enabled" : true, + "hosts_resolver_mode" : "ip-only", + "do_not_fail_on_forbidden_empty" : false + } +} \ No newline at end of file From b6987b8ab55e6ce20623c67b55f7456983e5af1e Mon Sep 17 00:00:00 2001 From: Sridhar <srnidhi@88665a377311.ant.amazon.com> Date: Mon, 13 Sep 2021 21:52:04 -0700 Subject: [PATCH 2/5] fixed issue due to which configv6 and configv7 tests failed Signed-off-by: Nidhi Sridhar <srnidhi@amazon.com> --- .../org/opensearch/security/securityconf/impl/v6/ConfigV6.java | 1 - .../org/opensearch/security/securityconf/impl/v7/ConfigV7.java | 1 - 2 files changed, 2 deletions(-) diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java b/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java index 54b49baa28..b04c1fb49f 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java @@ -83,7 +83,6 @@ public String toString() { } } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Kibana { public boolean multitenancy_enabled = true; diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index 2d91ff2cd1..0f8f0145e6 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -134,7 +134,6 @@ public String toString() { } } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Kibana { public boolean multitenancy_enabled = true; From 1e3a70bf337e7ca2ad3bd744e23d70596ff6db21 Mon Sep 17 00:00:00 2001 From: Sridhar <srnidhi@88665a377311.ant.amazon.com> Date: Fri, 24 Sep 2021 12:57:22 -0700 Subject: [PATCH 3/5] added newline at EOF securityconfig_nondefault.json Signed-off-by: Nidhi Sridhar <srnidhi@amazon.com> --- src/test/resources/restapi/securityconfig_nondefault.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/resources/restapi/securityconfig_nondefault.json b/src/test/resources/restapi/securityconfig_nondefault.json index 3870330044..c9e6aaec5e 100644 --- a/src/test/resources/restapi/securityconfig_nondefault.json +++ b/src/test/resources/restapi/securityconfig_nondefault.json @@ -170,4 +170,4 @@ "hosts_resolver_mode" : "ip-only", "do_not_fail_on_forbidden_empty" : false } -} \ No newline at end of file +} From 0536a2f42466210dbc86c6c8b8f1cb0df7d64b30 Mon Sep 17 00:00:00 2001 From: Nidhi Sridhar <srnidhi@amazon.com> Date: Wed, 29 Sep 2021 12:26:31 -0700 Subject: [PATCH 4/5] adding non-null annotation only to properties in ConfigV6 and ConfigV7 impacted by patch Signed-off-by: Nidhi Sridhar <srnidhi@amazon.com> --- .../securityconf/impl/v6/ConfigV6.java | 21 ++++++++----------- .../securityconf/impl/v7/ConfigV7.java | 19 +++++++---------- .../securityconf/impl/v6/ConfigV6Test.java | 14 ++----------- .../securityconf/impl/v7/ConfigV7Test.java | 14 ++----------- 4 files changed, 20 insertions(+), 48 deletions(-) diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java b/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java index b04c1fb49f..45704ea3e4 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v6/ConfigV6.java @@ -56,7 +56,6 @@ public String toString() { return "Config [dynamic=" + dynamic + "]"; } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Dynamic { @@ -85,6 +84,7 @@ public String toString() { public static class Kibana { + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean multitenancy_enabled = true; public String server_username = "kibanaserver"; public String opendistro_role = null; @@ -100,7 +100,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Http { public boolean anonymous_auth_enabled = false; public Xff xff = new Xff(); @@ -112,7 +111,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthFailureListeners { @JsonIgnore private final Map<String, AuthFailureListener> listeners = new HashMap<>(); @@ -130,7 +128,6 @@ public Map<String, AuthFailureListener> getListeners() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthFailureListener { public String type; public String authentication_backend; @@ -154,8 +151,8 @@ public String asJson() { } } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Xff { + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean enabled = true; public String internalProxies = Pattern.compile( "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}|" + @@ -177,7 +174,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Authc { @JsonIgnore @@ -201,10 +197,12 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthcDomain { + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean http_enabled= true; + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean transport_enabled= true; + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean enabled= true; public int order = 0; public HttpAuthenticator http_authenticator = new HttpAuthenticator(); @@ -218,8 +216,8 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class HttpAuthenticator { + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean challenge = true; public String type; public Map<String, Object> config = Collections.emptyMap(); @@ -241,7 +239,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthzBackend { public String type = "noop"; public Map<String, Object> config = Collections.emptyMap(); @@ -263,7 +260,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthcBackend { public String type = InternalAuthenticationBackend.class.getName(); public Map<String, Object> config = Collections.emptyMap(); @@ -285,7 +281,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Authz { @JsonIgnore private final Map<String, AuthzDomain> domains = new HashMap<>(); @@ -308,10 +303,12 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthzDomain { + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean http_enabled = true; + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean transport_enabled = true; + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean enabled = true; public AuthzBackend authorization_backend = new AuthzBackend(); @Override diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index 0f8f0145e6..64db7118ea 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -107,7 +107,6 @@ public String toString() { return "Config [dynamic=" + dynamic + "]"; } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Dynamic { @@ -122,6 +121,7 @@ public static class Dynamic { public Authz authz = new Authz(); public AuthFailureListeners auth_failure_listeners = new AuthFailureListeners(); public boolean do_not_fail_on_forbidden; + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean multi_rolespan_enabled = true; public String hosts_resolver_mode = "ip-only"; public String transport_userrname_attribute; @@ -136,6 +136,7 @@ public String toString() { public static class Kibana { + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean multitenancy_enabled = true; public String server_username = "kibanaserver"; public String opendistro_role = null; @@ -150,7 +151,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Http { public boolean anonymous_auth_enabled = false; public Xff xff = new Xff(); @@ -162,7 +162,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthFailureListeners { @JsonIgnore private final Map<String, AuthFailureListener> listeners = new HashMap<>(); @@ -180,7 +179,6 @@ public Map<String, AuthFailureListener> getListeners() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthFailureListener { public String type; public String authentication_backend; @@ -217,7 +215,6 @@ public String asJson() { } } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Xff { public boolean enabled = false; public String internalProxies = Pattern.compile( @@ -237,7 +234,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Authc { @JsonIgnore @@ -262,10 +258,11 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthcDomain { + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean http_enabled= true; + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean transport_enabled= true; //public boolean enabled= true; public int order = 0; @@ -301,8 +298,8 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class HttpAuthenticator { + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean challenge = true; public String type; public Map<String, Object> config = Collections.emptyMap(); @@ -337,7 +334,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthzBackend { public String type = "noop"; public Map<String, Object> config = Collections.emptyMap(); @@ -376,7 +372,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthcBackend { public String type = InternalAuthenticationBackend.class.getName(); public Map<String, Object> config = Collections.emptyMap(); @@ -415,7 +410,6 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class Authz { @JsonIgnore private final Map<String, AuthzDomain> domains = new HashMap<>(); @@ -438,9 +432,10 @@ public String toString() { } - @JsonInclude(JsonInclude.Include.NON_NULL) public static class AuthzDomain { + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean http_enabled = true; + @JsonInclude(JsonInclude.Include.NON_NULL) public boolean transport_enabled = true; public AuthzBackend authorization_backend = new AuthzBackend(); public String description; diff --git a/src/test/java/org/opensearch/security/securityconf/impl/v6/ConfigV6Test.java b/src/test/java/org/opensearch/security/securityconf/impl/v6/ConfigV6Test.java index 8ea28b957b..7d554daf8d 100644 --- a/src/test/java/org/opensearch/security/securityconf/impl/v6/ConfigV6Test.java +++ b/src/test/java/org/opensearch/security/securityconf/impl/v6/ConfigV6Test.java @@ -20,12 +20,7 @@ public static Iterable<Boolean> omitDefaults() { } public void assertEquals(ConfigV6.Kibana expected, JsonNode node) { - if (omitDefaults && !expected.multitenancy_enabled) { - // false (default) is not persisted - Assert.assertNull(node.get("multitenancy_enabled")); - } else { - Assert.assertEquals(expected.multitenancy_enabled, node.get("multitenancy_enabled").asBoolean()); - } + Assert.assertEquals(expected.multitenancy_enabled, node.get("multitenancy_enabled").asBoolean()); if (expected.server_username == null) { Assert.assertNull(node.get("server_username")); } else { @@ -51,12 +46,7 @@ public void assertEquals(ConfigV6.Kibana expected, JsonNode node) { } private void assertEquals(ConfigV6.Kibana expected, ConfigV6.Kibana actual) { - if (omitDefaults && !expected.multitenancy_enabled) { - // BUG: false is omitted and is restored to default (which is true) instead of false - Assert.assertTrue(actual.multitenancy_enabled); - } else { - Assert.assertEquals(expected.multitenancy_enabled, actual.multitenancy_enabled); - } + Assert.assertEquals(expected.multitenancy_enabled, actual.multitenancy_enabled); if (expected.server_username == null) { // null is restored to default instead of null Assert.assertEquals(new ConfigV6.Kibana().server_username, actual.server_username); diff --git a/src/test/java/org/opensearch/security/securityconf/impl/v7/ConfigV7Test.java b/src/test/java/org/opensearch/security/securityconf/impl/v7/ConfigV7Test.java index 43dc4e8e65..c6fc8d89e6 100644 --- a/src/test/java/org/opensearch/security/securityconf/impl/v7/ConfigV7Test.java +++ b/src/test/java/org/opensearch/security/securityconf/impl/v7/ConfigV7Test.java @@ -20,12 +20,7 @@ public static Iterable<Boolean> omitDefaults() { } public void assertEquals(ConfigV7.Kibana expected, JsonNode node) { - if (omitDefaults && !expected.multitenancy_enabled) { - // false (default) is not persisted - Assert.assertNull(node.get("multitenancy_enabled")); - } else { - Assert.assertEquals(expected.multitenancy_enabled, node.get("multitenancy_enabled").asBoolean()); - } + Assert.assertEquals(expected.multitenancy_enabled, node.get("multitenancy_enabled").asBoolean()); if (expected.server_username == null) { Assert.assertNull(node.get("server_username")); } else { @@ -45,12 +40,7 @@ public void assertEquals(ConfigV7.Kibana expected, JsonNode node) { } private void assertEquals(ConfigV7.Kibana expected, ConfigV7.Kibana actual) { - if (omitDefaults && !expected.multitenancy_enabled) { - // BUG: false is omitted and is restored to default (which is true) instead of false - Assert.assertTrue(actual.multitenancy_enabled); - } else { - Assert.assertEquals(expected.multitenancy_enabled, actual.multitenancy_enabled); - } + Assert.assertEquals(expected.multitenancy_enabled, actual.multitenancy_enabled); if (expected.server_username == null) { // null is restored to default instead of null Assert.assertEquals(new ConfigV7.Kibana().server_username, actual.server_username); From 53c066e46826438f330ea55986f31e4d2c278b37 Mon Sep 17 00:00:00 2001 From: Nidhi Sridhar <srnidhi@amazon.com> Date: Tue, 9 Nov 2021 15:09:55 -0800 Subject: [PATCH 5/5] Rename unit test to testSecurityConfigForHTTPPatch() as per review comment Signed-off-by: Nidhi Sridhar <srnidhi@amazon.com> --- .../security/dlic/rest/api/SecurityConfigApiTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java index 27d78ddd47..455f0bba6b 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java @@ -111,7 +111,7 @@ public void testSecurityConfigApiWrite() throws Exception { } @Test - public void testSecurityConfigPatch() throws Exception { + public void testSecurityConfigForHTTPPatch() throws Exception { Settings settings = Settings.builder().put(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build(); setup(settings); @@ -133,7 +133,6 @@ public void testSecurityConfigPatch() throws Exception { //get config response = rh.executeGetRequest(ENDPOINT + "/securityconfig", new Header[0]); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - System.out.print("nidhitest" + response.getBody()); // verify configs are same Assert.assertEquals(DefaultObjectMapper.readTree(updatedConfig), DefaultObjectMapper.readTree(response.getBody()).get("config"));