-
Notifications
You must be signed in to change notification settings - Fork 24.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Avoid NPE in set_security_user without security (#52691)
If security was disabled (explicitly), then the SecurityContext would be null, but the set_security_user processor was still registered. Attempting to define a pipeline that used that processor would fail with an (intentional) NPE. This behaviour, introduced in #52032, is a regression from previous releases where the pipeline was allowed, but was no usable. This change restores the previous behaviour (with a new warning).
- Loading branch information
Showing
9 changed files
with
260 additions
and
43 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* | ||
* This QA project tests the security plugin when security is explicitlt disabled. | ||
* It is intended to cover security functionality which is supposed to | ||
* function in a specific way even if security is disabled on the cluster | ||
* For example: If a cluster has a pipeline with the set_security_user processor | ||
* defined, it should be not fail | ||
*/ | ||
|
||
apply plugin: 'elasticsearch.testclusters' | ||
apply plugin: 'elasticsearch.standalone-rest-test' | ||
apply plugin: 'elasticsearch.rest-test' | ||
|
||
dependencies { | ||
testCompile project(path: xpackModule('core'), configuration: 'default') | ||
testCompile project(path: xpackModule('security'), configuration: 'testArtifacts') | ||
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts') | ||
} | ||
|
||
testClusters.integTest { | ||
testDistribution = 'DEFAULT' | ||
numberOfNodes = 2 | ||
|
||
setting 'xpack.ilm.enabled', 'false' | ||
setting 'xpack.ml.enabled', 'false' | ||
// We run with a trial license, but explicitly disable security. | ||
// This means the security plugin is loaded and all feature are permitted, but they are not enabled | ||
setting 'xpack.license.self_generated.type', 'trial' | ||
setting 'xpack.security.enabled', 'false' | ||
} |
48 changes: 48 additions & 0 deletions
48
...java/org/elasticsearch/xpack/security/SetSecurityUserProcessorWithSecurityDisabledIT.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
package org.elasticsearch.xpack.security; | ||
|
||
import org.apache.http.util.EntityUtils; | ||
import org.elasticsearch.client.Request; | ||
import org.elasticsearch.client.Response; | ||
import org.elasticsearch.client.ResponseException; | ||
import org.elasticsearch.test.rest.ESRestTestCase; | ||
|
||
import static org.hamcrest.Matchers.containsString; | ||
|
||
/** | ||
* Tests that it is possible to <em>define</em> a pipeline with the | ||
* {@link org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor} on a cluster with security disabled, but it is not possible | ||
* to use that pipeline for ingestion. | ||
*/ | ||
public class SetSecurityUserProcessorWithSecurityDisabledIT extends ESRestTestCase { | ||
|
||
public void testDefineAndUseProcessor() throws Exception { | ||
final String pipeline = "pipeline-" + getTestName(); | ||
final String index = "index-" + getTestName(); | ||
{ | ||
final Request putPipeline = new Request("PUT", "/_ingest/pipeline/" + pipeline); | ||
putPipeline.setJsonEntity("{" + | ||
" \"description\": \"Test pipeline (" + getTestName() + ")\"," + | ||
" \"processors\":[{" + | ||
" \"set_security_user\":{ \"field\": \"user\" }" + | ||
" }]" + | ||
"}"); | ||
final Response response = client().performRequest(putPipeline); | ||
assertOK(response); | ||
} | ||
|
||
{ | ||
final Request ingest = new Request("PUT", "/" + index + "/_doc/1?pipeline=" + pipeline); | ||
ingest.setJsonEntity("{\"field\":\"value\"}"); | ||
final ResponseException ex = expectThrows(ResponseException.class, () -> client().performRequest(ingest)); | ||
final Response response = ex.getResponse(); | ||
assertThat(EntityUtils.toString(response.getEntity()), | ||
containsString("Security (authentication) is not enabled on this cluster")); | ||
} | ||
} | ||
|
||
} |
28 changes: 28 additions & 0 deletions
28
x-pack/plugin/security/qa/security-not-enabled/build.gradle
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/* | ||
* This QA project tests the security plugin when security is not enabled. | ||
* It is intended to cover security functionality which is supposed to | ||
* function in a specific way even if security is not enabled on the cluster | ||
* For example: If a cluster has a pipeline with the set_security_user processor | ||
* defined, it should be not fail | ||
*/ | ||
|
||
apply plugin: 'elasticsearch.testclusters' | ||
apply plugin: 'elasticsearch.standalone-rest-test' | ||
apply plugin: 'elasticsearch.rest-test' | ||
|
||
dependencies { | ||
testCompile project(path: xpackModule('core'), configuration: 'default') | ||
testCompile project(path: xpackModule('security'), configuration: 'testArtifacts') | ||
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts') | ||
} | ||
|
||
testClusters.integTest { | ||
testDistribution = 'DEFAULT' | ||
numberOfNodes = 2 | ||
|
||
setting 'xpack.ilm.enabled', 'false' | ||
setting 'xpack.ml.enabled', 'false' | ||
// We run with a trial license, but do not enable security. | ||
// This means the security plugin is loaded and all feature are permitted, but they are not enabled | ||
setting 'xpack.license.self_generated.type', 'trial' | ||
} |
48 changes: 48 additions & 0 deletions
48
...va/org/elasticsearch/xpack/security/SetSecurityUserProcessorWithSecurityNotEnabledIT.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
package org.elasticsearch.xpack.security; | ||
|
||
import org.apache.http.util.EntityUtils; | ||
import org.elasticsearch.client.Request; | ||
import org.elasticsearch.client.Response; | ||
import org.elasticsearch.client.ResponseException; | ||
import org.elasticsearch.test.rest.ESRestTestCase; | ||
|
||
import static org.hamcrest.Matchers.containsString; | ||
|
||
/** | ||
* Tests that it is possible to <em>define</em> a pipeline with the | ||
* {@link org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor} on a cluster where security is not enabled, | ||
* but it is not possible to use that pipeline for ingestion. | ||
*/ | ||
public class SetSecurityUserProcessorWithSecurityNotEnabledIT extends ESRestTestCase { | ||
|
||
public void testDefineAndUseProcessor() throws Exception { | ||
final String pipeline = "pipeline-" + getTestName(); | ||
final String index = "index-" + getTestName(); | ||
{ | ||
final Request putPipeline = new Request("PUT", "/_ingest/pipeline/" + pipeline); | ||
putPipeline.setJsonEntity("{" + | ||
" \"description\": \"Test pipeline (" + getTestName() + ")\"," + | ||
" \"processors\":[{" + | ||
" \"set_security_user\":{ \"field\": \"user\" }" + | ||
" }]" + | ||
"}"); | ||
final Response response = client().performRequest(putPipeline); | ||
assertOK(response); | ||
} | ||
|
||
{ | ||
final Request ingest = new Request("PUT", "/" + index + "/_doc/1?pipeline=" + pipeline); | ||
ingest.setJsonEntity("{\"field\":\"value\"}"); | ||
final ResponseException ex = expectThrows(ResponseException.class, () -> client().performRequest(ingest)); | ||
final Response response = ex.getResponse(); | ||
assertThat(EntityUtils.toString(response.getEntity()), | ||
containsString("Security (authentication) is not enabled on this cluster")); | ||
} | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.