Skip to content

Commit 8a5b13b

Browse files
Rishav9852KumarRishav9852Kumar
committed
integ& audit fix
Signed-off-by: Rishav9852Kumar <rishavkumaraug20005212@gmail.com>
1 parent 56936d7 commit 8a5b13b

4 files changed

Lines changed: 215 additions & 76 deletions

File tree

‎src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ public void testDefault() {
4949
final WildcardMatcher defaultIgnoredUserMatcher = WildcardMatcher.from("kibanaserver");
5050
final EnumSet<AuditCategory> defaultDisabledRestCategories = EnumSet.of(AUTHENTICATED, GRANTED_PRIVILEGES);
5151
final EnumSet<AuditCategory> defaultDisabledTransportCategories = EnumSet.of(
52-
AUTHENTICATED, GRANTED_PRIVILEGES, AuditCategory.CLUSTER_SETTINGS_CHANGED, AuditCategory.INDEX_SETTINGS_CHANGED
52+
AUTHENTICATED,
53+
GRANTED_PRIVILEGES,
54+
AuditCategory.CLUSTER_SETTINGS_CHANGED,
55+
AuditCategory.INDEX_SETTINGS_CHANGED
5356
);
5457
// act
5558
final AuditConfig.Filter auditConfigFilter = AuditConfig.Filter.from(Settings.EMPTY);
@@ -208,7 +211,11 @@ public void fromSettingStringSet() {
208211
public void fromSettingParseAuditCategory() {
209212
final FilterEntries entry = FilterEntries.DISABLE_REST_CATEGORIES;
210213
final Function<Settings, Set<AuditCategory>> parse = (settings) -> AuditCategory.parse(
211-
AuditConfig.Filter.fromSettingStringSet(settings, entry, ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_REST_CATEGORIES_DEFAULT)
214+
AuditConfig.Filter.fromSettingStringSet(
215+
settings,
216+
entry,
217+
ConfigConstants.OPENDISTRO_SECURITY_AUDIT_DISABLED_REST_CATEGORIES_DEFAULT
218+
)
212219
);
213220

214221
final Settings noValues = Settings.builder().build();

‎src/test/java/org/opensearch/security/auditlog/config/AuditConfigSerializeTest.java‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ public void testDefaultSerialize() throws IOException {
6767
.field("enable_rest", true)
6868
.field("disabled_rest_categories", ImmutableList.of("AUTHENTICATED", "GRANTED_PRIVILEGES"))
6969
.field("enable_transport", true)
70-
.field("disabled_transport_categories", ImmutableList.of("AUTHENTICATED", "GRANTED_PRIVILEGES", "CLUSTER_SETTINGS_CHANGED", "INDEX_SETTINGS_CHANGED"))
70+
.field(
71+
"disabled_transport_categories",
72+
ImmutableList.of("AUTHENTICATED", "GRANTED_PRIVILEGES", "CLUSTER_SETTINGS_CHANGED", "INDEX_SETTINGS_CHANGED")
73+
)
7174
.field("resolve_bulk_requests", false)
7275
.field("log_request_body", true)
7376
.field("resolve_indices", true)
@@ -106,12 +109,14 @@ public void testDefaultDeserialize() throws IOException {
106109
assertTrue(audit.isTransportApiAuditEnabled());
107110
assertThat(
108111
audit.getDisabledTransportCategories(),
109-
is(EnumSet.of(
110-
AuditCategory.AUTHENTICATED,
111-
AuditCategory.GRANTED_PRIVILEGES,
112-
AuditCategory.CLUSTER_SETTINGS_CHANGED,
113-
AuditCategory.INDEX_SETTINGS_CHANGED
114-
))
112+
is(
113+
EnumSet.of(
114+
AuditCategory.AUTHENTICATED,
115+
AuditCategory.GRANTED_PRIVILEGES,
116+
AuditCategory.CLUSTER_SETTINGS_CHANGED,
117+
AuditCategory.INDEX_SETTINGS_CHANGED
118+
)
119+
)
115120
);
116121
assertFalse(audit.shouldResolveBulkRequests());
117122
assertTrue(audit.shouldLogRequestBody());
@@ -282,7 +287,10 @@ public void testNullSerialize() throws IOException {
282287
.field("enable_rest", true)
283288
.field("disabled_rest_categories", ImmutableList.of("AUTHENTICATED", "GRANTED_PRIVILEGES"))
284289
.field("enable_transport", true)
285-
.field("disabled_transport_categories", ImmutableList.of("AUTHENTICATED", "GRANTED_PRIVILEGES", "CLUSTER_SETTINGS_CHANGED", "INDEX_SETTINGS_CHANGED"))
290+
.field(
291+
"disabled_transport_categories",
292+
ImmutableList.of("AUTHENTICATED", "GRANTED_PRIVILEGES", "CLUSTER_SETTINGS_CHANGED", "INDEX_SETTINGS_CHANGED")
293+
)
286294
.field("resolve_bulk_requests", false)
287295
.field("log_request_body", true)
288296
.field("resolve_indices", true)

‎src/test/java/org/opensearch/security/auditlog/impl/SettingsChangeAuditTest.java‎

Lines changed: 27 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,9 @@
2222
import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest;
2323
import org.opensearch.cluster.ClusterName;
2424
import org.opensearch.cluster.ClusterState;
25-
import org.opensearch.cluster.metadata.IndexMetadata;
2625
import org.opensearch.cluster.metadata.Metadata;
2726
import org.opensearch.cluster.node.DiscoveryNode;
2827
import org.opensearch.cluster.service.ClusterService;
29-
import org.opensearch.common.settings.ClusterSettings;
3028
import org.opensearch.common.settings.Settings;
3129
import org.opensearch.security.auditlog.AuditTestUtils;
3230
import org.opensearch.security.auditlog.integration.TestAuditlogImpl;
@@ -155,9 +153,7 @@ public void testClusterBothPersistentAndTransientSettings() throws Exception {
155153
public void testClusterSettingResetToDefault() throws Exception {
156154
// Simulate existing persistent setting that is being reset (null value)
157155
final Metadata metadata = mock(Metadata.class);
158-
when(metadata.persistentSettings()).thenReturn(
159-
Settings.builder().put("cluster.max_shards_per_node", "2000").build()
160-
);
156+
when(metadata.persistentSettings()).thenReturn(Settings.builder().put("cluster.max_shards_per_node", "2000").build());
161157
when(metadata.transientSettings()).thenReturn(Settings.EMPTY);
162158
final ClusterState state = mock(ClusterState.class);
163159
when(state.metadata()).thenReturn(metadata);
@@ -183,9 +179,7 @@ public void testClusterSettingResetToDefault() throws Exception {
183179
public void testClusterSettingOldValueCaptured() throws Exception {
184180
// Mock current cluster state with an existing persistent setting
185181
final Metadata metadata = mock(Metadata.class);
186-
when(metadata.persistentSettings()).thenReturn(
187-
Settings.builder().put("cluster.max_shards_per_node", "1000").build()
188-
);
182+
when(metadata.persistentSettings()).thenReturn(Settings.builder().put("cluster.max_shards_per_node", "1000").build());
189183
when(metadata.transientSettings()).thenReturn(Settings.EMPTY);
190184
final ClusterState state = mock(ClusterState.class);
191185
when(state.metadata()).thenReturn(metadata);
@@ -308,9 +302,7 @@ public void testSensitiveSettingRedactionByPasswordPattern() throws Exception {
308302
final AbstractAuditLog auditLog = createAuditLog();
309303

310304
final ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
311-
request.persistentSettings(
312-
Settings.builder().put("plugins.security.ssl.transport.keystore_password", "mysecret").build()
313-
);
305+
request.persistentSettings(Settings.builder().put("plugins.security.ssl.transport.keystore_password", "mysecret").build());
314306

315307
auditLog.logSettingsChange("cluster:admin/settings/update", request, null);
316308
auditLog.close();
@@ -325,9 +317,7 @@ public void testSensitiveSettingRedactionBySecretPattern() throws Exception {
325317
final AbstractAuditLog auditLog = createAuditLog();
326318

327319
final ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
328-
request.persistentSettings(
329-
Settings.builder().put("some.plugin.client_secret", "topsecret").build()
330-
);
320+
request.persistentSettings(Settings.builder().put("some.plugin.client_secret", "topsecret").build());
331321

332322
auditLog.logSettingsChange("cluster:admin/settings/update", request, null);
333323
auditLog.close();
@@ -342,9 +332,7 @@ public void testSensitiveSettingRedactionByTokenPattern() throws Exception {
342332
final AbstractAuditLog auditLog = createAuditLog();
343333

344334
final ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
345-
request.persistentSettings(
346-
Settings.builder().put("some.plugin.auth_token", "abc123").build()
347-
);
335+
request.persistentSettings(Settings.builder().put("some.plugin.auth_token", "abc123").build());
348336

349337
auditLog.logSettingsChange("cluster:admin/settings/update", request, null);
350338
auditLog.close();
@@ -359,9 +347,7 @@ public void testNonSensitiveSettingNotRedacted() throws Exception {
359347
final AbstractAuditLog auditLog = createAuditLog();
360348

361349
final ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
362-
request.persistentSettings(
363-
Settings.builder().put("cluster.max_shards_per_node", "2000").build()
364-
);
350+
request.persistentSettings(Settings.builder().put("cluster.max_shards_per_node", "2000").build());
365351

366352
auditLog.logSettingsChange("cluster:admin/settings/update", request, null);
367353
auditLog.close();
@@ -386,9 +372,7 @@ public void testSensitiveOldValueAlsoRedacted() throws Exception {
386372
final AbstractAuditLog auditLog = createAuditLog();
387373

388374
final ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
389-
request.persistentSettings(
390-
Settings.builder().put("plugins.security.ssl.transport.keystore_password", "newsecret").build()
391-
);
375+
request.persistentSettings(Settings.builder().put("plugins.security.ssl.transport.keystore_password", "newsecret").build());
392376

393377
auditLog.logSettingsChange("cluster:admin/settings/update", request, null);
394378
auditLog.close();
@@ -436,7 +420,18 @@ public void testAuditMessageSettingsChangesField() {
436420
final AuditMessage msg = new AuditMessage(AuditCategory.CLUSTER_SETTINGS_CHANGED, cs, null, null);
437421

438422
final List<Map<String, Object>> changes = List.of(
439-
Map.of("setting", "cluster.max_shards_per_node", "old_value", "1000", "new_value", "2000", "operation", "set", "scope", "persistent")
423+
Map.of(
424+
"setting",
425+
"cluster.max_shards_per_node",
426+
"old_value",
427+
"1000",
428+
"new_value",
429+
"2000",
430+
"operation",
431+
"set",
432+
"scope",
433+
"persistent"
434+
)
440435
);
441436
msg.addSettingsChanges(changes);
442437

@@ -499,10 +494,7 @@ public void testMultipleSettingsInOneClusterRequest() throws Exception {
499494

500495
final ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
501496
request.persistentSettings(
502-
Settings.builder()
503-
.put("cluster.max_shards_per_node", "2000")
504-
.put("cluster.routing.allocation.enable", "all")
505-
.build()
497+
Settings.builder().put("cluster.max_shards_per_node", "2000").put("cluster.routing.allocation.enable", "all").build()
506498
);
507499

508500
auditLog.logSettingsChange("cluster:admin/settings/update", request, null);
@@ -516,24 +508,18 @@ public void testMultipleSettingsInOneClusterRequest() throws Exception {
516508
// --- ClusterSettings registry redaction test ---
517509

518510
/**
519-
* Verifies that isSensitiveSetting() uses ClusterSettings.isSensitiveSetting() when available,
520-
* before falling back to pattern matching. This covers the redaction bug fix where the registry
521-
* returning false would skip the pattern fallback.
511+
* Verifies that isSensitiveSetting() pattern fallback works even when ClusterSettings
512+
* registry is available but returns false (setting registered but not as SecureSetting).
513+
* The registry path returning true is covered by integration tests with a real cluster.
522514
*/
523515
@Test
524-
public void testSensitiveSettingRedactionViaRegistryThenPatternFallback() throws Exception {
525-
// Mock ClusterSettings where isSensitiveSetting returns false for a password setting
526-
// (registered but not as SecureSetting). Pattern fallback should still catch it.
527-
final ClusterSettings clusterSettings = mock(ClusterSettings.class);
528-
when(clusterSettings.isSensitiveSetting("plugins.security.ssl.transport.keystore_password")).thenReturn(false);
529-
when(cs.getClusterSettings()).thenReturn(clusterSettings);
530-
516+
public void testSensitiveSettingRedactionWhenRegistryReturnsFalse() throws Exception {
517+
// When getClusterSettings() is not mocked, it returns null → exception caught → pattern fallback runs.
518+
// This test verifies the pattern fallback catches "password" in the key name regardless.
531519
final AbstractAuditLog auditLog = createAuditLog();
532520

533521
final ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
534-
request.persistentSettings(
535-
Settings.builder().put("plugins.security.ssl.transport.keystore_password", "mysecret").build()
536-
);
522+
request.persistentSettings(Settings.builder().put("plugins.security.ssl.transport.keystore_password", "mysecret").build());
537523

538524
auditLog.logSettingsChange("cluster:admin/settings/update", request, null);
539525
auditLog.close();
@@ -543,31 +529,6 @@ public void testSensitiveSettingRedactionViaRegistryThenPatternFallback() throws
543529
assertThat(result, not(containsString("mysecret")));
544530
}
545531

546-
/**
547-
* Verifies that isSensitiveSetting() returns true directly from the ClusterSettings registry
548-
* when the setting is registered as a SecureSetting, without needing the pattern fallback.
549-
*/
550-
@Test
551-
public void testSensitiveSettingRedactionViaRegistryReturnsTrue() throws Exception {
552-
final ClusterSettings clusterSettings = mock(ClusterSettings.class);
553-
when(clusterSettings.isSensitiveSetting("some.secure.setting")).thenReturn(true);
554-
when(cs.getClusterSettings()).thenReturn(clusterSettings);
555-
556-
final AbstractAuditLog auditLog = createAuditLog();
557-
558-
final ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
559-
request.persistentSettings(
560-
Settings.builder().put("some.secure.setting", "secretvalue").build()
561-
);
562-
563-
auditLog.logSettingsChange("cluster:admin/settings/update", request, null);
564-
auditLog.close();
565-
566-
final String result = TestAuditlogImpl.sb.toString();
567-
assertThat(result, containsString("***REDACTED***"));
568-
assertThat(result, not(containsString("secretvalue")));
569-
}
570-
571532
// --- Cluster state unavailable fallback test ---
572533

573534
/**

0 commit comments

Comments
 (0)